Commit c2b9bd03 authored by rmistry@google.com's avatar rmistry@google.com

Find, upload and apply patchset dependencies.

Here is an explanation of the changes in each module:

* git_cl.py -
IF a local branch is being tracked AND a CL has been uploaded there THEN use the CL's issue number and latest patchset as a dependency.

* upload.py -
Uploads the patchset dependency, if it exists, to Rietveld (Rietveld will be able to parse this when https://codereview.chromium.org/1155513002/ lands).

* rietveld.py -
Adds utility methods to get patchset dependencies from the new Rietveld endpoint (the endpoint will exist when https://codereview.chromium.org/1155513002/ lands).

* apply_issue.py -
If CL3 depends on CL2 which in turn depends on CL1 then apply_issue will gather a list of all issues and patchsets to apply (Eg: [CL1:PS1, CL2:PS1, CL3:PS2]).
apply_issue will then loop over the list applying each dependency.
Note: The apply_issue.py diff looks much worse than it is. Please see my comment in
https://codereview.chromium.org/1149653002/diff/260001/apply_issue.py#oldcode169


Tested end-to-end using a test Git repository (https://skia.googlesource.com/skiabot-test/) and the following CLs created in my test Rietveld instance:
* https://skia-codereview-staging.appspot.com/931002  ('Branch1 CL')
* https://skia-codereview-staging.appspot.com/5001001 ('Branch2 CL')
* https://skia-codereview-staging.appspot.com/9881001 ('Branch3 CL')
* https://skia-codereview-staging.appspot.com/3951001 ('Branch3.1 CL')
Opt into the new UI and observe the new 'Depends on Patchset' and 'Dependent Patchsets' sections in the above CLs.


Design doc is here: https://docs.google.com/document/d/1KZGFKZpOPvco81sYVRCzwlnjGctup71RAzY0MSb0ntc/edit#heading=h.6r6lt4tsvssw

BUG=502255

Review URL: https://codereview.chromium.org/1149653002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@295778 0039d316-1c4b-4281-b951-d872f2087c98
parent 25b9ab2c
...@@ -166,15 +166,49 @@ def main(): ...@@ -166,15 +166,49 @@ def main():
options.patchset = properties['patchsets'][-1] options.patchset = properties['patchsets'][-1]
print('No patchset specified. Using patchset %d' % options.patchset) print('No patchset specified. Using patchset %d' % options.patchset)
print('Downloading the patch.') issues_patchsets_to_apply = [(options.issue, options.patchset)]
depends_on_info = obj.get_depends_on_patchset(options.issue, options.patchset)
while depends_on_info:
depends_on_issue = int(depends_on_info['issue'])
depends_on_patchset = int(depends_on_info['patchset'])
try: try:
patchset = obj.get_patch(options.issue, options.patchset) depends_on_info = obj.get_depends_on_patchset(depends_on_issue,
depends_on_patchset)
issues_patchsets_to_apply.insert(0, (depends_on_issue,
depends_on_patchset))
except urllib2.HTTPError:
print ('The patchset that was marked as a dependency no longer '
'exists: %s/%d/#ps%d' % (
options.server, depends_on_issue, depends_on_patchset))
print 'Therefore it is likely that this patch will not apply cleanly.'
print
depends_on_info = None
num_issues_patchsets_to_apply = len(issues_patchsets_to_apply)
if num_issues_patchsets_to_apply > 1:
print
print 'apply_issue.py found %d dependent CLs.' % (
num_issues_patchsets_to_apply - 1)
print 'They will be applied in the following order:'
num = 1
for issue_to_apply, patchset_to_apply in issues_patchsets_to_apply:
print ' #%d %s/%d/#ps%d' % (
num, options.server, issue_to_apply, patchset_to_apply)
num += 1
print
for issue_to_apply, patchset_to_apply in issues_patchsets_to_apply:
issue_url = '%s/%d/#ps%d' % (options.server, issue_to_apply,
patchset_to_apply)
print('Downloading patch from %s' % issue_url)
try:
patchset = obj.get_patch(issue_to_apply, patchset_to_apply)
except urllib2.HTTPError as e: except urllib2.HTTPError as e:
print( print(
'Failed to fetch the patch for issue %d, patchset %d.\n' 'Failed to fetch the patch for issue %d, patchset %d.\n'
'Try visiting %s/%d') % ( 'Try visiting %s/%d') % (
options.issue, options.patchset, issue_to_apply, patchset_to_apply,
options.server, options.issue) options.server, issue_to_apply)
return 1 return 1
if options.whitelist: if options.whitelist:
patchset.patches = [patch for patch in patchset.patches patchset.patches = [patch for patch in patchset.patches
...@@ -204,7 +238,7 @@ def main(): ...@@ -204,7 +238,7 @@ def main():
# chromium_commands.py?view=markup # chromium_commands.py?view=markup
open('.buildbot-patched', 'w').close() open('.buildbot-patched', 'w').close()
print('\nApplying the patch.') print('\nApplying the patch from %s' % issue_url)
try: try:
scm_obj.apply_patch(patchset, verbose=True) scm_obj.apply_patch(patchset, verbose=True)
except checkout.PatchApplicationFailed as e: except checkout.PatchApplicationFailed as e:
......
...@@ -2085,6 +2085,28 @@ def RietveldUpload(options, args, cl, change): ...@@ -2085,6 +2085,28 @@ def RietveldUpload(options, args, cl, change):
if target_ref: if target_ref:
upload_args.extend(['--target_ref', target_ref]) upload_args.extend(['--target_ref', target_ref])
# Look for dependent patchsets. See crbug.com/480453 for more details.
remote, upstream_branch = cl.FetchUpstreamTuple(cl.GetBranch())
upstream_branch = ShortBranchName(upstream_branch)
if remote is '.':
# A local branch is being tracked.
local_branch = ShortBranchName(upstream_branch)
auth_config = auth.extract_auth_config_from_options(options)
branch_cl = Changelist(branchref=local_branch, auth_config=auth_config)
branch_cl_issue_url = branch_cl.GetIssueURL()
branch_cl_issue = branch_cl.GetIssue()
branch_cl_patchset = branch_cl.GetPatchset()
if branch_cl_issue_url and branch_cl_issue and branch_cl_patchset:
upload_args.extend(
['--depends_on_patchset', '%s:%s' % (
branch_cl_issue, branch_cl_patchset)])
print
print ('The current branch (%s) is tracking a local branch (%s) with '
'an open CL.') % (cl.GetBranch(), local_branch)
print 'Adding %s/#ps%s as a dependency patchset.' % (
branch_cl_issue_url, branch_cl_patchset)
print
project = settings.GetProject() project = settings.GetProject()
if project: if project:
upload_args.extend(['--project', project]) upload_args.extend(['--project', project])
......
...@@ -84,6 +84,20 @@ class Rietveld(object): ...@@ -84,6 +84,20 @@ class Rietveld(object):
data['description'] = '\n'.join(data['description'].strip().splitlines()) data['description'] = '\n'.join(data['description'].strip().splitlines())
return data return data
def get_depends_on_patchset(self, issue, patchset):
"""Returns the patchset this patchset depends on if it exists."""
url = '/%d/patchset/%d/get_depends_on_patchset' % (issue, patchset)
resp = None
try:
resp = json.loads(self.get(url))
except urllib2.HTTPError:
# The get_depends_on_patchset endpoint does not exist on this Rietveld
# instance yet. Ignore the error and proceed.
# TODO(rmistry): Make this an error when all Rietveld instances have
# this endpoint.
pass
return resp
def get_patchset_properties(self, issue, patchset): def get_patchset_properties(self, issue, patchset):
"""Returns the patchset properties.""" """Returns the patchset properties."""
url = '/api/%d/%d' % (issue, patchset) url = '/api/%d/%d' % (issue, patchset)
...@@ -677,6 +691,9 @@ class ReadOnlyRietveld(object): ...@@ -677,6 +691,9 @@ class ReadOnlyRietveld(object):
def get_patchset_properties(self, issue, patchset): def get_patchset_properties(self, issue, patchset):
return self._rietveld.get_patchset_properties(issue, patchset) return self._rietveld.get_patchset_properties(issue, patchset)
def get_depends_on_patchset(self, issue, patchset):
return self._rietveld.get_depends_on_patchset(issue, patchset)
def get_patch(self, issue, patchset): def get_patch(self, issue, patchset):
return self._rietveld.get_patch(issue, patchset) return self._rietveld.get_patch(issue, patchset)
......
...@@ -635,6 +635,10 @@ group.add_option("--target_ref", action="store", dest="target_ref", ...@@ -635,6 +635,10 @@ group.add_option("--target_ref", action="store", dest="target_ref",
parser.add_option("--cq_dry_run", action="store_true", parser.add_option("--cq_dry_run", action="store_true",
help="Send the patchset to do a CQ dry run right after " help="Send the patchset to do a CQ dry run right after "
"upload.") "upload.")
parser.add_option("--depends_on_patchset", action="store",
dest="depends_on_patchset",
help="The uploaded patchset this patchset depends on. The "
"value will be in this format- issue_num:patchset_num")
group.add_option("--download_base", action="store_true", group.add_option("--download_base", action="store_true",
dest="download_base", default=False, dest="download_base", default=False,
help="Base files will be downloaded by the server " help="Base files will be downloaded by the server "
...@@ -2436,6 +2440,8 @@ def RealMain(argv, data=None): ...@@ -2436,6 +2440,8 @@ def RealMain(argv, data=None):
if options.cq_dry_run: if options.cq_dry_run:
form_fields.append(("cq_dry_run", "1")) form_fields.append(("cq_dry_run", "1"))
form_fields.append(("commit", "1")) form_fields.append(("commit", "1"))
if options.depends_on_patchset:
form_fields.append(("depends_on_patchset", options.depends_on_patchset))
# Process --message, --title and --file. # Process --message, --title and --file.
message = options.message or "" message = options.message or ""
......
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