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.

Author: rickeylev

.github/workflows/check_version_markers.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}"

.github/workflows/create_archive_and_notes.sh

@@ -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)

dev/release/create_rc.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.
 

dev/release/utils.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"]

tests/tools/private/release/create_rc_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() == []