From a736361719c9c422703f4374bfc29413875f7bfc Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 5 Jul 2026 18:53:10 +0000 Subject: [PATCH 1/6] chore(release): sync changelog and version markers to main during backports Collects news files and version marker diffs during backport cherry-picks, and proposes a PR to main to apply them (with auto-merge). - Add git diff/apply helpers. - Update GH helper for PR creation and auto-merge. - Update changelog utility for selective news and semver sorting. - Update process_backports to orchestrate the sync PR. --- .../private/release/changelog_news_test.py | 89 +++++++ tests/tools/private/release/git_test.py | 83 ++++++ tests/tools/private/release/prepare_test.py | 18 +- .../private/release/process_backports_test.py | 244 ++++++++++++++++-- tools/private/release/changelog_news.py | 90 +++++-- tools/private/release/gh.py | 31 ++- tools/private/release/git.py | 44 ++++ tools/private/release/prepare.py | 6 +- tools/private/release/process_backports.py | 218 +++++++++++++++- 9 files changed, 766 insertions(+), 57 deletions(-) diff --git a/tests/tools/private/release/changelog_news_test.py b/tests/tools/private/release/changelog_news_test.py index f8bd141a55..214ac1f815 100644 --- a/tests/tools/private/release/changelog_news_test.py +++ b/tests/tools/private/release/changelog_news_test.py @@ -408,6 +408,95 @@ 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) + 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..5403117e4b 100644 --- a/tests/tools/private/release/git_test.py +++ b/tests/tools/private/release/git_test.py @@ -82,5 +82,88 @@ 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) + + 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..5378914bee 100644 --- a/tests/tools/private/release/process_backports_test.py +++ b/tests/tools/private/release/process_backports_test.py @@ -20,6 +20,8 @@ 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 def test_process_backports_no_pending(self): args = argparse.Namespace( @@ -60,28 +62,72 @@ 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_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", + base="main", ) - self.mock_git.push.assert_called_once_with("origin", "release/2.0") + self.mock_gh.enable_auto_merge.assert_called_once_with(999) self.mock_gh.update_issue_body.assert_called_once() call_args = self.mock_gh.update_issue_body.call_args[0] @@ -115,28 +161,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("12345678"), + call("main_sha"), + ] + ) self.mock_git.push.assert_not_called() self.mock_gh.update_issue_body.assert_not_called() @@ -353,6 +426,141 @@ def mock_resolve(items): self.assertIn("- [ ] #125", call1_args[1]) self.assertIn("- [ ] invalid | status=error-invalid-pr", call1_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.mock_gh.get_issue_body.return_value = """ +## Checklist +- [ ] Prepare Release +- [ ] Create Release branch + +## 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" + ) + self.mock_gh.create_pr.assert_called_once_with( + title="chore(release): sync changelog for v2.0.0 backports", + body=expected_body, + base="main", + ) + self.mock_gh.enable_auto_merge.assert_called_once_with(999) + + # update_issue_body called twice (once per successful PR) + self.assertEqual(self.mock_gh.update_issue_body.call_count, 2) + if __name__ == "__main__": unittest.main() diff --git a/tools/private/release/changelog_news.py b/tools/private/release/changelog_news.py index b896af1f6b..21cde4ab1f 100644 --- a/tools/private/release/changelog_news.py +++ b/tools/private/release/changelog_news.py @@ -108,6 +108,32 @@ 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() + + return len(changelog_content) + + 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,27 +279,44 @@ 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)" - ) + # Find insertion point + insertion_point = _find_insertion_point(changelog_content, version) - if not re.search(pattern, changelog_content, re.DOTALL): - raise RuntimeError( - "Could not find active Unreleased section to replace in CHANGELOG.md" - ) + # Check if we are inserting at the top (before the first existing version) + matches = list(re.finditer(r"\{#v(?P\d+-\d+-\d+)\}", changelog_content)) + first_version_start = matches[0].start() if matches else len(changelog_content) - new_content = re.sub( - pattern, - replacement, - changelog_content, - count=1, - flags=re.DOTALL, - ) + if insertion_point == first_version_start: + # Insert at the top and reset Unreleased section + replacement = ( + f"{{#unreleased}}\n{_UNRELEASED_TEMPLATE_BODY}\n\n{new_release_block}\n" + ) + 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" + ) + new_content = re.sub( + pattern, + replacement, + changelog_content, + count=1, + flags=re.DOTALL, + ) + else: + # Insert in the middle or end, do NOT touch Unreleased section + if insertion_point == len(changelog_content): + new_content = ( + changelog_content.rstrip() + "\n\n" + new_release_block + "\n" + ) + else: + 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 +327,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 +344,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 +357,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 +369,5 @@ def update_changelog( version=version, release_date=release_date, delete_news=delete_news, + news_files=news_files, ) diff --git a/tools/private/release/gh.py b/tools/private/release/gh.py index 8990ef5abf..8a588aac72 100644 --- a/tools/private/release/gh.py +++ b/tools/private/release/gh.py @@ -263,24 +263,41 @@ 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") -> 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'). Returns: The URL of the created PR. """ output = self._gh_pr( "create", - f"--title=Prepare release v{version}", - f"--body=Work towards #{issue_num}", - "--base=main", + f"--title={title}", + f"--body={body}", + f"--base={base}", ) 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..275bbd6e04 100644 --- a/tools/private/release/git.py +++ b/tools/private/release/git.py @@ -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..cdf0b8a0b2 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 @@ -22,6 +26,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 +102,39 @@ 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 ( + f.startswith("news/") + and f.endswith(".md") + and f != "news/BUILD.bazel" + ): + 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 +145,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 +221,149 @@ 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: + 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("[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 = "\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, + ) + 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) + except Exception as e: + print(f"Warning: Failed to enable auto-merge: {e}") + finally: + if args.dry_run: + self.git.reset_hard(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") 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.""" @@ -318,8 +504,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,12 +519,25 @@ 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) + 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:") for pr in failed_prs: From ba62655be76751f288c4a9139da4570124b15e1d Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 5 Jul 2026 19:02:00 +0000 Subject: [PATCH 2/6] chore(release): preserve Unreleased static text in changelog Simplifies the changelog insertion logic to always insert the new release block at the insertion point, rather than replacing the Unreleased section. This preserves any user changes to the static Unreleased text in CHANGELOG.md. Also updates the tests to match the current static Unreleased section format. --- .../private/release/changelog_news_test.py | 18 ++----- tools/private/release/changelog_news.py | 49 +++---------------- 2 files changed, 11 insertions(+), 56 deletions(-) diff --git a/tests/tools/private/release/changelog_news_test.py b/tests/tools/private/release/changelog_news_test.py index 214ac1f815..5c10365059 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 diff --git a/tools/private/release/changelog_news.py b/tools/private/release/changelog_news.py index 21cde4ab1f..0901f8c8b8 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.""" @@ -283,40 +275,15 @@ def _add_news_to_changelog(input_path, output_path, version, entries, release_da # Find insertion point insertion_point = _find_insertion_point(changelog_content, version) - # Check if we are inserting at the top (before the first existing version) - matches = list(re.finditer(r"\{#v(?P\d+-\d+-\d+)\}", changelog_content)) - first_version_start = matches[0].start() if matches else len(changelog_content) - - if insertion_point == first_version_start: - # Insert at the top and reset Unreleased section - replacement = ( - f"{{#unreleased}}\n{_UNRELEASED_TEMPLATE_BODY}\n\n{new_release_block}\n" - ) - 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" - ) - new_content = re.sub( - pattern, - replacement, - changelog_content, - count=1, - flags=re.DOTALL, - ) + if insertion_point == len(changelog_content): + new_content = changelog_content.rstrip() + "\n\n" + new_release_block + "\n" else: - # Insert in the middle or end, do NOT touch Unreleased section - if insertion_point == len(changelog_content): - new_content = ( - changelog_content.rstrip() + "\n\n" + new_release_block + "\n" - ) - else: - new_content = ( - changelog_content[:insertion_point] - + new_release_block - + "\n\n" - + changelog_content[insertion_point:] - ) + new_content = ( + changelog_content[:insertion_point] + + new_release_block + + "\n\n" + + changelog_content[insertion_point:] + ) output_path.write_text(new_content, encoding="utf-8") From 21bc57ccdc838e0bd79c79e0c8856987b0ad87eb Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 5 Jul 2026 19:05:21 +0000 Subject: [PATCH 3/6] chore(release): raise error if no smaller version in changelog Updates _find_insertion_point to raise a ValueError if it cannot find an existing version in CHANGELOG.md that is smaller than the new version being inserted. This replaces the previous behavior of appending to the end of the file, which should not occur in practice. Also adds a test case to verify this error behavior. --- .../private/release/changelog_news_test.py | 34 +++++++++++++++++++ tools/private/release/changelog_news.py | 19 +++++------ 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/tests/tools/private/release/changelog_news_test.py b/tests/tools/private/release/changelog_news_test.py index 5c10365059..83ab2cc959 100644 --- a/tests/tools/private/release/changelog_news_test.py +++ b/tests/tools/private/release/changelog_news_test.py @@ -485,6 +485,40 @@ def test_update_changelog_insertion_point(self): 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/tools/private/release/changelog_news.py b/tools/private/release/changelog_news.py index 0901f8c8b8..e09b082ade 100644 --- a/tools/private/release/changelog_news.py +++ b/tools/private/release/changelog_news.py @@ -123,7 +123,9 @@ def _find_insertion_point(changelog_content, new_version): if parsed_ver < parsed_new_ver: return m.start() - return len(changelog_content) + 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): @@ -275,15 +277,12 @@ def _add_news_to_changelog(input_path, output_path, version, entries, release_da # Find insertion point insertion_point = _find_insertion_point(changelog_content, version) - if insertion_point == len(changelog_content): - new_content = changelog_content.rstrip() + "\n\n" + new_release_block + "\n" - else: - new_content = ( - changelog_content[:insertion_point] - + new_release_block - + "\n\n" - + changelog_content[insertion_point:] - ) + new_content = ( + changelog_content[:insertion_point] + + new_release_block + + "\n\n" + + changelog_content[insertion_point:] + ) output_path.write_text(new_content, encoding="utf-8") From ce58b87eebd93dea7c94f35043290c8ae7ddc5ad Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 6 Jul 2026 01:33:31 +0000 Subject: [PATCH 4/6] chore(release): address PR comments on sync PR - Use is_news_file helper to avoid accidental deletion of non-news files. - Make branch checkout robust by resetting existing branch to main. - Specify UTF-8 encoding when writing patch files. --- .../private/release/process_backports_test.py | 109 ++++++++++++++++++ tools/private/release/process_backports.py | 14 +-- 2 files changed, 116 insertions(+), 7 deletions(-) diff --git a/tests/tools/private/release/process_backports_test.py b/tests/tools/private/release/process_backports_test.py index 5378914bee..34030e94d0 100644 --- a/tests/tools/private/release/process_backports_test.py +++ b/tests/tools/private/release/process_backports_test.py @@ -134,6 +134,115 @@ def mock_resolve(items): self.assertEqual(call_args[0], 123) self.assertIn("- [x] #124 | status=done rc=rc0 commit= 12345678", call_args[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.mock_gh.get_issue_body.return_value = """ +## Checklist +- [ ] Prepare Release +- [ ] Create Release branch + +## 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("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", + base="main", + ) + self.mock_gh.enable_auto_merge.assert_called_once_with(999) + + 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]) + @patch("tools.private.release.process_backports.datetime") def test_process_backports_dry_run(self, mock_datetime): mock_datetime.date.today.return_value = datetime.date(2026, 7, 1) diff --git a/tools/private/release/process_backports.py b/tools/private/release/process_backports.py index cdf0b8a0b2..fd61c359cd 100644 --- a/tools/private/release/process_backports.py +++ b/tools/private/release/process_backports.py @@ -116,11 +116,7 @@ def _cherry_pick_and_update_prs( # Collect news files before they are deleted by update_changelog modified_files = self.git.get_modified_files("HEAD") for f in modified_files: - if ( - f.startswith("news/") - and f.endswith(".md") - and f != "news/BUILD.bazel" - ): + if changelog_news.is_news_file(f): collected_news_files.append(f) # Replace version markers FIRST to isolate diff @@ -258,7 +254,11 @@ def _sync_changelog_to_main( f"[DRY RUN] Would create and checkout branch {backport_branch} from {main_branch}" ) else: - self.git.checkout(backport_branch, create_branch=True) + if self.git.branch_exists(backport_branch): + self.git.checkout(backport_branch) + self.git.reset_hard(main_branch) + else: + self.git.checkout(backport_branch, create_branch=True) print( f"Updating CHANGELOG.md and removing news files on {backport_branch}..." @@ -347,7 +347,7 @@ def _apply_version_marker_diffs( ) patch_filepath = os.path.join(temp_dir, f"{pr_num}.patch") - with open(patch_filepath, "w") as f: + with open(patch_filepath, "w", encoding="utf-8") as f: f.write(diff_content) if self.git.apply_check(patch_filepath): From 24c1c10dcfd416f3169cf4ed5dd03da8e404e3a3 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 6 Jul 2026 01:42:43 +0000 Subject: [PATCH 5/6] refactor(release): modify reset_hard signature to enforce keyword argument - Renames the parameter to . - Requires it to be passed as a keyword argument. - Updates all callers and tests. --- tests/tools/private/release/git_test.py | 22 ++++++++++++++++++- .../private/release/process_backports_test.py | 6 ++--- tools/private/release/git.py | 8 +++---- tools/private/release/process_backports.py | 6 ++--- 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/tests/tools/private/release/git_test.py b/tests/tools/private/release/git_test.py index 5403117e4b..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): @@ -165,5 +165,25 @@ def test_apply_check_conflict(self): 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/process_backports_test.py b/tests/tools/private/release/process_backports_test.py index 34030e94d0..8780305c3f 100644 --- a/tests/tools/private/release/process_backports_test.py +++ b/tests/tools/private/release/process_backports_test.py @@ -193,7 +193,7 @@ def mock_resolve(items): # Verify reset_hard was called to reset the existing branch to main self.mock_git.reset_hard.assert_has_calls( [ - call("main"), + call(reset_to="main"), ] ) self.mock_git.cherry_pick.assert_called_once_with("abcdef12") @@ -315,8 +315,8 @@ def mock_resolve(items): ) self.mock_git.reset_hard.assert_has_calls( [ - call("12345678"), - call("main_sha"), + call(reset_to="12345678"), + call(reset_to="main_sha"), ] ) self.mock_git.push.assert_not_called() diff --git a/tools/private/release/git.py b/tools/private/release/git.py index 275bbd6e04..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. diff --git a/tools/private/release/process_backports.py b/tools/private/release/process_backports.py index fd61c359cd..7b832f2ed7 100644 --- a/tools/private/release/process_backports.py +++ b/tools/private/release/process_backports.py @@ -256,7 +256,7 @@ def _sync_changelog_to_main( else: if self.git.branch_exists(backport_branch): self.git.checkout(backport_branch) - self.git.reset_hard(main_branch) + self.git.reset_hard(reset_to=main_branch) else: self.git.checkout(backport_branch, create_branch=True) @@ -325,7 +325,7 @@ def _sync_changelog_to_main( print(f"Warning: Failed to enable auto-merge: {e}") finally: if args.dry_run: - self.git.reset_hard(main_start_sha) + self.git.reset_hard(reset_to=main_start_sha) self.git.checkout(release_branch) def _apply_version_marker_diffs( @@ -527,7 +527,7 @@ def _run_internal(self) -> int: 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( From f6e66feb66884ed7428ee2cf3c23073b30063b45 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 6 Jul 2026 02:13:28 +0000 Subject: [PATCH 6/6] feat(release): track Sync Changelog tasks in release issue Add 'Sync Changelog' tasks to checklist when adding backports. Automatically mark them as done when the sync PR is merged using a new complete-sync-changelog subcommand and GitHub Actions workflow. --- .github/workflows/on_pr_closed.yaml | 30 ++++ tests/tools/private/release/BUILD.bazel | 9 ++ .../private/release/add_backports_test.py | 3 + .../release/complete_sync_changelog_test.py | 120 ++++++++++++++++ .../private/release/process_backports_test.py | 136 ++++++++++++++---- .../private/release/release_issue_test.py | 61 ++++++++ tools/private/release/add_backports.py | 3 + .../release/complete_sync_changelog.py | 97 +++++++++++++ tools/private/release/gh.py | 17 ++- tools/private/release/process_backports.py | 38 ++++- tools/private/release/release.py | 2 + tools/private/release/release_issue.py | 61 ++++++++ 12 files changed, 543 insertions(+), 34 deletions(-) create mode 100644 tests/tools/private/release/complete_sync_changelog_test.py create mode 100644 tools/private/release/complete_sync_changelog.py 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/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/process_backports_test.py b/tests/tools/private/release/process_backports_test.py index 8780305c3f..d5df43ae46 100644 --- a/tests/tools/private/release/process_backports_test.py +++ b/tests/tools/private/release/process_backports_test.py @@ -23,11 +23,23 @@ def setUp(self): 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() @@ -42,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 @@ -124,15 +138,23 @@ def mock_resolve(items): 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", + 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.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.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): @@ -141,10 +163,12 @@ def test_process_backports_sync_branch_exists(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 @@ -233,15 +257,23 @@ def mock_resolve(items): 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", + 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.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.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): @@ -250,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 @@ -327,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 @@ -368,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 @@ -393,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 @@ -443,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 @@ -466,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] @@ -481,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) @@ -501,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 @@ -517,23 +562,42 @@ 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): @@ -542,10 +606,13 @@ def test_process_backports_version_sync_failure(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 +- [ ] Sync Changelog #125 +- [ ] Tag Final ## Backports - [ ] #124 | status=pending @@ -658,17 +725,26 @@ def mock_resolve(items): "Warning: These PRs failed to update their version markers:\n" "- #124\n" "\n" - "Work towards #123" + "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 twice (once per successful PR) - self.assertEqual(self.mock_gh.update_issue_body.call_count, 2) + # 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/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 8a588aac72..92cc3ca5c4 100644 --- a/tools/private/release/gh.py +++ b/tools/private/release/gh.py @@ -263,23 +263,34 @@ 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, title: str, body: str, base: str = "main") -> str: + def create_pr( + self, + title: str, + body: str, + base: str = "main", + labels: list[str] | None = None, + ) -> str: """Creates a pull request. Args: 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={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: diff --git a/tools/private/release/process_backports.py b/tools/private/release/process_backports.py index 7b832f2ed7..27eb3226b1 100644 --- a/tools/private/release/process_backports.py +++ b/tools/private/release/process_backports.py @@ -15,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, @@ -279,6 +280,12 @@ def _sync_changelog_to_main( 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: @@ -307,6 +314,7 @@ def _sync_changelog_to_main( 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}...") @@ -314,6 +322,7 @@ def _sync_changelog_to_main( title=pr_title, body=pr_body, base=main_branch, + labels=["type: sync-changelog"], ) print(f"Created PR: {pr_url}") @@ -321,8 +330,27 @@ def _sync_changelog_to_main( 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 enable auto-merge: {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) @@ -409,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( 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)