Merge #788 Fix bug and clean up scm.get_latest

module_build_service/scm.py

@@ -178,31 +178,37 @@ class SCM(object):
             raise RuntimeError("checkout: Unhandled SCM scheme.")
         return self.sourcedir
 
-    def get_latest(self, branch='master'):
-        """Get the latest commit ID.
+    def get_latest(self, ref='master'):
+        """ Get the latest commit hash based on the provided git ref
 
-        :returns: str -- the latest commit ID, e.g. the git $BRANCH HEAD
+        :param ref: a string of a git ref (either a branch or commit hash)
+        :returns: a string of the latest commit hash
         :raises: RuntimeError
         """
         if self.scheme == "git":
             log.debug("Getting/verifying commit hash for %s" % self.repository)
-            # check all branches on the remote
+            # get all the 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")]
+            # 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 = {}
-            # 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]
+            for branch_and_ref in output.strip().split("\n"):
+                # This grabs the last bit of text after the last "/", which is the branch name
+                cur_branch = branch_and_ref.split("\t")[-1].split("/")[-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 branch in branches:
-                return branches[branch]
+            if ref in branches:
+                return branches[ref]
             # 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
+                # 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:
             raise RuntimeError("get_latest: Unhandled SCM scheme.")
 
@@ -227,7 +233,7 @@ class SCM(object):
             td = None
             try:
                 td = tempfile.mkdtemp()
-                SCM._run(['git', 'clone', '-q', self.repository, td])
+                SCM._run(['git', 'clone', '-q', self.repository, td, '--bare'])
                 output = SCM._run(
                     ['git', 'rev-parse', commit_to_check], chdir=td)[1]
             finally:
@@ -237,7 +243,7 @@ class SCM(object):
             if output:
                 return str(output.strip('\n'))
 
-            raise RuntimeError(
+            raise UnprocessableEntity(
                 'The full commit hash of "{0}" for "{1}" could not be found'
                 .format(commit_hash, self.repository))
         else:

module_build_service/utils.py

@@ -614,7 +614,7 @@ def _scm_get_latest(pkg):
         # to the specific commit available at the time of
         # submission (now).
         pkgref = module_build_service.scm.SCM(
-            pkg.repository).get_latest(branch=pkg.ref)
+            pkg.repository).get_latest(pkg.ref)
     except Exception as e:
         log.exception(e)
         return {

tests/test_build/test_build.py

@@ -80,8 +80,8 @@ class FakeSCM(object):
 
         return self.sourcedir
 
-    def get_latest(self, branch='master'):
-        return branch
+    def get_latest(self, ref='master'):
+        return ref
 
     def get_module_yaml(self):
         return path.join(self.sourcedir, self.name + ".yaml")

tests/test_scm.py

@@ -53,7 +53,7 @@ class TestSCMModule(unittest.TestCase):
     def test_local_get_latest_is_sane(self):
         """ See that a hash is returned by scm.get_latest. """
         scm = module_build_service.scm.SCM(repo_path)
-        latest = scm.get_latest(branch='master')
+        latest = scm.get_latest('master')
         target = '5481faa232d66589e660cc301179867fb00842c9'
         assert latest == target, "%r != %r" % (latest, target)
 
@@ -66,7 +66,7 @@ class TestSCMModule(unittest.TestCase):
         assert scm.scheme == 'git', scm.scheme
         fname = tempfile.mktemp(suffix='mbs-scm-test')
         try:
-            scm.get_latest(branch='master; touch %s' % fname)
+            scm.get_latest('master; touch %s' % fname)
         except UnprocessableEntity:
             assert not os.path.exists(fname), "%r exists!  Vulnerable." % fname
 
@@ -121,22 +121,22 @@ class TestSCMModule(unittest.TestCase):
     @raises(UnprocessableEntity)
     def test_get_latest_incorect_component_branch(self):
         scm = module_build_service.scm.SCM(repo_path)
-        scm.get_latest(branch='foobar')
+        scm.get_latest('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)
+        commit = scm.get_latest(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)
+        commit = scm.get_latest(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')
+        scm.get_latest('15481faa232d66589e660cc301179867fb00842c9')

tests/test_utils/test_utils.py

@@ -69,8 +69,8 @@ class FakeSCM(object):
 
         return self.sourcedir
 
-    def get_latest(self, branch='master'):
-        return self.commit if self.commit else branch
+    def get_latest(self, ref='master'):
+        return self.commit if self.commit else ref
 
     def get_module_yaml(self):
         return path.join(self.sourcedir, self.name + ".yaml")
@@ -241,8 +241,8 @@ class TestUtils(unittest.TestCase):
             'f25': '76f9d8c8e87eed0aab91034b01d3d5ff6bd5b4cb'}
         original_refs = ["f23", "f24", "f25"]
 
-        def mocked_get_latest(branch="master"):
-            return hashes_returned[branch]
+        def mocked_get_latest(ref="master"):
+            return hashes_returned[ref]
 
         mocked_scm.return_value.get_latest = mocked_get_latest
         mmd = modulemd.ModuleMetadata()
@@ -640,8 +640,8 @@ class TestUtils(unittest.TestCase):
                 'f25': '4ceea43add2366d8b8c5a622a2fb563b625b9abf',
                 'f24': 'fbed359411a1baa08d4a88e0d12d426fbf8f602c'}
 
-            def mocked_get_latest(branch="master"):
-                return hashes_returned[branch]
+            def mocked_get_latest(ref="master"):
+                return hashes_returned[ref]
 
             mocked_scm.return_value.get_latest = mocked_get_latest
 

tests/test_views/test_views.py

@@ -105,8 +105,8 @@ class FakeSCM(object):
 
         return self.sourcedir
 
-    def get_latest(self, branch='master'):
-        return hashlib.sha1(branch).hexdigest()[:10]
+    def get_latest(self, ref='master'):
+        return hashlib.sha1(ref).hexdigest()[:10]
 
     def get_module_yaml(self):
         return path.join(self.sourcedir, self.name + ".yaml")