From 5a7ced5b7d48d7e6ca1718727f607fa77f1fe6cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Mon, 25 Feb 2019 14:12:21 +0100 Subject: [PATCH] util: Refactor resolving git url MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split out a smaller function that receives a git repo URL and ref and returns the commit id. The caching resolver class is modified to use this second function if branch is given to it. The new function checks if the ref received already looks like a commit ID, and if so it does not attempt to do anything with it. Signed-off-by: Lubomír Sedlář --- pungi/util.py | 56 +++++++++++++++++++++++++++++----------------- tests/test_util.py | 26 ++++++++++++++++++--- 2 files changed, 59 insertions(+), 23 deletions(-) diff --git a/pungi/util.py b/pungi/util.py index 83c07192..fa7dd9d7 100644 --- a/pungi/util.py +++ b/pungi/util.py @@ -250,6 +250,32 @@ class GitUrlResolveError(RuntimeError): pass +def resolve_git_ref(repourl, ref): + """Resolve a reference in a Git repo to a commit. + + Raises RuntimeError if there was an error. Most likely cause is failure to + run git command. + """ + if re.match(r"^[a-f0-9]{40}$", ref): + # This looks like a commit ID already. + return ref + + _, output = git_ls_remote(repourl, ref) + + lines = [line for line in output.split('\n') if line] + if len(lines) == 0: + # Branch does not exist in remote repo + raise GitUrlResolveError( + "Failed to resolve %s: ref does not exist in remote repo" % repourl + ) + if len(lines) != 1: + # This should never happen. HEAD can not match multiple commits in a + # single repo, and there can not be a repo without a HEAD. + raise GitUrlResolveError("Failed to resolve %s", repourl) + + return lines[0].split()[0] + + def resolve_git_url(url): """Given a url to a Git repo specifying HEAD or origin/ as a ref, replace that specifier with actual SHA1 of the commit. @@ -269,20 +295,8 @@ def resolve_git_url(url): scheme = r.scheme.replace('git+', '') baseurl = urllib.parse.urlunsplit((scheme, r.netloc, r.path, '', '')) - _, output = git_ls_remote(baseurl, ref) + fragment = resolve_git_ref(baseurl, ref) - lines = [line for line in output.split('\n') if line] - if len(lines) == 0: - # Branch does not exist in remote repo - raise GitUrlResolveError( - "Failed to resolve %s: ref does not exist in remote repo" % url - ) - if len(lines) != 1: - # This should never happen. HEAD can not match multiple commits in a - # single repo, and there can not be a repo without a HEAD. - raise GitUrlResolveError("Failed to resolve %s", url) - - fragment = lines[0].split()[0] result = urllib.parse.urlunsplit((r.scheme, r.netloc, r.path, r.query, fragment)) if '?#' in url: # The urllib library drops empty query string. This hack puts it back in. @@ -298,17 +312,19 @@ class GitUrlResolver(object): self.offline = offline self.cache = {} - def __call__(self, url): + def __call__(self, url, branch=None): if self.offline: return url - if url not in self.cache: + key = (url, branch) + if key not in self.cache: try: - self.cache[url] = resolve_git_url(url) + res = resolve_git_ref(url, branch) if branch else resolve_git_url(url) + self.cache[key] = res except GitUrlResolveError as exc: - self.cache[url] = exc - if isinstance(self.cache[url], GitUrlResolveError): - raise self.cache[url] - return self.cache[url] + self.cache[key] = exc + if isinstance(self.cache[key], GitUrlResolveError): + raise self.cache[key] + return self.cache[key] # fomat: {arch|*: [data]} diff --git a/tests/test_util.py b/tests/test_util.py index 34023a47..5a65df2c 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -43,6 +43,10 @@ class TestGitRefResolver(unittest.TestCase): run.assert_called_once_with(['git', 'ls-remote', 'https://git.example.com/repo.git', 'refs/heads/f24'], universal_newlines=True) + def test_resolve_ref_with_commit_id(self): + ref = util.resolve_git_ref("https://git.example.com/repo.git", "a" * 40) + self.assertEqual(ref, "a" * 40) + @mock.patch('pungi.util.run') def test_resolve_missing_spec(self, run): url = util.resolve_git_url('https://git.example.com/repo.git') @@ -121,18 +125,34 @@ class TestGitRefResolver(unittest.TestCase): ) self.assertEqual(mock_resolve.call_args_list, []) + @mock.patch("pungi.util.resolve_git_ref") @mock.patch("pungi.util.resolve_git_url") - def test_resolver_caches_calls(self, mock_resolve): + def test_resolver_caches_calls(self, mock_resolve_url, mock_resolve_ref): url1 = "http://example.com/repo.git#HEAD" url2 = "http://example.com/repo.git#master" - mock_resolve.side_effect = ["1", "2"] + url3 = "http://example.com/repo.git" + ref1 = "foo" + ref2 = "bar" + mock_resolve_url.side_effect = ["1", "2"] + mock_resolve_ref.side_effect = ["cafe", "beef"] resolver = util.GitUrlResolver() self.assertEqual(resolver(url1), "1") self.assertEqual(resolver(url1), "1") + self.assertEqual(resolver(url3, ref1), "cafe") + self.assertEqual(resolver(url3, ref2), "beef") self.assertEqual(resolver(url2), "2") + self.assertEqual(resolver(url3, ref1), "cafe") self.assertEqual(resolver(url1), "1") + self.assertEqual(resolver(url3, ref2), "beef") self.assertEqual(resolver(url2), "2") - self.assertEqual(mock_resolve.call_args_list, [mock.call(url1), mock.call(url2)]) + self.assertEqual(resolver(url3, ref2), "beef") + self.assertEqual( + mock_resolve_url.call_args_list, [mock.call(url1), mock.call(url2)] + ) + self.assertEqual( + mock_resolve_ref.call_args_list, + [mock.call(url3, ref1), mock.call(url3, ref2)], + ) @mock.patch("pungi.util.resolve_git_url") def test_resolver_caches_failure(self, mock_resolve):