Skip to content

C
Chromium_Depot_tools
Commit 2b40b89a authored by maruel@chromium.org's avatar maruel@chromium.org
Browse files
Options

Update upload.py @827fa087f74d, which includes support for svn 1.7 

It also removes the need of manually creating a subject argument.

Other related changes in this CL:
- Reenable the prompt for patchset title in gcl. I'm not sure why it was disabled.
- Remove git cl upload --desc_from_logs flag. --force is already meaningful.

R=cmp@chromium.org
BUG=
TEST=

Review URL: http://codereview.chromium.org/9193023

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@119066 0039d316-1c4b-4281-b951-d872f2087c98
parent d0adab57
Hide whitespace changes
Inline Side-by-side
Showing with 108 additions and 131 deletions
gcl.py
View file @ 2b40b89a
......@@ -285,7 +285,6 @@ class ChangeInfo(object):
self.issue = int(issue)
self.patchset = int(patchset)
self._description = None
self._subject = None
self._reviewers = None
self._set_description(description)
if files is None:
......@@ -310,19 +309,11 @@ class ChangeInfo(object):
parsed_lines = []
reviewers_re = re.compile(REVIEWERS_REGEX)
reviewers = ''
subject = ''
for l in description.splitlines():
if not subject:
subject = l
matched_reviewers = reviewers_re.match(l)
if matched_reviewers:
reviewers = matched_reviewers.group(1).split(',')
parsed_lines.append(l)
if len(subject) > 100:
subject = subject[:97] + '...'
self._subject = subject
self._reviewers = reviewers
self._description = '\n'.join(parsed_lines)
......@@ -332,10 +323,6 @@ class ChangeInfo(object):
def reviewers(self):
return self._reviewers
@property
def subject(self):
return self._subject
def NeedsUpload(self):
return self.needs_upload
......@@ -859,18 +846,11 @@ def CMDupload(change_info, args):
desc_file = None
try:
if change_info.issue: # Uploading a new patchset.
found_message = False
for arg in args:
if arg.startswith("--message") or arg.startswith("-m"):
found_message = True
break
if not found_message:
upload_arg.append("--message=''")
if change_info.issue:
# Uploading a new patchset.
upload_arg.append("--issue=%d" % change_info.issue)
else: # First time we upload.
else:
# First time we upload.
handle, desc_file = tempfile.mkstemp(text=True)
os.write(handle, change_info.description)
os.close(handle)
......@@ -888,9 +868,7 @@ def CMDupload(change_info, args):
cc_list = ','.join(filter(None, [cc_list] + watchers))
if cc_list:
upload_arg.append("--cc=" + cc_list)
upload_arg.append("--description_file=%s" % desc_file)
if change_info.subject:
upload_arg.append("--message=" + change_info.subject)
upload_arg.append("--file=%s" % desc_file)
if GetCodeReviewSetting("PRIVATE") == "True":
upload_arg.append("--private")
......
git_cl.py
View file @ 2b40b89a
......@@ -639,47 +639,42 @@ def GetCodereviewSettingsInteractively():
class ChangeDescription(object):
"""Contains a parsed form of the change description."""
def __init__(self, subject, log_desc, reviewers):
self.subject = subject
def __init__(self, log_desc, reviewers):
self.log_desc = log_desc
self.reviewers = reviewers
self.description = self.log_desc
def Update(self):
initial_text = """# Enter a description of the change.
def Prompt(self):
content = """# Enter a description of the change.
# This will displayed on the codereview site.
# The first line will also be used as the subject of the review.
"""
initial_text += self.description
content += self.description
if ('\nR=' not in self.description and
'\nTBR=' not in self.description and
self.reviewers):
initial_text += '\nR=' + self.reviewers
content += '\nR=' + self.reviewers
if '\nBUG=' not in self.description:
initial_text += '\nBUG='
content += '\nBUG='
if '\nTEST=' not in self.description:
initial_text += '\nTEST='
initial_text = initial_text.rstrip('\n') + '\n'
content = gclient_utils.RunEditor(initial_text, True)
content += '\nTEST='
content = content.rstrip('\n') + '\n'
content = gclient_utils.RunEditor(content, True)
if not content:
DieWithError('Running editor failed')
content = re.compile(r'^#.*$', re.MULTILINE).sub('', content).strip()
if not content:
DieWithError('No CL description, aborting')
self._ParseDescription(content)
self.description = content.strip('\n') + '\n'
def _ParseDescription(self, description):
"""Updates the list of reviewers and subject from the description."""
if not description:
self.description = description
return
self.description = description.strip('\n') + '\n'
self.subject = description.split('\n', 1)[0]
def ParseDescription(self):
"""Updates the list of reviewers."""
# Retrieves all reviewer lines
regexp = re.compile(r'^\s*(TBR|R)=(.+)$', re.MULTILINE)
self.reviewers = ','.join(
reviewers = ','.join(
i.group(2).strip() for i in regexp.finditer(self.description))
if reviewers:
self.reviewers = reviewers
def IsEmpty(self):
return not self.description
......@@ -925,9 +920,6 @@ def CMDupload(parser, args):
upload_args.extend(['--server', cl.GetRietveldServer()])
if options.emulate_svn_auto_props:
upload_args.append('--emulate_svn_auto_props')
if options.from_logs and not options.message:
print 'Must set message for subject line if using desc_from_logs'
return 1
change_desc = None
......@@ -938,18 +930,17 @@ def CMDupload(parser, args):
print ("This branch is associated with issue %s. "
"Adding patch to that issue." % cl.GetIssue())
else:
log_desc = CreateDescriptionFromLog(args)
change_desc = ChangeDescription(options.message, log_desc,
options.reviewers)
if not options.from_logs:
change_desc.Update()
message = options.message or CreateDescriptionFromLog(args)
change_desc = ChangeDescription(message, options.reviewers)
if not options.force:
change_desc.Prompt()
change_desc.ParseDescription()
if change_desc.IsEmpty():
print "Description is empty; aborting."
return 1
upload_args.extend(['--message', change_desc.subject])
upload_args.extend(['--description', change_desc.description])
upload_args.extend(['--message', change_desc.description])
if change_desc.reviewers:
upload_args.extend(['--reviewers', change_desc.reviewers])
if options.send_mail:
......
tests/gcl_unittest.py
View file @ 2b40b89a
......@@ -187,7 +187,6 @@ class ChangeInfoUnittest(GclTestsBase):
'UpdateRietveldDescription',
'description', 'issue', 'name',
'needs_upload', 'patch', 'patchset', 'reviewers', 'rietveld',
'subject',
]
# If this test fails, you should add the relevant test.
self.compareMembers(
......@@ -312,10 +311,10 @@ class CMDuploadUnittest(GclTestsBase):
gcl.os.chdir('proout')
change_info.GetFileNames().AndReturn(files)
gcl.GenerateDiff(files)
gcl.upload.RealMain(['upload.py', '-y', '--server=https://my_server',
'-r', 'georges@example.com',
'--message=\'\'', '--issue=1'],
change_info.patch).AndReturn(("1",
gcl.upload.RealMain(
[ 'upload.py', '-y', '--server=https://my_server',
'-r', 'georges@example.com', '--issue=1'],
change_info.patch).AndReturn(("1",
"2"))
change_info.GetLocalRoot().AndReturn('proout')
change_info.Save()
......@@ -358,7 +357,7 @@ class CMDuploadUnittest(GclTestsBase):
gcl.GenerateDiff(change_info.GetFileNames())
gcl.upload.RealMain(
[ 'upload.py', '-y', '--server=https://my_server', '--server=a',
'--description_file=descfile', '--message=deescription'],
'--file=descfile'],
change_info.patch).AndReturn(("1", "2"))
gcl.os.remove('descfile')
change_info.SendToRietveld("/lint/issue%s_%s" % ('1', '2'), timeout=1)
......@@ -398,9 +397,9 @@ class CMDuploadUnittest(GclTestsBase):
gcl.os.getcwd().AndReturn('somewhere')
gcl.os.chdir(change_info.GetLocalRoot())
gcl.GenerateDiff(change_info.GetFileNames())
gcl.upload.RealMain(['upload.py', '-y', '--server=https://my_server',
"--description_file=descfile",
"--message=deescription"], change_info.patch).AndReturn(("1", "2"))
gcl.upload.RealMain(
['upload.py', '-y', '--server=https://my_server', "--file=descfile"],
change_info.patch).AndReturn(("1", "2"))
gcl.os.remove('descfile')
change_info.SendToRietveld("/lint/issue%s_%s" % ('1', '2'), timeout=1)
gcl.os.chdir('somewhere')
......@@ -455,10 +454,10 @@ class CMDuploadUnittest(GclTestsBase):
change_info.GetLocalRoot().AndReturn('proout')
gcl.os.chdir('proout')
gcl.GenerateDiff(files)
gcl.upload.RealMain(['upload.py', '-y', '--server=https://my_server',
'--reviewers=georges@example.com',
'--message=\'\'', '--issue=1'],
change_info.patch).AndReturn(("1", "2"))
gcl.upload.RealMain(
[ 'upload.py', '-y', '--server=https://my_server',
'--reviewers=georges@example.com', '--issue=1'],
change_info.patch).AndReturn(("1", "2"))
change_info.Save()
change_info.PrimeLint()
gcl.os.chdir('somewhere')
......@@ -484,10 +483,10 @@ class CMDuploadUnittest(GclTestsBase):
gcl.os.getcwd().AndReturn('somewhere')
gcl.os.chdir('proout')
gcl.GenerateDiff(change_info.GetFileNames())
gcl.upload.RealMain(['upload.py', '-y', '--server=https://my_server',
'--reviewers=foo@example.com,bar@example.com',
'--message=\'\'', '--issue=1'],
change_info.patch).AndReturn(("1", "2"))
gcl.upload.RealMain(
[ 'upload.py', '-y', '--server=https://my_server',
'--reviewers=foo@example.com,bar@example.com', '--issue=1'],
change_info.patch).AndReturn(("1", "2"))
change_info.Save()
change_info.PrimeLint()
gcl.os.chdir('somewhere')
......
tests/git_cl_test.py
View file @ 2b40b89a
......@@ -201,12 +201,10 @@ class TestGitCl(TestCase):
@staticmethod
def _cmd_line(description, args):
"""Returns the upload command line passed to upload.RealMain()."""
msg = description.split('\n', 1)[0]
return [
'upload', '--assume_yes', '--server',
'https://codereview.example.com',
'--message', msg,
'--description', description
'--message', description
] + args + [
'--cc', 'joe@example.com',
'master...'
......
third_party/upload.py
View file @ 2b40b89a
#!/usr/bin/env python
# coding: utf-8
#
# Copyright 2007 Google Inc.
#
......@@ -215,7 +216,7 @@ class AbstractRpcServer(object):
def _CreateRequest(self, url, data=None):
"""Creates a new urllib request."""
logging.debug("Creating request for: '%s' with payload:\n%s", url, data)
req = urllib2.Request(url, data=data)
req = urllib2.Request(url, data=data, headers={"Accept": "text/plain"})
if self.host_override:
req.add_header("Host", self.host_override)
for key, value in self.extra_headers.iteritems():
......@@ -399,14 +400,13 @@ class AbstractRpcServer(object):
raise
elif e.code == 401 or e.code == 302:
self._Authenticate()
## elif e.code >= 500 and e.code < 600:
## # Server Error - try again.
## continue
elif e.code == 301:
# Handle permanent redirect manually.
url = e.info()["location"]
url_loc = urlparse.urlparse(url)
self.host = '%s://%s' % (url_loc[0], url_loc[1])
elif e.code >= 500:
ErrorExit(e.read())
else:
raise
finally:
......@@ -532,14 +532,13 @@ group.add_option("--account_type", action="store", dest="account_type",
"valid choices are 'GOOGLE' and 'HOSTED')."))
# 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",
group.add_option("-t", "--title", action="store", dest="title",
help="New issue subject or new patch set title")
group.add_option("-m", "--message", action="store", dest="message",
default=None,
help="Optional path of a file that contains "
"the description when creating an issue.")
help="New issue description or new patch set message")
group.add_option("-F", "--file", action="store", dest="file",
default=None, help="Read the message above from file.")
group.add_option("-r", "--reviewers", action="store", dest="reviewers",
metavar="REVIEWERS", default=None,
help="Add reviewers (comma separated email addresses).")
......@@ -551,10 +550,6 @@ group.add_option("--private", action="store_true", dest="private",
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.")
......@@ -1464,8 +1459,8 @@ class MercurialVCS(VersionControlSystem):
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, self.subdir)
return filename[len(self.subdir):].lstrip(r"\/")
absname = os.path.join(self.repo_dir, filename)
return os.path.relpath(absname)
def GenerateDiff(self, extra_args):
cmd = ["hg", "diff", "--git", "-r", self.base_rev] + extra_args
......@@ -1504,9 +1499,8 @@ class MercurialVCS(VersionControlSystem):
return unknown_files
def GetBaseFile(self, filename):
# "hg status" and "hg cat" both take a path relative to the current subdir
# rather than to the repo root, but "hg diff" has given us the full path
# to the repo root.
# "hg status" and "hg cat" both take a path relative to the current subdir,
# but "hg diff" has given us the path relative to the repo root.
base_content = ""
new_content = None
is_binary = False
......@@ -1575,15 +1569,15 @@ class PerforceVCS(VersionControlSystem):
ConfirmLogin()
if not options.message:
if not options.title:
description = self.RunPerforceCommand(["describe", self.p4_changelist],
marshal_output=True)
if description and "desc" in description:
# Rietveld doesn't support multi-line descriptions
raw_message = description["desc"].strip()
lines = raw_message.splitlines()
raw_title = description["desc"].strip()
lines = raw_title.splitlines()
if len(lines):
options.message = lines[0]
options.title = lines[0]
def GetGUID(self):
"""For now we don't know how to get repository ID for Perforce"""
......@@ -1974,10 +1968,12 @@ def GuessVCSName(options):
if res != None:
return res
# Subversion has a .svn in all working directories.
if os.path.isdir('.svn'):
logging.info("Guessed VCS = Subversion")
return (VCS_SUBVERSION, None)
# Subversion from 1.7 has a single centralized .svn folder
# ( see http://subversion.apache.org/docs/release-notes/1.7.html#wc-ng )
# That's why we use 'svn info' instead of checking for .svn dir
res = RunDetectCommand(VCS_SUBVERSION, ["svn", "info"])
if res != None:
return res
# 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.
......@@ -2219,20 +2215,13 @@ def RealMain(argv, data=None):
files = vcs.GetBaseFiles(data)
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.server,
options.email,
options.host,
options.save_cookies,
options.account_type)
form_fields = [("subject", message)]
form_fields = []
repo_guid = vcs.GetGUID()
if repo_guid:
form_fields.append(("repo_guid", repo_guid))
......@@ -2256,15 +2245,37 @@ def RealMain(argv, data=None):
for cc in options.cc.split(','):
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()
# Process --message, --title and --file.
message = options.message or ""
title = options.title or ""
if options.file:
if options.message:
ErrorExit("Can't specify both message and message file options")
file = open(options.file, 'r')
message = file.read()
file.close()
if description:
form_fields.append(("description", description))
if options.issue:
prompt = "Title describing this patch set: "
else:
prompt = "New issue subject: "
title = (
title or message.split('\n', 1)[0].strip() or raw_input(prompt)).strip()
if not title:
ErrorExit("A non-empty title is required")
if len(title) > 100:
title = title[:99] + '…'
if title and not options.issue:
message = message or title
form_fields.append(("subject", title))
if message:
if not options.issue:
form_fields.append(("description", message))
else:
# TODO: [ ] Use /<issue>/publish to add a commen
pass
# Send a hash of all the base file so the server can determine if a copy
# already exists in an earlier patchset.
base_hashes = ""
......@@ -2282,10 +2293,6 @@ def RealMain(argv, data=None):
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:
form_fields.append(("send_mail", "1"))
if not options.download_base:
form_fields.append(("content_upload", "1"))
if len(data) > MAX_UPLOAD_SIZE:
......@@ -2320,11 +2327,15 @@ def RealMain(argv, data=None):
if not options.download_base:
vcs.UploadBaseFiles(issue, rpc_server, patches, patchset, options, files)
if options.send_mail:
payload = ""
if options.send_patch:
payload=urllib.urlencode({"attach_patch": "yes"})
rpc_server.Send("/" + issue + "/mail", payload=payload)
payload = {} # payload for final request
if options.send_mail:
payload["send_mail"] = "yes"
if options.send_patch:
payload["attach_patch"] = "yes"
payload = urllib.urlencode(payload)
rpc_server.Send("/" + issue + "/upload_complete/" + (patchset or ""),
payload=payload)
return issue, patchset
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment