Update upload.py at b7cdf3e63c5e.

third_party/upload.py

Index: third_party/upload.py
diff --git a/third_party/upload.py b/third_party/upload.py
index 744f3d6794d9c1010b1b3cc0b853df3d91aa5116..31542663af90b4a3e344fac6512d6b29195d708b 100755
--- a/third_party/upload.py
+++ b/third_party/upload.py
@@ -89,6 +89,7 @@ DEFAULT_REVIEW_SERVER = "codereview.appspot.com"
 # 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"
@@ -97,16 +98,30 @@ VCS_PERFORCE = "Perforce"
 VCS_CVS = "CVS"
 VCS_UNKNOWN = "Unknown"
 
-VCS_ABBREVIATIONS = {
-  VCS_MERCURIAL.lower(): VCS_MERCURIAL,
-  "hg": VCS_MERCURIAL,
-  VCS_SUBVERSION.lower(): VCS_SUBVERSION,
-  "svn": VCS_SUBVERSION,
-  VCS_PERFORCE.lower(): VCS_PERFORCE,
-  "p4": VCS_PERFORCE,
-  VCS_GIT.lower(): VCS_GIT,
-  VCS_CVS.lower(): VCS_CVS,
-}
+VCS = [
+{
+    'name': VCS_MERCURIAL,
+    'aliases': ['hg', 'mercurial'],
+}, {
+    'name': VCS_SUBVERSION,
+    'aliases': ['svn', 'subversion'],
+}, {
+    'name': VCS_PERFORCE,
+    'aliases': ['p4', 'perforce'],
+}, {
+    'name': VCS_GIT,
+    'aliases': ['git'],
+}, {
+    'name': VCS_CVS,
+    'aliases': ['cvs'],
+}]
+
+VCS_SHORT_NAMES = []    # hg, svn, ...
+VCS_ABBREVIATIONS = {}  # alias: name, ...
+for vcs in VCS:
+  VCS_SHORT_NAMES.append(min(vcs['aliases'], key=len))
+  VCS_ABBREVIATIONS.update((alias, vcs['name']) for alias in vcs['aliases'])
+
 
 # OAuth 2.0-Related Constants
 LOCALHOST_IP = '127.0.0.1'
@@ -646,8 +661,8 @@ group.add_option("-p", "--send_patch", action="store_true",
                       "attachment, and prepend email subject with 'PATCH:'.")
 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)."))
+                 help=("Explicitly specify version control system (%s)"
+                       % ", ".join(VCS_SHORT_NAMES)))
 group.add_option("--emulate_svn_auto_props", action="store_true",
                  dest="emulate_svn_auto_props", default=False,
                  help=("Emulate Subversion's auto properties feature."))
@@ -1597,10 +1612,10 @@ class GitVCS(VersionControlSystem):
                       silent_ok=True)
     return status.splitlines()
 
-  def GetFileContent(self, file_hash, is_binary):
+  def GetFileContent(self, file_hash):
     """Returns the content of a file identified by its git hash."""
     data, retcode = RunShellWithReturnCode(["git", "show", file_hash],
-                                            universal_newlines=not is_binary)
+                                            universal_newlines=False)

[iannucci, 2013/09/19 17:25:20]

isn't universal_newlines=False the default?

[chrisphan, 2013/09/19 17:45:09]

universal_newlines is False on default for popen, but default to True in
RunShellWithReturnCode

     if retcode:
       ErrorExit("Got error status from 'git show %s'" % file_hash)
     return data
@@ -1625,18 +1640,22 @@ class GitVCS(VersionControlSystem):
     else:
       status = "M"
 
-    is_image = self.IsImage(filename)
-    is_binary = self.IsBinaryData(base_content) or is_image
-
     # Grab the before/after content if we need it.
     # 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)
+      base_content = self.GetFileContent(hash_before)
+
+    is_binary = self.IsImage(filename)
+    if base_content:
+      is_binary = is_binary or self.IsBinaryData(base_content)
+
     # 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)
-
+    if hash_after:
+      new_content = self.GetFileContent(hash_after)
+      is_binary = is_binary or self.IsBinaryData(new_content)
+      if not is_binary:
+        new_content = None
     return (base_content, new_content, is_binary, status)