From 4de54135e8be66b57014b08c36cc311a407733d0 Mon Sep 17 00:00:00 2001 From: Edward Lesmes Date: Tue, 5 May 2020 19:41:33 +0000 Subject: [PATCH] Revert "git-cl: turn GERRIT_SQUASH_UPLOADS into a warning" This reverts commit 31a538a3a9e45202471c7eb0b2bd11c039a78f0a. Reason for revert: Breaks ANGLE's workflow. Original change's description: > git-cl: turn GERRIT_SQUASH_UPLOADS into a warning > > Now that repos have dropped this from their codereview.settings file, > change this logic to issue a warning if it's ever seen again. > > Drop checking local gerrit.override-squash-uploads config too since > it's no longer relevant. > > Bug: 993518 > Change-Id: Id91bbc94b0890ca21c51a274f6acc41f2ae19b78 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1756159 > Commit-Queue: Mike Frysinger > Reviewed-by: Edward Lesmes TBR=vapier@chromium.org,ehmaldonado@chromium.org,infra-scoped@luci-project-accounts.iam.gserviceaccount.com Change-Id: Iaa8b3341b189f356082ae25a1557898e25820566 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 993518 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2182019 Reviewed-by: Edward Lesmes Commit-Queue: Edward Lesmes --- git_cl.py | 19 +++++++++++++++++-- tests/git_cl_test.py | 26 ++++++++++++++++++++++---- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/git_cl.py b/git_cl.py index 58d7b90b0..8bb03fc84 100755 --- a/git_cl.py +++ b/git_cl.py @@ -766,12 +766,27 @@ class Settings(object): def GetSquashGerritUploads(self): """Returns True if uploads to Gerrit should be squashed by default.""" + if self.squash_gerrit_uploads is None: + self.squash_gerrit_uploads = self.GetSquashGerritUploadsOverride() if self.squash_gerrit_uploads is None: # Default is squash now (http://crbug.com/611892#c23). self.squash_gerrit_uploads = self._GetConfig( 'gerrit.squash-uploads').lower() != 'false' return self.squash_gerrit_uploads + def GetSquashGerritUploadsOverride(self): + """Return True or False if codereview.settings should be overridden. + + Returns None if no override has been defined. + """ + # See also http://crbug.com/611892#c23 + result = self._GetConfig('gerrit.override-squash-uploads').lower() + if result == 'true': + return True + if result == 'false': + return False + return None + def GetGerritSkipEnsureAuthenticated(self): """Return True if EnsureAuthenticated should not be done for Gerrit uploads.""" @@ -2740,8 +2755,8 @@ def LoadCodereviewSettingsFromFile(fileobj): RunGit(['config', 'gerrit.host', keyvals['GERRIT_HOST']]) if 'GERRIT_SQUASH_UPLOADS' in keyvals: - print('WARNING: GERRIT_SQUASH_UPLOADS in codereview.settings is no longer ' - 'supported. Please delete it and update your ~/.gitconfig instead.') + RunGit(['config', 'gerrit.squash-uploads', + keyvals['GERRIT_SQUASH_UPLOADS']]) if 'GERRIT_SKIP_ENSURE_AUTHENTICATED' in keyvals: RunGit(['config', 'gerrit.skip-ensure-authenticated', diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 3c582842d..29c88d5d6 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -694,6 +694,10 @@ class TestGitCl(unittest.TestCase): calls = [] + if squash_mode in ('override_squash', 'override_nosquash'): + 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: calls += [ (('DownloadGerritHook', False), ''), @@ -1104,38 +1108,52 @@ class TestGitCl(unittest.TestCase): post_amend_description='desc ✔\n\nBUG=\n\nChange-Id: Ixxx', change_id='Ixxx') + def test_gerrit_upload_without_change_id_override_nosquash(self): + self._run_gerrit_upload_test( + [], + 'desc ✔\n\nBUG=\n', + [], + squash=False, + squash_mode='override_nosquash', + post_amend_description='desc ✔\n\nBUG=\n\nChange-Id: Ixxx', + change_id='Ixxx') + def test_gerrit_no_reviewer(self): self._run_gerrit_upload_test( - ['--no-squash'], + [], 'desc ✔\n\nBUG=\n\nChange-Id: I123456789\n', [], squash=False, + squash_mode='override_nosquash', change_id='I123456789') def test_gerrit_no_reviewer_non_chromium_host(self): # TODO(crbug/877717): remove this test case. self._run_gerrit_upload_test( - ['--no-squash'], + [], 'desc ✔\n\nBUG=\n\nChange-Id: I123456789\n', [], squash=False, + squash_mode='override_nosquash', short_hostname='other', change_id='I123456789') def test_gerrit_patchset_title_special_chars_nosquash(self): self._run_gerrit_upload_test( - ['--no-squash', '-f', '-t', 'We\'ll escape ^_ ^ special chars...@{u}'], + ['-f', '-t', 'We\'ll escape ^_ ^ special chars...@{u}'], 'desc ✔\n\nBUG=\n\nChange-Id: I123456789', squash=False, + squash_mode='override_nosquash', change_id='I123456789', title='We\'ll escape ^_ ^ special chars...@{u}') def test_gerrit_reviewers_cmd_line(self): self._run_gerrit_upload_test( - ['--no-squash', '-r', 'foo@example.com', '--send-mail'], + ['-r', 'foo@example.com', '--send-mail'], 'desc ✔\n\nBUG=\n\nChange-Id: I123456789', reviewers=['foo@example.com'], squash=False, + squash_mode='override_nosquash', notify=True, change_id='I123456789', final_description=(