diff --git a/.github/workflows/on_pr_closed.yaml b/.github/workflows/on_pr_closed.yaml index 5dfcb89748..94c49b6561 100644 --- a/.github/workflows/on_pr_closed.yaml +++ b/.github/workflows/on_pr_closed.yaml @@ -81,3 +81,33 @@ jobs: "$PR_NUMBER" \ --remote origin \ --no-dry-run + + complete_sync_changelog: + if: | + github.event.pull_request.merged == true && + contains(github.event.pull_request.labels.*.name, 'type: sync-changelog') + runs-on: ubuntu-latest + permissions: + contents: write + issues: write + pull-requests: read + steps: + - name: Checkout repository + uses: actions/checkout@v7 + with: + fetch-depth: 0 + + - name: Setup Bazel + uses: bazel-contrib/setup-bazel@0.19.0 + with: + bazelisk-version: 1.20.0 + + - name: Complete Sync Changelog + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + bazel run //tools/private/release -- complete-sync-changelog \ + --pr "$PR_NUMBER" + + diff --git a/tests/tools/private/release/BUILD.bazel b/tests/tools/private/release/BUILD.bazel index bc3b6f7e8b..633cfa328f 100644 --- a/tests/tools/private/release/BUILD.bazel +++ b/tests/tools/private/release/BUILD.bazel @@ -26,6 +26,15 @@ py_test( ], ) +py_test( + name = "complete_sync_changelog_test", + srcs = ["complete_sync_changelog_test.py"], + deps = [ + ":release_test_helper", + "//tools/private/release:release_lib", + ], +) + py_test( name = "create_rc_test", srcs = ["create_rc_test.py"], diff --git a/tests/tools/private/release/add_backports_test.py b/tests/tools/private/release/add_backports_test.py index 7a5d96a23b..75729e87fe 100644 --- a/tests/tools/private/release/add_backports_test.py +++ b/tests/tools/private/release/add_backports_test.py @@ -35,6 +35,8 @@ def test_add_backports_explicit_issue(self): self.assertIn("- [ ] #125", call_args[1]) # Should also auto-add Tag RC0 self.assertIn("- [ ] Tag RC0", call_args[1]) + self.assertIn("- [ ] Sync Changelog #124", call_args[1]) + self.assertIn("- [ ] Sync Changelog #125", call_args[1]) def test_add_backports_auto_discover_success(self): args = argparse.Namespace(issue=None, prs=["124"]) @@ -98,6 +100,7 @@ def test_add_backports_no_auto_add_rc_if_pending(self): self.assertNotIn("Tag RC1", call_args[1]) # Tag RC0 should still be there self.assertIn("- [ ] Tag RC0", call_args[1]) + self.assertIn("- [ ] Sync Changelog #124", call_args[1]) if __name__ == "__main__": diff --git a/tests/tools/private/release/changelog_news_test.py b/tests/tools/private/release/changelog_news_test.py index f8bd141a55..83ab2cc959 100644 --- a/tests/tools/private/release/changelog_news_test.py +++ b/tests/tools/private/release/changelog_news_test.py @@ -16,21 +16,9 @@ def test_update_changelog_with_news(self): [unreleased]: https://github.com/bazel-contrib/rules_python/releases/tag/unreleased -{#unreleased-removed} -### Removed -* Nothing removed. - -{#unreleased-changed} -### Changed -* Nothing changed. - -{#unreleased-fixed} -### Fixed -* Nothing fixed. - -{#unreleased-added} -### Added -* Nothing added. +Unreleased changes are tracked as individual files in the [news/](./news) +directory, or view the [latest generated +changelog](https://rules-python.readthedocs.io/en/latest/changelog.html). {#v2-0-2} ## [2.0.2] - 2026-05-14 @@ -408,6 +396,129 @@ def test_update_changelog_empty_news(self): self.assertNotIn("{#v3-0-0-fixed}", new_content) self.assertNotIn("{#v3-0-0-added}", new_content) + def test_update_changelog_selective_news_files(self): + # Arrange + changelog = """# Changelog + +{#unreleased} +## Unreleased + +[unreleased]: https://github.com/bazel-contrib/rules_python/releases/tag/unreleased + +{#v2-0-2} +## [2.0.2] - 2026-05-14 + +[2.0.2]: https://github.com/bazel-contrib/rules_python/releases/tag/2.0.2 +""" + changelog_path = self.tmpdir / "CHANGELOG.md" + changelog_path.write_text(changelog) + + news_dir = self.tmpdir / "news" + news_dir.mkdir() + + # Create news files + (news_dir / "123.fixed.md").write_text("Fix A") + (news_dir / "456.fixed.md").write_text("Fix B") + + # Act: Only process 123.fixed.md + changelog_news.update_changelog( + "2.0.3", + "2026-06-16", + changelog_path=changelog_path, + news_dir=news_dir, + news_files=[news_dir / "123.fixed.md"], + ) + + # Assert + # 1. Only 123.fixed.md should be deleted + self.assertFalse((news_dir / "123.fixed.md").exists()) + self.assertTrue((news_dir / "456.fixed.md").exists()) + + new_content = changelog_path.read_text() + + # 2. Only Fix A should be in the changelog + self.assertIn("Fix A", new_content) + self.assertNotIn("Fix B", new_content) + + def test_update_changelog_insertion_point(self): + # Arrange + changelog = """# Changelog + +{#unreleased} +## Unreleased + +[unreleased]: https://github.com/bazel-contrib/rules_python/releases/tag/unreleased + +{#v2-2-0} +## [2.2.0] - 2026-06-30 + +[2.2.0]: https://github.com/bazel-contrib/rules_python/releases/tag/2.2.0 + +{#v2-0-0} +## [2.0.0] - 2026-04-09 + +[2.0.0]: https://github.com/bazel-contrib/rules_python/releases/tag/2.0.0 +""" + changelog_path = self.tmpdir / "CHANGELOG.md" + changelog_path.write_text(changelog) + + news_dir = self.tmpdir / "news" + news_dir.mkdir() + (news_dir / "123.fixed.md").write_text("Fix in 2.1.0") + + # Act: Insert 2.1.0 + changelog_news.update_changelog( + "2.1.0", + "2026-06-17", + changelog_path=changelog_path, + news_dir=news_dir, + ) + + # Assert + new_content = changelog_path.read_text() + + # Verify 2.1.0 is inserted BEFORE 2.0.0 but AFTER 2.2.0 + idx_2_2_0 = new_content.index("{#v2-2-0}") + idx_2_1_0 = new_content.index("{#v2-1-0}") + idx_2_0_0 = new_content.index("{#v2-0-0}") + + self.assertTrue(idx_2_2_0 < idx_2_1_0 < idx_2_0_0) + self.assertIn("Fix in 2.1.0", new_content) + + def test_update_changelog_insertion_point_too_small(self): + # Arrange + changelog = """# Changelog + +{#unreleased} +## Unreleased + +[unreleased]: https://github.com/bazel-contrib/rules_python/releases/tag/unreleased + +{#v2-0-0} +## [2.0.0] - 2026-04-09 + +[2.0.0]: https://github.com/bazel-contrib/rules_python/releases/tag/2.0.0 +""" + changelog_path = self.tmpdir / "CHANGELOG.md" + changelog_path.write_text(changelog) + + news_dir = self.tmpdir / "news" + news_dir.mkdir() + (news_dir / "123.fixed.md").write_text("Fix in 1.0.0") + + # Act & Assert + with self.assertRaises(ValueError) as ctx: + changelog_news.update_changelog( + "1.0.0", + "2026-01-01", + changelog_path=changelog_path, + news_dir=news_dir, + ) + self.assertIn( + "Could not find a version in CHANGELOG.md smaller than 1.0.0", + str(ctx.exception), + ) + if __name__ == "__main__": unittest.main() diff --git a/tests/tools/private/release/complete_sync_changelog_test.py b/tests/tools/private/release/complete_sync_changelog_test.py new file mode 100644 index 0000000000..66c40c01d3 --- /dev/null +++ b/tests/tools/private/release/complete_sync_changelog_test.py @@ -0,0 +1,120 @@ +import argparse +import unittest +from unittest.mock import patch + +from tests.tools.private.release.release_test_helper import _mock_git_and_gh +from tools.private.release.complete_sync_changelog import CompleteSyncChangelog + + +class CompleteSyncChangelogTest(unittest.TestCase): + def setUp(self): + _mock_git_and_gh(self) + self.addCleanup(patch.stopall) + + # Dynamic mock for issue body + self.issue_body = "" + + def mock_get_body(issue_num): + return self.issue_body + + def mock_update_body(issue_num, body): + self.issue_body = body + + self.mock_gh.get_issue_body.side_effect = mock_get_body + self.mock_gh.update_issue_body.side_effect = mock_update_body + + def test_complete_sync_changelog_success(self): + args = argparse.Namespace(pr=999) + self.mock_gh.get_pr_info.return_value = { + "state": "MERGED", + "body": "Updates CHANGELOG.md\n\nRelease-Tracking-Issue: #123", + "mergeCommit": {"oid": "abcdef1234567890"}, + } + self.issue_body = """ +## Checklist +- [ ] Prepare Release +- [ ] Create Release branch +- [ ] Sync Changelog #124 | status=pending pr=#999 +- [ ] Sync Changelog #125 | status=pending pr=#999 +- [ ] Sync Changelog #126 | status=pending pr=#888 +- [ ] Tag Final + +## Backports +""" + result = CompleteSyncChangelog(args, self.mock_gh).run() + + self.assertEqual(result, 0) + self.mock_gh.get_pr_info.assert_called_once_with(999) + self.mock_gh.get_issue_body.assert_called_once_with(123) + self.mock_gh.update_issue_body.assert_called_once() + + # Check that only tasks pointing to #999 were marked checked=True and status=done + self.assertIn( + "- [x] Sync Changelog #124 | status=done pr=#999 commit= abcdef12", + self.issue_body, + ) + self.assertIn( + "- [x] Sync Changelog #125 | status=done pr=#999 commit= abcdef12", + self.issue_body, + ) + # Task pointing to #888 should remain unchanged + self.assertIn( + "- [ ] Sync Changelog #126 | status=pending pr=#888", + self.issue_body, + ) + + def test_complete_sync_changelog_not_merged(self): + args = argparse.Namespace(pr=999) + self.mock_gh.get_pr_info.return_value = { + "state": "OPEN", + "body": "Updates CHANGELOG.md\n\nRelease-Tracking-Issue: #123", + } + + result = CompleteSyncChangelog(args, self.mock_gh).run() + + self.assertEqual(result, 1) + self.mock_gh.get_pr_info.assert_called_once_with(999) + self.mock_gh.update_issue_body.assert_not_called() + + def test_complete_sync_changelog_missing_tracking_issue_link(self): + args = argparse.Namespace(pr=999) + self.mock_gh.get_pr_info.return_value = { + "state": "MERGED", + "body": "Updates CHANGELOG.md without tracking issue link", + "mergeCommit": {"oid": "abcdef1234567890"}, + } + + result = CompleteSyncChangelog(args, self.mock_gh).run() + + self.assertEqual(result, 1) + self.mock_gh.get_pr_info.assert_called_once_with(999) + self.mock_gh.update_issue_body.assert_not_called() + + def test_complete_sync_changelog_no_matching_tasks(self): + args = argparse.Namespace(pr=999) + self.mock_gh.get_pr_info.return_value = { + "state": "MERGED", + "body": "Updates CHANGELOG.md\n\nRelease-Tracking-Issue: #123", + "mergeCommit": {"oid": "abcdef1234567890"}, + } + # Checklist has no tasks pointing to #999 + self.issue_body = """ +## Checklist +- [ ] Prepare Release +- [ ] Create Release branch +- [ ] Sync Changelog #124 | status=pending pr=#888 +- [ ] Tag Final + +## Backports +""" + result = CompleteSyncChangelog(args, self.mock_gh).run() + + # Should log warning but return 0 (success/noop) + self.assertEqual(result, 0) + self.mock_gh.get_pr_info.assert_called_once_with(999) + self.mock_gh.get_issue_body.assert_called_once_with(123) + self.mock_gh.update_issue_body.assert_not_called() + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/tools/private/release/git_test.py b/tests/tools/private/release/git_test.py index 16667b8715..34643fc016 100644 --- a/tests/tools/private/release/git_test.py +++ b/tests/tools/private/release/git_test.py @@ -41,7 +41,7 @@ def test_checkout_track_remote_existing_branch( self.mock_run_git.assert_called_once_with( "checkout", "my-branch", capture_output=False ) - mock_reset_hard.assert_called_once_with("origin/my-branch") + mock_reset_hard.assert_called_once_with(reset_to="origin/my-branch") class GitFetchTest(unittest.TestCase): @@ -82,5 +82,108 @@ def test_fetch_all_options(self): ) +class GitGetModifiedFilesTest(unittest.TestCase): + def setUp(self): + self.git = Git(".") + self.patcher = patch.object(self.git, "_run_git") + self.mock_run_git = self.patcher.start() + self.addCleanup(self.patcher.stop) + + def test_get_modified_files(self): + self.mock_run_git.return_value = "file1.txt\nfile2.py\n\n" + files = self.git.get_modified_files("HEAD") + self.mock_run_git.assert_called_once_with( + "show", "--name-only", "--format=", "HEAD" + ) + self.assertEqual(files, ["file1.txt", "file2.py"]) + + def test_get_modified_files_empty(self): + self.mock_run_git.return_value = "" + files = self.git.get_modified_files("HEAD") + self.assertEqual(files, []) + + +class GitDiffTest(unittest.TestCase): + def setUp(self): + self.git = Git(".") + self.patcher = patch.object(self.git, "_run_git") + self.mock_run_git = self.patcher.start() + self.addCleanup(self.patcher.stop) + + def test_diff_has_changes(self): + self.mock_run_git.return_value = "some diff output" + output = self.git.diff() + self.mock_run_git.assert_called_once_with("diff") + self.assertEqual(output, "some diff output") + + def test_diff_empty(self): + self.mock_run_git.return_value = "" + output = self.git.diff() + self.mock_run_git.assert_called_once_with("diff") + self.assertEqual(output, "") + + +class GitApplyTest(unittest.TestCase): + def setUp(self): + self.git = Git(".") + self.patcher = patch.object(self.git, "_run_git") + self.mock_run_git = self.patcher.start() + self.addCleanup(self.patcher.stop) + + def test_apply(self): + self.git.apply("patch.patch") + self.mock_run_git.assert_called_once_with( + "apply", "patch.patch", capture_output=False + ) + + +class GitApplyCheckTest(unittest.TestCase): + def setUp(self): + self.git = Git(".") + self.patcher = patch.object(self.git, "_run_git") + self.mock_run_git = self.patcher.start() + self.addCleanup(self.patcher.stop) + + def test_apply_check_clean(self): + self.mock_run_git.return_value = "" + result = self.git.apply_check("patch.patch") + self.mock_run_git.assert_called_once_with( + "apply", "--check", "patch.patch", capture_output=False + ) + self.assertTrue(result) + + def test_apply_check_conflict(self): + import subprocess + + self.mock_run_git.side_effect = subprocess.CalledProcessError( + 1, ["git", "apply", "--check", "patch.patch"] + ) + result = self.git.apply_check("patch.patch") + self.mock_run_git.assert_called_once_with( + "apply", "--check", "patch.patch", capture_output=False + ) + self.assertFalse(result) + + +class GitResetHardTest(unittest.TestCase): + def setUp(self): + self.git = Git(".") + self.patcher = patch.object(self.git, "_run_git") + self.mock_run_git = self.patcher.start() + self.addCleanup(self.patcher.stop) + + def test_reset_hard_default(self): + self.git.reset_hard() + self.mock_run_git.assert_called_once_with( + "reset", "--hard", "HEAD", capture_output=False + ) + + def test_reset_hard_custom(self): + self.git.reset_hard(reset_to="my-commit") + self.mock_run_git.assert_called_once_with( + "reset", "--hard", "my-commit", capture_output=False + ) + + if __name__ == "__main__": unittest.main() diff --git a/tests/tools/private/release/prepare_test.py b/tests/tools/private/release/prepare_test.py index 3d92b131e2..45b92a636f 100644 --- a/tests/tools/private/release/prepare_test.py +++ b/tests/tools/private/release/prepare_test.py @@ -36,7 +36,11 @@ def test_prepare_success_existing_issue(self, mock_replace, mock_changelog): self.assertEqual(result, 0) self.mock_gh.get_release_tracking_issue.assert_called_once_with("2.0.0") self.mock_gh.create_tracking_issue.assert_not_called() - self.mock_gh.create_pr.assert_called_once_with("2.0.0", 123) + self.mock_gh.create_pr.assert_called_once_with( + title="Prepare release v2.0.0", + body="Work towards #123", + base="main", + ) self.mock_git.add_modified_and_deleted.assert_called_once() @patch("tools.private.release.prepare.changelog_news") @@ -67,7 +71,11 @@ def test_prepare_success_create_issue(self, mock_replace, mock_changelog): self.mock_gh.create_tracking_issue.assert_called_once_with( "2.0.0", "dummy template content" ) - self.mock_gh.create_pr.assert_called_once_with("2.0.0", 123) + self.mock_gh.create_pr.assert_called_once_with( + title="Prepare release v2.0.0", + body="Work towards #123", + base="main", + ) self.mock_git.add_modified_and_deleted.assert_called_once() @patch("tools.private.release.prepare.changelog_news") @@ -172,7 +180,11 @@ def test_prepare_create_pr_when_none_associated(self, mock_replace, mock_changel "origin", "prepare-2.0.0", set_upstream=True, force=True ) self.mock_gh.get_open_pr.assert_called_once_with("prepare-2.0.0") - self.mock_gh.create_pr.assert_called_once_with("2.0.0", 123) + self.mock_gh.create_pr.assert_called_once_with( + title="Prepare release v2.0.0", + body="Work towards #123", + base="main", + ) self.mock_gh.update_issue_body.assert_called_once() call_args = self.mock_gh.update_issue_body.call_args[0] self.assertIn("pr=#789", call_args[1]) diff --git a/tests/tools/private/release/process_backports_test.py b/tests/tools/private/release/process_backports_test.py index fe7bae256d..d5df43ae46 100644 --- a/tests/tools/private/release/process_backports_test.py +++ b/tests/tools/private/release/process_backports_test.py @@ -20,12 +20,26 @@ def setUp(self): self.mock_gh.resolve_pr_number.side_effect = lambda x: int( x.lstrip("#").split("/")[-1] ) + self.mock_git.diff.return_value = "" + self.mock_git.apply_check.return_value = True + + # Dynamic mock for issue body + self.issue_body = "" + + def mock_get_body(issue_num): + return self.issue_body + + def mock_update_body(issue_num, body): + self.issue_body = body + + self.mock_gh.get_issue_body.side_effect = mock_get_body + self.mock_gh.update_issue_body.side_effect = mock_update_body def test_process_backports_no_pending(self): args = argparse.Namespace( issue=123, remote="origin", dry_run=False, add=None, triggering_comment=None ) - self.mock_gh.get_issue_body.return_value = "No backports here" + self.issue_body = "No backports here" result = ProcessBackports(args, self.mock_git, self.mock_gh).run() @@ -40,10 +54,12 @@ def test_process_backports_success(self, mock_datetime): issue=123, remote="origin", dry_run=False, add=None, triggering_comment=None ) self.mock_gh.get_issue_title.return_value = "Release 2.0.0" - self.mock_gh.get_issue_body.return_value = """ + self.issue_body = """ ## Checklist - [ ] Prepare Release - [ ] Create Release branch +- [ ] Sync Changelog #124 +- [ ] Tag Final ## Backports - [ ] #124 | status=pending @@ -60,33 +76,204 @@ def mock_resolve(items): self.mock_gh.get_merge_commits_for_prs.side_effect = mock_resolve self.mock_git.sort_commits_chronologically.return_value = ["abcdef12"] - self.mock_git.get_commit_sha.return_value = "12345678" + self.mock_git.get_commit_sha.side_effect = ["12345678", "12345678", "main_sha"] self.mock_git.get_commit_message.return_value = 'Cherry-pick "fix bug"' + self.mock_git.get_modified_files.return_value = ["news/124.fixed.md"] + self.mock_git.diff.return_value = "version diff for 124" + self.mock_git.apply_check.return_value = True + self.mock_gh.create_pr.return_value = "https://github.com/foo/bar/pull/999" result = ProcessBackports(args, self.mock_git, self.mock_gh).run() self.assertEqual(result, 0) self.mock_git.fetch.assert_has_calls( - [call("origin", tags=True, force=True), call("origin")] + [ + call("origin", tags=True, force=True), + call("origin"), + call("origin", refspec="main"), + ] ) - self.mock_git.checkout.assert_called_once_with( - "release/2.0", track_remote="origin" + self.mock_git.checkout.assert_has_calls( + [ + call("release/2.0", track_remote="origin"), + call("main", track_remote="origin"), + call("prepare-2.0.0-backports-6affdae", create_branch=True), + call("release/2.0"), + ] ) self.mock_git.cherry_pick.assert_called_once_with("abcdef12") - self.mock_changelog_news.update_changelog.assert_called_once_with( - "2.0.0", "2026-07-01" + self.mock_git.diff.assert_called_once() + self.mock_git.apply_check.assert_called_once_with(unittest.mock.ANY) + self.mock_git.apply.assert_called_once_with(unittest.mock.ANY) + self.mock_changelog_news.update_changelog.assert_has_calls( + [ + call("2.0.0", "2026-07-01"), + call( + "2.0.0", + "2026-07-01", + news_files=["news/124.fixed.md"], + delete_news=True, + ), + ] ) - self.mock_git.add_modified_and_deleted.assert_called_once() + self.assertEqual(self.mock_git.add_modified_and_deleted.call_count, 2) self.mock_replace_version_next.assert_called_once_with("2.0.0") - self.mock_git.commit.assert_called_once_with( - 'Cherry-pick "fix bug"\n\nWork towards #123', amend=True + self.mock_git.commit.assert_has_calls( + [ + call('Cherry-pick "fix bug"\n\nWork towards #123', amend=True), + call("chore(release): sync changelog for v2.0.0 backports"), + ] + ) + self.mock_git.push.assert_has_calls( + [ + call("origin", "release/2.0"), + call( + "origin", + "prepare-2.0.0-backports-6affdae", + set_upstream=True, + force=True, + ), + ] ) - self.mock_git.push.assert_called_once_with("origin", "release/2.0") - self.mock_gh.update_issue_body.assert_called_once() - call_args = self.mock_gh.update_issue_body.call_args[0] - self.assertEqual(call_args[0], 123) - self.assertIn("- [x] #124 | status=done rc=rc0 commit= 12345678", call_args[1]) + self.mock_gh.create_pr.assert_called_once_with( + title="chore(release): sync changelog for v2.0.0 backports", + body="Updates CHANGELOG.md and removes news files for backports:\n- #124\n\nWork towards #123\nRelease-Tracking-Issue: #123", + base="main", + labels=["type: sync-changelog"], + ) + self.mock_gh.enable_auto_merge.assert_called_once_with(999) + + self.assertEqual(self.mock_gh.update_issue_body.call_count, 2) + call_args_list = self.mock_gh.update_issue_body.call_args_list + self.assertEqual(call_args_list[0][0][0], 123) + self.assertIn( + "- [x] #124 | status=done rc=rc0 commit= 12345678", call_args_list[0][0][1] + ) + self.assertEqual(call_args_list[1][0][0], 123) + self.assertIn( + "- [ ] Sync Changelog #124 | status=pending pr=#999", + call_args_list[1][0][1], + ) + + @patch("tools.private.release.process_backports.datetime") + def test_process_backports_sync_branch_exists(self, mock_datetime): + mock_datetime.date.today.return_value = datetime.date(2026, 7, 1) + args = argparse.Namespace( + issue=123, remote="origin", dry_run=False, add=None, triggering_comment=None + ) + self.mock_gh.get_issue_title.return_value = "Release 2.0.0" + self.issue_body = """ +## Checklist +- [ ] Prepare Release +- [ ] Create Release branch +- [ ] Sync Changelog #124 +- [ ] Tag Final + +## Backports +- [ ] #124 | status=pending +""" + self.mock_git.get_remote_tags.return_value = [] + + def mock_resolve(items): + for item in items: + if item.pr_ref == "#124": + item.commit = "abcdef12" + item.status = "done" + return items + + self.mock_gh.get_merge_commits_for_prs.side_effect = mock_resolve + + self.mock_git.sort_commits_chronologically.return_value = ["abcdef12"] + self.mock_git.get_commit_sha.side_effect = ["12345678", "12345678", "main_sha"] + self.mock_git.get_commit_message.return_value = 'Cherry-pick "fix bug"' + self.mock_git.get_modified_files.return_value = ["news/124.fixed.md"] + self.mock_git.diff.return_value = "version diff for 124" + self.mock_git.apply_check.return_value = True + self.mock_gh.create_pr.return_value = "https://github.com/foo/bar/pull/999" + + # Configure branch to exist + self.mock_git.branch_exists.return_value = True + + result = ProcessBackports(args, self.mock_git, self.mock_gh).run() + + self.assertEqual(result, 0) + self.mock_git.fetch.assert_has_calls( + [ + call("origin", tags=True, force=True), + call("origin"), + call("origin", refspec="main"), + ] + ) + self.mock_git.checkout.assert_has_calls( + [ + call("release/2.0", track_remote="origin"), + call("main", track_remote="origin"), + # Called without create_branch=True + call("prepare-2.0.0-backports-6affdae"), + call("release/2.0"), + ] + ) + # Verify reset_hard was called to reset the existing branch to main + self.mock_git.reset_hard.assert_has_calls( + [ + call(reset_to="main"), + ] + ) + self.mock_git.cherry_pick.assert_called_once_with("abcdef12") + self.mock_git.diff.assert_called_once() + self.mock_git.apply_check.assert_called_once_with(unittest.mock.ANY) + self.mock_git.apply.assert_called_once_with(unittest.mock.ANY) + self.mock_changelog_news.update_changelog.assert_has_calls( + [ + call("2.0.0", "2026-07-01"), + call( + "2.0.0", + "2026-07-01", + news_files=["news/124.fixed.md"], + delete_news=True, + ), + ] + ) + self.assertEqual(self.mock_git.add_modified_and_deleted.call_count, 2) + self.mock_replace_version_next.assert_called_once_with("2.0.0") + self.mock_git.commit.assert_has_calls( + [ + call('Cherry-pick "fix bug"\n\nWork towards #123', amend=True), + call("chore(release): sync changelog for v2.0.0 backports"), + ] + ) + self.mock_git.push.assert_has_calls( + [ + call("origin", "release/2.0"), + call( + "origin", + "prepare-2.0.0-backports-6affdae", + set_upstream=True, + force=True, + ), + ] + ) + + self.mock_gh.create_pr.assert_called_once_with( + title="chore(release): sync changelog for v2.0.0 backports", + body="Updates CHANGELOG.md and removes news files for backports:\n- #124\n\nWork towards #123\nRelease-Tracking-Issue: #123", + base="main", + labels=["type: sync-changelog"], + ) + self.mock_gh.enable_auto_merge.assert_called_once_with(999) + + self.assertEqual(self.mock_gh.update_issue_body.call_count, 2) + call_args_list = self.mock_gh.update_issue_body.call_args_list + self.assertEqual(call_args_list[0][0][0], 123) + self.assertIn( + "- [x] #124 | status=done rc=rc0 commit= 12345678", call_args_list[0][0][1] + ) + self.assertEqual(call_args_list[1][0][0], 123) + self.assertIn( + "- [ ] Sync Changelog #124 | status=pending pr=#999", + call_args_list[1][0][1], + ) @patch("tools.private.release.process_backports.datetime") def test_process_backports_dry_run(self, mock_datetime): @@ -95,10 +282,12 @@ def test_process_backports_dry_run(self, mock_datetime): issue=123, remote="origin", dry_run=True, add=None, triggering_comment=None ) self.mock_gh.get_issue_title.return_value = "Release 2.0.0" - self.mock_gh.get_issue_body.return_value = """ + self.issue_body = """ ## Checklist - [ ] Prepare Release - [ ] Create Release branch +- [ ] Sync Changelog #124 +- [ ] Tag Final ## Backports - [ ] #124 | status=pending @@ -115,28 +304,55 @@ def mock_resolve(items): self.mock_gh.get_merge_commits_for_prs.side_effect = mock_resolve self.mock_git.sort_commits_chronologically.return_value = ["abcdef12"] - self.mock_git.get_commit_sha.return_value = "12345678" + self.mock_git.get_commit_sha.side_effect = ["12345678", "main_sha"] self.mock_git.get_commit_message.return_value = 'Cherry-pick "fix bug"' + self.mock_git.get_modified_files.return_value = ["news/124.fixed.md"] + self.mock_git.diff.return_value = "version diff for 124" + self.mock_git.apply_check.return_value = True result = ProcessBackports(args, self.mock_git, self.mock_gh).run() self.assertEqual(result, 0) self.mock_git.fetch.assert_has_calls( - [call("origin", tags=True, force=True), call("origin")] + [ + call("origin", tags=True, force=True), + call("origin"), + call("origin", refspec="main"), + ] ) - self.mock_git.checkout.assert_called_once_with( - "release/2.0", track_remote="origin" + self.mock_git.checkout.assert_has_calls( + [ + call("release/2.0", track_remote="origin"), + call("main", track_remote="origin"), + call("release/2.0"), + ] ) self.mock_git.cherry_pick.assert_called_once_with("abcdef12") - self.mock_changelog_news.update_changelog.assert_called_once_with( - "2.0.0", "2026-07-01" + self.mock_git.diff.assert_called_once() + self.mock_git.apply_check.assert_called_once_with(unittest.mock.ANY) + self.mock_git.apply.assert_not_called() + self.mock_changelog_news.update_changelog.assert_has_calls( + [ + call("2.0.0", "2026-07-01"), + call( + "2.0.0", + "2026-07-01", + news_files=["news/124.fixed.md"], + delete_news=True, + ), + ] ) - self.mock_git.add_modified_and_deleted.assert_called_once() + self.assertEqual(self.mock_git.add_modified_and_deleted.call_count, 1) self.mock_replace_version_next.assert_called_once_with("2.0.0") self.mock_git.commit.assert_called_once_with( 'Cherry-pick "fix bug"\n\nWork towards #123', amend=True ) - self.mock_git.reset_hard.assert_called_once_with("12345678") + self.mock_git.reset_hard.assert_has_calls( + [ + call(reset_to="12345678"), + call(reset_to="main_sha"), + ] + ) self.mock_git.push.assert_not_called() self.mock_gh.update_issue_body.assert_not_called() @@ -145,7 +361,7 @@ def test_process_backports_ignored_and_failed_states(self): issue=123, remote="origin", dry_run=False, add=None, triggering_comment=None ) self.mock_gh.get_issue_title.return_value = "Release 2.0.0" - self.mock_gh.get_issue_body.return_value = """ + self.issue_body = """ ## Checklist - [ ] Prepare Release - [ ] Create Release branch @@ -186,7 +402,7 @@ def test_process_backports_ignored_error_status(self): issue=123, remote="origin", dry_run=False, add=None, triggering_comment=None ) self.mock_gh.get_issue_title.return_value = "Release 2.0.0" - self.mock_gh.get_issue_body.return_value = """ + self.issue_body = """ ## Checklist - [ ] Prepare Release - [ ] Create Release branch @@ -211,7 +427,7 @@ def test_process_backports_cherry_pick_failed(self, mock_datetime): issue=123, remote="origin", dry_run=False, add=None, triggering_comment=None ) self.mock_gh.get_issue_title.return_value = "Release 2.0.0" - self.mock_gh.get_issue_body.return_value = """ + self.issue_body = """ ## Checklist - [ ] Prepare Release - [ ] Create Release branch @@ -261,7 +477,7 @@ def test_process_backports_add_backports_and_auto_add_rc_task(self, mock_datetim triggering_comment=None, ) self.mock_gh.get_issue_title.return_value = "Release 2.0.0" - self.mock_gh.get_issue_body.return_value = """ + self.issue_body = """ ## Checklist - [x] Prepare Release | status=done pr=#122 commit=abcdef12 - [x] Create Release branch | status=done branch=release/2.0 commit=abcdef12 @@ -284,14 +500,18 @@ def mock_resolve(items): self.mock_gh.get_merge_commits_for_prs.side_effect = mock_resolve self.mock_git.sort_commits_chronologically.return_value = ["abcdef12"] + # Mock create_pr to return a string to avoid int(MagicMock) returning 1 + self.mock_gh.create_pr.return_value = "https://github.com/foo/bar/pull/999" + result = ProcessBackports(args, self.mock_git, self.mock_gh).run() self.assertEqual(result, 0) - # update_issue_body should be called twice: + # update_issue_body should be called 3 times: # 1. When adding backports and auto-adding Tag RC1 task. # 2. When updating the backport status to done. - self.assertEqual(self.mock_gh.update_issue_body.call_count, 2) + # 3. When updating the sync task status to pending. + self.assertEqual(self.mock_gh.update_issue_body.call_count, 3) call1_args = self.mock_gh.update_issue_body.call_args_list[0][0] call2_args = self.mock_gh.update_issue_body.call_args_list[1][0] @@ -299,15 +519,22 @@ def mock_resolve(items): self.assertEqual(call1_args[0], 123) self.assertIn("- [ ] #124", call1_args[1]) self.assertIn("- [ ] Tag RC1", call1_args[1]) + self.assertIn("- [ ] Sync Changelog #124", call1_args[1]) self.assertIn( "- [x] Tag RC0 | status=done tag=2.0.0-rc0 commit=abcdef12\n- [ ]" - " Tag RC1\n- [ ] Tag Final", + " Tag RC1\n- [ ] Sync Changelog #124\n- [ ] Tag Final", call1_args[1].strip(), ) self.assertEqual(call2_args[0], 123) self.assertIn("- [x] #124 | status=done rc=rc1 commit= 12345678", call2_args[1]) + call3_args = self.mock_gh.update_issue_body.call_args_list[2][0] + self.assertEqual(call3_args[0], 123) + self.assertIn( + "- [ ] Sync Changelog #124 | status=pending pr=#999", call3_args[1] + ) + @patch("tools.private.release.process_backports.datetime") def test_process_backports_add_backports_marks_invalid(self, mock_datetime): mock_datetime.date.today.return_value = datetime.date(2026, 7, 1) @@ -319,7 +546,7 @@ def test_process_backports_add_backports_marks_invalid(self, mock_datetime): triggering_comment=None, ) self.mock_gh.get_issue_title.return_value = "Release 2.0.0" - self.mock_gh.get_issue_body.return_value = """ + self.issue_body = """ ## Checklist - [x] Prepare Release | status=done pr=#122 commit=abcdef12 - [x] Create Release branch | status=done branch=release/2.0 commit=abcdef12 @@ -335,23 +562,189 @@ def test_process_backports_add_backports_marks_invalid(self, mock_datetime): def mock_resolve(items): # Both 124 and 125 should be processed, 'invalid' should be ignored (it has error status) for item in items: - if item.pr_ref in ("#124", "#125"): - item.commit = "abcdef12" + if item.pr_ref == "#124": + item.commit = "sha_124" + item.status = "done" + elif item.pr_ref == "#125": + item.commit = "sha_125" item.status = "done" return items self.mock_gh.get_merge_commits_for_prs.side_effect = mock_resolve - self.mock_git.sort_commits_chronologically.return_value = ["abcdef12"] + self.mock_git.sort_commits_chronologically.return_value = ["sha_124", "sha_125"] + self.mock_gh.create_pr.return_value = "https://github.com/foo/bar/pull/999" result = ProcessBackports(args, self.mock_git, self.mock_gh).run() self.assertEqual(result, 0) # Should have updated body to add 124, 125, and invalid - self.assertEqual(self.mock_gh.update_issue_body.call_count, 2) + # update_issue_body should be called 4 times: + # 1. When adding backports. + # 2. When updating 124 status to done. + # 3. When updating 125 status to done. + # 4. When updating sync tasks status to pending. + self.assertEqual(self.mock_gh.update_issue_body.call_count, 4) call1_args = self.mock_gh.update_issue_body.call_args_list[0][0] self.assertIn("- [ ] #124", call1_args[1]) self.assertIn("- [ ] #125", call1_args[1]) self.assertIn("- [ ] invalid | status=error-invalid-pr", call1_args[1]) + self.assertIn("- [ ] Sync Changelog #124", call1_args[1]) + self.assertIn("- [ ] Sync Changelog #125", call1_args[1]) + + call4_args = self.mock_gh.update_issue_body.call_args_list[3][0] + self.assertIn( + "- [ ] Sync Changelog #124 | status=pending pr=#999", call4_args[1] + ) + self.assertIn( + "- [ ] Sync Changelog #125 | status=pending pr=#999", call4_args[1] + ) + + @patch("tools.private.release.process_backports.datetime") + def test_process_backports_version_sync_failure(self, mock_datetime): + mock_datetime.date.today.return_value = datetime.date(2026, 7, 1) + args = argparse.Namespace( + issue=123, remote="origin", dry_run=False, add=None, triggering_comment=None + ) + self.mock_gh.get_issue_title.return_value = "Release 2.0.0" + self.issue_body = """ +## Checklist +- [ ] Prepare Release +- [ ] Create Release branch +- [ ] Sync Changelog #124 +- [ ] Sync Changelog #125 +- [ ] Tag Final + +## Backports +- [ ] #124 | status=pending +- [ ] #125 | status=pending +""" + self.mock_git.get_remote_tags.return_value = [] + + def mock_resolve(items): + for item in items: + if item.pr_ref in ("#124", "#125"): + item.commit = "sha_" + item.pr_ref.lstrip("#") + item.status = "done" + return items + + self.mock_gh.get_merge_commits_for_prs.side_effect = mock_resolve + + self.mock_git.sort_commits_chronologically.return_value = ["sha_124", "sha_125"] + self.mock_git.get_commit_sha.side_effect = [ + "12345678", + "sha_124_amended", + "sha_125_amended", + "main_sha", + ] + self.mock_git.get_commit_message.return_value = 'Cherry-pick "fix bug"' + self.mock_git.get_modified_files.side_effect = [ + ["news/124.fixed.md"], + ["news/125.fixed.md"], + ] + self.mock_git.diff.side_effect = ["diff 124", "diff 125"] + self.mock_git.apply_check.side_effect = [False, True] + self.mock_gh.create_pr.return_value = "https://github.com/foo/bar/pull/999" + + result = ProcessBackports(args, self.mock_git, self.mock_gh).run() + + self.assertEqual(result, 0) + self.mock_git.fetch.assert_has_calls( + [ + call("origin", tags=True, force=True), + call("origin"), + call("origin", refspec="main"), + ] + ) + self.mock_git.checkout.assert_has_calls( + [ + call("release/2.0", track_remote="origin"), + call("main", track_remote="origin"), + call("prepare-2.0.0-backports-b552a96", create_branch=True), + call("release/2.0"), + ] + ) + self.mock_git.cherry_pick.assert_has_calls( + [ + call("sha_124"), + call("sha_125"), + ] + ) + # diff should be called for each successful cherry-pick + self.assertEqual(self.mock_git.diff.call_count, 2) + # apply_check should be called for both patches + self.assertEqual(self.mock_git.apply_check.call_count, 2) + # apply should only be called for 125 (since 124 failed check) + self.mock_git.apply.assert_called_once_with(unittest.mock.ANY) + + self.mock_changelog_news.update_changelog.assert_has_calls( + [ + call("2.0.0", "2026-07-01"), + call("2.0.0", "2026-07-01"), + call( + "2.0.0", + "2026-07-01", + news_files=["news/124.fixed.md", "news/125.fixed.md"], + delete_news=True, + ), + ] + ) + # add_modified_and_deleted called: + # - once per cherry-pick (2) + # - once on main backport branch (1) + # Total = 3 + self.assertEqual(self.mock_git.add_modified_and_deleted.call_count, 3) + # replace_version_next called once per cherry-pick + self.assertEqual(self.mock_replace_version_next.call_count, 2) + + self.mock_git.commit.assert_has_calls( + [ + call('Cherry-pick "fix bug"\n\nWork towards #123', amend=True), + call('Cherry-pick "fix bug"\n\nWork towards #123', amend=True), + call("chore(release): sync changelog for v2.0.0 backports"), + ] + ) + self.mock_git.push.assert_has_calls( + [ + call("origin", "release/2.0"), + call("origin", "release/2.0"), + call( + "origin", + "prepare-2.0.0-backports-b552a96", + set_upstream=True, + force=True, + ), + ] + ) + + # PR body should contain warning about 124 + expected_body = ( + "Updates CHANGELOG.md and removes news files for backports:\n" + "- #124\n" + "- #125\n" + "\n" + "Warning: These PRs failed to update their version markers:\n" + "- #124\n" + "\n" + "Work towards #123\n" + "Release-Tracking-Issue: #123" + ) + self.mock_gh.create_pr.assert_called_once_with( + title="chore(release): sync changelog for v2.0.0 backports", + body=expected_body, + base="main", + labels=["type: sync-changelog"], + ) + self.mock_gh.enable_auto_merge.assert_called_once_with(999) + + # update_issue_body called 3 times (twice for backports, once for sync tasks) + self.assertEqual(self.mock_gh.update_issue_body.call_count, 3) + call3_args = self.mock_gh.update_issue_body.call_args_list[2][0] + self.assertIn( + "- [ ] Sync Changelog #124 | status=pending pr=#999", call3_args[1] + ) + self.assertIn( + "- [ ] Sync Changelog #125 | status=pending pr=#999", call3_args[1] + ) if __name__ == "__main__": diff --git a/tests/tools/private/release/release_issue_test.py b/tests/tools/private/release/release_issue_test.py index cfc6da6ae4..36f0dbd704 100644 --- a/tests/tools/private/release/release_issue_test.py +++ b/tests/tools/private/release/release_issue_test.py @@ -2,7 +2,9 @@ from tools.private.release.release_issue import ( add_backports_to_body, + add_sync_changelog_task_to_body, format_metadata_line, + parse_checklist_state, parse_metadata_line, ) @@ -96,6 +98,65 @@ def test_add_backports_to_body(self): """ self.assertEqual(updated_body.strip(), expected_body.strip()) + def test_add_sync_changelog_task_to_body(self): + body = """ +## Checklist +- [ ] Prepare Release +- [ ] Create Release branch +- [ ] Tag Final +""" + # Insert first task (should go before Tag Final) + body = add_sync_changelog_task_to_body(body, 124) + expected = """ +## Checklist +- [ ] Prepare Release +- [ ] Create Release branch +- [ ] Sync Changelog #124 +- [ ] Tag Final +""" + self.assertEqual(body.strip(), expected.strip()) + + # Insert second task (should go after the last Sync Changelog task) + body = add_sync_changelog_task_to_body(body, 125) + expected = """ +## Checklist +- [ ] Prepare Release +- [ ] Create Release branch +- [ ] Sync Changelog #124 +- [ ] Sync Changelog #125 +- [ ] Tag Final +""" + self.assertEqual(body.strip(), expected.strip()) + + # Insert duplicate (should be ignored) + body = add_sync_changelog_task_to_body(body, 124) + self.assertEqual(body.strip(), expected.strip()) + + def test_parse_checklist_state_with_sync_changelogs(self): + body = """ +## Checklist +- [x] Prepare Release | status=done pr=#122 commit=abcdef12 +- [x] Create Release branch | status=done branch=release/2.0 +- [ ] Sync Changelog #124 | status=pending pr=#125 +- [ ] Sync Changelog #126 +- [ ] Tag Final +""" + state = parse_checklist_state(body) + self.assertIn(124, state["sync_changelogs"]) + self.assertIn(126, state["sync_changelogs"]) + + task_124 = state["sync_changelogs"][124] + self.assertEqual(task_124.name, "Sync Changelog #124") + self.assertFalse(task_124.checked) + self.assertEqual(task_124.status, "pending") + self.assertEqual(task_124.pr, "#125") + + task_126 = state["sync_changelogs"][126] + self.assertEqual(task_126.name, "Sync Changelog #126") + self.assertFalse(task_126.checked) + self.assertIsNone(task_126.status) + self.assertIsNone(task_126.pr) + if __name__ == "__main__": unittest.main() diff --git a/tools/private/release/add_backports.py b/tools/private/release/add_backports.py index c25f509b9c..63c70af7d7 100644 --- a/tools/private/release/add_backports.py +++ b/tools/private/release/add_backports.py @@ -4,6 +4,7 @@ from tools.private.release.release_issue import ( add_backports_to_body, add_rc_task_to_body, + add_sync_changelog_task_to_body, parse_checklist_state, ) @@ -60,6 +61,8 @@ def run(self) -> int: body = self.gh.get_issue_body(issue_num) items_to_add = [{"ref": f"#{pr}"} for pr in resolved_prs] body = add_backports_to_body(body, items_to_add) + for pr in resolved_prs: + body = add_sync_changelog_task_to_body(body, pr) state = parse_checklist_state(body) rc_tags = state.get("rc_tags", {}) has_pending_rc = any( diff --git a/tools/private/release/changelog_news.py b/tools/private/release/changelog_news.py index b896af1f6b..e09b082ade 100644 --- a/tools/private/release/changelog_news.py +++ b/tools/private/release/changelog_news.py @@ -3,14 +3,6 @@ import pathlib import re -_UNRELEASED_TEMPLATE_BODY = """## Unreleased - -[unreleased]: https://github.com/bazel-contrib/rules_python/releases/tag/unreleased - -Unreleased changes are tracked as individual files in the [news/](./news) -directory, or view the [latest generated -changelog](https://rules-python.readthedocs.io/en/latest/changelog.html).""" - def _get_sub_category(content): """Extracts the sub-category in parentheses from the entry content.""" @@ -108,6 +100,34 @@ def generate_release_block(version, release_date, news_entries): return "\n".join(lines) +def _parse_simple_version(ver_str): + """Parses a version string (X-Y-Z or X.Y.Z) into a tuple of ints.""" + normalized = ver_str.replace("-", ".") + return tuple(int(x) for x in normalized.split(".")) + + +def _find_insertion_point(changelog_content, new_version): + """Finds the character index to insert the new version block. + + It should be inserted before the first version in the changelog that is + smaller than new_version. + """ + # Find all version anchors: {#vX-Y-Z} + matches = list(re.finditer(r"\{#v(?P\d+-\d+-\d+)\}", changelog_content)) + + parsed_new_ver = _parse_simple_version(new_version) + + for m in matches: + ver_str = m.group("ver") + parsed_ver = _parse_simple_version(ver_str) + if parsed_ver < parsed_new_ver: + return m.start() + + raise ValueError( + f"Could not find a version in CHANGELOG.md smaller than {new_version} to insert before." + ) + + def _add_news_to_changelog(input_path, output_path, version, entries, release_date): """Adds or merges news entries into CHANGELOG.md.""" input_path = pathlib.Path(input_path) @@ -253,26 +273,15 @@ def _add_news_to_changelog(input_path, output_path, version, entries, release_da " release section from news entries..." ) new_release_block = generate_release_block(version, release_date, entries) - replacement = ( - f"{{#unreleased}}\n{_UNRELEASED_TEMPLATE_BODY}\n\n{new_release_block}\n" - ) - - # Replace the active Unreleased section (from {#unreleased} to the first release anchor) - pattern = ( - r"(?P\{#unreleased\})(?P.*?)(?=\n\s*\{#v\d+-\d+-\d+\}|\Z)" - ) - if not re.search(pattern, changelog_content, re.DOTALL): - raise RuntimeError( - "Could not find active Unreleased section to replace in CHANGELOG.md" - ) + # Find insertion point + insertion_point = _find_insertion_point(changelog_content, version) - new_content = re.sub( - pattern, - replacement, - changelog_content, - count=1, - flags=re.DOTALL, + new_content = ( + changelog_content[:insertion_point] + + new_release_block + + "\n\n" + + changelog_content[insertion_point:] ) output_path.write_text(new_content, encoding="utf-8") @@ -284,9 +293,13 @@ def merge_new_into_changelog( version, release_date, delete_news=False, + news_files=None, ): """Merges news entries from news_dir into changelog_path and writes to output_path.""" - news_files = _get_news_files(news_dir) + if news_files is None: + news_files = _get_news_files(news_dir) + else: + news_files = [pathlib.Path(f) for f in news_files] entries = _parse_new_files(news_files) _add_news_to_changelog( input_path=changelog_path, @@ -297,7 +310,8 @@ def merge_new_into_changelog( ) if delete_news: for p in news_files: - p.unlink() + if p.exists(): + p.unlink() if news_files: print(f"Removed {len(news_files)} processed news files.") @@ -309,6 +323,7 @@ def update_changelog( output_path=None, news_dir="news", delete_news=True, + news_files=None, ): """Performs the version replacements in CHANGELOG.md.""" if output_path is None: @@ -320,4 +335,5 @@ def update_changelog( version=version, release_date=release_date, delete_news=delete_news, + news_files=news_files, ) diff --git a/tools/private/release/complete_sync_changelog.py b/tools/private/release/complete_sync_changelog.py new file mode 100644 index 0000000000..9c8d9028f0 --- /dev/null +++ b/tools/private/release/complete_sync_changelog.py @@ -0,0 +1,97 @@ +"""Subcommand to mark sync changelog tasks as complete.""" + +import re + +from tools.private.release.gh import GitHub +from tools.private.release.release_issue import ( + parse_checklist_state, + update_task_in_body, +) + + +class CompleteSyncChangelog: + """Class to mark sync changelog tasks as complete.""" + + def __init__(self, args, gh: GitHub): + self.args = args + self.gh = gh + + def run(self) -> int: + """Executes the complete-sync-changelog subcommand.""" + args = self.args + print(f"Completing sync changelog for PR #{args.pr}...") + + pr_info = self.gh.get_pr_info(args.pr) + if not pr_info or pr_info.get("state") != "MERGED": + state = pr_info.get("state", "UNKNOWN") + print(f"Error: PR #{args.pr} is not merged yet (state: {state}).") + return 1 + + # Resolve issue number from PR body using Release-Tracking-Issue: # + pr_body = pr_info.get("body") or "" + match = re.search(r"Release-Tracking-Issue:\s*#(\d+)", pr_body) + if not match: + print( + f"Error: Could not find 'Release-Tracking-Issue: #' in" + f" PR #{args.pr} body: {pr_body}" + ) + return 1 + + issue_num = int(match.group(1)) + print(f"Resolved tracking issue #{issue_num} from PR #{args.pr} body.") + + commit_sha = pr_info["mergeCommit"]["oid"] + short_commit = commit_sha[:8] + print( + f"PR #{args.pr} merged at commit {commit_sha}. Updating tracking issue..." + ) + + # Update checklist: mark all Sync Changelog tasks pointing to this PR as done + body = self.gh.get_issue_body(issue_num) + state = parse_checklist_state(body) + sync_changelogs = state.get("sync_changelogs", {}) + + updated_any = False + for pr_num, task in sync_changelogs.items(): + # Check if this task points to our merged PR + task_pr = task.metadata.get("pr") + if task_pr == f"#{args.pr}": + print(f"Marking task '{task.name}' as complete...") + metadata = { + "status": "done", + "pr": f"#{args.pr}", + "commit": short_commit, + } + body = update_task_in_body( + body, task.name, checked=True, metadata=metadata + ) + updated_any = True + + if not updated_any: + print(f"Warning: No 'Sync Changelog' tasks found pointing to PR #{args.pr}") + return 0 + + self.gh.update_issue_body(issue_num, body) + print("Sync changelog tasks marked complete successfully!") + return 0 + + @classmethod + def add_parser(cls, subparsers): + """Adds parser for complete-sync-changelog subcommand.""" + parser = subparsers.add_parser( + "complete-sync-changelog", + help="Mark the Sync Changelog tasks as complete in the tracking issue.", + ) + parser.add_argument( + "--pr", + type=int, + required=True, + help="The merged sync changelog PR number.", + ) + parser.set_defaults(command=cls.run_from_args) + + @classmethod + def run_from_args(cls, args): + """Instantiates and runs the command from parsed args.""" + gh = GitHub() + return cls(args, gh).run() diff --git a/tools/private/release/gh.py b/tools/private/release/gh.py index 8990ef5abf..92cc3ca5c4 100644 --- a/tools/private/release/gh.py +++ b/tools/private/release/gh.py @@ -263,24 +263,52 @@ def update_issue_body(self, issue_num: int, body: str) -> None: if os.path.exists(temp_path): os.unlink(temp_path) - def create_pr(self, version: str, issue_num: int) -> str: - """Creates a pull request for release preparation. + def create_pr( + self, + title: str, + body: str, + base: str = "main", + labels: list[str] | None = None, + ) -> str: + """Creates a pull request. Args: - version: The version being prepared. - issue_num: The associated tracking issue number. + title: The title of the PR. + body: The body of the PR. + base: The base branch to merge into (default: 'main'). + labels: Optional list of labels to add to the PR. Returns: The URL of the created PR. """ - output = self._gh_pr( + cmd = [ "create", - f"--title=Prepare release v{version}", - f"--body=Work towards #{issue_num}", - "--base=main", - ) + f"--title={title}", + f"--body={body}", + f"--base={base}", + ] + if labels: + for label in labels: + cmd.append(f"--label={label}") + output = self._gh_pr(*cmd) return output if output else "" + def enable_auto_merge(self, pr_num: int, method: str = "squash") -> None: + """Enables auto-merge for a PR. + + Args: + pr_num: The PR number. + method: The merge method ('squash', 'rebase', or 'merge'). + """ + cmd = ["merge", str(pr_num), "--auto"] + if method == "squash": + cmd.append("--squash") + elif method == "rebase": + cmd.append("--rebase") + elif method == "merge": + cmd.append("--merge") + self._gh_pr(*cmd, capture_output=False) + def get_open_pr(self, branch_name: str) -> dict | None: """Returns PR info if an open PR exists for the given branch. diff --git a/tools/private/release/git.py b/tools/private/release/git.py index 3e7f77f39a..4305de855f 100644 --- a/tools/private/release/git.py +++ b/tools/private/release/git.py @@ -80,7 +80,7 @@ def checkout( self._run_git(*cmd, capture_output=False) if should_reset_hard: - self.reset_hard(f"{track_remote}/{ref}") + self.reset_hard(reset_to=f"{track_remote}/{ref}") def add(self, *files: str) -> None: """Stages files for commit. @@ -191,13 +191,13 @@ def cherry_pick_abort(self) -> None: """Aborts an in-progress cherry-pick operation.""" self._run_git("cherry-pick", "--abort", capture_output=False) - def reset_hard(self, ref: str = "HEAD") -> None: + def reset_hard(self, *, reset_to: str = "HEAD") -> None: """Resets the index and working tree to a specific reference. Args: - ref: The git reference to reset to. Defaults to 'HEAD'. + reset_to: The git reference to reset to. Defaults to 'HEAD'. """ - self._run_git("reset", "--hard", ref, capture_output=False) + self._run_git("reset", "--hard", reset_to, capture_output=False) def status(self) -> str: """Returns the output of git status --porcelain. @@ -353,3 +353,47 @@ def get_remote_tags(self, remote: str) -> list[str]: if not tag.endswith("^{}"): tags.append(tag) return tags + + def get_modified_files(self, ref: str) -> list[str]: + """Returns a list of files modified in a given reference. + + Args: + ref: The git reference. + + Returns: + A list of file paths. + """ + output = self._run_git("show", "--name-only", "--format=", ref) + return [line for line in output.splitlines() if line.strip()] if output else [] + + def diff(self) -> str: + """Returns the diff of unstaged changes. + + Returns: + The diff output as a string. + """ + output = self._run_git("diff") + return output if output else "" + + def apply(self, patch_file: str) -> None: + """Applies a patch file. + + Args: + patch_file: The path to the patch file. + """ + self._run_git("apply", patch_file, capture_output=False) + + def apply_check(self, patch_file: str) -> bool: + """Verifies if a patch can be applied cleanly. + + Args: + patch_file: The path to the patch file. + + Returns: + True if the patch can be applied cleanly, False otherwise. + """ + try: + self._run_git("apply", "--check", patch_file, capture_output=False) + return True + except subprocess.CalledProcessError: + return False diff --git a/tools/private/release/prepare.py b/tools/private/release/prepare.py index 610f2369e7..fd19362bec 100644 --- a/tools/private/release/prepare.py +++ b/tools/private/release/prepare.py @@ -171,7 +171,11 @@ def run(self) -> int: ) pr_num = "" else: - pr_url = self.gh.create_pr(version, issue_num) + pr_url = self.gh.create_pr( + title=f"Prepare release v{version}", + body=f"Work towards #{issue_num}", + base="main", + ) pr_num = pr_url.split("/")[-1] print(f"Created Pull Request: {pr_url} (PR #{pr_num})") diff --git a/tools/private/release/process_backports.py b/tools/private/release/process_backports.py index c8a936eab5..27eb3226b1 100644 --- a/tools/private/release/process_backports.py +++ b/tools/private/release/process_backports.py @@ -2,6 +2,10 @@ import argparse import datetime +import hashlib +import os +import tempfile +from dataclasses import dataclass from typing import Any from tools.private.release import changelog_news @@ -11,6 +15,7 @@ RELEASE_TITLE_RE, add_backports_to_body, add_rc_task_to_body, + add_sync_changelog_task_to_body, parse_backports, parse_checklist_state, update_task_in_body, @@ -22,6 +27,20 @@ ) +@dataclass +class CherryPickAndUpdatePrsResult: + # List of PR references that failed to cherry-pick. + failed_prs: list[str] + # List of news files collected from the successful cherry-picks. + collected_news_files: list[str] + # List of PR numbers that were successfully cherry-picked. + successful_pr_nums: list[int] + # List of tuples mapping successful PR numbers to their version marker diffs. + collected_diffs: list[tuple[int, str]] + # The updated checklist body for the release tracking issue. + body: str + + class ProcessBackports: """Class to process pending backports.""" @@ -84,23 +103,35 @@ def _cherry_pick_and_update_prs( version, branch_name, next_rc_suffix, - ) -> tuple[list[str], str]: + ) -> CherryPickAndUpdatePrsResult: failed_prs = [] + collected_news_files = [] + successful_pr_nums = [] + collected_diffs = [] for sha in sorted_shas: item = sha_to_item[sha] print(f"Cherry-picking {item.pr_ref} / {sha}...") try: self.git.cherry_pick(sha) + # Collect news files before they are deleted by update_changelog + modified_files = self.git.get_modified_files("HEAD") + for f in modified_files: + if changelog_news.is_news_file(f): + collected_news_files.append(f) + + # Replace version markers FIRST to isolate diff + print(f"Replacing version markers for PR {item.pr_ref}...") + replace_version_next(version) + + # Get diff of unstaged changes (version marker replacement) + diff_content = self.git.diff() + # Perform news processing (merging news/ files into the changelog) print(f"Merging news fragments into changelog for PR {item.pr_ref}...") release_date = datetime.date.today().strftime("%Y-%m-%d") changelog_news.update_changelog(version, release_date) - # Replace version markers that might have been introduced by the backport - print(f"Replacing version markers for PR {item.pr_ref}...") - replace_version_next(version) - # Stage changelog changes, news/ deletions, and version placeholder updates self.git.add_modified_and_deleted() @@ -111,6 +142,16 @@ def _cherry_pick_and_update_prs( new_msg = f"{current_msg.strip()}\n\nWork towards #{issue}" self.git.commit(new_msg, amend=True) + try: + pr_num = self.gh.resolve_pr_number(item.pr_ref) + if diff_content: + collected_diffs.append((pr_num, diff_content)) + successful_pr_nums.append(pr_num) + except Exception as e: + print( + f"Warning: Failed to resolve PR number for {item.pr_ref}: {e}" + ) + if not dry_run: # Push amended commit self.git.push(remote, branch_name) @@ -177,7 +218,180 @@ def _cherry_pick_and_update_prs( f"ERROR: Failed to update tracking issue for" f" failed PR {item.pr_ref}: {e}" ) - return failed_prs, body + return CherryPickAndUpdatePrsResult( + failed_prs=failed_prs, + collected_news_files=collected_news_files, + successful_pr_nums=successful_pr_nums, + collected_diffs=collected_diffs, + body=body, + ) + + def _sync_changelog_to_main( + self, + version: str, + collected_news_files: list[str], + successful_pr_nums: list[int], + collected_diffs: list[tuple[int, str]], + release_branch: str, + ) -> None: + args = self.args + sorted_prs = sorted(successful_pr_nums) + prs_str = ",".join(str(n) for n in sorted_prs) + prs_hash = hashlib.sha256(prs_str.encode()).hexdigest()[:7] + + main_branch = "main" + backport_branch = f"prepare-{version}-backports-{prs_hash}" + + print(f"Syncing changelog to {main_branch} via branch {backport_branch}...") + + self.git.fetch(args.remote, refspec=main_branch) + self.git.checkout(main_branch, track_remote=args.remote) + main_start_sha = self.git.get_commit_sha("HEAD") + + failed_version_sync_prs = [] + try: + if args.dry_run: + print( + f"[DRY RUN] Would create and checkout branch {backport_branch} from {main_branch}" + ) + else: + if self.git.branch_exists(backport_branch): + self.git.checkout(backport_branch) + self.git.reset_hard(reset_to=main_branch) + else: + self.git.checkout(backport_branch, create_branch=True) + + print( + f"Updating CHANGELOG.md and removing news files on {backport_branch}..." + ) + release_date = datetime.date.today().strftime("%Y-%m-%d") + changelog_news.update_changelog( + version, + release_date, + news_files=collected_news_files, + delete_news=True, + ) + + # Apply version marker diffs + failed_version_sync_prs = self._apply_version_marker_diffs(collected_diffs) + + if args.dry_run: + print( + f"[DRY RUN] Would commit: 'chore(release): sync changelog for v{version} backports'" + ) + print(f"[DRY RUN] Would push {backport_branch} to {args.remote}") + print( + f"[DRY RUN] Would create PR to {main_branch} with label 'type: sync-changelog'" + ) + print( + f"[DRY RUN] Would update tracking issue #{args.issue} checklist tasks 'Sync Changelog #' to PENDING" + ) + print("[DRY RUN] Diff of changes:") + print(self.git.status()) + else: + self.git.add_modified_and_deleted() + self.git.commit( + f"chore(release): sync changelog for v{version} backports" + ) + self.git.push( + args.remote, backport_branch, set_upstream=True, force=True + ) + + pr_title = f"chore(release): sync changelog for v{version} backports" + pr_body_lines = [ + "Updates CHANGELOG.md and removes news files for backports:", + ] + for pr_num in sorted_prs: + pr_body_lines.append(f"- #{pr_num}") + + if failed_version_sync_prs: + pr_body_lines.append("") + pr_body_lines.append( + "Warning: These PRs failed to update their version markers:" + ) + for pr_num in sorted(failed_version_sync_prs): + pr_body_lines.append(f"- #{pr_num}") + + pr_body_lines.append("") + pr_body_lines.append(f"Work towards #{args.issue}") + pr_body_lines.append(f"Release-Tracking-Issue: #{args.issue}") + pr_body = "\n".join(pr_body_lines) + + print(f"Creating PR to {main_branch}...") + pr_url = self.gh.create_pr( + title=pr_title, + body=pr_body, + base=main_branch, + labels=["type: sync-changelog"], + ) + print(f"Created PR: {pr_url}") + + try: + pr_num = int(pr_url.split("/")[-1]) + print(f"Enabling auto-merge for PR #{pr_num}...") + self.gh.enable_auto_merge(pr_num) + + print( + f"Updating tracking issue #{args.issue} checklist with" + " Sync Changelog tasks..." + ) + issue_body = self.gh.get_issue_body(args.issue) + for pr in successful_pr_nums: + task_name = f"Sync Changelog #{pr}" + metadata = {"status": "pending", "pr": f"#{pr_num}"} + issue_body = update_task_in_body( + issue_body, + task_name, + checked=False, + metadata=metadata, + ) + self.gh.update_issue_body(args.issue, issue_body) + except Exception as e: + print( + f"Warning: Failed to update tracking issue or enable" + f" auto-merge: {e}" + ) + finally: + if args.dry_run: + self.git.reset_hard(reset_to=main_start_sha) + self.git.checkout(release_branch) + + def _apply_version_marker_diffs( + self, + collected_diffs: list[tuple[int, str]], + ) -> list[int]: + """Applies version marker diffs on main branch and returns failed PR numbers.""" + args = self.args + failed_version_sync_prs = [] + if not collected_diffs: + return failed_version_sync_prs + + with tempfile.TemporaryDirectory() as temp_dir: + print(f"Applying {len(collected_diffs)} version marker patches...") + for pr_num, diff_content in collected_diffs: + if args.dry_run: + print( + f"[DRY RUN] Would check and apply version marker patch for PR #{pr_num}" + ) + + patch_filepath = os.path.join(temp_dir, f"{pr_num}.patch") + with open(patch_filepath, "w", encoding="utf-8") as f: + f.write(diff_content) + + if self.git.apply_check(patch_filepath): + if args.dry_run: + print( + f"[DRY RUN] Version marker patch for PR #{pr_num} applies cleanly." + ) + else: + print(f"Applying version marker patch for PR #{pr_num}...") + self.git.apply(patch_filepath) + else: + print( + f"Warning: Version marker patch for PR #{pr_num} could not be applied cleanly to main. Skipping." + ) + failed_version_sync_prs.append(pr_num) + return failed_version_sync_prs def run(self) -> int: """Executes the process-backports subcommand.""" @@ -223,6 +437,14 @@ def _run_internal(self) -> int: print(f"Adding backports {items_to_add} to tracking issue #{args.issue}...") try: body = add_backports_to_body(body, items_to_add) + for item in items_to_add: + if ( + "metadata" in item + and item["metadata"].get("status") == "error-invalid-pr" + ): + continue + pr_num = int(item["ref"].lstrip("#")) + body = add_sync_changelog_task_to_body(body, pr_num) state = parse_checklist_state(body) rc_tags = state.get("rc_tags", {}) has_pending_rc = any( @@ -318,8 +540,11 @@ def _run_internal(self) -> int: self.git.checkout(branch_name, track_remote=args.remote) start_sha = self.git.get_commit_sha("HEAD") + collected_news_files = [] + successful_pr_nums = [] + collected_diffs = [] try: - new_failed_prs, body = self._cherry_pick_and_update_prs( + result = self._cherry_pick_and_update_prs( sorted_shas, sha_to_item, body, @@ -330,11 +555,24 @@ def _run_internal(self) -> int: branch_name, next_rc_suffix, ) - failed_prs.extend(new_failed_prs) + failed_prs.extend(result.failed_prs) + collected_news_files.extend(result.collected_news_files) + successful_pr_nums.extend(result.successful_pr_nums) + collected_diffs.extend(result.collected_diffs) + body = result.body finally: if args.dry_run: print(f"[DRY RUN] Resetting branch {branch_name} to {start_sha}") - self.git.reset_hard(start_sha) + self.git.reset_hard(reset_to=start_sha) + + if successful_pr_nums: + self._sync_changelog_to_main( + version, + collected_news_files, + successful_pr_nums, + collected_diffs, + branch_name, + ) if failed_prs: print("ERROR: One or more cherry-picks/resolutions failed:") diff --git a/tools/private/release/release.py b/tools/private/release/release.py index 8ad9c2e33e..f008c093b1 100644 --- a/tools/private/release/release.py +++ b/tools/private/release/release.py @@ -6,6 +6,7 @@ from tools.private.release.add_backports import AddBackports from tools.private.release.complete_prepare import CompletePrepare +from tools.private.release.complete_sync_changelog import CompleteSyncChangelog from tools.private.release.create_rc import CreateRc from tools.private.release.create_release_branch import CreateReleaseBranch from tools.private.release.create_release_issue import CreateReleaseIssue @@ -20,6 +21,7 @@ CreateReleaseIssue, Prepare, CompletePrepare, + CompleteSyncChangelog, CreateReleaseBranch, AddBackports, ProcessBackports, diff --git a/tools/private/release/release_issue.py b/tools/private/release/release_issue.py index 359249d11c..220348bd47 100644 --- a/tools/private/release/release_issue.py +++ b/tools/private/release/release_issue.py @@ -161,6 +161,7 @@ def parse_checklist_state(body): "create_branch": ReleaseTask("Create Release branch", False), "tag_final": ReleaseTask("Tag Final", False), "rc_tags": {}, # Dynamically mapped: int -> ReleaseTask + "sync_changelogs": {}, # Dynamically mapped: int -> ReleaseTask } lines = body.splitlines() @@ -214,6 +215,19 @@ def parse_checklist_state(body): commit=meta.get("commit"), metadata=meta, ) + else: + # Match Sync Changelog # + sync_match = re.match(r"Sync Changelog #(\d+)", name, re.IGNORECASE) + if sync_match: + pr_num = int(sync_match.group(1)) + state["sync_changelogs"][pr_num] = ReleaseTask( + name=name, + checked=checked, + status=meta.get("status"), + pr=meta.get("pr"), + commit=meta.get("commit"), + metadata=meta, + ) return state @@ -325,3 +339,50 @@ def add_rc_task_to_body(body: str, rc_num: int) -> str: lines.insert(last_rc_idx + 1, new_task_line) return "\n".join(lines) + + +def add_sync_changelog_task_to_body(body: str, pr_num: int) -> str: + """Adds a new 'Sync Changelog #' task to the checklist in the issue body.""" + body = body.replace("\r\n", "\n") + + # Check if already exists + task_name = f"Sync Changelog #{pr_num}" + lines = body.splitlines() + for line in lines: + parsed = parse_metadata_line(line) + if parsed and parsed["name"].lower() == task_name.lower(): + print(f"Task '{task_name}' already exists. Skipping.") + return body + + # Find the index of the last "Sync Changelog #" line + last_sync_idx = -1 + for i, line in enumerate(lines): + parsed = parse_metadata_line(line) + if parsed and re.match(r"Sync Changelog #\d+", parsed["name"], re.IGNORECASE): + last_sync_idx = i + + if last_sync_idx == -1: + # If no Sync Changelog task found, insert before "Tag Final" + for i, line in enumerate(lines): + parsed = parse_metadata_line(line) + if parsed and parsed["name"].lower() == "tag final": + last_sync_idx = i - 1 + break + + if last_sync_idx == -1: + # If "Tag Final" not found, insert after "Create Release branch" + for i, line in enumerate(lines): + parsed = parse_metadata_line(line) + if parsed and parsed["name"].lower() == "create release branch": + last_sync_idx = i + break + + if last_sync_idx == -1: + raise ValueError( + "Could not find a place to insert the new Sync Changelog task." + ) + + new_task_line = f"- [ ] Sync Changelog #{pr_num}" + lines.insert(last_sync_idx + 1, new_task_line) + + return "\n".join(lines)