diff --git a/tagbot/action/repo.py b/tagbot/action/repo.py index 2655164..38f60e1 100644 --- a/tagbot/action/repo.py +++ b/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.""" diff --git a/test/action/test_repo.py b/test/action/test_repo.py index 5deac34..c6a744b 100644 --- a/test/action/test_repo.py +++ b/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()