opendev/git-review
Code Issues Proposed changes

Handle http queries below /

Browse Source 
Previously git review could not properly query Gerrit servers via http
if the root of the Gerrit api was below /. This is because it was always
rooting the changes query api at /changes instead of /foo/changes if
hosted at /foo.

To fix this we read the project name from the config so that we can
remove the project name suffix from the urls then append changes/ to the
resulting url to get a properly rooted query url.

Note this was never a problem with ssh because ssh can't be hosted as
some subpath. Everything is rooted with ssh and gerrit.

Change-Id: I46e21dfdbbb7f60aa89a7b028501c0d953ae1d7f
This commit is contained in:
Clark Boylan 2017-06-27 17:23:37 -07:00
parent ef47e1f553
commit f918bf76d2
3 changed files with 51 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
git_review/cmd.py
View File

@ -180,7 +180,7 @@ def git_credentials(url):
"""Return credentials using git credential or None."""
cmd = 'git', 'credential', 'fill'
stdin = 'url=%s' % url
rc, out = run_command_status(*cmd, stdin=stdin)
rc, out = run_command_status(*cmd, stdin=stdin.encode('utf-8'))
if rc:
return None
data = dict(l.split('=', 1) for l in out.splitlines())
@ -558,29 +558,49 @@ def parse_gerrit_ssh_params_from_git_url(git_url):
return (hostname, username, port, project_name)
def query_reviews(remote_url, change=None, current_patch_set=True,
exception=CommandFailed, parse_exc=Exception):
def query_reviews(remote_url, project=None, change=None,
current_patch_set=True, exception=CommandFailed,
parse_exc=Exception):
if remote_url.startswith('http://') or remote_url.startswith('https://'):
query = query_reviews_over_http
else:
query = query_reviews_over_ssh
return query(remote_url,
project=project,
change=change,
current_patch_set=current_patch_set,
exception=exception,
parse_exc=parse_exc)
def query_reviews_over_http(remote_url, change=None, current_patch_set=True,
exception=CommandFailed, parse_exc=Exception):
url = urljoin(remote_url, '/changes/')
def query_reviews_over_http(remote_url, project=None, change=None,
current_patch_set=True, exception=CommandFailed,
parse_exc=Exception):
if project:
# Remove any trailing .git suffixes for project to url comparison
clean_url = os.path.splitext(remote_url)[0]
clean_project = os.path.splitext(project)[0]
if clean_url.endswith(clean_project):
# Get the "root" url for gerrit by removing the project from the
# url. For example:
# https://example.com/foo/project.git gets truncated to
# https://example.com/foo/ regardless of whether or not none,
# either, or both of the remote_url or project strings end
# with .git.
remote_url = clean_url[:-len(clean_project)]
url = urljoin(remote_url, 'changes/')
if change:
if current_patch_set:
url += '?q=%s&o=CURRENT_REVISION' % change
else:
url += '?q=%s&o=ALL_REVISIONS' % change
else:
project_name = re.sub(r"^/|(\.git$)", "", urlparse(remote_url).path)
if project:
project_name = re.sub(r"^/|(\.git$)", "",
project)
else:
project_name = re.sub(r"^/|(\.git$)", "",
urlparse(remote_url).path)
params = urlencode({'q': 'project:%s status:open' % project_name})
url += '?' + params
@ -611,8 +631,9 @@ def query_reviews_over_http(remote_url, change=None, current_patch_set=True,
return reviews
def query_reviews_over_ssh(remote_url, change=None, current_patch_set=True,
exception=CommandFailed, parse_exc=Exception):
def query_reviews_over_ssh(remote_url, project=None, change=None,
current_patch_set=True, exception=CommandFailed,
parse_exc=Exception):
(hostname, username, port, project_name) = \
parse_gerrit_ssh_params_from_git_url(remote_url)
@ -1074,11 +1095,12 @@ class ReviewsPrinter(object):
print("Found %d items for review" % total_reviews)
def list_reviews(remote, with_topic=False):
def list_reviews(remote, project, with_topic=False):
remote_url = get_remote_url(remote)
reviews = []
for r in query_reviews(remote_url,
project=project,
exception=CannotQueryOpenChangesets,
parse_exc=CannotParseOpenChangesets):
reviews.append(Review(r))
@ -1163,7 +1185,7 @@ class BranchTrackingMismatch(GitReviewException):
EXIT_CODE = 70
def fetch_review(review, masterbranch, remote):
def fetch_review(review, masterbranch, remote, project):
remote_url = get_remote_url(remote)
review_arg = review
@ -1171,6 +1193,7 @@ def fetch_review(review, masterbranch, remote):
current_patch_set = patchset_number is None
review_infos = query_reviews(remote_url,
project=project,
change=review,
current_patch_set=current_patch_set,
exception=CannotQueryPatchSet,
@ -1556,7 +1579,8 @@ def _main():
branch, remote, options.rebase)
return
local_branch, remote_branch = fetch_review(options.changeidentifier,
branch, remote)
branch, remote,
config['project'])
if options.download:
checkout_review(local_branch, remote, remote_branch)
else:
@ -1569,7 +1593,7 @@ def _main():
return
elif options.list:
with_topic = options.list > 1
list_reviews(remote, with_topic=with_topic)
list_reviews(remote, config['project'], with_topic=with_topic)
return
if options.custom_script:

2
git_review/tests/test_git_review.py
View File

@ -486,6 +486,8 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase):
'test/test_project2')
self._run_git('fetch', project2_uri, 'HEAD')
self._run_git('checkout', 'FETCH_HEAD')
# We have to rewrite the .gitreview file after this checkout.
self._create_gitreview_file()
self._simple_change('project2: test1', 'project2: change1, open')
self._run_git('push', project2_uri, 'HEAD:refs/for/master')

18
git_review/tests/test_unit.py
View File

@ -110,7 +110,7 @@ class GitReviewConsole(testtools.TestCase, fixtures.TestWithFixtures):
mock_query.return_value = self.reviews
with mock.patch('sys.stdout', new_callable=io.StringIO) as output:
cmd.list_reviews(None)
cmd.list_reviews(None, None)
console_output = output.getvalue().split('\n')
self.assertEqual(
@ -126,7 +126,7 @@ class GitReviewConsole(testtools.TestCase, fixtures.TestWithFixtures):
mock_query.return_value = self.reviews
with mock.patch('sys.stdout', new_callable=io.StringIO) as output:
cmd.list_reviews(None, with_topic=True)
cmd.list_reviews(None, None, with_topic=True)
console_output = output.getvalue().split('\n')
self.assertEqual(
@ -142,7 +142,7 @@ class GitReviewConsole(testtools.TestCase, fixtures.TestWithFixtures):
mock_query.return_value = self.reviews
with mock.patch('sys.stdout', new_callable=io.StringIO) as output:
cmd.list_reviews(None)
cmd.list_reviews(None, None)
console_output = output.getvalue().split('\n')
wrapper = textwrap.TextWrapper(replace_whitespace=False,
@ -295,8 +295,10 @@ class GitReviewUnitTest(testtools.TestCase):
url = 'http://user@gerrit.example.com'
cmd.run_http_exc(FakeException, url)
# This gets encoded to utf8 which means the type passed down
# is bytes.
mock_run.assert_called_once_with('git', 'credential', 'fill',
stdin='url=%s' % url)
stdin=b'url=%s' % url.encode('utf-8'))
calls = [mock.call(url), mock.call(url, auth=('user', 'pass'))]
mock_get.assert_has_calls(calls)
@ -311,8 +313,10 @@ class GitReviewUnitTest(testtools.TestCase):
self.fails('Exception expected')
except FakeException as err:
self.assertEqual(cmd.http_code_2_return_code(401), err.code)
# This gets encoded to utf8 which means the type passed down
# is bytes.
mock_run.assert_called_once_with('git', 'credential', 'fill',
stdin='url=%s' % url)
stdin=b'url=%s' % url.encode('utf-8'))
calls = [mock.call(url), mock.call(url, auth=('user', 'pass'))]
mock_get.assert_has_calls(calls)
@ -327,8 +331,10 @@ class GitReviewUnitTest(testtools.TestCase):
self.fails('Exception expected')
except FakeException as err:
self.assertEqual(cmd.http_code_2_return_code(401), err.code)
# This gets encoded to utf8 which means the type passed down
# is bytes.
mock_run.assert_called_once_with('git', 'credential', 'fill',
stdin='url=%s' % url)
stdin=b'url=%s' % url.encode('utf-8'))
mock_get.assert_called_once_with(url)
@mock.patch('sys.argv', ['argv0', '--track', 'branch'])