From 997c20bb379ed5d7b143133dffd955aaa1df4b5e Mon Sep 17 00:00:00 2001 From: Cedric Brandily Date: Mon, 18 Apr 2016 19:26:55 +0200 Subject: [PATCH] Support git without git credential Old versions (typically 1.7.9) of git don't provide git credential subcommand but it's required in some cases by git-review over http(s). This change updates git-review in order to handle correctly such case. Change-Id: I6db5aca67818ae48f1d7d84bcdffff9c0e48767a --- git_review/cmd.py | 14 +++++++++----- git_review/tests/test_unit.py | 29 ++++++++++++++++++++++------- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/git_review/cmd.py b/git_review/cmd.py index 93512427..5f3801d2 100755 --- a/git_review/cmd.py +++ b/git_review/cmd.py @@ -163,11 +163,13 @@ def run_command_exc(klazz, *argv, **env): return output -def git_credentials(klazz, url): - """Get credentials using git credential.""" +def git_credentials(url): + """Return credentials using git credential or None.""" cmd = 'git', 'credential', 'fill' stdin = 'url=%s' % url - out = run_command_exc(klazz, *cmd, stdin=stdin) + rc, out = run_command_status(*cmd, stdin=stdin) + if rc: + return None data = dict(l.split('=', 1) for l in out.splitlines()) return data['username'], data['password'] @@ -192,8 +194,10 @@ def run_http_exc(klazz, url, **env): try: res = requests.get(url, **env) if res.status_code == 401: - env['auth'] = git_credentials(klazz, url) - res = requests.get(url, **env) + creds = git_credentials(url) + if creds: + env['auth'] = creds + res = requests.get(url, **env) except klazz: raise except Exception as err: diff --git a/git_review/tests/test_unit.py b/git_review/tests/test_unit.py index 05113fe1..b4cbbc78 100644 --- a/git_review/tests/test_unit.py +++ b/git_review/tests/test_unit.py @@ -241,7 +241,7 @@ class GitReviewUnitTest(testtools.TestCase): self.assertEqual(255, err.code) mock_get.assert_called_once_with(url) - @mock.patch('git_review.cmd.run_command_exc') + @mock.patch('git_review.cmd.run_command_status') @mock.patch('requests.get', return_value=FakeResponse(200)) def test_run_http_exc_without_auth(self, mock_get, mock_run): url = 'http://user@gerrit.example.com' @@ -250,21 +250,21 @@ class GitReviewUnitTest(testtools.TestCase): self.assertFalse(mock_run.called) mock_get.assert_called_once_with(url) - @mock.patch('git_review.cmd.run_command_exc', - return_value=FAKE_GIT_CREDENTIAL_FILL) + @mock.patch('git_review.cmd.run_command_status', + return_value=(0, FAKE_GIT_CREDENTIAL_FILL)) @mock.patch('requests.get', side_effect=[FakeResponse(401), FakeResponse(200)]) def test_run_http_exc_with_auth(self, mock_get, mock_run): url = 'http://user@gerrit.example.com' cmd.run_http_exc(FakeException, url) - mock_run.assert_called_once_with(mock.ANY, 'git', 'credential', 'fill', + mock_run.assert_called_once_with('git', 'credential', 'fill', stdin='url=%s' % url) calls = [mock.call(url), mock.call(url, auth=('user', 'pass'))] mock_get.assert_has_calls(calls) - @mock.patch('git_review.cmd.run_command_exc', - return_value=FAKE_GIT_CREDENTIAL_FILL) + @mock.patch('git_review.cmd.run_command_status', + return_value=(0, FAKE_GIT_CREDENTIAL_FILL)) @mock.patch('requests.get', return_value=FakeResponse(401)) def test_run_http_exc_with_failing_auth(self, mock_get, mock_run): url = 'http://user@gerrit.example.com' @@ -274,11 +274,26 @@ class GitReviewUnitTest(testtools.TestCase): self.fails('Exception expected') except FakeException as err: self.assertEqual(cmd.http_code_2_return_code(401), err.code) - mock_run.assert_called_once_with(mock.ANY, 'git', 'credential', 'fill', + mock_run.assert_called_once_with('git', 'credential', 'fill', stdin='url=%s' % url) calls = [mock.call(url), mock.call(url, auth=('user', 'pass'))] mock_get.assert_has_calls(calls) + @mock.patch('git_review.cmd.run_command_status', + return_value=(1, '')) + @mock.patch('requests.get', return_value=FakeResponse(401)) + def test_run_http_exc_with_failing_git_creds(self, mock_get, mock_run): + url = 'http://user@gerrit.example.com' + + try: + cmd.run_http_exc(FakeException, url) + self.fails('Exception expected') + except FakeException as err: + self.assertEqual(cmd.http_code_2_return_code(401), err.code) + mock_run.assert_called_once_with('git', 'credential', 'fill', + stdin='url=%s' % url) + mock_get.assert_called_once_with(url) + @mock.patch('sys.argv', ['argv0', '--track', 'branch']) @mock.patch('git_review.cmd.check_remote') @mock.patch('git_review.cmd.resolve_tracking')