From 2825353c97acf778ef249a10dae1aababf8921a1 Mon Sep 17 00:00:00 2001 From: "tandrii@chromium.org" Date: Thu, 14 Apr 2016 13:46:56 +0000 Subject: [PATCH] Allow to skip EnsureAuthenticated for Gerrit. Adding this line to codereview.settings should do the trick: GERRIT_SKIP_ENSURE_AUTHENTICATED: true And I also cleaned up tests so they don't spill too much to stdout. BUG=603378 R=machenbach@chromium.org,andybons@chromium.org CC=shinyak@chromium.org Review URL: https://codereview.chromium.org/1884173003 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@299926 0039d316-1c4b-4281-b951-d872f2087c98 --- git_cl.py | 18 ++++++++++++++++++ tests/git_cl_test.py | 31 +++++++++++++++++++++++++------ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/git_cl.py b/git_cl.py index 9bc7d52e7..eb66039a7 100755 --- a/git_cl.py +++ b/git_cl.py @@ -589,6 +589,7 @@ class Settings(object): self.updated = False self.is_gerrit = None self.squash_gerrit_uploads = None + self.gerrit_skip_ensure_authenticated = None self.git_editor = None self.project = None self.force_https_commit_url = None @@ -771,6 +772,15 @@ class Settings(object): error_ok=True).strip() == 'true') return self.squash_gerrit_uploads + def GetGerritSkipEnsureAuthenticated(self): + """Return True if EnsureAuthenticated should not be done for Gerrit + uploads.""" + if self.gerrit_skip_ensure_authenticated is None: + self.gerrit_skip_ensure_authenticated = ( + RunGit(['config', '--bool', 'gerrit.skip_ensure_authenticated'], + error_ok=True).strip() == 'true') + return self.gerrit_skip_ensure_authenticated + def GetGitEditor(self): """Return the editor specified in the git config, or None if none is.""" if self.git_editor is None: @@ -2022,6 +2032,10 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): def EnsureAuthenticated(self, force): """Best effort check that user is authenticated with Gerrit server.""" + if settings.GetGerritSkipEnsureAuthenticated(): + # For projects with unusual authentication schemes. + # See http://crbug.com/603378. + return # Lazy-loader to identify Gerrit and Git hosts. if gerrit_util.GceAuthenticator.is_gce(): return @@ -2717,6 +2731,10 @@ def LoadCodereviewSettingsFromFile(fileobj): RunGit(['config', 'gerrit.squash-uploads', keyvals['GERRIT_SQUASH_UPLOADS']]) + if 'GERRIT_SKIP_ENSURE_AUTHENTICATED' in keyvals: + RunGit(['config', 'gerrit.skip_ensure_authenticated', + keyvals['GERRIT_SKIP_ENSURE_AUTHENTICATED']]) + if 'PUSH_URL_CONFIG' in keyvals and 'ORIGIN_URL_CONFIG' in keyvals: #should be of the form #PUSH_URL_CONFIG: url.ssh://gitrw.chromium.org.pushinsteadof diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 80021904d..ed24c84ca 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -527,6 +527,7 @@ class TestGitCl(TestCase): reviewers, private=False): """Generic reviewer test framework.""" + self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) try: similarity = upload_args[upload_args.index('--similarity')+1] except ValueError: @@ -660,6 +661,7 @@ class TestGitCl(TestCase): 'Must specify reviewers to send email.\n', stderr.getvalue()) def test_dcommit(self): + self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) self.calls = ( self._dcommit_calls_1() + self._git_sanity_checks('fake_ancestor_sha', 'working') + @@ -668,6 +670,7 @@ class TestGitCl(TestCase): git_cl.main(['dcommit']) def test_dcommit_bypass_hooks(self): + self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) self.calls = ( self._dcommit_calls_1() + self._dcommit_calls_bypassed() + @@ -676,8 +679,12 @@ class TestGitCl(TestCase): @classmethod - def _gerrit_ensure_auth_calls(cls, issue=None): - calls = [] + def _gerrit_ensure_auth_calls(cls, issue=None, skip_auth_check=False): + cmd = ['git', 'config', '--bool', 'gerrit.skip_ensure_authenticated'] + if skip_auth_check: + return [((cmd, ), 'true')] + + calls = [((cmd, ), '', subprocess2.CalledProcessError(1, '', '', '', ''))] if issue: calls.extend([ ((['git', 'config', 'branch.master.gerritserver'],), ''), @@ -849,6 +856,7 @@ class TestGitCl(TestCase): issue=None): """Generic gerrit upload test framework.""" reviewers = reviewers or [] + self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) self.mock(git_cl.gerrit_util, "CookiesAuthenticator", CookiesAuthenticatorMockFactory(same_cookie='same_cred')) self.calls = self._gerrit_base_calls(issue=issue) @@ -926,6 +934,7 @@ class TestGitCl(TestCase): issue=123456) def test_upload_branch_deps(self): + self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) def mock_run_git(*args, **_kwargs): if args[0] == ['for-each-ref', '--format=%(refname:short) %(upstream:short)', @@ -1094,6 +1103,7 @@ class TestGitCl(TestCase): self.assertNotEqual(git_cl.main(['diff']), 0) def _patch_common(self, is_gerrit=False, force_codereview=False): + self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) self.mock(git_cl._RietveldChangelistImpl, 'GetMostRecentPatchset', lambda x: '60001') self.mock(git_cl._RietveldChangelistImpl, 'GetPatchSetDiff', @@ -1265,11 +1275,13 @@ class TestGitCl(TestCase): def test_checkout_not_found(self): """Tests git cl checkout .""" + self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) self.calls = self._checkout_calls() self.assertEqual(1, git_cl.main(['checkout', '99999'])) def test_checkout_no_branch_issues(self): """Tests git cl checkout .""" + self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) self.calls = [ ((['git', 'config', '--local', '--get-regexp', 'branch\\..*\\.rietveldissue'], ), '', @@ -1281,17 +1293,18 @@ class TestGitCl(TestCase): ] self.assertEqual(1, git_cl.main(['checkout', '99999'])) - def _test_gerrit_ensure_authenticated_common(self, auth): + def _test_gerrit_ensure_authenticated_common(self, auth, + skip_auth_check=False): self.mock(git_cl.gerrit_util, 'CookiesAuthenticator', CookiesAuthenticatorMockFactory(hosts_with_creds=auth)) self.mock(git_cl, 'DieWithError', lambda msg: self._mocked_call(['DieWithError', msg])) self.mock(git_cl, 'ask_for_data', lambda msg: self._mocked_call(['ask_for_data', msg])) - self.calls = [ - ((['git', 'symbolic-ref', 'HEAD'],), 'master') - ] + self._gerrit_ensure_auth_calls() + self.calls = self._gerrit_ensure_auth_calls(skip_auth_check=skip_auth_check) cl = git_cl.Changelist(codereview='gerrit') + cl.branch = 'master' + cl.branchref = 'refs/heads/master' cl.lookedup_issue = True return cl @@ -1309,6 +1322,7 @@ class TestGitCl(TestCase): self.assertIsNone(cl.EnsureAuthenticated(force=False)) def test_gerrit_ensure_authenticated_conflict(self): + self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) cl = self._test_gerrit_ensure_authenticated_common(auth={ 'chromium.googlesource.com': 'one', 'chromium-review.googlesource.com': 'other', @@ -1325,6 +1339,11 @@ class TestGitCl(TestCase): }) self.assertIsNone(cl.EnsureAuthenticated(force=False)) + def test_gerrit_ensure_authenticated_skipped(self): + cl = self._test_gerrit_ensure_authenticated_common( + auth={}, skip_auth_check=True) + self.assertIsNone(cl.EnsureAuthenticated(force=False)) + def test_cmd_set_commit_rietveld(self): self.mock(git_cl._RietveldChangelistImpl, 'SetFlag', lambda _, f, v: self._mocked_call(['SetFlag', f, v]))