git support

static/upload.py

 #!/usr/bin/env python
 #
 # Copyright 2007 Google Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
 #
 #     http://www.apache.org/licenses/LICENSE-2.0
 #
 # Unless required by applicable law or agreed to in writing, software
 # distributed under the License is distributed on an "AS IS" BASIS,
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Tool for uploading diffs to the codereview app.

[Andi Albrecht, 2008/08/19 04:59:03]

I think we should make clear that it uploads diffs between a working copy and a
repository (and not diffs produced by the diff command).

+"""Tool for uploading diffs from a version control system to the codereview app.
 
 Usage summary: upload.py [options] [-- diff_options]
 
-With Subversion, diff_options are passed to "svn diff".
-With Git, diff_options are passed to "git diff".

[Andi Albrecht, 2008/08/19 04:59:03]

I'd unify this message to something like "diff_options are passed to the diff
command of the version control system."

+Diff options are passed to the diff command of the underlying system.
+
+Supported version control systems:
+  Git
+  Subversion
+
+(It is important for Git users to specify a tree-ish to diff against.)
 """
 # This code is derived from appcfg.py in the App Engine SDK (open source),
 # and from ASPN recipe #146306.
 
 import cookielib
 import getpass
 import logging
 import md5
 import mimetypes
 import optparse
 import os
 import re
 import socket
+import subprocess
 import sys
 import urllib
 import urllib2
 import urlparse
 
 try:
   import readline
 except ImportError:
   pass
 
@@ skipping 440 matching lines @@
   """Helper to guess the content-type from the filename."""
   return mimetypes.guess_type(filename)[0] or 'application/octet-stream'
 
 
 def RunShell(command, args=(), silent_ok=False):
   command = "%s %s" % (command, " ".join(args))
   logging.info("Running %s", command)
   stream = os.popen(command, "r")
   data = stream.read()
   if stream.close():
-    ErrorExit("Got error status from %s" % (command + str(args)))
+    ErrorExit("Got error status from %s" % command)
   if not silent_ok and not data:
     ErrorExit("No output from %s" % command)
   return data
 
 
 class VersionControlSystem(object):
   """Abstract base class providing an interface to the VCS."""
 
   def GenerateDiff(self, args):
     """Return the current diff as a string.
@@ skipping 81 matching lines @@
     Args:
       required: If true, exits if the url can't be guessed, otherwise None is
         returned.
     """
     info = RunShell("svn info")
     for line in info.splitlines():
       words = line.split()
       if len(words) == 2 and words[0] == "URL:":
         url = words[1]
         scheme, netloc, path, params, query, fragment = urlparse.urlparse(url)
+        username, netloc = urllib.splituser(netloc)
+        if username:
+          logging.info("Removed username from base URL")
         if netloc.endswith("svn.python.org"):
           if netloc == "svn.python.org":
             if path.startswith("/projects/"):
               path = path[9:]
           elif netloc != "pythondev@svn.python.org":
             ErrorExit("Unrecognized Python URL: %s" % url)
           base = "http://svn.python.org/view/*checkout*%s/" % path
           logging.info("Guessed Python base = %s", base)
         elif netloc.endswith("svn.collab.net"):
           if path.startswith("/repos/"):
             path = path[6:]
           base = "http://svn.collab.net/viewvc/*checkout*%s/" % path
           logging.info("Guessed CollabNet base = %s", base)
         elif netloc.endswith(".googlecode.com"):
-          base = url + "/"
-          if base.startswith("https"):
-            base = "http" + base[5:]
+          path = path + "/"
+          base = urlparse.urlunparse(("http", netloc, path, params,
+                                      query, fragment))
           logging.info("Guessed Google Code base = %s", base)
         else:
-          base = url + "/"
+          path = path + "/"
+          base = urlparse.urlunparse((scheme, netloc, path, params,
+                                      query, fragment))
           logging.info("Guessed base = %s", base)
         return base
     if required:
       ErrorExit("Can't find URL in output from svn info")
     return None
 
   def GenerateDiff(self, args):
     cmd = "svn diff"
     if not sys.platform.startswith("win"):
       cmd += " --diff-cmd=diff"
@@ skipping 94 matching lines @@
 
   def GenerateDiff(self, extra_args):
     # This is more complicated than svn's GenerateDiff because we must convert
     # the diff output to include an svn-style "Index:" line as well as record
     # the hashes of the base files, so we can upload them along with our diff.
     gitdiff = RunShell("git diff", ["--full-index"] + extra_args)
     svndiff = []
     filecount = 0
     filename = None
     for line in gitdiff.splitlines():
-      match = re.match(r"diff --git a/(.*) b/.*", line)

[Andi Albrecht, 2008/08/19 04:59:03]

We should wrap the regular expression in '^...$' to avoid matching the pattern
somewhere else, e.g. in comments.

+      match = re.match(r"diff --git a/(.*) b/.*$", line)
       if match:
         filecount += 1
         filename = match.group(1)
         svndiff.append("Index: %s\n" % filename)
       else:
         # The "index" line in a git diff looks like this (long hashes elided):
         #   index 82c0d44..b2cee3f 100755
         # We want to save the left hash, as that identifies the base file.
         match = re.match(r"index (\w+)\.\.", line)

[Andi Albrecht, 2008/08/19 04:59:03]

'^...$' here too.

         if match:
           self.base_hashes[filename] = match.group(1)
       svndiff.append(line + "\n")
     if not filecount:

[Andi Albrecht, 2008/08/19 04:59:03]

Do we need to run an additional sanity check here? If the above first regex
matches, but the second not, we have an index line in svndiff but no
corresponding entry in base_hashes.
I can't tell if this is a real problem... I was just thinking about diffs that
are somehow broken or if the pattern for the git-style index line doesn't match
for some reason.

       ErrorExit("No valid patches found in output from git diff")
     return "".join(svndiff)
 
   def GetUnknownFiles(self):
     status = RunShell("git ls-files --others", silent_ok=True)
     return status.splitlines()
 
   def GetBaseFile(self, filename):
     hash = self.base_hashes[filename]
     if hash == "0" * 40:  # All-zero hash indicates no base file.
@@ skipping 69 matching lines @@
       sys.exit(False)
     rv.append([lines[1], patch[0]])
   return rv
 
 
 def GuessVCS():
   """Helper to guess the version control system.
 
   This examines the current directory, guesses which VersionControlSystem
   we're using, and returns an instance of the appropriate class.  Exit with an
   error if we can't figure it out.

[Andi Albrecht, 2008/08/19 19:48:34]

Thanks for adding some more docstrings and comments!

 
   Returns:
     A VersionControlSystem instance. Exits if the VCS can't be guessed.
   """
-
   # Subversion has a .svn in all working directories.
-  if os.path.isdir(os.path.join(os.getcwd(), '.svn')):
+  if os.path.isdir('.svn'):
     logging.info("Guessed VCS = Subversion")
     return SubversionVCS()
 
   # Git has a command to test if you're in a git tree.
-  # This will silently fail if you don't have git installed.
-  if os.system("git rev-parse --is-inside-work-tree >/dev/null 2>&1") == 0:

[Andi Albrecht, 2008/08/19 04:59:03]

What about Windows users? I'm not sure how the syntax on a Windows shell looks
like, but I think they have no /dev/null ;-)

-    return GitVCS()
+  # Try running it, but don't die if we don't have git installed.
+  try:
+    subproc = subprocess.Popen(["git", "rev-parse", "--is-inside-work-tree"],
+                               stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    if subproc.wait() == 0:
+      return GitVCS()
+  except OSError, (errno, message):
+    if errno != 2:  # ENOENT -- they don't have git installed.
+      raise
 
   ErrorExit(("Could not guess version control system. "
              "Are you in a working copy directory?"))
 
 
 def RealMain(argv, data=None):
   logging.basicConfig(format=("%(asctime).19s %(levelname)s %(filename)s:"
                               "%(lineno)s %(message)s "))
   os.environ['LC_ALL'] = 'C'
   options, args = parser.parse_args(argv[1:])
   global verbosity
   verbosity = options.verbose
   if verbosity >= 3:
     logging.getLogger().setLevel(logging.DEBUG)
   elif verbosity >= 2:
     logging.getLogger().setLevel(logging.INFO)
   vcs = GuessVCS()
   if isinstance(vcs, SubversionVCS):
     # base field is only allowed for Subversion.
     # Note: Fetching base files may become deprecated in future releases.
     base = vcs.GuessBase(not options.local_base)
   else:
     base = None
   if not base and not options.local_base:
+    # TODO(andi): Enable local_base for other VCS by default.
     # For future use. SubversionVCS.GuessBase() already checks this

[Andi Albrecht, 2008/08/19 19:48:34]

I think we can enable local_base here now for all other VCS. In my current
Bazaar patch this part is:

if not base and not options.local_base:
  options.local_base = True
  logging.info("Enabled upload of base file")

     # condition.
     ErrorExit("Use '--local_base' to upload base files.")
   if not options.assume_yes:
     vcs.CheckForUnknownFiles()
   if not data:
     data = vcs.GenerateDiff(args)
   if verbosity >= 1:
     print "Upload server:", options.server, "(change with -s/--server)"
   if options.issue:
     prompt = "Message describing this patch set: "
@@ skipping 53 matching lines @@
       msg = response_body
   else:
     msg = response_body
   StatusUpdate(msg)
   if not response_body.startswith("Issue created.") and \
   not response_body.startswith("Issue updated."):
     sys.exit(0)
   issue = msg[msg.rfind("/")+1:]
 
   if not files:
-    rv = UploadSeparatePatches(issue, rpc_server, patchset, data, options)
+    result = UploadSeparatePatches(issue, rpc_server, patchset, data, options)
     if options.local_base:
-      patches = rv
+      patches = result
 
   if options.local_base:
     vcs.UploadBaseFiles(issue, rpc_server, patches, patchset, options)
     if options.send_mail:
       rpc_server.Send("/" + issue + "/mail")
   return issue
 
 
 def main():
   try:
     RealMain(sys.argv)
   except KeyboardInterrupt:
     print
     StatusUpdate("Interrupted.")
     sys.exit(1)
 
 
 if __name__ == "__main__":
   main()