Fix subdirectory tree SHA lookups in TagBot

tagbot/action/repo.py

@@ -50,8 +50,8 @@
 
     GitlabClient = _GitlabClient
     GitlabUnknown = _GitlabUnknown
-except Exception:
-    # Optional import: ignore errors if .gitlab is not available or fails to import.
+except ImportError:
+    # Optional import: ignore import errors if .gitlab is not available.
     pass
 
 # Build a tuple of UnknownObjectException classes for both GitHub and GitLab
@@ -128,10 +128,10 @@ def __init__(
             self._registry_ssh_key = registry_ssh
             logger.debug("Will access registry via Git clone")
             self._clone_registry = True
-        except Exception:
+        except (GithubException, RequestException) as exc:
             # This is an awful hack to let me avoid properly fixing the tests...
             if "pytest" in sys.modules:
-                logger.warning("'awful hack' in use")
+                logger.warning("'awful hack' in use", exc_info=exc)
                 self._registry = self._gh.get_repo(registry, lazy=True)
                 self._clone_registry = False
             else:
@@ -312,8 +312,7 @@ def _commit_sha_from_registry_pr(self, version: str, tree: str) -> Optional[str]
         # Handle special case of tagging packages in a repo subdirectory, in which
         # case the Julia package tree hash does not match the git commit tree hash
         if self.__subdir:
-            arg = f"{commit.sha}:{self.__subdir}"
-            subdir_tree_hash = self._git.command("rev-parse", arg)
+            subdir_tree_hash = self._subdir_tree_hash(commit.sha, suppress_abort=False)
             if subdir_tree_hash == tree:
                 return cast(str, commit.sha)
             else:
@@ -332,8 +331,15 @@ def _commit_sha_of_tree_from_branch(
     ) -> Optional[str]:
         """Look up the commit SHA of a tree with the given SHA on one branch."""
         for commit in self._repo.get_commits(sha=branch, since=since):
-            if commit.commit.tree.sha == tree:
-                return cast(str, commit.sha)
+            if self.__subdir:
+                subdir_tree_hash = self._subdir_tree_hash(
+                    commit.sha, suppress_abort=True
+                )
+                if subdir_tree_hash == tree:
+                    return cast(str, commit.sha)
+            else:
+                if commit.commit.tree.sha == tree:
+                    return cast(str, commit.sha)
         return None
 
     def _commit_sha_of_tree(self, tree: str) -> Optional[str]:
@@ -351,7 +357,33 @@ def _commit_sha_of_tree(self, tree: str) -> Optional[str]:
         # For a valid tree SHA, the only time that we reach here is when a release
         # has been made long after the commit was made, which is reasonably rare.
         # Fall back to cloning the repo in that case.
-        return self._git.commit_sha_of_tree(tree)
+        if self.__subdir:
+            # For subdirectories, we need to check the subdirectory tree hash.
+            # Limit to 10000 commits to avoid performance issues on large repos.
+            for line in self._git.command(
+                "log", "--all", "--format=%H", "-n", "10000"
+            ).splitlines():
+                subdir_tree_hash = self._subdir_tree_hash(line, suppress_abort=True)
+                if subdir_tree_hash == tree:
+                    return line
+            return None
+        else:
+            return self._git.commit_sha_of_tree(tree)
+
+    def _subdir_tree_hash(
+        self, commit_sha: str, *, suppress_abort: bool
+    ) -> Optional[str]:
+        """Return subdir tree hash for a commit; optionally suppress Abort."""
+        if not self.__subdir:
+            return None
+        arg = f"{commit_sha}:{self.__subdir}"
+        try:
+            return self._git.command("rev-parse", arg)
+        except Abort:
+            if suppress_abort:
+                logger.debug("rev-parse failed while inspecting %s", arg)
+                return None
+            raise
 
     def _commit_sha_of_tag(self, version_tag: str) -> Optional[str]:
         """Look up the commit SHA of a given tag."""

test/action/test_repo.py

@@ -269,6 +269,48 @@ def test_commit_sha_of_tree_from_branch():
     assert r._commit_sha_of_tree_from_branch("master", "tree", since) is None
 
 
+@patch("tagbot.action.repo.logger")
+def test_commit_sha_of_tree_from_branch_subdir(logger):
+    r = _repo(subdir="path/to/package")
+    since = datetime.now(timezone.utc)
+    commits = [Mock(sha="abc"), Mock(sha="sha")]
+    r._repo.get_commits = Mock(return_value=commits)
+    r._git.command = Mock(side_effect=["other", "tree_hash"])
+
+    assert r._commit_sha_of_tree_from_branch("master", "tree_hash", since) == "sha"
+
+    r._repo.get_commits.assert_called_with(sha="master", since=since)
+    r._git.command.assert_has_calls(
+        [
+            call("rev-parse", "abc:path/to/package"),
+            call("rev-parse", "sha:path/to/package"),
+        ]
+    )
+    logger.debug.assert_not_called()
+
+
+@patch("tagbot.action.repo.logger")
+def test_commit_sha_of_tree_from_branch_subdir_rev_parse_failure(logger):
+    r = _repo(subdir="path/to/package")
+    since = datetime.now(timezone.utc)
+    commits = [Mock(sha="abc"), Mock(sha="sha")]
+    r._repo.get_commits = Mock(return_value=commits)
+    r._git.command = Mock(side_effect=[Abort("missing"), "tree_hash"])
+
+    assert r._commit_sha_of_tree_from_branch("master", "tree_hash", since) == "sha"
+
+    r._repo.get_commits.assert_called_with(sha="master", since=since)
+    logger.debug.assert_called_with(
+        "rev-parse failed while inspecting %s", "abc:path/to/package"
+    )
+    r._git.command.assert_has_calls(
+        [
+            call("rev-parse", "abc:path/to/package"),
+            call("rev-parse", "sha:path/to/package"),
+        ]
+    )
+
+
 def test_commit_sha_of_tree():
     r = _repo()
     now = datetime.now(timezone.utc)
@@ -290,6 +332,41 @@ def test_commit_sha_of_tree():
     assert r._commit_sha_of_tree("tree") is None
 
 
+def test_commit_sha_of_tree_subdir_fallback():
+    """Test subdirectory fallback when branch lookups fail."""
+    r = _repo(subdir="path/to/package")
+    now = datetime.now(timezone.utc)
+    r._repo = Mock(default_branch="master")
+    branches = r._repo.get_branches.return_value = [Mock()]
+    branches[0].name = "master"
+    r._lookback = Mock(__rsub__=lambda x, y: now)
+    # Branch lookups return None (fail)
+    r._commit_sha_of_tree_from_branch = Mock(return_value=None)
+    # git log returns commit SHAs
+    r._git.command = Mock(return_value="abc123\ndef456\nghi789")
+    # _subdir_tree_hash called via helper, simulate finding match on second commit
+    with patch.object(r, "_subdir_tree_hash", side_effect=[None, "tree_hash", "other"]):
+        assert r._commit_sha_of_tree("tree_hash") == "def456"
+        # Verify it iterated through commits
+        assert r._subdir_tree_hash.call_count == 2
+
+
+def test_commit_sha_of_tree_subdir_fallback_no_match():
+    """Test subdirectory fallback returns None when no match found."""
+    r = _repo(subdir="path/to/package")
+    now = datetime.now(timezone.utc)
+    r._repo = Mock(default_branch="master")
+    branches = r._repo.get_branches.return_value = [Mock()]
+    branches[0].name = "master"
+    r._lookback = Mock(__rsub__=lambda x, y: now)
+    r._commit_sha_of_tree_from_branch = Mock(return_value=None)
+    r._git.command = Mock(return_value="abc123\ndef456")
+    # No matches found
+    with patch.object(r, "_subdir_tree_hash", return_value=None):
+        assert r._commit_sha_of_tree("tree_hash") is None
+        assert r._subdir_tree_hash.call_count == 2
+
+
 def test_commit_sha_of_tag():
     r = _repo()
     r._repo.get_git_ref = Mock()