cli: modify way in which empty git repos are identified (#916)

Small tweak on how empty Git repos are identified. Uses `git rev-list 
-n 1 -all` to check if there are commits in the currently checked out 
branch.

The value of the `GIT_TAG` environment variable when the currently 
checked out commit is not tagged is now an empty string, instead of 
`None`.

A small fix to the try-catch statement in `HostRunner._exec_cmd` 
function is also included.

src/popper/runner.py

@@ -226,7 +226,8 @@ def _prepare_environment(self, step, env={}):
                     "GIT_BRANCH": self._config.git_branch,
                     "GIT_SHA_SHORT": self._config.git_sha_short,
                     "GIT_REMOTE_ORIGIN_URL": self._config.git_remote_origin_url,
-                    "GIT_TAG": self._config.git_tag,
+                    # git_tag might be None, so we use empty string instead
+                    "GIT_TAG": self._config.get("git_tag", ""),
                 }
             )
         return step_env

src/popper/runner_host.py

@@ -63,6 +63,7 @@ def stop_running_tasks(self):
     @staticmethod
     def _exec_cmd(cmd, env=None, cwd=os.getcwd(), pids=set(), logging=True):
         pid = 0
+        ecode = None
         try:
             with Popen(
                 cmd,
@@ -91,7 +92,8 @@ def _exec_cmd(cmd, env=None, cwd=os.getcwd(), pids=set(), logging=True):
 
         except SubprocessError as ex:
             output = ""
-            ecode = ex.returncode
+            if not ecode:
+                ecode = 1
             log.step_info(f"Command '{cmd[0]}' failed with: {ex}")
         except Exception as ex:
             output = ""

src/popper/scm.py

@@ -18,12 +18,19 @@ def new_repo(gitrepo_dir=None):
     """
     if not gitrepo_dir or not os.path.isdir(gitrepo_dir):
         return None
+
+    repo = None
+
     try:
-        return git.Repo(gitrepo_dir, search_parent_directories=True)
+        repo = git.Repo(gitrepo_dir, search_parent_directories=True)
     except git.InvalidGitRepositoryError:
         # Optimistically assume that this is due to .git/ folder not existing
         pass
-    return None
+
+    if not repo or is_empty(repo):
+        return None
+
+    return repo
 
 
 def get_remote_url(repo=None):
@@ -159,26 +166,24 @@ def parse(url):
     return service_url, service, user, repo, step_dir, version
 
 
+def is_empty(repo):
+    """True if the currently checked out branch has no commits."""
+    return repo.git.rev_list("-n 1", "--all") == ""
+
+
 def get_sha(repo, short=None):
     """Returns the commit id for the currently checked out version on the
     given repository object. If short is given, it is interpreted as the number
     of characters from the SHA that get returned. E.g. short=7 returns the
     first 7 characters, otherwise it returns the entire SHA1 string.
     """
-    if not repo:
+    if not repo or is_empty(repo):
         return None
 
-    try:
-        if short:
-            sha = repo.git.rev_parse(repo.head.object.hexsha, short=short)
-        else:
-            sha = repo.git.rev_parse(repo.head.object.hexsha)
-
-    except ValueError:
-        sha = None
-        log.warning(
-            f"Could not obtain commit ID (SHA1) due to the Git repository at {repo.git_dir} being empty."
-        )
+    if short:
+        sha = repo.git.rev_parse(repo.head.object.hexsha, short=short)
+    else:
+        sha = repo.git.rev_parse(repo.head.object.hexsha)
 
     return sha
 
@@ -188,7 +193,7 @@ def get_branch(repo):
     for environment variables commonly used in CI services: TRAVIS_BRANCH,
     GIT_BRANCH (Jenkins), CIRCLE_BRANCH and CI_COMMIT_REF_NAME (Gitlab)
     """
-    if not repo:
+    if not repo or is_empty(repo):
         return None
 
     if not repo.head.is_detached:
@@ -203,11 +208,11 @@ def get_branch(repo):
 
 
 def get_tag(repo):
-    """Get name of tag for current commit. If the repo is in detached head
-    state, it looks for for environment variables commonly used in CI services:
+    """Get name of tag for current commit. If current commit is not tagged,
+    it looks for for environment variables commonly used in CI services:
     TRAVIS_TAG, CIRCLE_TAG, GIT_TAG (Jenkins) and CI_COMMIT_REF_NAME (Gitlab).
     """
-    if not repo:
+    if not repo or is_empty(repo):
         return None
 
     tags = repo.git.tag("--points-at", "HEAD").split()
@@ -220,4 +225,4 @@ def get_tag(repo):
         if tag:
             return tag
 
-    return ""
+    return None

src/test/test_scm.py

@@ -18,6 +18,18 @@ def setUpClass(self):
     def tearDownClass(self):
         log.setLevel("NOTSET")
 
+    def test_empty_repo(self):
+        tempdir = tempfile.mkdtemp()
+        repo = git.Repo.init(tempdir)
+        self.assertTrue(scm.is_empty(repo))
+
+        self.assertIsNone(scm.get_sha(repo))
+        self.assertIsNone(scm.get_branch(repo))
+        self.assertIsNone(scm.get_tag(repo))
+
+        repo = self.mk_repo()
+        self.assertFalse(scm.is_empty(repo))
+
     def test_get_remote_url(self):
         repo = self.mk_repo()
         url = scm.get_remote_url(repo)
@@ -53,16 +65,6 @@ def test_get_branch_in_detached_head_state(self):
         self.assertIsNone(scm.get_sha(None, short=8))
         self.assertIsNone(scm.get_branch(None))
 
-        # drop head commit
-        with self.assertLogs("popper", level="WARNING") as cm:
-            repo.git.update_ref("-d", "HEAD")
-            self.assertEqual(None, scm.get_sha(repo))
-            self.assertEqual(1, len(cm.output))
-            self.assertTrue(
-                f"WARNING:popper:Could not obtain commit ID (SHA1) due to the Git repository at {repo.git_dir} being empty."
-                in cm.output[0]
-            )
-
     def test_get_tag(self):
         self.assertIsNone(scm.get_tag(None))
 
@@ -86,7 +88,7 @@ def test_get_tag(self):
         os.environ.pop("CI_COMMIT_REF_NAME")
 
         # without any of the above, it should be empty
-        self.assertEqual("", scm.get_tag(repo))
+        self.assertIsNone(scm.get_tag(repo))
 
         # test with a tagged commit
         repo = self.mk_repo(tag="foo")