chore(release): reuse existing PRs and avoid early exit in prep tool (#3870)
Fix the release preparation tool to prevent it from exiting early when
no local changes are present, ensuring the pipeline can always proceed.
The tool now detects and reuses existing open branch PRs or PRs already
associated with the tracking issue, avoiding the creation of duplicate
PRs.
diff --git a/tests/tools/private/release/release_test.py b/tests/tools/private/release/release_test.py
index 534aa2a..a1467d8 100644
--- a/tests/tools/private/release/release_test.py
+++ b/tests/tools/private/release/release_test.py
@@ -35,6 +35,7 @@
mock_git.branch_exists.return_value = False
mock_git.tag_exists.return_value = False
mock_gh.get_release_tracking_issue.side_effect = NoTrackingIssueError("Not found")
+ mock_gh.get_open_pr.return_value = None
class TempDirTestCase(unittest.TestCase):
@@ -801,6 +802,100 @@
@patch("tools.private.release.prepare.changelog_news")
@patch("tools.private.release.prepare.replace_version_next")
+ def test_prepare_use_associated_pr_from_tracking_issue(
+ self, mock_replace, mock_changelog
+ ):
+ # Arrange
+ args = MagicMock(version="2.0.0", issue=None, dry_run=False)
+ self.mock_git.status.side_effect = ["", ""]
+ self.mock_git.branch_exists.return_value = True
+ self.mock_gh.get_release_tracking_issue.side_effect = None
+ self.mock_gh.get_release_tracking_issue.return_value = 123
+ self.mock_gh.get_open_pr.return_value = None
+ # PR #456 is already associated in the tracking issue
+ self.mock_gh.get_issue_body.return_value = (
+ "- [ ] Prepare Release | status=pending pr=#456"
+ )
+
+ # Act
+ result = releaser.cmd_prepare(args)
+
+ # Assert
+ self.assertEqual(result, 0)
+ self.mock_git.checkout.assert_called_once_with("prepare-2.0.0")
+ self.mock_git.commit.assert_not_called()
+ self.mock_git.push.assert_called_once_with(
+ "origin", "prepare-2.0.0", set_upstream=True
+ )
+ self.mock_gh.get_open_pr.assert_called_once_with("prepare-2.0.0")
+ self.mock_gh.create_pr.assert_not_called() # Should NOT create a new PR
+ self.mock_gh.update_issue_body.assert_called_once()
+ call_args = self.mock_gh.update_issue_body.call_args[0]
+ self.assertIn("pr=#456", call_args[1])
+
+ @patch("tools.private.release.prepare.changelog_news")
+ @patch("tools.private.release.prepare.replace_version_next")
+ def test_prepare_create_pr_when_none_associated(self, mock_replace, mock_changelog):
+ # Arrange
+ args = MagicMock(version="2.0.0", issue=None, dry_run=False)
+ self.mock_git.status.side_effect = ["", ""]
+ self.mock_git.branch_exists.return_value = True
+ self.mock_gh.get_release_tracking_issue.side_effect = None
+ self.mock_gh.get_release_tracking_issue.return_value = 123
+ self.mock_gh.get_open_pr.return_value = None
+ # No PR associated in the tracking issue
+ self.mock_gh.get_issue_body.return_value = "- [ ] Prepare Release"
+ self.mock_gh.create_pr.return_value = "https://github.com/foo/bar/pull/789"
+
+ # Act
+ result = releaser.cmd_prepare(args)
+
+ # Assert
+ self.assertEqual(result, 0)
+ self.mock_git.checkout.assert_called_once_with("prepare-2.0.0")
+ self.mock_git.commit.assert_not_called()
+ self.mock_git.push.assert_called_once_with(
+ "origin", "prepare-2.0.0", set_upstream=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.update_issue_body.assert_called_once()
+ call_args = self.mock_gh.update_issue_body.call_args[0]
+ self.assertIn("pr=#789", call_args[1])
+
+ @patch("tools.private.release.prepare.changelog_news")
+ @patch("tools.private.release.prepare.replace_version_next")
+ def test_prepare_reuse_existing_pr(self, mock_replace, mock_changelog):
+ # Arrange
+ args = MagicMock(version="2.0.0", issue=None, dry_run=False)
+ self.mock_git.status.side_effect = ["", ""]
+ self.mock_git.branch_exists.return_value = True
+ self.mock_gh.get_release_tracking_issue.side_effect = None
+ self.mock_gh.get_release_tracking_issue.return_value = 123
+ self.mock_gh.get_open_pr.return_value = {
+ "number": 456,
+ "url": "https://github.com/foo/bar/pull/456",
+ }
+ self.mock_gh.get_issue_body.return_value = "- [ ] Prepare Release"
+
+ # Act
+ result = releaser.cmd_prepare(args)
+
+ # Assert
+ self.assertEqual(result, 0)
+ self.mock_git.checkout.assert_called_once_with("prepare-2.0.0")
+ self.mock_git.commit.assert_not_called()
+ self.mock_git.push.assert_called_once_with(
+ "origin", "prepare-2.0.0", set_upstream=True
+ )
+ self.mock_gh.get_open_pr.assert_called_once_with("prepare-2.0.0")
+ self.mock_gh.create_pr.assert_not_called()
+ self.mock_gh.update_issue_body.assert_called_once()
+ call_args = self.mock_gh.update_issue_body.call_args[0]
+ self.assertIn("pr=#456", call_args[1])
+
+ @patch("tools.private.release.prepare.changelog_news")
+ @patch("tools.private.release.prepare.replace_version_next")
def test_prepare_dry_run_no_issue(self, mock_replace, mock_changelog):
# Arrange
template_dir = self.tmpdir / ".github" / "ISSUE_TEMPLATE"
diff --git a/tools/private/release/gh.py b/tools/private/release/gh.py
index f37d87f..da86415 100644
--- a/tools/private/release/gh.py
+++ b/tools/private/release/gh.py
@@ -162,6 +162,22 @@
)
+def get_open_pr(branch_name):
+ """Returns PR info if an open PR exists for the given branch, else None."""
+ cmd = [
+ "gh",
+ "pr",
+ "list",
+ f"--repo={_REPO}",
+ f"--head={branch_name}",
+ "--state=open",
+ "--json=number,url",
+ ]
+ output = run_cmd(*cmd)
+ prs = json.loads(output) if output else []
+ return prs[0] if prs else None
+
+
def get_pr_info(pr_num):
"""Gets information about a PR, including state, merge commit, and body."""
output = run_cmd(
diff --git a/tools/private/release/prepare.py b/tools/private/release/prepare.py
index 1d946bd..c9012e0 100644
--- a/tools/private/release/prepare.py
+++ b/tools/private/release/prepare.py
@@ -2,7 +2,10 @@
import pathlib
from tools.private.release import changelog_news, gh, git
-from tools.private.release.release_issue import update_task_in_body
+from tools.private.release.release_issue import (
+ parse_checklist_state,
+ update_task_in_body,
+)
from tools.private.release.utils import (
determine_next_version,
replace_version_next,
@@ -102,26 +105,47 @@
print(f"[DRY RUN] Would push branch {branch_name} to origin")
else:
modified_files = git.status()
- if not modified_files:
+ if modified_files:
+ # Stage all modified and deleted tracked files
+ git.add_modified_and_deleted()
+ git.commit(f"Prepare release {version}")
+ else:
print("No files modified by the release tool. Nothing to commit.")
- return 0
- # Stage all modified and deleted tracked files
- git.add_modified_and_deleted()
-
- git.commit(f"Prepare release {version}")
+ print(f"Pushing branch {branch_name} to origin...")
git.push("origin", branch_name, set_upstream=True)
# --- Create PR ---
- if args.dry_run:
- target_issue = f"#{issue_num}" if issue_num else "<NEW_ISSUE>"
+ # Determine if we need to create a PR or reuse an existing one
+ open_pr = gh.get_open_pr(branch_name)
+ associated_pr = None
+
+ if not open_pr and issue_num:
+ body = gh.get_issue_body(issue_num)
+ state = parse_checklist_state(body)
+ associated_pr = state["prepare_release"]["pr"]
+
+ if open_pr:
+ pr_num = open_pr["number"]
+ pr_url = open_pr["url"]
+ print(f"Open Pull Request already exists: {pr_url} (PR #{pr_num})")
+ elif associated_pr:
+ pr_num = associated_pr.lstrip("#")
+ pr_url = f"https://github.com/bazel-contrib/rules_python/pull/{pr_num}"
print(
- f"[DRY RUN] Would create Pull Request for branch {branch_name} targeting issue {target_issue}"
+ f"PR #{pr_num} is already associated in tracking issue #{issue_num}. Using it."
)
else:
- pr_url = gh.create_pr(version, issue_num)
- pr_num = pr_url.split("/")[-1]
- print(f"Created Pull Request: {pr_url} (PR #{pr_num})")
+ if args.dry_run:
+ target_issue = f"#{issue_num}" if issue_num else "<NEW_ISSUE>"
+ print(
+ f"[DRY RUN] Would create Pull Request for branch {branch_name} targeting issue {target_issue}"
+ )
+ pr_num = "<NEW_PR>"
+ else:
+ pr_url = gh.create_pr(version, issue_num)
+ pr_num = pr_url.split("/")[-1]
+ print(f"Created Pull Request: {pr_url} (PR #{pr_num})")
# --- Update checklist ---
if args.dry_run:
diff --git a/tools/private/release/release.py b/tools/private/release/release.py
index d9a225d..dab99e6 100644
--- a/tools/private/release/release.py
+++ b/tools/private/release/release.py
@@ -10,6 +10,7 @@
from tools.private.release import changelog_news, gh, git
from tools.private.release.prepare import cmd_prepare
from tools.private.release.release_issue import (
+ parse_checklist_state,
parse_metadata_line,
update_task_in_body,
)
@@ -35,72 +36,6 @@
# ==============================================================================
-def parse_checklist_state(body):
- """Parses the main checklist tasks and their metadata."""
- state = {
- "prepare_release": {
- "checked": False,
- "status": None,
- "pr": None,
- "commit": None,
- },
- "create_branch": {
- "checked": False,
- "status": None,
- "branch": None,
- "commit": None,
- },
- "tag_final": {"checked": False, "status": None, "tag": None, "commit": None},
- "rc_tags": {}, # Dynamically mapped: int -> metadata dict
- }
-
- lines = body.splitlines()
- for line in lines:
- parsed = parse_metadata_line(line)
- if not parsed:
- continue
-
- name = parsed["name"].strip()
- meta = parsed["metadata"]
- checked = parsed["checked"]
- name_lower = name.lower()
-
- if "prepare release" in name_lower:
- state["prepare_release"] = {
- "checked": checked,
- "status": meta.get("status"),
- "pr": meta.get("pr"),
- "commit": meta.get("commit"),
- }
- elif "create release branch" in name_lower:
- state["create_branch"] = {
- "checked": checked,
- "status": meta.get("status"),
- "branch": meta.get("branch"),
- "commit": meta.get("commit"),
- }
- elif "tag final" in name_lower:
- state["tag_final"] = {
- "checked": checked,
- "status": meta.get("status"),
- "tag": meta.get("tag"),
- "commit": meta.get("commit"),
- }
- else:
- # Match Tag RC<num>
- rc_match = re.match(r"Tag RC(\d+)", name, re.IGNORECASE)
- if rc_match:
- rc_num = int(rc_match.group(1))
- state["rc_tags"][rc_num] = {
- "checked": checked,
- "status": meta.get("status"),
- "tag": meta.get("tag"),
- "commit": meta.get("commit"),
- }
-
- return state
-
-
def parse_backports(body):
"""Parses the ## Backports checklist section."""
body = body.replace("\r\n", "\n")
diff --git a/tools/private/release/release_issue.py b/tools/private/release/release_issue.py
index 27bcf3d..f96b917 100644
--- a/tools/private/release/release_issue.py
+++ b/tools/private/release/release_issue.py
@@ -60,3 +60,69 @@
)
return "\n".join(updated_lines)
+
+
+def parse_checklist_state(body):
+ """Parses the main checklist tasks and their metadata."""
+ state = {
+ "prepare_release": {
+ "checked": False,
+ "status": None,
+ "pr": None,
+ "commit": None,
+ },
+ "create_branch": {
+ "checked": False,
+ "status": None,
+ "branch": None,
+ "commit": None,
+ },
+ "tag_final": {"checked": False, "status": None, "tag": None, "commit": None},
+ "rc_tags": {}, # Dynamically mapped: int -> metadata dict
+ }
+
+ lines = body.splitlines()
+ for line in lines:
+ parsed = parse_metadata_line(line)
+ if not parsed:
+ continue
+
+ name = parsed["name"].strip()
+ meta = parsed["metadata"]
+ checked = parsed["checked"]
+ name_lower = name.lower()
+
+ if "prepare release" in name_lower:
+ state["prepare_release"] = {
+ "checked": checked,
+ "status": meta.get("status"),
+ "pr": meta.get("pr"),
+ "commit": meta.get("commit"),
+ }
+ elif "create release branch" in name_lower:
+ state["create_branch"] = {
+ "checked": checked,
+ "status": meta.get("status"),
+ "branch": meta.get("branch"),
+ "commit": meta.get("commit"),
+ }
+ elif "tag final" in name_lower:
+ state["tag_final"] = {
+ "checked": checked,
+ "status": meta.get("status"),
+ "tag": meta.get("tag"),
+ "commit": meta.get("commit"),
+ }
+ else:
+ # Match Tag RC<num>
+ rc_match = re.match(r"Tag RC(\d+)", name, re.IGNORECASE)
+ if rc_match:
+ rc_num = int(rc_match.group(1))
+ state["rc_tags"][rc_num] = {
+ "checked": checked,
+ "status": meta.get("status"),
+ "tag": meta.get("tag"),
+ "commit": meta.get("commit"),
+ }
+
+ return state
