From ab982d49dd4f096c472827f9f949c5a9f3f5a216 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 26 Sep 2026 15:04:09 -0700 Subject: [PATCH 1/4] docs(changelog): sync v2.3.4 changelog entry for #4156 (#4174) PR #4156 added win32 Python runtimes to the release/2.3 branch without a changelog entry. Add the v2.3.4 release section to CHANGELOG.md on main to keep the changelog synchronized with the 2.3.4 release. Work towards #4173 Release-Tracking-Issue: #4173 --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 013d33c686..72cf0d4e0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,16 @@ changelog](https://rules-python.readthedocs.io/en/latest/changelog.html). package directory, set it to `None` or `""` to set it to the root of the repo ([#4029](https://github.com/bazel-contrib/rules_python/issues/4029)). +{#v2-3-4} +## [2.3.4] - 2026-09-21 + +[2.3.4]: https://github.com/bazel-contrib/rules_python/releases/tag/2.3.4 + +{#v2-3-4-added} +### Added +* (toolchains) Added win32 (`i686-pc-windows-msvc`) Python runtimes for + versions 3.9.25, 3.10.20, 3.11.15, 3.12.13, 3.13.13, and 3.14.4 + ([#4156](https://github.com/bazel-contrib/rules_python/pull/4156)). {#v2-3-3} ## [2.3.3] - 2026-09-04 From 73473fb43f915654d153960e7f37026c1f9e81d5 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 26 Sep 2026 15:23:32 -0700 Subject: [PATCH 2/4] build(release): exclude devguide.md from version marker checks (#4186) `docs/devguide.md` documents how to use `VERSION_NEXT_*` markers, which causes the release workflow version marker check to fail on release branches. Exclude `devguide.md` from the version marker check script and the release tool replacement patterns. Work towards #4175 --- .github/workflows/check_version_markers.sh | 4 +++- dev/release/utils.py | 1 + tests/tools/private/release/utils_test.py | 7 +++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/check_version_markers.sh b/.github/workflows/check_version_markers.sh index 15a0a67dc8..b5a27f02f3 100755 --- a/.github/workflows/check_version_markers.sh +++ b/.github/workflows/check_version_markers.sh @@ -17,9 +17,11 @@ fi grep_exit_code=0 # Exclude dot directories, specifically, this file so that we don't # find the substring we're looking for in our own file. -# Exclude CONTRIBUTING.md, RELEASING.md because they document how to use these strings. +# Exclude CONTRIBUTING.md, RELEASING.md, and devguide.md because they document +# how to use these strings. grep --exclude=CONTRIBUTING.md \ --exclude=RELEASING.md \ + --exclude=devguide.md \ --exclude-dir=.* \ --exclude-dir=release \ VERSION_NEXT_ -r || grep_exit_code=$? diff --git a/dev/release/utils.py b/dev/release/utils.py index 407044aeba..5fd17dbf86 100644 --- a/dev/release/utils.py +++ b/dev/release/utils.py @@ -32,6 +32,7 @@ def semver_type(value): "./bazel-*/*", "./CONTRIBUTING.md", "./RELEASING.md", + "./docs/devguide.md", "./dev/release/*", "./tests/tools/private/release/*", ] diff --git a/tests/tools/private/release/utils_test.py b/tests/tools/private/release/utils_test.py index accae6379c..7c42027043 100644 --- a/tests/tools/private/release/utils_test.py +++ b/tests/tools/private/release/utils_test.py @@ -258,6 +258,10 @@ def test_replace_version_next_excludes_bazel_dirs(release_tool_env): tests_dir.mkdir(parents=True) (tests_dir / "mock_file.bzl").write_text(mock_file_content) + docs_dir = release_tool_env.git_root / "docs" + docs_dir.mkdir(parents=True) + (docs_dir / "devguide.md").write_text(mock_file_content) + version = "0.28.0" # Act @@ -276,6 +280,9 @@ def test_replace_version_next_excludes_bazel_dirs(release_tool_env): new_content = (tests_dir / "mock_file.bzl").read_text() assert "VERSION_NEXT_FEATURE" in new_content + new_content = (docs_dir / "devguide.md").read_text() + assert "VERSION_NEXT_FEATURE" in new_content + def test_determine_next_version_ignores_agents_markers(mocker, release_tool_env): mocker.patch("dev.release.git.Git.get_current_branch", return_value="main") From 3ed114ad604c4c69e81e2a161238c70638721794 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 26 Sep 2026 15:43:34 -0700 Subject: [PATCH 3/4] agents: report deleted resources in conversation-done skill (#4187) Users had no consolidated summary of which resources were removed when running the conversation-done cleanup workflow. Track deleted resources during cleanup execution, print a summary list upon completion, and instruct the agent in the skill documentation to report the deleted resources to the user. --- .agents/skills/conversation-done/SKILL.md | 13 +++++++ .../conversation-done/scripts/cleanup.py | 33 +++++++++++++---- .../conversation-done/scripts/cleanup_test.py | 36 +++++++++++++++++++ 3 files changed, 76 insertions(+), 6 deletions(-) diff --git a/.agents/skills/conversation-done/SKILL.md b/.agents/skills/conversation-done/SKILL.md index 11a5b21309..f11c78b1c8 100644 --- a/.agents/skills/conversation-done/SKILL.md +++ b/.agents/skills/conversation-done/SKILL.md @@ -93,3 +93,16 @@ Fine-tune what gets cleaned up using selective leave flags: * **Upstream Remote Protection**: Never deletes branches on `upstream`. * **No Expunge Rule**: Never runs `bazel clean --expunge`; shuts down servers gracefully and deletes the specific output base directory directly. + +## Reporting Results + +After running the cleanup script, always report to the user what resources were +deleted (or would be deleted in `--dry-run` mode), including: + +* **Bazel Output Base(s)**: Paths and reclaimed disk space sizes. +* **Git Worktree**: Removed worktree directory path. +* **Local Git Branch**: Deleted local branch name. +* **Remote Git Branch**: Deleted remote branch (`/`). + +If any resources were skipped, left intact via `--leave-*` flags, or not found, +note that briefly in the summary. diff --git a/.agents/skills/conversation-done/scripts/cleanup.py b/.agents/skills/conversation-done/scripts/cleanup.py index 24d13a3cd2..1496b1f939 100755 --- a/.agents/skills/conversation-done/scripts/cleanup.py +++ b/.agents/skills/conversation-done/scripts/cleanup.py @@ -519,15 +519,16 @@ async def execute_cleanup_target( leave_branch: bool = False, leave_remote: bool = False, dry_run: bool = False, -) -> None: - """Executes cleanup on a single target.""" +) -> list[str]: + """Executes cleanup on a single target and returns deleted resource descriptions.""" log(f"\nCleaning up target resources for: {target.worktree_path or target.branch}") + deleted: list[str] = [] if target.is_main_worktree: log_error( "Target is the main repository! Refusing to clean up main repository." ) - return + return deleted # 1. Clean up Bazel output bases if not leave_bazel_output_base and target.bazel_output_bases: @@ -535,7 +536,8 @@ async def execute_cleanup_target( for ob in target.bazel_output_bases: size_str = await get_path_size_human(ob) log(f" - Output base: {ob} ({size_str})") - await shutdown_and_remove_bazel_output_base(ob, dry_run=dry_run) + if await shutdown_and_remove_bazel_output_base(ob, dry_run=dry_run): + deleted.append(f"Bazel output base: {ob} ({size_str})") elif leave_bazel_output_base: log("1. Leaving Bazel output base intact.") else: @@ -544,7 +546,8 @@ async def execute_cleanup_target( # 2. Clean up Git worktree if not leave_worktree and target.worktree_path: log("2. Cleaning up Git worktree:") - await remove_git_worktree(main_repo, target.worktree_path, dry_run=dry_run) + if await remove_git_worktree(main_repo, target.worktree_path, dry_run=dry_run): + deleted.append(f"Git worktree: {target.worktree_path}") elif leave_worktree: log("2. Leaving Git worktree directory on disk.") else: @@ -558,6 +561,7 @@ async def execute_cleanup_target( ) if success: log(f" {msg}") + deleted.append(f"Local branch: {target.branch}") else: log_warn(f" {msg}") elif leave_branch: @@ -578,6 +582,7 @@ async def execute_cleanup_target( ) if success: log(f" {msg}") + deleted.append(f"Remote branch: {target.remote}/{target.branch}") else: log_warn(f" {msg}") elif leave_remote: @@ -585,6 +590,8 @@ async def execute_cleanup_target( elif target.branch and target.remote: log(f"4. Remote branch '{target.branch}' not present on '{target.remote}'.") + return deleted + async def list_resources(main_repo: str) -> None: """Lists all worktrees and their associated resources.""" @@ -759,8 +766,9 @@ async def async_main() -> None: log("Aborted.") return + deleted_resources: list[str] = [] for target in targets: - await execute_cleanup_target( + deleted = await execute_cleanup_target( main_repo, target, leave_bazel_output_base=args.leave_bazel_output_base, @@ -769,6 +777,19 @@ async def async_main() -> None: leave_remote=args.leave_remote, dry_run=args.dry_run, ) + deleted_resources.extend(deleted) + + summary_header = ( + "\nResources that would be deleted (dry run):" + if args.dry_run + else "\nDeleted resources:" + ) + log(summary_header) + if deleted_resources: + for resource in deleted_resources: + log(f" - {resource}") + else: + log(" None") log("\nCleanup completed.") diff --git a/.agents/skills/conversation-done/scripts/cleanup_test.py b/.agents/skills/conversation-done/scripts/cleanup_test.py index 18a38b1ef3..79abfa9a9a 100644 --- a/.agents/skills/conversation-done/scripts/cleanup_test.py +++ b/.agents/skills/conversation-done/scripts/cleanup_test.py @@ -148,6 +148,42 @@ async def test_build_cleanup_target_branches(self): self.assertEqual(target.remote, "origin") self.assertTrue(target.remote_branch_exists) + async def test_execute_cleanup_target_reports_deleted_resources(self): + """Verifies execute_cleanup_target returns the list of deleted resources.""" + target = cleanup.CleanupTarget( + worktree_path="/repo/worktrees/feat1", + branch="feat1", + remote="origin", + remote_branch_exists=True, + bazel_output_bases=["/cache/bazel/ob1"], + is_main_worktree=False, + ) + with mock.patch( + "cleanup.get_path_size_human", return_value="1.50 GB" + ), mock.patch( + "cleanup.shutdown_and_remove_bazel_output_base", return_value=True + ), mock.patch("cleanup.remove_git_worktree", return_value=True), mock.patch( + "cleanup.delete_local_branch", + return_value=(True, "Deleted local branch 'feat1'."), + ), mock.patch( + "cleanup.delete_remote_branch", + return_value=( + True, + "Deleted remote branch 'feat1' from 'origin'.", + ), + ): + deleted = await cleanup.execute_cleanup_target("/repo/main", target) + + self.assertEqual( + deleted, + [ + "Bazel output base: /cache/bazel/ob1 (1.50 GB)", + "Git worktree: /repo/worktrees/feat1", + "Local branch: feat1", + "Remote branch: origin/feat1", + ], + ) + if __name__ == "__main__": unittest.main() From faee3a79f66a8fa86ba2058b47aaa8325f91603a Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 26 Sep 2026 15:56:01 -0700 Subject: [PATCH 4/4] build(release): check VERSION_NEXT_ markers during create-rc (#4188) Checking `VERSION_NEXT_` markers during post-tag archive and release notes generation fails too late in the release process, after the RC tag is already created and pushed. Have `release.py create-rc` check the release branch for leftover `VERSION_NEXT_` markers before tagging or updating the tracking issue, and remove `check_version_markers.sh` from release archive generation. --- .github/workflows/check_version_markers.sh | 33 -------------- .github/workflows/create_archive_and_notes.sh | 3 -- dev/release/create_rc.py | 19 ++++++-- dev/release/utils.py | 21 +++++++++ tests/tools/private/release/create_rc_test.py | 44 ++++++++++++++++++- tests/tools/private/release/utils_test.py | 39 ++++++++++++++++ 6 files changed, 118 insertions(+), 41 deletions(-) delete mode 100755 .github/workflows/check_version_markers.sh diff --git a/.github/workflows/check_version_markers.sh b/.github/workflows/check_version_markers.sh deleted file mode 100755 index b5a27f02f3..0000000000 --- a/.github/workflows/check_version_markers.sh +++ /dev/null @@ -1,33 +0,0 @@ -#!/usr/bin/env bash - -set -o nounset -set -o pipefail -set -o errexit - -set -x - -TAG=${1:-} -if [ -n "$TAG" ]; then - # If the workflow checks out one commit, but is releasing another - git fetch origin tag "$TAG" - # Update our local state so the grep command below searches what we expect - git checkout "$TAG" -fi - -grep_exit_code=0 -# Exclude dot directories, specifically, this file so that we don't -# find the substring we're looking for in our own file. -# Exclude CONTRIBUTING.md, RELEASING.md, and devguide.md because they document -# how to use these strings. -grep --exclude=CONTRIBUTING.md \ - --exclude=RELEASING.md \ - --exclude=devguide.md \ - --exclude-dir=.* \ - --exclude-dir=release \ - VERSION_NEXT_ -r || grep_exit_code=$? - -if [[ $grep_exit_code -eq 0 ]]; then - echo - echo "Found VERSION_NEXT markers indicating version needs to be specified" - exit 1 -fi diff --git a/.github/workflows/create_archive_and_notes.sh b/.github/workflows/create_archive_and_notes.sh index 26973ac0e5..d3e0b583ed 100755 --- a/.github/workflows/create_archive_and_notes.sh +++ b/.github/workflows/create_archive_and_notes.sh @@ -26,10 +26,7 @@ if [ -z "$TAG" ]; then fi # If the workflow checks out one commit, but is releasing another git fetch origin tag "$TAG" - -# Update our local state so that check_version_markers searches what we expect git checkout "$TAG" -$(dirname $0)/check_version_markers.sh # A prefix is added to better match the GitHub generated archives. PREFIX="rules_python-${TAG}" diff --git a/dev/release/create_rc.py b/dev/release/create_rc.py index b4e26190b1..fabf0c54e4 100644 --- a/dev/release/create_rc.py +++ b/dev/release/create_rc.py @@ -16,6 +16,7 @@ ) from dev.release.utils import ( REPO_URL, + find_version_markers, get_latest_rc_tag, set_github_output, ) @@ -122,6 +123,21 @@ def _run_internal(self) -> int: next_rc_num = rc_num + 1 next_rc = f"{version}-rc{next_rc_num}" + target_ref = f"{args.remote}/{branch_name}" + commit_sha = self.git.get_commit_sha(target_ref) + self.git.checkout(target_ref) + + print("Checking for leftover VERSION_NEXT_ markers...") + markers = find_version_markers() + if markers: + for marker in markers: + print(marker) + print( + "Error: Found VERSION_NEXT markers indicating version needs to" + " be specified." + ) + return 1 + # Precheck: next RC number must exist and be unchecked in the checklist rc_tags = state.get("rc_tags", {}) if next_rc_num not in rc_tags: @@ -137,9 +153,6 @@ def _run_internal(self) -> int: ) return 1 - target_ref = f"{args.remote}/{branch_name}" - commit_sha = self.git.get_commit_sha(target_ref) - print(f"Tagging and pushing next RC: {next_rc}...") self.git.tag(next_rc, target_ref) self.git.push(args.remote, next_rc) diff --git a/dev/release/utils.py b/dev/release/utils.py index 5fd17dbf86..186a65f30a 100644 --- a/dev/release/utils.py +++ b/dev/release/utils.py @@ -211,6 +211,27 @@ def replace_version_next(version: str) -> list[pathlib.Path]: return replace_version_next_in_files(_iter_version_placeholder_files(), version) +def find_version_markers() -> list[str]: + """Finds any remaining VERSION_NEXT_ markers in non-excluded files. + + Returns: + A list of strings in the format 'filepath:line_number: line_content' + for each line containing 'VERSION_NEXT_'. + """ + matches: list[str] = [] + for filepath in _iter_version_placeholder_files(): + try: + content = filepath.read_text(encoding="utf-8") + except (IOError, UnicodeDecodeError): + continue + + if "VERSION_NEXT_" in content: + for line_num, line in enumerate(content.splitlines(), start=1): + if "VERSION_NEXT_" in line: + matches.append(f"{filepath}:{line_num}: {line}") + return matches + + def parse_pr_list(value: str) -> list[str]: """Parses a comma or space separated list of PR references. diff --git a/tests/tools/private/release/create_rc_test.py b/tests/tools/private/release/create_rc_test.py index f5b8f1cb72..70905beb79 100644 --- a/tests/tools/private/release/create_rc_test.py +++ b/tests/tools/private/release/create_rc_test.py @@ -4,11 +4,18 @@ import tempfile from unittest.mock import call +import pytest + from dev.release.create_rc import CreateRc pytest_plugins = ["tests.tools.private.release.release_test_helper"] +@pytest.fixture(name="isolate_cwd", autouse=True) +def fixture_isolate_cwd(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + + def test_create_rc_success_first_rc(mocker, mock_git, mock_gh): # Arrange args = argparse.Namespace( @@ -41,7 +48,7 @@ def test_create_rc_success_first_rc(mocker, mock_git, mock_gh): mock_git.fetch.assert_has_calls( [call("my-remote"), call("my-remote", tags=True, force=True)] ) - mock_git.checkout.assert_not_called() + mock_git.checkout.assert_called_once_with("my-remote/release/2.0") mock_git.tag.assert_called_once_with("2.0.0-rc0", "my-remote/release/2.0") mock_git.push.assert_called_once_with("my-remote", "2.0.0-rc0") mock_git.get_commit_sha.assert_called_once_with("my-remote/release/2.0") @@ -138,7 +145,7 @@ def test_create_rc_success_next_rc(mock_git, mock_gh): mock_git.fetch.assert_has_calls( [call("my-remote"), call("my-remote", tags=True, force=True)] ) - mock_git.checkout.assert_not_called() + mock_git.checkout.assert_called_once_with("my-remote/release/2.0") mock_git.tag.assert_called_once_with("2.0.0-rc1", "my-remote/release/2.0") mock_git.push.assert_called_once_with("my-remote", "2.0.0-rc1") mock_git.get_commit_sha.assert_called_once_with("my-remote/release/2.0") @@ -418,3 +425,36 @@ def test_create_rc_precondition_failure_reacts_to_comment(mocker, mock_git, mock # Assert assert result == 1 assert mock_gh.reactions.get(456) == ["-1"] + + +def test_create_rc_fails_on_version_markers(tmp_path, mock_git, mock_gh): + # Arrange + args = argparse.Namespace( + issue=123, remote="my-remote", triggering_comment=456, dry_run=False + ) + initial_body = """ +## Checklist +- [x] Prepare Release | status=done pr=#122 commit=abcdef12 +- [x] Create Release branch | status=done branch=release/2.0 commit=abcdef12 +- [ ] Tag RC0 | status=pending +""" + mock_gh.issues[123] = { + "title": "Release 2.0.0", + "body": initial_body, + "labels": ["type: release"], + } + mock_git.get_remote_tags.return_value = [] + mock_git.get_commit_sha.return_value = "1234567890" + (tmp_path / "dirty.bzl").write_text(":::{versionadded} VERSION_NEXT_FEATURE\n") + + # Act + result = CreateRc(args, mock_git, mock_gh).run() + + # Assert + assert result == 1 + mock_git.checkout.assert_called_once_with("my-remote/release/2.0") + mock_git.tag.assert_not_called() + mock_git.push.assert_not_called() + assert mock_gh.get_issue_body(123) == initial_body + assert 123 not in mock_gh.issue_comments + assert mock_gh.reactions.get(456) == ["-1"] diff --git a/tests/tools/private/release/utils_test.py b/tests/tools/private/release/utils_test.py index 7c42027043..bb0eb21ea7 100644 --- a/tests/tools/private/release/utils_test.py +++ b/tests/tools/private/release/utils_test.py @@ -345,3 +345,42 @@ def test_format_exception_empty_message_with_notes(): e = Exception() e.add_note("Note only") assert utils.format_exception(e) == "Note only" + + +def test_find_version_markers_none(release_tool_env): + (release_tool_env.git_root / "clean.bzl").write_text(":::{versionadded} 1.2.0\n") + assert utils.find_version_markers() == [] + + +def test_find_version_markers_found(release_tool_env): + (release_tool_env.git_root / "dirty.bzl").write_text( + "line 1\n" + ":::{versionadded} VERSION_NEXT_FEATURE\n" + ":::{versionchanged} VERSION_NEXT_PATCH\n" + ) + markers = utils.find_version_markers() + assert len(markers) == 2 + assert any( + "dirty.bzl:2: :::{versionadded} VERSION_NEXT_FEATURE" in m for m in markers + ) + assert any( + "dirty.bzl:3: :::{versionchanged} VERSION_NEXT_PATCH" in m for m in markers + ) + + +def test_find_version_markers_excludes_paths(release_tool_env): + content = ":::{versionadded} VERSION_NEXT_FEATURE\n" + for rel_path in [ + ".agents/rule.md", + "bazel-out/file.bzl", + "dev/release/tool.py", + "tests/tools/private/release/test.py", + "docs/devguide.md", + "CONTRIBUTING.md", + "RELEASING.md", + ]: + p = release_tool_env.git_root / rel_path + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(content) + + assert utils.find_version_markers() == []