Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(252)

Unified Diff: third_party/upload.py

Issue 7709021: Update upload.py from ed59464f8468. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/tools/depot_tools
Patch Set: Created 9 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: third_party/upload.py
diff --git a/third_party/upload.py b/third_party/upload.py
index cdeae6bf26e1640da4bc2a9f7052c52833d3f0a2..53c2c93656f67abf3e37f23d37920ba92e2893d4 100755
--- a/third_party/upload.py
+++ b/third_party/upload.py
@@ -94,12 +94,6 @@ VCS_PERFORCE = "Perforce"
VCS_CVS = "CVS"
VCS_UNKNOWN = "Unknown"
-# 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/json',
- 'application/x-javascript', 'application/xml',
- 'application/x-freemind', 'application/x-sh']
-
VCS_ABBREVIATIONS = {
VCS_MERCURIAL.lower(): VCS_MERCURIAL,
"hg": VCS_MERCURIAL,
@@ -548,6 +542,12 @@ group.add_option("--rev", action="store", dest="revision",
group.add_option("--send_mail", action="store_true",
dest="send_mail", default=False,
help="Send notification email to reviewers.")
+group.add_option("-p", "--send_patch", action="store_true",
+ dest="send_patch", default=False,
+ help="Send notification email to reviewers, with a diff of "
+ "the changes included as an attachment instead of "
+ "inline. Also prepends 'PATCH:' to the email subject. "
+ "(implies --send_mail)")
group.add_option("--vcs", action="store", dest="vcs",
metavar="VCS", default=None,
help=("Version control system (optional, usually upload.py "
@@ -876,15 +876,11 @@ class VersionControlSystem(object):
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/")
+ def IsBinaryData(self, data):
+ """Returns true if data contains a null byte."""
+ # Derived from how Mercurial's heuristic, see
+ # http://selenic.com/hg/file/848a6658069e/mercurial/util.py#l229
+ return bool(data and "\0" in data)
class SubversionVCS(VersionControlSystem):
@@ -941,6 +937,12 @@ class SubversionVCS(VersionControlSystem):
ErrorExit("Can't find URL in output from svn info")
return None
+ def _EscapeFilename(self, filename):
+ """Escapes filename for SVN commands."""
+ if "@" in filename and not filename.endswith("@"):
+ filename = "%s@" % filename
+ return filename
+
def GenerateDiff(self, args):
cmd = ["svn", "diff"]
if self.options.revision:
@@ -1008,7 +1010,8 @@ class SubversionVCS(VersionControlSystem):
def GetStatus(self, filename):
"""Returns the status of a file."""
if not self.options.revision:
- status = RunShell(["svn", "status", "--ignore-externals", filename])
+ status = RunShell(["svn", "status", "--ignore-externals",
+ self._EscapeFilename(filename)])
if not status:
ErrorExit("svn status returned no output for %s" % filename)
status_lines = status.splitlines()
@@ -1027,7 +1030,8 @@ class SubversionVCS(VersionControlSystem):
else:
dirname, relfilename = os.path.split(filename)
if dirname not in self.svnls_cache:
- cmd = ["svn", "list", "-r", self.rev_start, dirname or "."]
+ cmd = ["svn", "list", "-r", self.rev_start,
+ self._EscapeFilename(dirname) or "."]
out, err, returncode = RunShellWithReturnCodeAndStderr(cmd)
if returncode:
# Directory might not yet exist at start revison
@@ -1041,7 +1045,7 @@ class SubversionVCS(VersionControlSystem):
args = ["svn", "list"]
if self.rev_end:
args += ["-r", self.rev_end]
- cmd = args + [dirname or "."]
+ cmd = args + [self._EscapeFilename(dirname) or "."]
out, returncode = RunShellWithReturnCode(cmd)
if returncode:
ErrorExit("Failed to run command %s" % cmd)
@@ -1067,8 +1071,8 @@ class SubversionVCS(VersionControlSystem):
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)
+ mimetype = RunShell(["svn", "propget", "svn:mime-type",
+ self._EscapeFilename(filename)], silent_ok=True)
base_content = ""
is_binary = bool(mimetype) and not mimetype.startswith("text/")
if is_binary and self.IsImage(filename):
@@ -1078,6 +1082,7 @@ class SubversionVCS(VersionControlSystem):
(status[0] == " " and status[1] == "M")): # Property change.
args = []
if self.options.revision:
+ # filename must not be escaped. We already add an ampersand here.
url = "%s/%s@%s" % (self.svn_base, filename, self.rev_start)
else:
# Don't change filename, it's needed later.
@@ -1092,9 +1097,12 @@ class SubversionVCS(VersionControlSystem):
else:
mimetype = mimetype.strip()
get_base = False
+ # this test for binary is exactly the test prescribed by the
+ # official SVN docs at
+ # http://subversion.apache.org/faq.html#binary-files
is_binary = (bool(mimetype) and
not mimetype.startswith("text/") and
- not mimetype in TEXT_MIMETYPES)
+ mimetype not in ("image/x-xbitmap", "image/x-xpixmap"))
if status[0] == " ":
# Empty base content just to force an upload.
base_content = ""
@@ -1127,7 +1135,8 @@ class SubversionVCS(VersionControlSystem):
silent_ok=True)
else:
base_content, ret_code = RunShellWithReturnCode(
- ["svn", "cat", filename], universal_newlines=universal_newlines)
+ ["svn", "cat", self._EscapeFilename(filename)],
+ universal_newlines=universal_newlines)
if ret_code and status[0] == "R":
# It's a replaced file without local history (see issue208).
# The base file needs to be fetched from the server.
@@ -1241,7 +1250,9 @@ class GitVCS(VersionControlSystem):
# review when a file is renamed. So first get the diff of all deleted files,
# then the diff of everything except deleted files with rename and copy
# support enabled.
- cmd = ["git", "diff", "--no-ext-diff", "--full-index"]
+ cmd = [
+ "git", "diff", "--no-ext-diff", "--full-index", "--ignore-submodules"
+ ]
diff = RunShell(cmd + ["--diff-filter=D"] + extra_args, env=env,
silent_ok=True)
diff += RunShell(cmd + ["-C", "--diff-filter=ACMRT"] + extra_args, env=env,
@@ -1267,7 +1278,6 @@ class GitVCS(VersionControlSystem):
hash_before, hash_after = self.hashes.get(filename, (None,None))
base_content = None
new_content = None
- is_binary = self.IsBinary(filename)
status = None
if filename in self.renames:
@@ -1283,6 +1293,7 @@ class GitVCS(VersionControlSystem):
else:
status = "M"
+ is_binary = self.IsBinaryData(base_content)
is_image = self.IsImage(filename)
# Grab the before/after content if we need it.
@@ -1314,7 +1325,6 @@ class CVSVCS(VersionControlSystem):
def GetBaseFile(self, filename):
base_content = None
new_content = None
- is_binary = False
status = "A"
output, retcode = RunShellWithReturnCode(["cvs", "status", filename])
@@ -1334,7 +1344,7 @@ class CVSVCS(VersionControlSystem):
status = "D"
base_content = self.GetOriginalContent_(filename)
- return (base_content, new_content, is_binary, status)
+ return (base_content, new_content, self.IsBinaryData(base_content), status)
def GenerateDiff(self, extra_args):
cmd = ["cvs", "diff", "-u", "-N"]
@@ -1451,10 +1461,10 @@ class MercurialVCS(VersionControlSystem):
if status != "A":
base_content = RunShell(["hg", "cat", "-r", base_rev, oldrelpath],
silent_ok=True)
- is_binary = "\0" in base_content # Mercurial's heuristic
+ is_binary = self.IsBinaryData(base_content)
if status != "R":
new_content = open(relpath, "rb").read()
- is_binary = is_binary or "\0" in new_content
+ is_binary = is_binary or self.IsBinaryData(new_content)
if is_binary and base_content:
# Fetch again without converting newlines
base_content = RunShell(["hg", "cat", "-r", base_rev, oldrelpath],
@@ -1468,7 +1478,7 @@ class PerforceVCS(VersionControlSystem):
"""Implementation of the VersionControlSystem interface for Perforce."""
def __init__(self, options):
-
+
def ConfirmLogin():
# Make sure we have a valid perforce session
while True:
@@ -1480,21 +1490,21 @@ class PerforceVCS(VersionControlSystem):
break
print "Enter perforce password: "
self.RunPerforceCommandWithReturnCode(["login"])
-
+
super(PerforceVCS, self).__init__(options)
-
+
self.p4_changelist = options.p4_changelist
if not self.p4_changelist:
ErrorExit("A changelist id is required")
if (options.revision):
ErrorExit("--rev is not supported for perforce")
-
+
self.p4_port = options.p4_port
self.p4_client = options.p4_client
self.p4_user = options.p4_user
-
+
ConfirmLogin()
-
+
if not options.message:
description = self.RunPerforceCommand(["describe", self.p4_changelist],
marshal_output=True)
@@ -1504,7 +1514,7 @@ class PerforceVCS(VersionControlSystem):
lines = raw_message.splitlines()
if len(lines):
options.message = lines[0]
-
+
def RunPerforceCommandWithReturnCode(self, extra_args, marshal_output=False,
universal_newlines=True):
args = ["p4"]
@@ -1518,13 +1528,13 @@ class PerforceVCS(VersionControlSystem):
if self.p4_user:
args.extend(["-u", self.p4_user])
args.extend(extra_args)
-
+
data, retcode = RunShellWithReturnCode(
args, print_output=False, universal_newlines=universal_newlines)
if marshal_output and data:
data = marshal.loads(data)
return data, retcode
-
+
def RunPerforceCommand(self, extra_args, marshal_output=False,
universal_newlines=True):
# This might be a good place to cache call results, since things like
@@ -1538,7 +1548,7 @@ class PerforceVCS(VersionControlSystem):
def GetFileProperties(self, property_key_prefix = "", command = "describe"):
description = self.RunPerforceCommand(["describe", self.p4_changelist],
marshal_output=True)
-
+
changed_files = {}
file_index = 0
# Try depotFile0, depotFile1, ... until we don't find a match
@@ -1567,9 +1577,6 @@ class PerforceVCS(VersionControlSystem):
def IsPendingBinary(self, filename):
return self.IsBinaryHelper(filename, "describe")
- def IsBinary(self, filename):
- ErrorExit("IsBinary is not safe: call IsBaseBinary or IsPendingBinary")
-
def IsBinaryHelper(self, filename, command):
file_types = self.GetFileProperties("type", command)
if not filename in file_types:
@@ -1593,52 +1600,52 @@ class PerforceVCS(VersionControlSystem):
"branch", # p4 integrate (to a new file), similar to hg "add"
"add", # p4 integrate (to a new file), after modifying the new file
]
-
+
# We only see a different base for "add" if this is a downgraded branch
- # after a file was branched (integrated), then edited.
+ # after a file was branched (integrated), then edited.
if self.GetAction(filename) in actionsWithDifferentBases:
# -Or shows information about pending integrations/moves
fstat_result = self.RunPerforceCommand(["fstat", "-Or", filename],
marshal_output=True)
-
+
baseFileKey = "resolveFromFile0" # I think it's safe to use only file0
if baseFileKey in fstat_result:
return fstat_result[baseFileKey]
-
+
return filename
def GetBaseRevision(self, filename):
base_filename = self.GetBaseFilename(filename)
-
+
have_result = self.RunPerforceCommand(["have", base_filename],
marshal_output=True)
if "haveRev" in have_result:
return have_result["haveRev"]
-
+
def GetLocalFilename(self, filename):
where = self.RunPerforceCommand(["where", filename], marshal_output=True)
if "path" in where:
return where["path"]
- def GenerateDiff(self, args):
+ def GenerateDiff(self, args):
class DiffData:
def __init__(self, perforceVCS, filename, action):
self.perforceVCS = perforceVCS
self.filename = filename
self.action = action
self.base_filename = perforceVCS.GetBaseFilename(filename)
-
+
self.file_body = None
self.base_rev = None
self.prefix = None
self.working_copy = True
self.change_summary = None
-
+
def GenerateDiffHeader(diffData):
header = []
header.append("Index: %s" % diffData.filename)
header.append("=" * 67)
-
+
if diffData.base_filename != diffData.filename:
if diffData.action.startswith("move"):
verb = "rename"
@@ -1646,7 +1653,7 @@ class PerforceVCS(VersionControlSystem):
verb = "copy"
header.append("%s from %s" % (verb, diffData.base_filename))
header.append("%s to %s" % (verb, diffData.filename))
-
+
suffix = "\t(revision %s)" % diffData.base_rev
header.append("--- " + diffData.base_filename + suffix)
if diffData.working_copy:
@@ -1655,14 +1662,14 @@ class PerforceVCS(VersionControlSystem):
if diffData.change_summary:
header.append(diffData.change_summary)
return header
-
+
def GenerateMergeDiff(diffData, args):
# -du generates a unified diff, which is nearly svn format
diffData.file_body = self.RunPerforceCommand(
["diff", "-du", diffData.filename] + args)
diffData.base_rev = self.GetBaseRevision(diffData.filename)
diffData.prefix = ""
-
+
# We have to replace p4's file status output (the lines starting
# with +++ or ---) to match svn's diff format
lines = diffData.file_body.splitlines()
@@ -1691,13 +1698,13 @@ class PerforceVCS(VersionControlSystem):
diffData.change_summary += " @@"
diffData.prefix = "+"
return diffData
-
+
def GenerateDeleteDiff(diffData):
diffData.base_rev = self.GetBaseRevision(diffData.filename)
is_base_binary = self.IsBaseBinary(diffData.filename)
# For deletes, base_filename == filename
- diffData.file_body = self.GetFileContent(diffData.base_filename,
- None,
+ diffData.file_body = self.GetFileContent(diffData.base_filename,
+ None,
is_base_binary)
# Replicate svn's list of changed lines
line_count = len(diffData.file_body.splitlines())
@@ -1707,16 +1714,16 @@ class PerforceVCS(VersionControlSystem):
diffData.change_summary += " +0,0 @@"
diffData.prefix = "-"
return diffData
-
+
changed_files = self.GetChangedFiles()
-
+
svndiff = []
filecount = 0
for (filename, action) in changed_files.items():
svn_status = self.PerforceActionToSvnStatus(action)
if svn_status == "SKIP":
continue
-
+
diffData = DiffData(self, filename, action)
# Is it possible to diff a branched file? Stackoverflow says no:
# http://stackoverflow.com/questions/1771314/in-perforce-command-line-how-to-diff-a-file-reopened-for-add
@@ -1729,9 +1736,9 @@ class PerforceVCS(VersionControlSystem):
else:
ErrorExit("Unknown file action %s (svn action %s)." % \
(action, svn_status))
-
+
svndiff += GenerateDiffHeader(diffData)
-
+
for line in diffData.file_body.splitlines():
svndiff.append(diffData.prefix + line)
filecount += 1
@@ -1757,16 +1764,16 @@ class PerforceVCS(VersionControlSystem):
changed_files = self.GetChangedFiles()
if not filename in changed_files:
ErrorExit("Trying to get base version of unknown file %s." % filename)
-
+
return changed_files[filename]
def GetBaseFile(self, filename):
base_filename = self.GetBaseFilename(filename)
base_content = ""
new_content = None
-
+
status = self.PerforceActionToSvnStatus(self.GetAction(filename))
-
+
if status != "A":
revision = self.GetBaseRevision(base_filename)
if not revision:
@@ -1775,13 +1782,13 @@ class PerforceVCS(VersionControlSystem):
base_content = self.GetFileContent(base_filename,
revision,
is_base_binary)
-
+
is_binary = self.IsPendingBinary(filename)
if status != "D" and status != "SKIP":
relpath = self.GetLocalFilename(filename)
if is_binary and self.IsImage(relpath):
new_content = open(relpath, "rb").read()
-
+
return base_content, new_content, is_binary, status
# NOTE: The SplitPatch function is duplicated in engine.py, keep them in sync.
@@ -1871,10 +1878,10 @@ def GuessVCSName(options):
for attribute, value in options.__dict__.iteritems():
if attribute.startswith("p4") and value != None:
return (VCS_PERFORCE, None)
-
+
def RunDetectCommand(vcs_type, command):
"""Helper to detect VCS by executing command.
-
+
Returns:
A pair (vcs, output) or None. Throws exception on error.
"""
@@ -1885,7 +1892,7 @@ def GuessVCSName(options):
except OSError, (errcode, message):
if errcode != errno.ENOENT: # command not found code
raise
-
+
# 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.
@@ -2187,6 +2194,8 @@ def RealMain(argv, data=None):
print "Warning: Private flag ignored when updating an existing issue."
else:
form_fields.append(("private", "1"))
+ if options.send_patch:
+ options.send_mail = True
# 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:
@@ -2226,7 +2235,10 @@ def RealMain(argv, data=None):
if not options.download_base:
vcs.UploadBaseFiles(issue, rpc_server, patches, patchset, options, files)
if options.send_mail:
- rpc_server.Send("/" + issue + "/mail", payload="")
+ payload = ""
+ if options.send_patch:
+ payload=urllib.urlencode({"attach_patch": "yes"})
+ rpc_server.Send("/" + issue + "/mail", payload=payload)
return issue, patchset
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698