Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion src/ghstack/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,29 @@ def cherry_pick(stack: bool, pull_request: str) -> None:

@main.command("land")
@click.option("--force", is_flag=True, help="force land even if the PR is closed")
@click.option(
"--validate-rules",
is_flag=True,
help="validate against merge rules defined in .github/merge_rules.yaml",
)
@click.option(
"--dry-run",
is_flag=True,
help="validate merge rules but don't actually merge",
)
@click.option(
"--comment-on-failure",
is_flag=True,
help="post validation errors as a PR comment",
)
@click.argument("pull_request", metavar="PR")
def land(force: bool, pull_request: str) -> None:
def land(
force: bool,
validate_rules: bool,
dry_run: bool,
comment_on_failure: bool,
pull_request: str,
) -> None:
"""
Land a PR stack
"""
Expand All @@ -177,6 +198,9 @@ def land(force: bool, pull_request: str) -> None:
github_url=config.github_url,
remote_name=config.remote_name,
force=force,
validate_rules=validate_rules,
dry_run=dry_run,
comment_on_failure=comment_on_failure,
)


Expand Down
97 changes: 73 additions & 24 deletions src/ghstack/land.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@

import logging
import re
from typing import List, Tuple
from typing import List, Optional, Tuple

import ghstack.git
import ghstack.github
import ghstack.github_utils
import ghstack.merge_rules
import ghstack.shell
from ghstack.diff import PullRequestResolved
from ghstack.types import GitCommitHash
Expand Down Expand Up @@ -50,6 +51,10 @@ def main(
github_url: str,
*,
force: bool = False,
validate_rules: bool = False,
dry_run: bool = False,
comment_on_failure: bool = False,
rules_file: Optional[str] = None,
) -> None:

# We land the entire stack pointed to by a URL.
Expand All @@ -60,19 +65,22 @@ def main(
params = ghstack.github_utils.parse_pull_request(
pull_request, sh=sh, remote_name=remote_name
)
owner = params["owner"]
name = params["name"]

default_branch = ghstack.github_utils.get_github_repo_info(
github=github,
sh=sh,
repo_owner=params["owner"],
repo_name=params["name"],
repo_owner=owner,
repo_name=name,
github_url=github_url,
remote_name=remote_name,
)["default_branch"]

needs_force = False
try:
protection = github.get(
f"repos/{params['owner']}/{params['name']}/branches/{default_branch}/protection"
f"repos/{owner}/{name}/branches/{default_branch}/protection"
)
if not protection["allow_force_pushes"]["enabled"]:
raise RuntimeError(
Expand All @@ -91,12 +99,12 @@ def main(

orig_ref, closed = lookup_pr_to_orig_ref_and_closed(
github,
owner=params["owner"],
name=params["name"],
owner=owner,
name=name,
number=params["number"],
)

if closed:
if closed and not force:
raise RuntimeError("PR is already closed, cannot land it!")

if sh is None:
Expand All @@ -117,6 +125,64 @@ def main(
github_url=github_url,
)

# Compute the metadata for each commit
stack_orig_refs: List[Tuple[str, PullRequestResolved]] = []
for s in stack:
pr_resolved = s.pull_request_resolved
# We got this from GitHub, this better not be corrupted
assert pr_resolved is not None

ref, stack_closed = lookup_pr_to_orig_ref_and_closed(
github,
owner=pr_resolved.owner,
name=pr_resolved.repo,
number=pr_resolved.number,
)
if stack_closed and not force:
continue
stack_orig_refs.append((ref, pr_resolved))

# Validate merge rules if requested
if validate_rules:
logging.info("Validating merge rules for PR stack...")

# Load merge rules
loader = ghstack.merge_rules.MergeRulesLoader(github, owner, name)
if rules_file:
rules = loader.load_from_file(rules_file)
else:
rules = loader.load_from_repo()

if not rules:
logging.warning("No merge rules found, skipping validation")
else:
# Validate each PR in the stack
validator = ghstack.merge_rules.MergeValidator(github, owner, name)
for _, pr_resolved in stack_orig_refs:
result = validator.validate_pr(pr_resolved.number, rules)
if not result.valid:
logging.error(
f"Validation failed for PR #{pr_resolved.number}: "
f"{', '.join(result.errors)}"
)
if comment_on_failure:
comment_body = (
ghstack.merge_rules.format_validation_error_comment(result)
)
github.post_issue_comment(
owner, name, pr_resolved.number, comment_body
)
raise ghstack.merge_rules.MergeValidationError(result)
logging.info(
f"PR #{pr_resolved.number} passed validation (rule: {result.rule_name})"
)

logging.info("All PRs in stack passed merge rules validation")

if dry_run:
logging.info("Dry run complete - no changes made")
return

# Switch working copy
try:
prev_ref = sh.git("symbolic-ref", "--short", "HEAD")
Expand All @@ -127,23 +193,6 @@ def main(
sh.git("checkout", f"{remote_name}/{default_branch}")

try:
# Compute the metadata for each commit
stack_orig_refs: List[Tuple[str, PullRequestResolved]] = []
for s in stack:
pr_resolved = s.pull_request_resolved
# We got this from GitHub, this better not be corrupted
assert pr_resolved is not None

ref, closed = lookup_pr_to_orig_ref_and_closed(
github,
owner=pr_resolved.owner,
name=pr_resolved.repo,
number=pr_resolved.number,
)
if closed and not force:
continue
stack_orig_refs.append((ref, pr_resolved))

# OK, actually do the land now
for orig_ref, pr_resolved in stack_orig_refs:
try:
Expand Down