From 36d937d16d8bd346f6b01bef64b87f954bfecf32 Mon Sep 17 00:00:00 2001 From: Gavin Mak Date: Tue, 26 Sep 2023 19:52:40 +0000 Subject: [PATCH] Clear existing Change-Ids from description if issue is 0 git cl upload may not create a new change even if issue is 0 because of a leftover Change-Id footer in the commit message. Bug: 1484251 Change-Id: Ibee0542e0f5a2cbf930b1882892fbb7640054b69 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4880971 Reviewed-by: Aravind Vasudevan Commit-Queue: Gavin Mak --- git_cl.py | 8 +++++++- tests/git_cl_test.py | 16 ++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/git_cl.py b/git_cl.py index d1936ecea..cfb62348e 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1788,6 +1788,12 @@ class Changelist(object): bug = options.bug fixed = options.fixed if not self.GetIssue(): + # There isn't any issue attached, so we shouldn't keep existing + # Change-Ids in the description. + if git_footers.get_footer_change_id(description): + description = git_footers.remove_footer(description, + 'Change-Id') + # Extract bug number from branch name, but only if issue is being # created. It must start with bug or fix, followed by _ or - and # number. Optionally, it may contain _ or - after number with @@ -1829,7 +1835,7 @@ class Changelist(object): def _GetTitleForUpload(self, options, multi_change_upload=False): # type: (optparse.Values, Optional[bool]) -> str - # Getting titles for multipl commits is not supported so we return the + # Getting titles for multiple commits is not supported so we return the # default. if not options.squash or multi_change_upload or options.title: return options.title diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 1a146bd44..fa4f30ddf 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -32,7 +32,6 @@ import gerrit_util import git_cl import git_common import git_footers -import git_new_branch import owners_client import scm import subprocess2 @@ -847,7 +846,8 @@ class TestGitCl(unittest.TestCase): self.mockGit.config['gerrit.override-squash-uploads'] = ( 'true' if squash_mode == 'override_squash' else 'false') - if not git_footers.get_footer_change_id(description) and not squash: + has_change_id = git_footers.get_footer_change_id(description) + if not squash and (not has_change_id or not issue): calls += [ (('DownloadGerritHook', False), ''), ] @@ -1340,6 +1340,8 @@ class TestGitCl(unittest.TestCase): self._run_gerrit_upload_test( ['-r', 'foo@example.com', '--send-mail'], 'desc ✔\n\nBUG=\n\nChange-Id: I123456789', + post_amend_description= + 'desc ✔\n\nBUG=\nR=foo@example.com\n\nChange-Id: I123456789', reviewers=['foo@example.com'], squash=False, squash_mode='override_nosquash', @@ -1352,6 +1354,8 @@ class TestGitCl(unittest.TestCase): self._run_gerrit_upload_test( ['-r', 'foo@example.com', '--send-email'], 'desc ✔\n\nBUG=\n\nChange-Id: I123456789', + post_amend_description= + 'desc ✔\n\nBUG=\nR=foo@example.com\n\nChange-Id: I123456789', reviewers=['foo@example.com'], squash=False, squash_mode='override_nosquash', @@ -1378,6 +1382,14 @@ class TestGitCl(unittest.TestCase): title='Title', change_id='Ixxxx') + def test_gerrit_upload_resets_change_id(self): + self._run_gerrit_upload_test( + [], + 'desc=\n\nChange-Id: Iyyy', [], + log_description='desc=\n\nChange-Id: Ixxx', + issue=None, + change_id='Iyyy') + def test_gerrit_upload_force_sets_fixed(self): self._run_gerrit_upload_test( ['-x', '10000', '-f'],