From 9e9fc992803c0c95402c190fb0bd15b787966855 Mon Sep 17 00:00:00 2001 From: Daniel Rubery Date: Fri, 5 Jul 2024 20:36:17 +0000 Subject: [PATCH] Fallback to looking up CL issue from triplet_id In nosquash mode, `git cl` is pretty challenging to use since we don't have issue numbers. All three parts of the triplet id are readily available though. This CL adds a fallback to `git cl` which looks up the issue number by getting the change details by triplet_id. Change-Id: I0839fe75bcb4bc8d60ff36b4da26dc0e419a1493 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5633186 Commit-Queue: Daniel Rubery Reviewed-by: Gavin Mak --- gerrit_util.py | 5 ++-- git_cl.py | 61 ++++++++++++++++++++++++++++++++++---------- tests/git_cl_test.py | 24 +++++++++++++++-- 3 files changed, 72 insertions(+), 18 deletions(-) diff --git a/gerrit_util.py b/gerrit_util.py index a039258987..09e8d57d08 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -1209,10 +1209,11 @@ def GetChangeUrl(host, change): return '%s://%s/a/changes/%s' % (GERRIT_PROTOCOL, host, change) -def GetChange(host, change): +def GetChange(host, change, accept_statuses: Container[int] = frozenset([200])): """Queries a Gerrit server for information about a single change.""" path = 'changes/%s' % change - return ReadHttpJsonResponse(CreateHttpConn(host, path)) + return ReadHttpJsonResponse(CreateHttpConn(host, path), + accept_statuses=accept_statuses) def GetChangeDetail(host, change, o_params=None): diff --git a/git_cl.py b/git_cl.py index 09bb387377..9b42e3ccb9 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1526,11 +1526,41 @@ class Changelist(object): self._cached_remote_url = (True, url) return url + def _GetIssueFromTripletId(self): + project = self.GetGerritProject() + remote, branch = self.GetRemoteBranch() + remote_ref = GetTargetRef(remote, branch, + None).replace("refs/heads/", "") + # _create_description_from_log calls into `git log + # HEAD^..`. This will exclude everything reachable from `HEAD^`, + # leaving just `HEAD`. + change_ids = git_footers.get_footer_change_id( + _create_description_from_log(["HEAD^"])) + if len(change_ids) != 1: + return None + change_id = change_ids[0] + triplet_id = urllib.parse.quote("%s~%s~%s" % + (project, remote_ref, change_id), + safe='') + + # GetGerritHost() would attempt to lookup the host from the + # issue first, leading to infinite recursion. Instead, use the + # fallback of getting the host from the remote URL. + gerrit_host = self._GetGerritHostFromRemoteUrl() + change = gerrit_util.GetChange(gerrit_host, + triplet_id, + accept_statuses=[200, 404]) + return change.get('_number') + def GetIssue(self): """Returns the issue number as a int or None if not set.""" if self.issue is None and not self.lookedup_issue: if self.GetBranch(): self.issue = self._GitGetBranchConfigValue(ISSUE_CONFIG_KEY) + + if not settings.GetSquashGerritUploads() and self.issue is None: + self.issue = self._GetIssueFromTripletId() + if self.issue is not None: self.issue = int(self.issue) self.lookedup_issue = True @@ -2223,6 +2253,22 @@ class Changelist(object): return None return urllib.parse.urlparse(remote_url).netloc + def _GetGerritHostFromRemoteUrl(self): + url = urllib.parse.urlparse(self.GetRemoteUrl()) + parts = url.netloc.split('.') + + # We assume repo to be hosted on Gerrit, and hence Gerrit server + # has "-review" suffix for lowest level subdomain. + parts[0] = parts[0] + '-review' + + if url.scheme == 'sso' and len(parts) == 1: + # sso:// uses abbreivated hosts, eg. sso://chromium instead + # of chromium.googlesource.com. Hence, for code review + # server, they need to be expanded. + parts[0] += '.googlesource.com' + + return '.'.join(parts) + def GetCodereviewServer(self): if not self._gerrit_server: # If we're on a branch then get the server potentially associated @@ -2234,20 +2280,7 @@ class Changelist(object): self._gerrit_host = urllib.parse.urlparse( self._gerrit_server).netloc if not self._gerrit_server: - url = urllib.parse.urlparse(self.GetRemoteUrl()) - parts = url.netloc.split('.') - - # We assume repo to be hosted on Gerrit, and hence Gerrit server - # has "-review" suffix for lowest level subdomain. - parts[0] = parts[0] + '-review' - - if url.scheme == 'sso' and len(parts) == 1: - # sso:// uses abbreivated hosts, eg. sso://chromium instead - # of chromium.googlesource.com. Hence, for code review - # server, they need to be expanded. - parts[0] += '.googlesource.com' - - self._gerrit_host = '.'.join(parts) + self._gerrit_host = self._GetGerritHostFromRemoteUrl() self._gerrit_server = 'https://%s' % self._gerrit_host return self._gerrit_server diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 210ca72dd4..4b0706cf9b 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -933,7 +933,7 @@ class TestGitCl(unittest.TestCase): metrics_arguments.append('wip') # If issue is given, then description is fetched from Gerrit instead. - if not title: + if not title and squash_mode != "override_nosquash": if issue is None: if squash: title = 'Initial upload' @@ -1249,7 +1249,17 @@ class TestGitCl(unittest.TestCase): if issue else None) self.mockGit.config['remote.origin.url'] = ( 'https://%s.googlesource.com/my/repo' % short_hostname) - self.mockGit.config['user.email'] = 'me@example.com' + self.mockGit.config['user.email'] = 'owner@example.com' + + self.calls = [] + if squash_mode == "override_nosquash": + if issue: + mock.patch('gerrit_util.GetChange', + return_value={ + '_number': issue + }).start() + else: + mock.patch('gerrit_util.GetChange', return_value={}).start() self.calls = self._gerrit_base_calls( issue=issue, @@ -3590,6 +3600,16 @@ class TestGitCl(unittest.TestCase): change_id='I123456789', default_branch='main') + def test_gerrit_nosquash_with_issue(self): + self._run_gerrit_upload_test( + [], + 'desc ✔\n\nBUG=\n\nChange-Id: I123456789\n', [], + squash=False, + squash_mode='override_nosquash', + issue=123456, + change_id='I123456789', + default_branch='main') + class ChangelistTest(unittest.TestCase): LAST_COMMIT_SUBJECT = 'Fixes goat teleporter destination to be Australia'