1917: post-review doesn't play nice with msysgit on Windows

joshua*******@gmai***** (Google Code) (Is this you? Claim this profile.)
March 31, 2011
What version are you running?
1.5 Beta 1

When trying to post a review from a git repository on Windows using msysgit, post-review will not pick up git because msysgit installs a 'git.cmd' into the user's PATH. But post-review uses subprocess.Popen to launch commands, which on Windows delegates to CreateProcess, which does not automatically append .cmd extensions, only .exe (see MSDN for an explanation of this behavior.) However, CreateProcess will resolve things in the PATH, though, so subprocess.Popen('git.cmd', ...) does work.

The fix is to try running 'git.cmd' explicitly, only on Windows. a patch is attached.
diff --git a/rbtools/postreview.py b/rbtools/postreview.py
index 64d7fa2..a3610ce 100755
--- a/rbtools/postreview.py
+++ b/rbtools/postreview.py
@@ -2242,15 +2242,26 @@ class GitClient(SCMClient):
     compatible diffs. This will attempt to generate a diff suitable for the
     remote repository, whether git, SVN or Perforce.
+    def __init__(self):
+        SCMClient.__init__(self)
+        # Store the 'correct' way to invoke git, just plain old 'git' by default
+        self.git = 'git'
     def _strip_heads_prefix(self, ref):
         """ Strips prefix from ref name, if possible """
         return re.sub(r'^refs/heads/', '', ref)
     def get_repository_info(self):
         if not check_install('git --help'):
-            return None
+            # CreateProcess (launched via subprocess, used by check_install)
+            # does not automatically append .cmd for things it finds in PATH.
+            # If we're on Windows, and this works, save it for further use.
#1 joshua*******@gmai***** (Google Code) (Is this you? Claim this profile.)
Ping, any progress on this?
#2 david
Please put this on http://reviews.reviewboard.org/
  • +PendingReview
#3 joshua*******@gmai***** (Google Code) (Is this you? Claim this profile.)
OK, http://reviews.reviewboard.org/r/2226/
#4 david
Fixed in master as d0138a7.
  • -PendingReview