Update upload.py to revision 488 and remove HOSTED support.

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
 #
@@ skipping 27 matching lines @@
 import optparse
 import os
 import re
 import socket
 import subprocess
 import sys
 import urllib
 import urllib2
 import urlparse
 
-# Work-around for md5 module deprecation warning in python 2.5+:
-try: 
-  # Try to load hashlib (python 2.5+)
+# The md5 module was deprecated in Python 2.5.
+try:
   from hashlib import md5
 except ImportError:
-  # If hashlib cannot be imported, load md5.new instead.
-  from md5 import new as md5
+  from md5 import md5
 
 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
 
+# Constants for version control names.  Used by GuessVCSName.
+VCS_GIT = "Git"
+VCS_MERCURIAL = "Mercurial"
+VCS_SUBVERSION = "Subversion"
+VCS_UNKNOWN = "Unknown"
 
-def GetEmail():
+# whitelist for non-binary filetypes which do not start with "text/"
+# .mm (Objective-C) shows up as application/x-freemind on my Linux box.
+TEXT_MIMETYPES = ['application/javascript', 'application/x-javascript',
+                  'application/x-freemind']
+
+VCS_ABBREVIATIONS = {
+  VCS_MERCURIAL.lower(): VCS_MERCURIAL,
+  "hg": VCS_MERCURIAL,
+  VCS_SUBVERSION.lower(): VCS_SUBVERSION,
+  "svn": VCS_SUBVERSION,
+  VCS_GIT.lower(): VCS_GIT,
+}
+
+
+def GetEmail(prompt):
   """Prompts the user for their email address and returns it.
 
   The last used email address is saved to a file and offered up as a suggestion
   to the user. If the user presses enter without typing in anything the last
   used email address is used. If the user enters a new address, it is saved
   for next time we prompt.
 
   """
-  last_email_file_name = os.path.expanduser(
-      os.path.join("~", ".last_codereview_email_address"))
+  last_email_file_name = os.path.expanduser("~/.last_codereview_email_address")
   last_email = ""
-  prompt = "Email: "
   if os.path.exists(last_email_file_name):
     try:
       last_email_file = open(last_email_file_name, "r")
       last_email = last_email_file.readline().strip("\n")
       last_email_file.close()
-      prompt = "Email [%s]: " % last_email
+      prompt += " [%s]" % last_email
     except IOError, e:
       pass
-  email = raw_input(prompt).strip()
+  email = raw_input(prompt + ": ").strip()
   if email:
     try:
       last_email_file = open(last_email_file_name, "w")
       last_email_file.write(email)
       last_email_file.close()
     except IOError, e:
       pass
   else:
     email = last_email
   return email
@@ skipping 82 matching lines @@
       password: The user's password
 
     Raises:
       ClientLoginError: If there was an error authenticating with ClientLogin.
       HTTPError: If there was some other form of HTTP error.
 
     Returns:
       The authentication token returned by ClientLogin.
     """
     account_type = "GOOGLE"
-    if self.host.endswith(".google.com"):
-      # Needed for use inside Google.
-      account_type = "HOSTED"
     req = self._CreateRequest(
         url="https://www.google.com/accounts/ClientLogin",
         data=urllib.urlencode({
             "Email": email,
             "Passwd": password,
             "service": "ah",
             "source": "rietveld-codereview-upload",
             "accountType": account_type,
         }),
     )
@@ skipping 41 matching lines @@
 
     The authentication process works as follows:
      1) We get a username and password from the user
      2) We use ClientLogin to obtain an AUTH token for the user
         (see http://code.google.com/apis/accounts/AuthForInstalledApps.html).
      3) We pass the auth token to /_ah/login on the server to obtain an
         authentication cookie. If login was successful, it tries to redirect
         us to the URL we provided.
 
     If we attempt to access the upload API without first obtaining an
-    authentication cookie, it returns a 401 response and directs us to
-    authenticate ourselves with ClientLogin.
+    authentication cookie, it returns a 401 response (or a 302) and
+    directs us to authenticate ourselves with ClientLogin.
     """
     for i in range(3):
       credentials = self.auth_function()
       try:
         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":
@@ skipping 60 matching lines @@
         req = self._CreateRequest(url=url, data=payload)
         req.add_header("Content-Type", content_type)
         try:
           f = self.opener.open(req)
           response = f.read()
           f.close()
           return response
         except urllib2.HTTPError, e:
           if tries > 3:
             raise
-          elif e.code == 401:
+          elif e.code == 401 or e.code == 302:
             self._Authenticate()
 ##           elif e.code >= 500 and e.code < 600:
 ##             # Server Error - try again.
 ##             continue
           else:
             raise
     finally:
       socket.setdefaulttimeout(old_timeout)
 
 
@@ skipping 14 matching lines @@
       A urllib2.OpenerDirector object.
     """
     opener = urllib2.OpenerDirector()
     opener.add_handler(urllib2.ProxyHandler())
     opener.add_handler(urllib2.UnknownHandler())
     opener.add_handler(urllib2.HTTPHandler())
     opener.add_handler(urllib2.HTTPDefaultErrorHandler())
     opener.add_handler(urllib2.HTTPSHandler())
     opener.add_handler(urllib2.HTTPErrorProcessor())
     if self.save_cookies:
-      self.cookie_file = os.path.expanduser(
-          os.path.join("~", ".codereview_upload_cookies"))
+      self.cookie_file = os.path.expanduser("~/.codereview_upload_cookies")
       self.cookie_jar = cookielib.MozillaCookieJar(self.cookie_file)
       if os.path.exists(self.cookie_file):
         try:
           self.cookie_jar.load()
           self.authenticated = True
           StatusUpdate("Loaded authentication cookies from %s" %
                        self.cookie_file)
         except (cookielib.LoadError, IOError):
           # Failed to load cookies - just ignore them.
           pass
@@ skipping 22 matching lines @@
                  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'."))
+                       "Defaults to '%default'."))
 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).")
+group.add_option("--private", action="store_true", dest="private",
+                 default=False,
+                 help="Make the issue restricted to reviewers and those CCed")
 # 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("--download_base", action="store_true",
                  dest="download_base", default=False,
                  help="Base files will be downloaded by the server "
                  "(side-by-side diffs may not work on files with CRs).")
 group.add_option("--rev", action="store", dest="revision",
                  metavar="REV", default=None,
                  help="Branch/tree/revision to diff against (used by DVCS).")
 group.add_option("--send_mail", action="store_true",
                  dest="send_mail", default=False,
                  help="Send notification email to reviewers.")
+group.add_option("--vcs", action="store", dest="vcs",
+                 metavar="VCS", default=None,
+                 help=("Version control system (optional, usually upload.py "
+                       "already guesses the right VCS)."))
 
 
 def GetRpcServer(options):
   """Returns an instance of an AbstractRpcServer.
 
   Returns:
     A new AbstractRpcServer, on which RPC calls can be made.
   """
 
   rpc_server_class = HttpRpcServer
 
   def GetUserCredentials():
     """Prompts the user for a username and password."""
     email = options.email
     if email is None:
-      email = GetEmail()
+      email = GetEmail("Email (login for uploading to %s)" % options.server)
     password = getpass.getpass("Password for %s: " % email)
     return (email, password)
 
   # If this is the dev_appserver, use fake authentication.
   host = (options.host or options.server).lower()
   if host == "localhost" or host.startswith("localhost:"):
     email = options.email
     if email is None:
       email = "test@example.com"
       logging.info("Using debug user %s.  Override with --email" % email)
@@ skipping 50 matching lines @@
 
 def GetContentType(filename):
   """Helper to guess the content-type from the filename."""
   return mimetypes.guess_type(filename)[0] or 'application/octet-stream'
 
 
 # Use a shell for subcommands on Windows to get a PATH search.
 use_shell = sys.platform.startswith("win")
 
 def RunShellWithReturnCode(command, print_output=False,
-                           universal_newlines=True):
+                           universal_newlines=True,
+                           env=os.environ):
   """Executes a command and returns the output from stdout and the return code.
 
   Args:
     command: Command to execute.
     print_output: If True, the output is printed to stdout.
                   If False, both stdout and stderr are ignored.
     universal_newlines: Use universal_newlines flag (default: True).
 
   Returns:
     Tuple (output, return code)
   """
   logging.info("Running %s", command)
   p = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                       shell=use_shell, universal_newlines=universal_newlines)
+                       shell=use_shell, universal_newlines=universal_newlines,
+                       env=env)
   if print_output:
     output_array = []
     while True:
       line = p.stdout.readline()
       if not line:
         break
       print line.strip("\n")
       output_array.append(line)
     output = "".join(output_array)
   else:
     output = p.stdout.read()
   p.wait()
   errout = p.stderr.read()
   if print_output and errout:
     print >>sys.stderr, errout
   p.stdout.close()
   p.stderr.close()
   return output, p.returncode
 
 
 def RunShell(command, silent_ok=False, universal_newlines=True,
-             print_output=False):
+             print_output=False, env=os.environ):
   data, retcode = RunShellWithReturnCode(command, print_output,
-                                         universal_newlines)
+                                         universal_newlines, env)
   if retcode:
     ErrorExit("Got error status from %s:\n%s" % (command, data))
   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."""
 
@@ skipping 119 matching lines @@
       if new_content != None:
         UploadFile(filename, file_id, new_content, is_binary, status, False)
 
   def IsImage(self, filename):
     """Returns true if the filename has an image extension."""
     mimetype =  mimetypes.guess_type(filename)[0]
     if not mimetype:
       return False
     return mimetype.startswith("image/")
 
+  def IsBinary(self, filename):
+    """Returns true if the guessed mimetyped isnt't in text group."""
+    mimetype = mimetypes.guess_type(filename)[0]
+    if not mimetype:
+      return False  # e.g. README, "real" binaries usually have an extension
+    # special case for text files which don't start with text/
+    if mimetype in TEXT_MIMETYPES:
+      return False
+    return not mimetype.startswith("text/")
+
 
 class SubversionVCS(VersionControlSystem):
   """Implementation of the VersionControlSystem interface for Subversion."""
 
   def __init__(self, options):
     super(SubversionVCS, self).__init__(options)
     if self.options.revision:
       match = re.match(r"(\d+)(:(\d+))?", self.options.revision)
       if not match:
         ErrorExit("Invalid Subversion revision %s." % self.options.revision)
@@ skipping 173 matching lines @@
     # 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] != "+":
       # We'll need to upload the new content if we're adding a binary file
       # since diff's output won't contain it.
       mimetype = RunShell(["svn", "propget", "svn:mime-type", filename],
                           silent_ok=True)
       base_content = ""
-      is_binary = mimetype and not mimetype.startswith("text/")
+      is_binary = bool(mimetype) and not mimetype.startswith("text/")
       if is_binary and self.IsImage(filename):
         new_content = self.ReadFile(filename)
     elif (status[0] in ("M", "D", "R") or
           (status[0] == "A" and status[3] == "+") or  # Copied file.
           (status[0] == " " and status[1] == "M")):  # Property change.
       args = []
       if self.options.revision:
         url = "%s/%s@%s" % (self.svn_base, filename, self.rev_start)
       else:
         # Don't change filename, it's needed later.
         url = filename
         args += ["-r", "BASE"]
       cmd = ["svn"] + args + ["propget", "svn:mime-type", url]
       mimetype, returncode = RunShellWithReturnCode(cmd)
       if returncode:
         # File does not exist in the requested revision.
         # Reset mimetype, it contains an error message.
         mimetype = ""
       get_base = False
-      is_binary = mimetype and not mimetype.startswith("text/")
+      is_binary = bool(mimetype) and not mimetype.startswith("text/")
       if status[0] == " ":
         # Empty base content just to force an upload.
         base_content = ""
       elif is_binary:
         if self.IsImage(filename):
           get_base = True
           if status[0] == "M":
             if not self.rev_end:
               new_content = self.ReadFile(filename)
             else:
@@ skipping 36 matching lines @@
       StatusUpdate("svn status returned unexpected output: %s" % status)
       sys.exit(1)
     return base_content, new_content, is_binary, status[0:5]
 
 
 class GitVCS(VersionControlSystem):
   """Implementation of the VersionControlSystem interface for Git."""
 
   def __init__(self, options):
     super(GitVCS, self).__init__(options)
-    # Map of filename -> hash of base file.
-    self.base_hashes = {}
+    # Map of filename -> (hash before, hash after) of base file.
+    # Hashes for "no such file" are represented as None.
+    self.hashes = {}
+    # Map of new filename -> old filename for renames.
+    self.renames = {}
 
   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.
+    # the hashes of the files, so we can upload them along with our diff.
+
+    # Special used by git to indicate "no such content".
+    NULL_HASH = "0"*40
+
+    extra_args = extra_args[:]
     if self.options.revision:
       extra_args = [self.options.revision] + extra_args
-    gitdiff = RunShell(["git", "diff", "--no-ext-diff", "--full-index"] +
-                       extra_args)
+
+    # --no-ext-diff is broken in some versions of Git, so try to work around
+    # this by overriding the environment (but there is still a problem if the
+    # git config key "diff.external" is used).
+    env = os.environ.copy()
+    if 'GIT_EXTERNAL_DIFF' in env: del env['GIT_EXTERNAL_DIFF']
+    gitdiff = RunShell(["git", "diff", "--no-ext-diff", "--full-index", "-M"]
+                       + extra_args, env=env)
     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
-        filename = match.group(1)
+        # Intentionally use the "after" filename so we can show renames.
+        filename = match.group(2)
         svndiff.append("Index: %s\n" % filename)
+        if match.group(1) != match.group(2):
+          self.renames[match.group(2)] = match.group(1)
       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)
+        match = re.match(r"index (\w+)\.\.(\w+)", line)
         if match:
-          self.base_hashes[filename] = match.group(1)
+          before, after = (match.group(1), match.group(2))
+          if before == NULL_HASH:
+            before = None
+          if after == NULL_HASH:
+            after = None
+          self.hashes[filename] = (before, after)
       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", "--exclude-standard", "--others"],
                       silent_ok=True)
     return status.splitlines()
 
+  def GetFileContent(self, file_hash, is_binary):
+    """Returns the content of a file identified by its git hash."""
+    data, retcode = RunShellWithReturnCode(["git", "show", file_hash],
+                                            universal_newlines=not is_binary)
+    if retcode:
+      ErrorExit("Got error status from 'git show %s'" % file_hash)
+    return data
+
   def GetBaseFile(self, filename):
-    hash = self.base_hashes[filename]
+    hash_before, hash_after = self.hashes.get(filename, (None,None))
     base_content = None
     new_content = None
-    is_binary = False
-    if hash == "0" * 40:  # All-zero hash indicates no base file.
+    is_binary = self.IsBinary(filename)
+    status = None
+
+    if filename in self.renames:
+      status = "A +"  # Match svn attribute name for renames.
+      if filename not in self.hashes:
+        # If a rename doesn't change the content, we never get a hash.
+        base_content = RunShell(["git", "show", filename])
+    elif not hash_before:
       status = "A"
       base_content = ""
+    elif not hash_after:
+      status = "D"
     else:
       status = "M"
-      base_content = RunShell(["git", "show", hash])
+
+    is_image = self.IsImage(filename)
+
+    # Grab the before/after content if we need it.
+    # We should include file contents if it's text or it's an image.
+    if not is_binary or is_image:
+      # Grab the base content if we don't have it already.
+      if base_content is None and hash_before:
+        base_content = self.GetFileContent(hash_before, is_binary)
+      # Only include the "after" file if it's an image; otherwise it
+      # it is reconstructed from the diff.
+      if is_image and hash_after:
+        new_content = self.GetFileContent(hash_after, is_binary)
+
     return (base_content, new_content, is_binary, status)
 
 
 class MercurialVCS(VersionControlSystem):
   """Implementation of the VersionControlSystem interface for Mercurial."""
 
   def __init__(self, options, repo_dir):
     super(MercurialVCS, self).__init__(options)
     # Absolute path to repository (we can be in a subdir)
     self.repo_dir = os.path.normpath(repo_dir)
     # Compute the subdir
     cwd = os.path.normpath(os.getcwd())
     assert cwd.startswith(self.repo_dir)
     self.subdir = cwd[len(self.repo_dir):].lstrip(r"\/")
     if self.options.revision:
       self.base_rev = self.options.revision
     else:
       self.base_rev = RunShell(["hg", "parent", "-q"]).split(':')[1].strip()
 
   def _GetRelPath(self, filename):
     """Get relative path of a file according to the current directory,
     given its logical path in the repo."""
-    assert filename.startswith(self.subdir), filename
+    assert filename.startswith(self.subdir), (filename, self.subdir)
     return filename[len(self.subdir):].lstrip(r"\/")
 
   def GenerateDiff(self, extra_args):
     # If no file specified, restrict to the current subdir
     extra_args = extra_args or ["."]
     cmd = ["hg", "diff", "--git", "-r", self.base_rev] + extra_args
     data = RunShell(cmd, silent_ok=True)
     svndiff = []
     filecount = 0
     for line in data.splitlines():
@@ skipping 42 matching lines @@
     # the working copy
     if out[0].startswith('%s: ' % relpath):
       out = out[1:]
     if len(out) > 1:
       # Moved/copied => considered as modified, use old filename to
       # retrieve base contents
       oldrelpath = out[1].strip()
       status = "M"
     else:
       status, _ = out[0].split(' ', 1)
+    if ":" in self.base_rev:
+      base_rev = self.base_rev.split(":", 1)[0]
+    else:
+      base_rev = self.base_rev
     if status != "A":
-      base_content = RunShell(["hg", "cat", "-r", self.base_rev, oldrelpath],
+      base_content = RunShell(["hg", "cat", "-r", base_rev, oldrelpath],
         silent_ok=True)
       is_binary = "\0" in base_content  # Mercurial's heuristic
     if status != "R":
       new_content = open(relpath, "rb").read()
       is_binary = is_binary or "\0" in new_content
     if is_binary and base_content:
       # Fetch again without converting newlines
-      base_content = RunShell(["hg", "cat", "-r", self.base_rev, oldrelpath],
+      base_content = RunShell(["hg", "cat", "-r", base_rev, oldrelpath],
         silent_ok=True, universal_newlines=False)
     if not is_binary or not self.IsImage(relpath):
       new_content = None
     return base_content, new_content, is_binary, status
 
 
 # NOTE: The SplitPatch function is duplicated in engine.py, keep them in sync.
 def SplitPatch(data):
   """Splits a patch into separate pieces for each file.
 
@@ skipping 55 matching lines @@
     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(1)
     rv.append([lines[1], patch[0]])
   return rv
 
 
-def GuessVCS(options):
+def GuessVCSName():
   """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.
+  we're using, and returns an string indicating which VCS is detected.
 
   Returns:
-    A VersionControlSystem instance. Exits if the VCS can't be guessed.
+    A pair (vcs, output).  vcs is a string indicating which VCS was detected
+    and is one of VCS_GIT, VCS_MERCURIAL, VCS_SUBVERSION, or VCS_UNKNOWN.
+    output is a string containing any interesting output from the vcs
+    detection routine, or None if there is nothing interesting.
   """
   # Mercurial has a command to get the base directory of a repository
   # Try running it, but don't die if we don't have hg installed.
   # NOTE: we try Mercurial first as it can sit on top of an SVN working copy.
   try:
     out, returncode = RunShellWithReturnCode(["hg", "root"])
     if returncode == 0:
-      return MercurialVCS(options, out.strip())
+      return (VCS_MERCURIAL, out.strip())
   except OSError, (errno, message):
     if errno != 2:  # ENOENT -- they don't have hg installed.
       raise
 
   # Subversion has a .svn in all working directories.
   if os.path.isdir('.svn'):
     logging.info("Guessed VCS = Subversion")
-    return SubversionVCS(options)
+    return (VCS_SUBVERSION, None)
 
   # Git has a command to test if you're in a git tree.
   # Try running it, but don't die if we don't have git installed.
   try:
     out, returncode = RunShellWithReturnCode(["git", "rev-parse",
                                               "--is-inside-work-tree"])
     if returncode == 0:
-      return GitVCS(options)
+      return (VCS_GIT, None)
   except OSError, (errno, message):
     if errno != 2:  # ENOENT -- they don't have git installed.
       raise
 
+  return (VCS_UNKNOWN, None)
+
+
+def GuessVCS(options):
+  """Helper to guess the version control system.
+
+  This verifies any user-specified VersionControlSystem (by command line
+  or environment variable).  If the user didn't specify one, 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.
+
+  Returns:
+    A VersionControlSystem instance. Exits if the VCS can't be guessed.
+  """
+  vcs = options.vcs
+  if not vcs:
+    vcs = os.environ.get("CODEREVIEW_VCS")
+  if vcs:
+    v = VCS_ABBREVIATIONS.get(vcs.lower())
+    if v is None:
+      ErrorExit("Unknown version control system %r specified." % vcs)
+    (vcs, extra_output) = (v, None)
+  else:
+    (vcs, extra_output) = GuessVCSName()
+
+  if vcs == VCS_MERCURIAL:
+    if extra_output is None:
+      extra_output = RunShell(["hg", "root"]).strip()
+    return MercurialVCS(options, extra_output)
+  elif vcs == VCS_SUBVERSION:
+    return SubversionVCS(options)
+  elif vcs == VCS_GIT:
+    return GitVCS(options)
+
   ErrorExit(("Could not guess version control system. "
              "Are you in a working copy directory?"))
 
 
+def CheckReviewer(reviewer):
+  """Validate a reviewer -- either a nickname or an email addres.
+
+  Args:
+    reviewer: A nickname or an email address.
+
+  Calls ErrorExit() if it is an invalid email address.
+  """
+  if "@" not in reviewer:
+    return  # Assume nickname
+  parts = reviewer.split("@")
+  if len(parts) > 2:
+    ErrorExit("Invalid email address: %r" % reviewer)
+  assert len(parts) == 2
+  if "." not in parts[1]:
+    ErrorExit("Invalid email address: %r" % reviewer)
+
+
 def RealMain(argv, data=None):
+  """The real main function.
+
+  Args:
+    argv: Command line arguments.
+    data: Diff contents. If None (default) the diff is generated by
+      the VersionControlSystem implementation returned by GuessVCS().
+
+  Returns:
+    A 2-tuple (issue id, patchset id).
+    The patchset id is None if the base files are not uploaded by this
+    script (applies only to SVN checkouts).
+  """
   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)
@@ skipping 24 matching lines @@
   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 "@" in reviewer and not reviewer.split("@")[1].count(".") == 1:
-        ErrorExit("Invalid email address: %s" % reviewer)
+      CheckReviewer(reviewer)
     form_fields.append(("reviewers", options.reviewers))
   if options.cc:
     for cc in options.cc.split(','):
-      if "@" in cc and not cc.split("@")[1].count(".") == 1:
-        ErrorExit("Invalid email address: %s" % cc)
+      CheckReviewer(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))
   # Send a hash of all the base file so the server can determine if a copy
   # already exists in an earlier patchset.
   base_hashes = ""
   for file, info in files.iteritems():
     if not info[0] is None:
       checksum = md5(info[0]).hexdigest()
       if base_hashes:
         base_hashes += "|"
       base_hashes += checksum + ":" + file
   form_fields.append(("base_hashes", base_hashes))
+  if options.private:
+    if options.issue:
+      print "Warning: Private flag ignored when updating an existing issue."
+    else:
+      form_fields.append(("private", "1"))
   # 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 options.download_base:
     form_fields.append(("send_mail", "1"))
   if not options.download_base:
     form_fields.append(("content_upload", "1"))
   if len(data) > MAX_UPLOAD_SIZE:
     print "Patch is large, so uploading file patches separately."
     uploaded_diff_file = []
     form_fields.append(("separate_patches", "1"))
   else:
     uploaded_diff_file = [("data", "data.diff", data)]
   ctype, body = EncodeMultipartFormData(form_fields, uploaded_diff_file)
   response_body = rpc_server.Send("/upload", body, content_type=ctype)
+  patchset = None
   if not options.download_base or not uploaded_diff_file:
     lines = response_body.splitlines()
     if len(lines) >= 2:
       msg = lines[0]
       patchset = lines[1].strip()
       patches = [x.split(" ", 1) for x in lines[2:]]
-      for patch_pair in patches:
-        # On Windows if a file has property changes its filename uses '\'
-        # instead of '/'.  Perhaps this change should be made (also) on the
-        # server when it is decoding the patch file sent by the client, but
-        # we do it here as well to be safe.
-        patch_pair[1] = patch_pair[1].replace('\\', '/')
     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:]
 
@@ skipping 13 matching lines @@
   try:
     RealMain(sys.argv)
   except KeyboardInterrupt:
     print
     StatusUpdate("Interrupted.")
     sys.exit(1)
 
 
 if __name__ == "__main__":
   main()