Fix changelog generation for backport releases

tagbot/action/changelog.py

@@ -90,6 +90,43 @@ def _previous_release(self, version_tag: str) -> Optional[GitRelease]:
                 prev_ver = ver
         return prev_rel
 
+    def _previous_release_chronological(
+        self, version_tag: str, commit_date: datetime
+    ) -> Optional[GitRelease]:
+        """Get the chronologically previous release."""
+        tag_prefix = self._repo._tag_prefix()
+        i_start = len(tag_prefix)
+        cur_ver = VersionInfo.parse(version_tag[i_start:])
+        prev_rel = None
+        latest_time = datetime.min.replace(tzinfo=timezone.utc)
+        tags = self._repo.get_all_tags()
+
+        for tag_name in tags:
+            if not tag_name.startswith(tag_prefix):
+                continue
+            try:
+                ver = VersionInfo.parse(tag_name[i_start:])
+            except ValueError:
+                continue
+            if ver.prerelease or ver.build:
+                continue
+            if ver > cur_ver or ver == cur_ver:
+                continue
+            # Get the release time
+            try:
+                rel = self._repo._repo.get_release(tag_name)
+                rel_time = _ensure_utc(rel.created_at)
+            except UnknownObjectException:
+                rel_time = _ensure_utc(self._repo._git.time_of_commit(tag_name))
+            if rel_time < commit_date and rel_time > latest_time:
+                prev_rel = type(
+                    "obj",
+                    (object,),
+                    {"tag_name": tag_name, "created_at": rel_time},
+                )()
+                latest_time = rel_time
+        return prev_rel
+
     def _is_backport(self, version: str, tags: Optional[List[str]] = None) -> bool:
         """Determine whether or not the version is a backport."""
         try:
@@ -218,6 +255,17 @@ def _pulls(self, start: datetime, end: datetime) -> List[PullRequest]:
             p for p in self._issues_and_pulls(start, end) if isinstance(p, PullRequest)
         ]
 
+    def _pulls_on_branches(
+        self, start: datetime, end: datetime, branches: List[str]
+    ) -> List[PullRequest]:
+        """Collect PRs in the interval merged into one of the given branches."""
+        branch_set = set(branches)
+        return [
+            p
+            for p in self._issues_and_pulls(start, end)
+            if isinstance(p, PullRequest) and p.base.ref in branch_set
+        ]
+
     def _custom_release_notes(self, version_tag: str) -> Optional[str]:
         """Look up a version's custom release notes."""
         logger.debug("Looking up custom release notes")
@@ -288,29 +336,83 @@ def _format_pull(self, pull: PullRequest) -> Dict[str, object]:
 
     def _collect_data(self, version_tag: str, sha: str) -> Dict[str, object]:
         """Collect data needed to create the changelog."""
-        previous = self._previous_release(version_tag)
-        start = datetime.fromtimestamp(0, timezone.utc)
-        prev_tag = None
-        compare = None
-        if previous:
-            if previous.created_at:
-                start = _ensure_utc(previous.created_at)
-            prev_tag = previous.tag_name
-            compare = f"{self._repo._repo.html_url}/compare/{prev_tag}...{version_tag}"
-        # When the last commit is a PR merge, the commit happens a second or two before
-        # the PR and associated issues are closed.
         commit = self._repo._repo.get_commit(sha)
-        end = _ensure_utc(commit.commit.author.date) + timedelta(minutes=1)
-        logger.debug(f"Previous version: {prev_tag}")
-        logger.debug(f"Start date: {start}")
-        logger.debug(f"End date: {end}")
-        issues = self._issues(start, end)
-        pulls = self._pulls(start, end)
+        try:
+            commit_date = commit.commit.author.date
+        except (AttributeError, TypeError, ValueError) as exc:
+            logger.warning(
+                f"Failed to read commit author date for {sha}: {exc}. "
+                "Falling back to current time."
+            )
+            commit_date = datetime.now(timezone.utc)
+        if commit_date is None:
+            logger.warning(
+                f"Commit author date is None for {sha}. Falling back to current time."
+            )
+            commit_date = datetime.now(timezone.utc)
+        else:
+            commit_date = _ensure_utc(commit_date)
+        end = commit_date + timedelta(minutes=1)
+
+        release_branches = self._repo.branches_of_commit(sha)
+        if release_branches:
+            # For backport releases (commit on a non-default branch):
+            # - compare URL and previous_release use the SemVer predecessor
+            # - PRs are filtered to those merged into the release branch(es),
+            #   windowed from the SemVer predecessor (catches cherry-picks that
+            #   predate the last main-line release)
+            # - Issues use the chronological predecessor's window (no base-branch
+            #   signal available for issues)
+            previous_semver = self._previous_release(version_tag)
+            previous_chrono = self._previous_release_chronological(
+                version_tag, commit_date
+            )
+            prev_tag = previous_semver.tag_name if previous_semver else None
+            compare = (
+                f"{self._repo._repo.html_url}/compare/{prev_tag}...{version_tag}"
+                if prev_tag
+                else None
+            )
+            start_issues = (
+                _ensure_utc(previous_chrono.created_at)
+                if previous_chrono and previous_chrono.created_at
+                else datetime.fromtimestamp(0, timezone.utc)
+            )
+            start_pulls = (
+                _ensure_utc(previous_semver.created_at)
+                if previous_semver and previous_semver.created_at
+                else datetime.fromtimestamp(0, timezone.utc)
+            )
+            logger.debug(f"Backport release branches: {release_branches}")
+            logger.debug(f"Previous version (SemVer): {prev_tag}")
+            logger.debug(f"Start date (issues/chronological): {start_issues}")
+            logger.debug(f"Start date (PRs/SemVer): {start_pulls}")
+            logger.debug(f"End date: {end}")
+            issues = self._issues(start_issues, end)
+            pulls = self._pulls_on_branches(start_pulls, end, release_branches)
+        else:
+            previous = self._previous_release(version_tag)
+            start = datetime.fromtimestamp(0, timezone.utc)
+            prev_tag = None
+            compare = None
+            if previous:
+                if previous.created_at:
+                    start = _ensure_utc(previous.created_at)
+                prev_tag = previous.tag_name
+                compare = (
+                    f"{self._repo._repo.html_url}/compare/{prev_tag}...{version_tag}"
+                )
+            logger.debug(f"Previous version: {prev_tag}")
+            logger.debug(f"Start date: {start}")
+            logger.debug(f"End date: {end}")
+            issues = self._issues(start, end)
+            pulls = self._pulls(start, end)
+
         is_yanked = self._repo.is_version_yanked(version_tag)
         return {
             "compare_url": compare,
             "custom": self._custom_release_notes(version_tag),
-            "backport": self._is_backport(version_tag),
+            "backport": self._is_backport(version_tag) or bool(release_branches),
             "issues": [self._format_issue(i) for i in issues],
             "package": self._repo._project("name"),
             "previous_release": prev_tag,

tagbot/action/repo.py

@@ -1635,3 +1635,23 @@ def commit_sha_of_version(self, version: str) -> Optional[str]:
             return None
         tree = versions[version]["git-tree-sha1"]
         return self._commit_sha_of_tree(tree)
+
+    def branches_of_commit(self, sha: str) -> List[str]:
+        """Return short names of non-default remote branches that contain sha."""
+        try:
+            output = self._git.command("branch", "-r", "--contains", sha)
+            default = f"origin/{self._repo.default_branch}"
+            prefix = "origin/"
+            prefix_len = len(prefix)
+            return [
+                b.strip()[prefix_len:]
+                for b in output.splitlines()
+                if b.strip() and " -> " not in b and b.strip() != default
+            ]
+        except Abort:
+            logger.debug("Failed to get branches for commit", exc_info=True)
+            return []
+
+    def is_backport_commit(self, sha: str) -> bool:
+        """Check if the commit is on a non-default branch."""
+        return bool(self.branches_of_commit(sha))

test/action/test_changelog.py

@@ -306,6 +306,7 @@ def test_collect_data():
     c._repo._repo = Mock(full_name="A/B.jl", html_url="https://github.com/A/B.jl")
     c._repo._project = Mock(return_value="B")
     c._repo.is_version_yanked = Mock(return_value=False)
+    c._repo.branches_of_commit = Mock(return_value=[])
     c._previous_release = Mock(
         side_effect=[
             Mock(tag_name="v1.2.2", created_at=datetime.now(timezone.utc)),
@@ -315,7 +316,6 @@ def test_collect_data():
     c._is_backport = Mock(return_value=False)
     commit = Mock(author=Mock(date=datetime.now(timezone.utc)))
     c._repo._repo.get_commit = Mock(return_value=Mock(commit=commit))
-    # TODO: Put stuff here.
     c._issues = Mock(return_value=[])
     c._pulls = Mock(return_value=[])
     c._custom_release_notes = Mock(return_value="custom")
@@ -337,6 +337,68 @@ def test_collect_data():
     assert data["previous_release"] is None
 
 
+def test_collect_data_backport():
+    c = _changelog()
+    c._repo._repo = Mock(full_name="A/B.jl", html_url="https://github.com/A/B.jl")
+    c._repo._project = Mock(return_value="B")
+    c._repo.is_version_yanked = Mock(return_value=False)
+    c._repo.branches_of_commit = Mock(return_value=["release-1.0"])
+    semver_prev = Mock(
+        tag_name="v1.0.0", created_at=datetime(2023, 1, 1, tzinfo=timezone.utc)
+    )
+    chrono_prev = Mock(
+        tag_name="v2.0.0", created_at=datetime(2023, 3, 1, tzinfo=timezone.utc)
+    )
+    c._previous_release = Mock(return_value=semver_prev)
+    c._previous_release_chronological = Mock(return_value=chrono_prev)
+    c._is_backport = Mock(return_value=True)
+    commit_date = datetime(2023, 4, 1, tzinfo=timezone.utc)
+    commit = Mock(commit=Mock(author=Mock(date=commit_date)))
+    c._repo._repo.get_commit = Mock(return_value=commit)
+    c._issues = Mock(return_value=[])
+    c._pulls_on_branches = Mock(return_value=[])
+    c._custom_release_notes = Mock(return_value=None)
+
+    data = c._collect_data("v1.0.1", "abcdef")
+
+    # compare/previous_release come from SemVer predecessor
+    assert data["previous_release"] == "v1.0.0"
+    assert data["compare_url"] == "https://github.com/A/B.jl/compare/v1.0.0...v1.0.1"
+    assert data["backport"] is True
+
+    end = commit_date + timedelta(minutes=1)
+    # Issues windowed from chronological predecessor
+    c._issues.assert_called_once_with(datetime(2023, 3, 1, tzinfo=timezone.utc), end)
+    # PRs filtered to release branches, windowed from SemVer predecessor
+    c._pulls_on_branches.assert_called_once_with(
+        datetime(2023, 1, 1, tzinfo=timezone.utc), end, ["release-1.0"]
+    )
+
+
+def test_pulls_on_branches():
+    c = _changelog()
+    pr_main = Mock(spec=PullRequest)
+    pr_main.base = Mock(ref="main")
+    pr_release = Mock(spec=PullRequest)
+    pr_release.base = Mock(ref="release-1.0")
+    pr_other = Mock(spec=PullRequest)
+    pr_other.base = Mock(ref="release-2.0")
+    issue = Mock(spec=Issue)
+    c._issues_and_pulls = Mock(return_value=[pr_main, pr_release, pr_other, issue])
+
+    start = datetime(2023, 1, 1, tzinfo=timezone.utc)
+    end = datetime(2023, 12, 1, tzinfo=timezone.utc)
+
+    result = c._pulls_on_branches(start, end, ["release-1.0"])
+    assert result == [pr_release]
+    c._issues_and_pulls.assert_called_once_with(start, end)
+
+    # Multiple branches
+    c._issues_and_pulls.reset_mock()
+    result = c._pulls_on_branches(start, end, ["release-1.0", "release-2.0"])
+    assert result == [pr_release, pr_other]
+
+
 def test_is_backport():
     c = _changelog()
     assert c._is_backport("v1.2.3", ["v1.2.1", "v1.2.2"]) is False
@@ -487,3 +549,46 @@ def test_get():
     c._collect_data = Mock(return_value={"version": "Foo-v1.2.3"})
     assert c.get("Foo-v1.2.3", "abc") == "Foo-v1.2.3"
     c._collect_data.assert_called_once_with("Foo-v1.2.3", "abc")
+
+
+def test_previous_release_chronological():
+    """Test finding chronologically previous releases."""
+    c = _changelog()
+    c._repo.get_all_tags = Mock(return_value=["v1.0.0", "v2.0.0", "v1.5.0"])
+    c._repo._repo.get_release = Mock()
+    c._repo._git.time_of_commit = Mock()
+
+    # Mock releases with different dates
+    early_date = datetime(2023, 1, 1, tzinfo=timezone.utc)
+    middle_date = datetime(2023, 6, 1, tzinfo=timezone.utc)
+    late_date = datetime(2023, 12, 1, tzinfo=timezone.utc)
+
+    # v1.0.0 released early
+    rel1 = Mock()
+    rel1.created_at = early_date
+    # v1.5.0 released middle
+    rel2 = Mock()
+    rel2.created_at = middle_date
+
+    # For v2.0.0, should find v1.5.0 (latest before late_date)
+    c._repo._repo.get_release.side_effect = [rel1, rel2]  # v1.0.0, v1.5.0
+    result = c._previous_release_chronological("v2.0.0", late_date)
+    assert result.tag_name == "v1.5.0"
+    assert result.created_at == middle_date
+
+    # No previous release before commit_date
+    early_commit = datetime(2022, 1, 1, tzinfo=timezone.utc)
+    c._repo._repo.get_release.side_effect = [rel1]  # only v1.0.0 < v1.5.0
+    result = c._previous_release_chronological("v1.5.0", early_commit)
+    assert result is None
+
+    # Skip versions >= current (no get_release calls expected)
+    c._repo._repo.get_release.side_effect = []
+    result = c._previous_release_chronological("v1.0.0", late_date)
+    assert result is None
+
+    # Handle prerelease versions (should be skipped)
+    c._repo.get_all_tags.return_value = ["v1.0.0", "v1.5.0-alpha", "v1.5.0"]
+    c._repo._repo.get_release.side_effect = [rel1, rel2]  # v1.0.0, v1.5.0
+    result = c._previous_release_chronological("v2.0.0", late_date)
+    assert result.tag_name == "v1.5.0"  # Should skip v1.5.0-alpha

test/action/test_repo.py

@@ -1446,3 +1446,70 @@ def test_is_version_yanked(mock_github):
     # Empty cache (package not registered)
     r._Repo__versions_toml_cache = {}
     assert r.is_version_yanked("v1.0.0") is False
+
+
+def test_is_backport_commit():
+    """Test detection of backport commits based on branch containment."""
+    r = _repo()
+    r._repo = Mock(default_branch="main")
+    r._git = Mock()
+
+    # Commit on default branch only
+    r._git.command.return_value = "  origin/main\n"
+    assert not r.is_backport_commit("abc123")
+
+    # Commit on default branch and another branch
+    r._git.command.return_value = "  origin/main\n  origin/release-1.0\n"
+    assert r.is_backport_commit("abc123")
+
+    # Commit on non-default branch only
+    r._git.command.return_value = "  origin/release-1.0\n"
+    assert r.is_backport_commit("abc123")
+
+    # Commit on multiple non-default branches
+    r._git.command.return_value = "  origin/release-1.0\n  origin/hotfix\n"
+    assert r.is_backport_commit("abc123")
+
+    # Git command fails
+    r._git.command.side_effect = Abort("git error")
+    assert not r.is_backport_commit("abc123")
+
+    # Empty output
+    r._git.command.side_effect = None
+    r._git.command.return_value = ""
+    assert not r.is_backport_commit("abc123")
+
+    # Only whitespace
+    r._git.command.return_value = "   \n  \n"
+    assert not r.is_backport_commit("abc123")
+
+    # HEAD symbolic ref line (origin/HEAD -> origin/main) must be filtered out
+    r._git.command.return_value = "  origin/HEAD -> origin/main\n  origin/main\n"
+    assert not r.is_backport_commit("abc123")
+
+
+def test_branches_of_commit():
+    """Test extracting non-default branch names containing a commit."""
+    r = _repo()
+    r._repo = Mock(default_branch="main")
+    r._git = Mock()
+
+    # Default branch only → empty
+    r._git.command.return_value = "  origin/main\n"
+    assert r.branches_of_commit("abc123") == []
+
+    # One non-default branch
+    r._git.command.return_value = "  origin/main\n  origin/release-1.0\n"
+    assert r.branches_of_commit("abc123") == ["release-1.0"]
+
+    # Multiple non-default branches, no default
+    r._git.command.return_value = "  origin/release-1.0\n  origin/hotfix\n"
+    assert sorted(r.branches_of_commit("abc123")) == ["hotfix", "release-1.0"]
+
+    # HEAD symbolic ref filtered out
+    r._git.command.return_value = "  origin/HEAD -> origin/main\n  origin/main\n"
+    assert r.branches_of_commit("abc123") == []
+
+    # Git command fails → empty list
+    r._git.command.side_effect = Abort("git error")
+    assert r.branches_of_commit("abc123") == []