From 400e989b1b91f6dc195266a40edb6a89bc9ee721 Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Wed, 12 Jul 2017 15:31:21 -0700 Subject: [PATCH] 'git cl issue 0': Remove Change-Id Although git-cl-upload warns when uploading a new patchset to a change owned by someone else, if the uploader has run 'git cl issue 0', then git-cl believes they'll be uploading a new change, so it doesn't bother checking. However, once the upload begins, Gerrit notices the Change-Id in the commit message, and instead adds a new patchset to someone else's review (if the uploader is a committer). This change introduces some logic to git-cl-issue to also remove any Change-Id from the commit message when a user tries to clear the metadata about their branch. Bug: 741648 Change-Id: I6c7c3b24a7fc09c68220c8200b732fbdf9cf1fd3 Reviewed-on: https://chromium-review.googlesource.com/568267 Reviewed-by: Robbie Iannucci Commit-Queue: Aaron Gable --- git_cl.py | 6 ++++++ tests/git_cl_test.py | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/git_cl.py b/git_cl.py index 3d3589a785..2846913572 100755 --- a/git_cl.py +++ b/git_cl.py @@ -1471,6 +1471,12 @@ class Changelist(object): self._codereview_impl.CodereviewServerConfigKey(), codereview_server) else: + desc = self.GetDescription() + if git_footers.get_footer_change_id(desc): + print('WARNING: The change patched into this branch has a Change-Id. ' + 'Removing it.') + RunGit(['commit', '--amend', '-m', + git_footers.remove_footer(desc, 'Change-Id')]) # Reset all of these just to be clean. reset_suffixes = [ 'last-upload-hash', diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index a3b3830026..cea27588f7 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -2647,10 +2647,33 @@ class TestGitCl(TestCase): def test_cmd_issue_erase_existing(self): out = StringIO.StringIO() self.mock(git_cl.sys, 'stdout', out) + self.mock(git_cl.Changelist, 'GetDescription', + lambda _: 'This is a description') + self.calls = [ + ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), + ((['git', 'config', 'branch.feature.rietveldissue'],), CERR1), + ((['git', 'config', 'branch.feature.gerritissue'],), '123'), + # Let this command raise exception (retcode=1) - it should be ignored. + ((['git', 'config', '--unset', 'branch.feature.last-upload-hash'],), + CERR1), + ((['git', 'config', '--unset', 'branch.feature.gerritissue'],), ''), + ((['git', 'config', '--unset', 'branch.feature.gerritpatchset'],), ''), + ((['git', 'config', '--unset', 'branch.feature.gerritserver'],), ''), + ((['git', 'config', '--unset', 'branch.feature.gerritsquashhash'],), + ''), + ] + self.assertEqual(0, git_cl.main(['issue', '0'])) + + def test_cmd_issue_erase_existing_with_change_id(self): + out = StringIO.StringIO() + self.mock(git_cl.sys, 'stdout', out) + self.mock(git_cl.Changelist, 'GetDescription', + lambda _: 'This is a description\n\nChange-Id: Ideadbeef') self.calls = [ ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), ((['git', 'config', 'branch.feature.rietveldissue'],), CERR1), ((['git', 'config', 'branch.feature.gerritissue'],), '123'), + ((['git', 'commit', '--amend', '-m', 'This is a description\n'],), ''), # Let this command raise exception (retcode=1) - it should be ignored. ((['git', 'config', '--unset', 'branch.feature.last-upload-hash'],), CERR1),