Skip to content

Commit 6f590d8

Browse files
committed
Add --validate-rules, --dry-run, --comment-on-failure to land command
Extends ghstack land with merge rules validation: CLI options: - --validate-rules: Validate PRs against .github/merge_rules.yaml - --dry-run: Validate without actually merging - --comment-on-failure: Post validation errors as PR comments The validation happens before any git operations: 1. Load merge rules from repository 2. For each PR in the stack, validate approvers and CI checks 3. If any PR fails validation, abort with detailed error 4. Optionally post error details as a PR comment Example usage: ghstack land --validate-rules --comment-on-failure <PR_URL> ghstack-source-id: a063004 ghstack-comment-id: 3807838942 Pull-Request: #318
1 parent 4a6ffc2 commit 6f590d8

File tree

2 files changed

+98
-25
lines changed

2 files changed

+98
-25
lines changed

src/ghstack/cli.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,29 @@ def cherry_pick(stack: bool, pull_request: str) -> None:
164164

165165
@main.command("land")
166166
@click.option("--force", is_flag=True, help="force land even if the PR is closed")
167+
@click.option(
168+
"--validate-rules",
169+
is_flag=True,
170+
help="validate against merge rules defined in .github/merge_rules.yaml",
171+
)
172+
@click.option(
173+
"--dry-run",
174+
is_flag=True,
175+
help="validate merge rules but don't actually merge",
176+
)
177+
@click.option(
178+
"--comment-on-failure",
179+
is_flag=True,
180+
help="post validation errors as a PR comment",
181+
)
167182
@click.argument("pull_request", metavar="PR")
168-
def land(force: bool, pull_request: str) -> None:
183+
def land(
184+
force: bool,
185+
validate_rules: bool,
186+
dry_run: bool,
187+
comment_on_failure: bool,
188+
pull_request: str,
189+
) -> None:
169190
"""
170191
Land a PR stack
171192
"""
@@ -177,6 +198,9 @@ def land(force: bool, pull_request: str) -> None:
177198
github_url=config.github_url,
178199
remote_name=config.remote_name,
179200
force=force,
201+
validate_rules=validate_rules,
202+
dry_run=dry_run,
203+
comment_on_failure=comment_on_failure,
180204
)
181205

182206

src/ghstack/land.py

Lines changed: 73 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@
22

33
import logging
44
import re
5-
from typing import List, Tuple
5+
from typing import List, Optional, Tuple
66

77
import ghstack.git
88
import ghstack.github
99
import ghstack.github_utils
10+
import ghstack.merge_rules
1011
import ghstack.shell
1112
from ghstack.diff import PullRequestResolved
1213
from ghstack.types import GitCommitHash
@@ -50,6 +51,10 @@ def main(
5051
github_url: str,
5152
*,
5253
force: bool = False,
54+
validate_rules: bool = False,
55+
dry_run: bool = False,
56+
comment_on_failure: bool = False,
57+
rules_file: Optional[str] = None,
5358
) -> None:
5459

5560
# We land the entire stack pointed to by a URL.
@@ -60,19 +65,22 @@ def main(
6065
params = ghstack.github_utils.parse_pull_request(
6166
pull_request, sh=sh, remote_name=remote_name
6267
)
68+
owner = params["owner"]
69+
name = params["name"]
70+
6371
default_branch = ghstack.github_utils.get_github_repo_info(
6472
github=github,
6573
sh=sh,
66-
repo_owner=params["owner"],
67-
repo_name=params["name"],
74+
repo_owner=owner,
75+
repo_name=name,
6876
github_url=github_url,
6977
remote_name=remote_name,
7078
)["default_branch"]
7179

7280
needs_force = False
7381
try:
7482
protection = github.get(
75-
f"repos/{params['owner']}/{params['name']}/branches/{default_branch}/protection"
83+
f"repos/{owner}/{name}/branches/{default_branch}/protection"
7684
)
7785
if not protection["allow_force_pushes"]["enabled"]:
7886
raise RuntimeError(
@@ -91,12 +99,12 @@ def main(
9199

92100
orig_ref, closed = lookup_pr_to_orig_ref_and_closed(
93101
github,
94-
owner=params["owner"],
95-
name=params["name"],
102+
owner=owner,
103+
name=name,
96104
number=params["number"],
97105
)
98106

99-
if closed:
107+
if closed and not force:
100108
raise RuntimeError("PR is already closed, cannot land it!")
101109

102110
if sh is None:
@@ -117,6 +125,64 @@ def main(
117125
github_url=github_url,
118126
)
119127

128+
# Compute the metadata for each commit
129+
stack_orig_refs: List[Tuple[str, PullRequestResolved]] = []
130+
for s in stack:
131+
pr_resolved = s.pull_request_resolved
132+
# We got this from GitHub, this better not be corrupted
133+
assert pr_resolved is not None
134+
135+
ref, stack_closed = lookup_pr_to_orig_ref_and_closed(
136+
github,
137+
owner=pr_resolved.owner,
138+
name=pr_resolved.repo,
139+
number=pr_resolved.number,
140+
)
141+
if stack_closed and not force:
142+
continue
143+
stack_orig_refs.append((ref, pr_resolved))
144+
145+
# Validate merge rules if requested
146+
if validate_rules:
147+
logging.info("Validating merge rules for PR stack...")
148+
149+
# Load merge rules
150+
loader = ghstack.merge_rules.MergeRulesLoader(github, owner, name)
151+
if rules_file:
152+
rules = loader.load_from_file(rules_file)
153+
else:
154+
rules = loader.load_from_repo()
155+
156+
if not rules:
157+
logging.warning("No merge rules found, skipping validation")
158+
else:
159+
# Validate each PR in the stack
160+
validator = ghstack.merge_rules.MergeValidator(github, owner, name)
161+
for _, pr_resolved in stack_orig_refs:
162+
result = validator.validate_pr(pr_resolved.number, rules)
163+
if not result.valid:
164+
logging.error(
165+
f"Validation failed for PR #{pr_resolved.number}: "
166+
f"{', '.join(result.errors)}"
167+
)
168+
if comment_on_failure:
169+
comment_body = (
170+
ghstack.merge_rules.format_validation_error_comment(result)
171+
)
172+
github.post_issue_comment(
173+
owner, name, pr_resolved.number, comment_body
174+
)
175+
raise ghstack.merge_rules.MergeValidationError(result)
176+
logging.info(
177+
f"PR #{pr_resolved.number} passed validation (rule: {result.rule_name})"
178+
)
179+
180+
logging.info("All PRs in stack passed merge rules validation")
181+
182+
if dry_run:
183+
logging.info("Dry run complete - no changes made")
184+
return
185+
120186
# Switch working copy
121187
try:
122188
prev_ref = sh.git("symbolic-ref", "--short", "HEAD")
@@ -127,23 +193,6 @@ def main(
127193
sh.git("checkout", f"{remote_name}/{default_branch}")
128194

129195
try:
130-
# Compute the metadata for each commit
131-
stack_orig_refs: List[Tuple[str, PullRequestResolved]] = []
132-
for s in stack:
133-
pr_resolved = s.pull_request_resolved
134-
# We got this from GitHub, this better not be corrupted
135-
assert pr_resolved is not None
136-
137-
ref, closed = lookup_pr_to_orig_ref_and_closed(
138-
github,
139-
owner=pr_resolved.owner,
140-
name=pr_resolved.repo,
141-
number=pr_resolved.number,
142-
)
143-
if closed and not force:
144-
continue
145-
stack_orig_refs.append((ref, pr_resolved))
146-
147196
# OK, actually do the land now
148197
for orig_ref, pr_resolved in stack_orig_refs:
149198
try:

0 commit comments

Comments
 (0)