From b577b0f9e15d519e99f329de83c1785bef4d3910 Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Nov 07 2018 03:02:40 +0000
Subject: Refactor SCM.get_latest


git-ls-remote accepts paramter refs to just return refs for specific
ones. This refactor uses this parameter to avoid handling full list of
refs from remote repository.

In original code, option --exit-code is passed to git-ls-remote, which
would cause command return exit code 2 when no ref is found from remote
repository. SCM._run raises an error if that happens, but the raised
error is not handled. This refactor catches this error to ensure the
original behavior happens, that is if a ref is not found, treat it as a
commit hash and call SCM.get_full_commit_hash.

Signed-off-by: Chenxiong Qi <cqi@redhat.com>

---

diff --git a/module_build_service/scm.py b/module_build_service/scm.py
index 5cdd034..b678756 100644
--- a/module_build_service/scm.py
+++ b/module_build_service/scm.py
@@ -196,31 +196,20 @@ class SCM(object):
             ref = 'master'
         if self.scheme == "git":
             log.debug("Getting/verifying commit hash for %s" % self.repository)
-            # get all the branches on the remote
-            output = SCM._run(["git", "ls-remote", "--exit-code", self.repository])[1]
-            # pair branch names and their latest refs into a dict. The output of the above command
-            # is multiple lines of "bf028e573e7c18533d89c7873a411de92d4d913e	refs/heads/master".
-            # So the dictionary ends up in the format of
-            # {"master": "bf028e573e7c18533d89c7873a411de92d4d913e"...}.
-            branches = {}
-            for branch_and_ref in output.decode('utf-8').strip().split("\n"):
-                # Only look at refs/heads/* and not refs/remotes/origin/*
-                if "refs/heads/" not in branch_and_ref:
-                    continue
-                # This grabs the last bit of text after the last "/", which is the branch name
-                cur_branch = branch_and_ref.split("\t")[-1].split("refs/heads/")[-1]
-                # This grabs the text before the first tab, which is the commit hash
-                cur_ref = branch_and_ref.split("\t")[0]
-                branches[cur_branch] = cur_ref
-            # first check if the branch name is in the repo
-            if ref in branches:
-                return branches[ref]
-            # if the branch is not in the repo it may be a ref.
-            else:
+            try:
+                _, output, _ = SCM._run([
+                    "git", "ls-remote", "--exit-code", self.repository, 'refs/heads/' + ref
+                ])
+            except UnprocessableEntity:
                 # The call below will either return the commit hash as is (if a full one was
                 # provided) or the full commit hash (if a short hash was provided). If ref is not
                 # a commit hash, then this will raise an exception.
                 return self.get_full_commit_hash(commit_hash=ref)
+            else:
+                # git-ls-remote prints output like this, where the first commit
+                # hash is what to return.
+                # bf028e573e7c18533d89c7873a411de92d4d913e	refs/heads/master
+                return output.split()[0].decode('utf-8')
         else:
             raise RuntimeError("get_latest: Unhandled SCM scheme.")
 
diff --git a/tests/test_scm.py b/tests/test_scm.py
index d79901d..e272c63 100644
--- a/tests/test_scm.py
+++ b/tests/test_scm.py
@@ -25,7 +25,6 @@ import shutil
 import tempfile
 
 import pytest
-from mock import patch
 
 import module_build_service.scm
 from module_build_service.errors import ValidationError, UnprocessableEntity
@@ -138,15 +137,3 @@ class TestSCMModule:
         scm = module_build_service.scm.SCM(repo_url)
         with pytest.raises(UnprocessableEntity):
             scm.get_latest('15481faa232d66589e660cc301179867fb00842c9')
-
-    @patch.object(module_build_service.scm.SCM, '_run')
-    def test_get_latest_ignore_origin(self, mock_run):
-        output = b"""\
-58379ef7887cbc91b215bacd32430628c92bc869\tHEAD
-58379ef7887cbc91b215bacd32430628c92bc869\trefs/heads/master
-10a651f39911a07d85fe87fcfe91999545e44ae0\trefs/remotes/origin/master
-"""
-        mock_run.return_value = (0, output, '')
-        scm = module_build_service.scm.SCM(repo_url)
-        commit = scm.get_latest(None)
-        assert commit == '58379ef7887cbc91b215bacd32430628c92bc869'