From bf2369e8e034f7d2575c61932392a9c2bddcbc8b Mon Sep 17 00:00:00 2001 From: Martin Curlej Date: Thu, 9 Nov 2017 12:46:58 +0100 Subject: [PATCH] Fixed issue with getting the latest ref Signed-off-by: Martin Curlej --- module_build_service/scm.py | 27 ++++++++++++++++++--------- tests/test_scm.py | 29 +++++++++++++++++++++++++++-- tests/test_views/test_views.py | 2 +- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/module_build_service/scm.py b/module_build_service/scm.py index 4098197a..2ef18606 100644 --- a/module_build_service/scm.py +++ b/module_build_service/scm.py @@ -125,7 +125,7 @@ class SCM(object): return None @staticmethod - @module_build_service.utils.retry(wait_on=RuntimeError) + @module_build_service.utils.retry(wait_on=UnprocessableEntity) def _run(cmd, chdir=None, log_stdout=False): proc = sp.Popen(cmd, stdout=sp.PIPE, stderr=sp.PIPE, cwd=chdir) stdout, stderr = proc.communicate() @@ -134,7 +134,7 @@ class SCM(object): if stderr: log.warning(stderr) if proc.returncode != 0: - raise RuntimeError("Failed on %r, retcode %r, out %r, err %r" % ( + raise UnprocessableEntity("Failed on %r, retcode %r, out %r, err %r" % ( cmd, proc.returncode, stdout, stderr)) return proc.returncode, stdout, stderr @@ -186,13 +186,22 @@ class SCM(object): """ if self.scheme == "git": log.debug("Getting/verifying commit hash for %s" % self.repository) - output = SCM._run(["git", "ls-remote", self.repository, branch])[1] - if output: - self.commit = output.split("\t")[0] - return self.commit - - # Hopefully `branch` is really a commit hash. Code later needs to verify this. - if self.is_available(True): + # check all branches on the remote + output = SCM._run(["git", "ls-remote", "--exit-code", self.repository])[1] + branch_data = [b.split("\t") for b in output.strip().split("\n")] + branches = {} + # pair branch names and their latest refs into a dict + for data in branch_data: + branch_name = data[1].split("/")[-1] + branches[branch_name] = data[0] + # first check if the branch name is in the repo + if branch in branches: + return branches[branch] + # if the branch is not in the repo it may be a ref. + else: + # if the ref does not exist in the repo, _run will raise and UnprocessableEntity + # error. + SCM._run(["git", "fetch", "--dry-run", self.repository, branch]) return branch else: raise RuntimeError("get_latest: Unhandled SCM scheme.") diff --git a/tests/test_scm.py b/tests/test_scm.py index 2f500a2e..01bb50fb 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -65,8 +65,10 @@ class TestSCMModule(unittest.TestCase): scm = module_build_service.scm.SCM(repo_path) assert scm.scheme == 'git', scm.scheme fname = tempfile.mktemp(suffix='mbs-scm-test') - scm.get_latest(branch='master; touch %s' % fname) - assert not os.path.exists(fname), "%r exists! Vulnerable." % fname + try: + scm.get_latest(branch='master; touch %s' % fname) + except UnprocessableEntity: + assert not os.path.exists(fname), "%r exists! Vulnerable." % fname def test_local_extract_name(self): scm = module_build_service.scm.SCM(repo_path) @@ -115,3 +117,26 @@ class TestSCMModule(unittest.TestCase): scm.checkout(self.tempdir) scm.verify() scm.get_module_yaml() + + @raises(UnprocessableEntity) + def test_get_latest_incorect_component_branch(self): + scm = module_build_service.scm.SCM(repo_path) + scm.get_latest(branch='foobar') + + def test_get_latest_component_branch(self): + ref = "5481faa232d66589e660cc301179867fb00842c9" + branch = "master" + scm = module_build_service.scm.SCM(repo_path) + commit = scm.get_latest(branch=branch) + assert commit == ref + + def test_get_latest_component_ref(self): + ref = "5481faa232d66589e660cc301179867fb00842c9" + scm = module_build_service.scm.SCM(repo_path) + commit = scm.get_latest(branch=ref) + assert commit == ref + + @raises(UnprocessableEntity) + def test_get_latest_incorect_component_ref(self): + scm = module_build_service.scm.SCM(repo_path) + scm.get_latest(branch='15481faa232d66589e660cc301179867fb00842c9') diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 04b4b431..97e961c1 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -81,7 +81,7 @@ class FakeSCM(object): self.mocked_scm.return_value.commit = self.commit if get_latest_raise: self.mocked_scm.return_value.get_latest.side_effect = \ - RuntimeError("Failed to get_latest commit") + UnprocessableEntity("Failed to get_latest commit") else: self.mocked_scm.return_value.get_latest = self.get_latest self.mocked_scm.return_value.repository_root = "git://pkgs.stg.fedoraproject.org/modules/"