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
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=295778
Review URL: https://codereview.chromium.org/1149653002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@295799 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/apply_issue.py b/apply_issue.py
index 41e133b..ba38987 100755
--- a/apply_issue.py
+++ b/apply_issue.py
@@ -166,52 +166,86 @@
options.patchset = properties['patchsets'][-1]
print('No patchset specified. Using patchset %d' % options.patchset)
- print('Downloading the patch.')
- try:
- patchset = obj.get_patch(options.issue, options.patchset)
- except urllib2.HTTPError as e:
- print(
- 'Failed to fetch the patch for issue %d, patchset %d.\n'
- 'Try visiting %s/%d') % (
- options.issue, options.patchset,
- options.server, options.issue)
- return 1
- if options.whitelist:
- patchset.patches = [patch for patch in patchset.patches
- if patch.filename in options.whitelist]
- if options.blacklist:
- patchset.patches = [patch for patch in patchset.patches
- if patch.filename not in options.blacklist]
- for patch in patchset.patches:
- print(patch)
- full_dir = os.path.abspath(options.root_dir)
- scm_type = scm.determine_scm(full_dir)
- if scm_type == 'svn':
- scm_obj = checkout.SvnCheckout(full_dir, None, None, None, None)
- elif scm_type == 'git':
- scm_obj = checkout.GitCheckout(full_dir, None, None, None, None)
- elif scm_type == None:
- scm_obj = checkout.RawCheckout(full_dir, None, None)
- else:
- parser.error('Couldn\'t determine the scm')
+ 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:
+ 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
- # TODO(maruel): HACK, remove me.
- # When run a build slave, make sure buildbot knows that the checkout was
- # modified.
- if options.root_dir == 'src' and getpass.getuser() == 'chrome-bot':
- # See sourcedirIsPatched() in:
- # http://src.chromium.org/viewvc/chrome/trunk/tools/build/scripts/slave/
- # chromium_commands.py?view=markup
- open('.buildbot-patched', 'w').close()
+ 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
- print('\nApplying the patch.')
- try:
- scm_obj.apply_patch(patchset, verbose=True)
- except checkout.PatchApplicationFailed as e:
- print(str(e))
- print('CWD=%s' % os.getcwd())
- print('Checkout path=%s' % scm_obj.project_path)
- return 1
+ 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:
+ print(
+ 'Failed to fetch the patch for issue %d, patchset %d.\n'
+ 'Try visiting %s/%d') % (
+ issue_to_apply, patchset_to_apply,
+ options.server, issue_to_apply)
+ return 1
+ if options.whitelist:
+ patchset.patches = [patch for patch in patchset.patches
+ if patch.filename in options.whitelist]
+ if options.blacklist:
+ patchset.patches = [patch for patch in patchset.patches
+ if patch.filename not in options.blacklist]
+ for patch in patchset.patches:
+ print(patch)
+ full_dir = os.path.abspath(options.root_dir)
+ scm_type = scm.determine_scm(full_dir)
+ if scm_type == 'svn':
+ scm_obj = checkout.SvnCheckout(full_dir, None, None, None, None)
+ elif scm_type == 'git':
+ scm_obj = checkout.GitCheckout(full_dir, None, None, None, None)
+ elif scm_type == None:
+ scm_obj = checkout.RawCheckout(full_dir, None, None)
+ else:
+ parser.error('Couldn\'t determine the scm')
+
+ # TODO(maruel): HACK, remove me.
+ # When run a build slave, make sure buildbot knows that the checkout was
+ # modified.
+ if options.root_dir == 'src' and getpass.getuser() == 'chrome-bot':
+ # See sourcedirIsPatched() in:
+ # http://src.chromium.org/viewvc/chrome/trunk/tools/build/scripts/slave/
+ # chromium_commands.py?view=markup
+ open('.buildbot-patched', 'w').close()
+
+ print('\nApplying the patch from %s' % issue_url)
+ try:
+ scm_obj.apply_patch(patchset, verbose=True)
+ except checkout.PatchApplicationFailed as e:
+ print(str(e))
+ print('CWD=%s' % os.getcwd())
+ print('Checkout path=%s' % scm_obj.project_path)
+ return 1
if ('DEPS' in map(os.path.basename, patchset.filenames)
and not options.ignore_deps):
diff --git a/git_cl.py b/git_cl.py
index f719254..288e281 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -2185,6 +2185,28 @@
if 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()
if project:
upload_args.extend(['--project', project])
diff --git a/rietveld.py b/rietveld.py
index eddf77e..ec5d781 100644
--- a/rietveld.py
+++ b/rietveld.py
@@ -84,6 +84,20 @@
data['description'] = '\n'.join(data['description'].strip().splitlines())
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, ValueError):
+ # 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):
"""Returns the patchset properties."""
url = '/api/%d/%d' % (issue, patchset)
@@ -677,6 +691,9 @@
def get_patchset_properties(self, 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):
return self._rietveld.get_patch(issue, patchset)
diff --git a/third_party/upload.py b/third_party/upload.py
index eac2a3d..002a0d6 100755
--- a/third_party/upload.py
+++ b/third_party/upload.py
@@ -635,6 +635,10 @@
parser.add_option("--cq_dry_run", action="store_true",
help="Send the patchset to do a CQ dry run right after "
"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",
dest="download_base", default=False,
help="Base files will be downloaded by the server "
@@ -2436,6 +2440,8 @@
if options.cq_dry_run:
form_fields.append(("cq_dry_run", "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.
message = options.message or ""