#1070 Refactor SCM.get_latest
Merged 8 months ago by mprahl. Opened 8 months ago by cqi.
cqi/fm-orchestrator refactor-SCM.get_latest  into  master

file modified
+10 -21

@@ -196,31 +196,20 @@ 

              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.")

  

file modified
-13

@@ -25,7 +25,6 @@ 

  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 @@ 

          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'

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

rebased onto 14b08c2ef1f4e21e30f7435e0a6fb5dc109d9928

8 months ago

This comment no longer applies.

@cqi, this is a much better approach. Thank you for the PR.

The tests pass locally, so once you remove the stale comment, this is good to merge.

Pretty please pagure-ci rebuild

rebased onto b577b0f

8 months ago

Commit f28766e fixes this pull-request

Pull-Request has been merged by mprahl

8 months ago

Pull-Request has been merged by mprahl

8 months ago