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.
+"""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 is the tree-ish to diff against, defaulting to master.
+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
 
 # The logging verbosity:
 #  0: Errors only.
 #  1: Status messages.
 #  2: Info logs.
 #  3: Debug logs.
 verbosity = 1
+
+# Max size of patch or base file.
+MAX_UPLOAD_SIZE = 900 * 1024
 
 
 def StatusUpdate(msg):
   """Print a status message to stdout.
 
   If 'verbosity' is greater than 0, print the message.
 
   Args:
     msg: The string to print.
   """
@@ skipping 148 matching lines @@
         auth_token = self._GetAuthToken(credentials[0], credentials[1])
       except ClientLoginError, e:
         if e.reason == "BadAuthentication":
           print >>sys.stderr, "Invalid username or password."
           continue
         if e.reason == "CaptchaRequired":
           print >>sys.stderr, (
               "Please go to\n"
               "https://www.google.com/accounts/DisplayUnlockCaptcha\n"
               "and verify you are a human.  Then try again.")
-          break;
+          break
         if e.reason == "NotVerified":
           print >>sys.stderr, "Account not verified."
           break
         if e.reason == "TermsNotAgreed":
           print >>sys.stderr, "User has not agreed to TOS."
           break
         if e.reason == "AccountDeleted":
           print >>sys.stderr, "The user account has been deleted."
           break
         if e.reason == "AccountDisabled":
           print >>sys.stderr, "The user account has been disabled."
           break
         if e.reason == "ServiceDisabled":
           print >>sys.stderr, ("The user's access to the service has been "
                                "disabled.")
           break
         if e.reason == "ServiceUnavailable":
           print >>sys.stderr, "The service is not available; try again later."
           break
         raise
       self._GetAuthCookie(auth_token)
       return
 
-  def Send(self, request_path, payload="",
+  def Send(self, request_path, payload=None,
            content_type="application/octet-stream",
            timeout=None,
            **kwargs):
     """Sends an RPC and returns the response.
 
     Args:
       request_path: The path to send the request to, eg /api/appversion/create.
       payload: The body of the request, or None to send an empty request.
       content_type: The Content-Type header to use.
       timeout: timeout in seconds; default None i.e. no timeout.
@@ skipping 81 matching lines @@
       # Always chmod the cookie file
       os.chmod(self.cookie_file, 0600)
     else:
       # Don't save cookies across runs of update.py.
       self.cookie_jar = cookielib.CookieJar()
     opener.add_handler(urllib2.HTTPCookieProcessor(self.cookie_jar))
     return opener
 
 
 parser = optparse.OptionParser(usage="%prog [options] [-- diff_options]")
-parser.add_option("-q", "--quiet", action="store_const", const=0,
-                  dest="verbose", help="Print errors only.")
-parser.add_option("-v", "--verbose", action="store_const", const=2,
-                  dest="verbose", default=1,
-                  help="Print info level logs.")
-parser.add_option("--noisy", action="store_const", const=3,
-                  dest="verbose", help="Print all logs.")
-parser.add_option("-s", "--server", action="store", dest="server",
-                  default="codereview.appspot.com",
-                  metavar="SERVER",
-                  help="The server to upload to. The format is host[:port].")
-parser.add_option("-e", "--email", action="store", dest="email",
-                  metavar="EMAIL", default=None,
-                  help="The username to use. Will prompt if omitted.")
-parser.add_option("-H", "--host", action="store", dest="host",
-                  metavar="HOST", default=None,
-                  help="Overrides the Host header sent with all RPCs.")
-parser.add_option("--no_cookies", action="store_false",
-                  dest="save_cookies", default=True,
-                  help="Do not save authentication cookies to local disk.")
-parser.add_option("-m", "--message", action="store", dest="message",
-                  metavar="MESSAGE", default=None,
-                  help="A message to identify the patch. "
-                       "Will prompt if omitted.")
-parser.add_option("-i", "--issue", type="int", action="store",
-                  metavar="ISSUE", default=None,
-                  help="Issue number to which to add. Defaults to new issue.")
 parser.add_option("-y", "--assume_yes", action="store_true",
                   dest="assume_yes", default=False,
                   help="Assume that the answer to yes/no questions is 'yes'.")
-parser.add_option("-l", "--local_base", action="store_true",
-                  dest="local_base", default=False,
-                  help="Base files will be uploaded.")
-parser.add_option("-r", "--reviewers", action="store", dest="reviewers",
-                  metavar="REVIEWERS", default=None,
-                  help="Add reviewers (comma separated email addresses).")
-parser.add_option("--cc", action="store", dest="cc",
-                  metavar="CC", default=None,
-                  help="Add CC (comma separated email addresses).")
+# Logging
+group = parser.add_option_group("Logging options")
+group.add_option("-q", "--quiet", action="store_const", const=0,
+                 dest="verbose", help="Print errors only.")
+group.add_option("-v", "--verbose", action="store_const", const=2,
+                 dest="verbose", default=1,
+                 help="Print info level logs (default).")
+group.add_option("--noisy", action="store_const", const=3,
+                 dest="verbose", help="Print all logs.")
+# Review server
+group = parser.add_option_group("Review server options")
+group.add_option("-s", "--server", action="store", dest="server",
+                 default="codereview.appspot.com",
+                 metavar="SERVER",
+                 help=("The server to upload to. The format is host[:port]. "
+                       "Defaults to 'codereview.appspot.com'."))
+group.add_option("-e", "--email", action="store", dest="email",
+                 metavar="EMAIL", default=None,
+                 help="The username to use. Will prompt if omitted.")
+group.add_option("-H", "--host", action="store", dest="host",
+                 metavar="HOST", default=None,
+                 help="Overrides the Host header sent with all RPCs.")
+group.add_option("--no_cookies", action="store_false",
+                 dest="save_cookies", default=True,
+                 help="Do not save authentication cookies to local disk.")
+# Issue
+group = parser.add_option_group("Issue options")
+group.add_option("-d", "--description", action="store", dest="description",
+                 metavar="DESCRIPTION", default=None,
+                 help="Optional description when creating an issue.")
+group.add_option("-f", "--description_file", action="store",
+                 dest="description_file", metavar="DESCRIPTION_FILE",
+                 default=None,
+                 help="Optional path of a file that contains "
+                      "the description when creating an issue.")
+group.add_option("-r", "--reviewers", action="store", dest="reviewers",
+                 metavar="REVIEWERS", default=None,
+                 help="Add reviewers (comma separated email addresses).")
+group.add_option("--cc", action="store", dest="cc",
+                 metavar="CC", default=None,
+                 help="Add CC (comma separated email addresses).")
+# Upload options
+group = parser.add_option_group("Patch options")
+group.add_option("-m", "--message", action="store", dest="message",
+                 metavar="MESSAGE", default=None,
+                 help="A message to identify the patch. "
+                      "Will prompt if omitted.")
+group.add_option("-i", "--issue", type="int", action="store",
+                 metavar="ISSUE", default=None,
+                 help="Issue number to which to add. Defaults to new issue.")
+group.add_option("-l", "--local_base", action="store_true",
+                 dest="local_base", default=False,
+                 help="Base files will be uploaded.")
+group.add_option("--send_mail", action="store_true",
+                 dest="send_mail", default=False,
+                 help="Send notification email to reviewers.")
 
 
 def GetRpcServer(options):
   """Returns an instance of an AbstractRpcServer.
 
   Returns:
     A new AbstractRpcServer, on which RPC calls can be made.
   """
 
   rpc_server_class = HttpRpcServer
@@ skipping 77 matching lines @@
   if stream.close():
     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 __init__(self, args):
-    """
+  def GenerateDiff(self, args):
+    """Return the current diff as a string.
+
     Args:
       args: Extra arguments to pass to the diff command.
     """
     raise NotImplementedError(
         "abstract method -- subclass %s must override" % self.__class__)
-
-  def GuessBase(self, required):
-    """Returns the SVN base URL.
-
-    Args:
-      required: If true, exits if the url can't be guessed, otherwise None is
-        returned.
-    """
-    if required:
-      ErrorExit("Cannot guess base URL; use the --local_base flag instead.")
-
-  def GenerateDiff(self):
-    """Return the current diff as a string."""
-    raise NotImplementedError(
-          "abstract method -- subclass %s must override" % self.__class__)
 
   def GetUnknownFiles(self):
     """Return a list of files unknown to the VCS."""
     raise NotImplementedError(
         "abstract method -- subclass %s must override" % self.__class__)
 
   def CheckForUnknownFiles(self):
     """Show an "are you sure?" prompt if there are unknown files."""
     unknown_files = self.GetUnknownFiles()
     if unknown_files:
       print "The following files are not added to version control:"
       for line in unknown_files:
         print line
       prompt = "Are you sure to continue?(y/N) "
       answer = raw_input(prompt).strip()
       if answer != "y":
         ErrorExit("User aborted")
 
   def GetBaseFile(self, filename):
-    """Get the content of the upstream version of a file."""
+    """Get the content of the upstream version of a file.
+
+    Returns:
+      A tuple (content, status) representing the file content and the status of
+      the file.
+    """
+
     raise NotImplementedError(
         "abstract method -- subclass %s must override" % self.__class__)
 
   def UploadBaseFiles(self, issue, rpc_server, patch_list, patchset, options):
     """Uploads the base files."""
     patches = dict()
     [patches.setdefault(v, k) for k, v in patch_list]
     for filename in patches.keys():
-      content = self.GetBaseFile(filename)
+      content, status = self.GetBaseFile(filename)
+      if len(content) > MAX_UPLOAD_SIZE:
+        print ("Not uploading the base file for " + filename +
+               " because the file is too large.")
+        continue
       checksum = md5.new(content).hexdigest()
       parts = []
       while content:
         parts.append(content[:800000])
         content = content[800000:]
       if not parts:
         parts = [""] # empty file
       for part, data in enumerate(parts):
         if options.verbose > 0:
           print "Uploading %s (%d/%d)" % (filename, part+1, len(parts))
         url = "/%d/upload_content/%d/%d" % (int(issue), int(patchset),
                                             int(patches.get(filename)))
         current_checksum = md5.new(data).hexdigest()
         form_fields = [("filename", filename),
+                       ("status", status),
                        ("num_parts", str(len(parts))),
                        ("checksum", checksum),
                        ("current_part", str(part)),
                        ("current_checksum", current_checksum),]
         if options.email:
           form_fields.append(("user", options.email))
         ctype, body = EncodeMultipartFormData(form_fields,
                                               [("data", filename, data)])
         response_body = rpc_server.Send(url, body,
                                         content_type=ctype)
         if not response_body.startswith("OK"):
           StatusUpdate("  --> %s" % response_body)
           sys.exit(False)
 
 
 class SubversionVCS(VersionControlSystem):
   """Implementation of the VersionControlSystem interface for Subversion."""
 
-  def __init__(self, extra_args):
-    self.diff_args = extra_args
-
   def GuessBase(self, required):
+    """Returns the SVN base URL.
+
+    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):
+  def GenerateDiff(self, args):
     cmd = "svn diff"
     if not sys.platform.startswith("win"):
       cmd += " --diff-cmd=diff"
-    data = RunShell(cmd, self.diff_args)
+    data = RunShell(cmd, args)
     count = 0
     for line in data.splitlines():
-      if line.startswith("Index:"):
+      if line.startswith("Index:") or line.startswith("Property changes on:"):
         count += 1
         logging.info(line)
     if not count:
       ErrorExit("No valid patches found in output from svn diff")
     return data
 
   def _CollapseKeywords(self, content, keyword_str):
     """Collapses SVN keywords."""
     # svn cat translates keywords but svn diff doesn't. As a result of this
     # behavior patching.PatchChunks() fails with a chunk mismatch error.
@@ skipping 39 matching lines @@
     status_lines = status.splitlines()
     # If file is in a cl, the output will begin with
     # "\n--- Changelist 'cl_name':\n".  See
     # http://svn.collab.net/repos/svn/trunk/notes/changelist-design.txt
     if (len(status_lines) == 3 and
         not status_lines[0] and
         status_lines[1].startswith("--- Changelist")):
       status = status_lines[2]
     else:
       status = status_lines[0]
-    if status[0] == "A":
+    # If a file is copied its status will be "A  +", which signifies
+    # "addition-with-history".  See "svn st" for more information.  We need to
+    # upload the original file or else diff parsing will fail if the file was
+    # edited.
+    if ((status[0] == "A" and status[3] != "+") or
+        (status[0] == " " and status[1] == "M")):  # property changed
       content = ""
-    elif status[0] in ["M", "D"]:
+    elif (status[0] in ("M", "D", "R") or
+          (status[0] == "A" and status[3] == "+")):
       mimetype = RunShell("svn -rBASE propget svn:mime-type", [filename],
                           silent_ok=True)
       if mimetype.startswith("application/octet-stream"):
         content = ""
       else:
         content = RunShell("svn cat", [filename])
       keywords = RunShell("svn -rBASE propget svn:keywords", [filename],
                           silent_ok=True)
       if keywords:
         content = self._CollapseKeywords(content, keywords)
     else:
       StatusUpdate("svn status returned unexpected output: %s" % status)
       sys.exit(False)
-    return content
+    return content, status[0:5]
 
 
 class GitVCS(VersionControlSystem):
   """Implementation of the VersionControlSystem interface for Git."""
 
-  def __init__(self, extra_args):
-    if len(extra_args) == 0:
-      self.diff_from = "master"

[joachim, 2008/07/31 19:37:20]

"master" is not a reserved name in git.  It seems like HEAD^ would be a very
viable alternative.  Alternatively, require exactly one argument?  Alternatively
yet, if zero arguments are given, should the diff be produced "from" the git
index, just like git diff would?

More broadly, this class requires that the uploaded diff be visible in the
working directory (well, the "to" part of that diff).  This might be an
annoyance in some cases:  Maybe you want to upload the diff between, say,
HEAD^^^ and HEAD^^, without first having to checkout (reset --hard to) HEAD^^. 
(In particular, this might screw up the mtime of various files, which might
interact badly with your build system.)  So it seems like you might want for
both ends of the diff to be any of:
- some named commit;
- the index; or
- the state of the working directory.

For simplicity's sake, then, I would suggest that the "to" part of the diff
always be taken from the index (i.e., add "--cached" to all your invocations of
git-diff), and that the "from" part be HEAD, unless specified in this one
optional argument here.  What do you think?

-    elif len(extra_args) == 1:
-      self.diff_from = extra_args[0]
-    else:
-      ErrorExit("git-based upload takes at most one argument")
-
-  def GenerateDiff(self):
-    gitdiff = RunShell("git diff", [self.diff_from])

[joachim, 2008/07/31 19:37:20]

It would seem to be more robust if you used GIT_EXTERNAL_DIFF as described in
git(7) rather than parsing the diff that git itself produces.  I'm not sure,
however, how well that would tie in with the interface here.

+  def __init__(self):
+    # Map of filename -> hash of base file.
+    self.base_hashes = {}
+
+  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)
+      match = re.match(r"diff --git a/(.*) b/.*$", line)
       if match:
         filecount += 1
-        # We must fix up the diff so it looks like a Subversion one.
-        # The "Index:" line in a Subversion diff refers to the original file,
-        # which is the filename after "a/" in the diff --git line.
-        svndiff.append("Index: " + match.group(1) + "\n")
+        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)
+        if match:
+          self.base_hashes[filename] = match.group(1)
       svndiff.append(line + "\n")
     if not filecount:
       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()
 

[joachim, 2008/07/31 19:37:20]

You should probably have a somewhat more elaborate version of
CheckForUnknownFiles().  If you go with the current design, you should check
that "git diff --cached" is empty.  If you go with my proposed design above,
GetUnknownFiles() would always return an empty set of files, and
CheckForUnknownFiles() should ask the user if they are sure of what they are
doing if "git diff" (without the --cached) is non-empty.

   def GetBaseFile(self, filename):
-    status = RunShell("git diff --abbrev=40 --raw", [self.diff_from, filename])

[joachim, 2008/07/31 19:37:20]

You probably want an additional "--" between the diff_from and the filename.

Also, if I understand the intent of this function correctly, don't you need the
-M (and, possibly, the -C and/or the --find-copies-harder) option?  If so, then,
by restricting the diff to a single file, though, you would never see a rename. 
It seems like you want the full (raw) diff, from which you would then grep for
the correct filename.

-    # The "raw diff" output looks like this (elided text marked with [...]):
-    #   :100755 100755 7a78c8e[...] 000000000[...] M      static/upload.py
-    # The interesting bit is the third column, which is the hash of the
-    # filename the diff is based against.  (It'll be zero if it's a new file.)
-    hash = status.split(' ')[2]
-    if hash == "0" * 40:
-      return ""  # No file to base against.
+    hash = self.base_hashes[filename]
+    if hash == "0" * 40:  # All-zero hash indicates no base file.
+      return ("", "A")
     else:
-      return RunShell("git show", [hash])
-
-
-def GuessVCSClass():
-  """Guess and return the proper VersionControlSystem class for this tree.
+      return (RunShell("git show", [hash]), "M")
+
+
+# NOTE: this function is duplicated in engine.py, keep them in sync.
+def SplitPatch(data):
+  """Splits a patch into separate pieces for each file.
+
+  Args:
+    data: A string containing the output of svn diff.
+
+  Returns:
+    A list of 2-tuple (filename, text) where text is the svn diff output
+      pertaining to filename.
+  """
+  patches = []
+  filename = None
+  diff = []
+  for line in data.splitlines(True):
+    new_filename = None
+    if line.startswith('Index:'):
+      unused, new_filename = line.split(':', 1)
+      new_filename = new_filename.strip()
+    elif line.startswith('Property changes on:'):
+      unused, temp_filename = line.split(':', 1)
+      # When a file is modified, paths use '/' between directories, however
+      # when a property is modified '\' is used on Windows.  Make them the same
+      # otherwise the file shows up twice.
+      temp_filename = temp_filename.strip().replace('\\', '/')
+      if temp_filename != filename:
+        # File has property changes but no modifications, create a new diff.
+        new_filename = temp_filename
+    if new_filename:
+      if filename and diff:
+        patches.append((filename, ''.join(diff)))
+      filename = new_filename
+      diff = [line]
+      continue
+    if diff is not None:
+      diff.append(line)
+  if filename and diff:
+    patches.append((filename, ''.join(diff)))
+  return patches
+
+
+def UploadSeparatePatches(issue, rpc_server, patchset, data, options):
+  """Uploads a separate patch for each file in the diff output.
+
+  Returns a list of [patch_key, filename] for each file.
+  """
+  patches = SplitPatch(data)
+  rv = []
+  for patch in patches:
+    if len(patch[1]) > MAX_UPLOAD_SIZE:
+      print ("Not uploading the patch for " + patch[0] +
+             " because the file is too large.")
+      continue
+    form_fields = [("filename", patch[0])]
+    if options.local_base:
+      form_fields.append(("content_upload", "1"))
+    files = [("data", "data.diff", patch[1])]
+    ctype, body = EncodeMultipartFormData(form_fields, files)
+    url = "/%d/upload_patch/%d" % (int(issue), int(patchset))
+    print "Uploading patch for " + patch[0]
+    response_body = rpc_server.Send(url, body, content_type=ctype)
+    lines = response_body.splitlines()
+    if not lines or lines[0] != "OK":
+      StatusUpdate("  --> %s" % response_body)
+      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 the appropriate class.  Exit with an error if we
-  can't figure it out.
+  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.exists(".svn"):
-    return SubversionVCS
+  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:
-    return GitVCS
-
-  ErrorExit("can't identify whether you're using svn or git")
-
-
-def RealMain(argv):
+  # 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)
-  vcsclass = GuessVCSClass()
-  vcs = vcsclass(args)
-  base = vcs.GuessBase(not options.local_base)
+  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()
-  data = vcs.GenerateDiff()
+  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: "
   else:
     prompt = "New issue subject: "
   message = options.message or raw_input(prompt).strip()
   if not message:
     ErrorExit("A non-empty message is required")
   rpc_server = GetRpcServer(options)
   form_fields = [("subject", message)]
   if base:
     form_fields.append(("base", base))
   if options.issue:
     form_fields.append(("issue", str(options.issue)))
   if options.email:
     form_fields.append(("user", options.email))
   if options.reviewers:
     for reviewer in options.reviewers.split(','):
       if reviewer.count("@") != 1 or "." not in reviewer.split("@")[1]:
         ErrorExit("Invalid email address: %s" % reviewer)
     form_fields.append(("reviewers", options.reviewers))
   if options.cc:
     for cc in options.cc.split(','):
       if cc.count("@") != 1 or "." not in cc.split("@")[1]:
         ErrorExit("Invalid email address: %s" % cc)
     form_fields.append(("cc", options.cc))
+  description = options.description
+  if options.description_file:
+    if options.description:
+      ErrorExit("Can't specify description and description_file")
+    file = open(options.description_file, 'r')
+    description = file.read()
+    file.close()
+  if description:
+    form_fields.append(("description", description))
+  # If we're uploading base files, don't send the email before the uploads, so
+  # that it contains the file status.
+  if options.send_mail and not options.local_base:
+    form_fields.append(("send_mail", "1"))
   if options.local_base:
     form_fields.append(("content_upload", "1"))
-  ctype, body = EncodeMultipartFormData(form_fields,
-                                        [("data", "data.diff", data)])
+  if len(data) > MAX_UPLOAD_SIZE:
+    print "Patch is large, so uploading file patches separately."
+    files = []
+    form_fields.append(("separate_patches", "1"))
+  else:
+    files = [("data", "data.diff", data)]
+  ctype, body = EncodeMultipartFormData(form_fields, files)
   response_body = rpc_server.Send("/upload", body, content_type=ctype)
-  if options.local_base:
+  if options.local_base or not files:
     lines = response_body.splitlines()
-    if len(lines) > 2:
+    if len(lines) >= 2:
       msg = lines[0]
       patchset = lines[1].strip()
       patches = [x.split(" ", 1) for x in lines[2:]]
     else:
       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:
+    result = UploadSeparatePatches(issue, rpc_server, patchset, data, options)
+    if options.local_base:
+      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()