Skip to content
Merged
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
32 changes: 31 additions & 1 deletion pr_split/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
push_branch,
remove_worktree,
)
from .git_ops.branches import commit_exists, run_git
from .git_ops.branches import commit_exists, require_tools, run_git
from .git_ops.prs import (
branch_has_merged,
close_pr,
Expand Down Expand Up @@ -169,6 +169,13 @@ def _add_children(parent_id: str, prefix: str) -> None:
return f"## Dependency graph\n\nMerge in this order:\n\n```\n{tree_block}\n```"


def _require_cli_tools(*tools: str) -> None:
missing = require_tools(*tools)
if missing is not None:
console.print(f"[red]{ErrorMsg.TOOL_NOT_FOUND(tool=missing)}[/red]")
raise typer.Exit(1)


def _require_local_branch(base: str) -> None:
"""Exit unless ``base`` names a local branch head.

Expand Down Expand Up @@ -1304,13 +1311,20 @@ def split(
fork_info: ForkPRInfo | None = None
select_plan(dev_branch_arg)

# Say "git/gh is not installed" before any helper turns that into a
# misleading "branch not found" or "authentication failed".
_require_cli_tools("git", *(() if dry_run else ("gh",)))

# A branch that exists only as origin/<name> (fresh clone or worktree)
# is adopted as a local branch, as `git checkout <name>` would.
for name in (dev_branch, base):
if not (name.lstrip("#").isdigit() or ":" in name):
adopt_remote_branch(name)

if not branch_exists(dev_branch):
# A PR number or user:branch has to be fetched with gh even for a
# dry run.
_require_cli_tools("gh")
if not check_gh_auth():
console.print(f"[red]{ErrorMsg.GH_AUTH_FAILED()}[/red]")
raise typer.Exit(1)
Expand Down Expand Up @@ -1339,6 +1353,7 @@ def split(
console.print(
"[red]Warning: this will permanently close PRs and delete remote branches.[/red]"
)
_require_cleanup_tools(existing.git_state)
if typer.confirm("Clean up and proceed with re-splitting?"):
closed_prs, deleted_branches = _cleanup_git_state(existing.git_state)
logger.success(
Expand Down Expand Up @@ -1572,6 +1587,14 @@ def status() -> None:
)


def _require_cleanup_tools(git_state: GitState) -> None:
# Every close/delete failure inside _cleanup_git_state is swallowed as a
# warning, so a missing binary would otherwise look like a successful
# cleanup that then deletes the plan. Both callers (clean and the
# re-split prompt in split) go through here before asking to proceed.
_require_cli_tools("git", *(("gh",) if git_state.prs else ()))


def _cleanup_git_state(git_state: GitState) -> tuple[int, int]:
closed_prs = 0
for pr_record in git_state.prs:
Expand Down Expand Up @@ -1812,6 +1835,8 @@ def clean() -> None:
plan_file = _load_plan_or_exit()
git_state = plan_file.git_state

_require_cleanup_tools(git_state)

typer.confirm("Delete all pr-split branches and close PRs?", abort=True)

closed_prs, deleted_branches = _cleanup_git_state(git_state)
Expand Down Expand Up @@ -1936,6 +1961,7 @@ def execute(
" Re-run 'pr-split split --dry-run' to regenerate.[/red]"
)
raise typer.Exit(1)
_require_cli_tools("git", "gh")
if not commit_exists(plan.merge_base_sha):
console.print(
f"[red]Plan's merge base {plan.merge_base_sha} is not in this repository "
Expand Down Expand Up @@ -2108,6 +2134,10 @@ def merge_all(
console.print("[yellow]No PRs found in plan. Nothing to merge.[/yellow]")
raise typer.Exit(0)

# Without gh every PR would be skipped as a "fetch error" and the run
# would still report success.
_require_cli_tools("gh")

if _base_has_merged(plan.base_branch):
console.print(
f"[red]{logs.BASE_ALREADY_MERGED.format(base=escape(plan.base_branch))}[/red]"
Expand Down
26 changes: 18 additions & 8 deletions pr_split/diff_ops/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from unidiff import PatchSet

from .. import logs
from ..exceptions import DiffParseError, GitOperationError
from ..exceptions import DiffParseError, ErrorMsg, GitOperationError
from ..types_defs import DiffStats, FileSummary, HunkInfo
from .new_file_split import split_new_file

Expand Down Expand Up @@ -39,13 +39,23 @@ def extract_diff(dev_branch: str, base_branch: str) -> str:
# Capture bytes: text mode applies universal-newline translation, which
# turns CRLF into LF and would make every sub-PR rewrite the file's
# line endings.
result = subprocess.run(
# core.quotePath=false: otherwise git octal-escapes and quotes any
# non-ASCII path ("b/caf\303\251.txt"), which unidiff cannot parse for
# new/deleted files and mis-reports as a literal quoted path otherwise.
["git", "-c", "core.quotePath=false", "diff", *DIFF_ARGS, f"{base_branch}...{dev_branch}"],
capture_output=True,
)
try:
result = subprocess.run(
# core.quotePath=false: otherwise git octal-escapes and quotes any
# non-ASCII path ("b/caf\303\251.txt"), which unidiff cannot parse for
# new/deleted files and mis-reports as a literal quoted path otherwise.
[
"git",
"-c",
"core.quotePath=false",
"diff",
*DIFF_ARGS,
f"{base_branch}...{dev_branch}",
],
capture_output=True,
)
except FileNotFoundError as exc:
raise GitOperationError(ErrorMsg.TOOL_NOT_FOUND(tool="git")) from exc
if result.returncode != 0:
raise GitOperationError(result.stderr.decode("utf-8", errors="replace").strip())
# Git diffs any NUL-free file as text, so legacy latin-1 sources reach us
Expand Down
13 changes: 8 additions & 5 deletions pr_split/diff_ops/reconstructor.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from .. import logs
from ..constants import AssignmentType
from ..exceptions import GitOperationError
from ..exceptions import ErrorMsg, GitOperationError
from ..schemas import Group, GroupAssignment
from .parser import ParsedDiff

Expand Down Expand Up @@ -58,10 +58,13 @@ def merge_chain_assignments(
def _get_base_file_content(file_path: str, ref: str) -> str:
# Bytes, not text mode: universal newlines would strip the CR from CRLF
# files and the reconstructed file would come out with LF endings.
result = subprocess.run(
["git", "show", f"{ref}:{file_path}"],
capture_output=True,
)
try:
result = subprocess.run(
["git", "show", f"{ref}:{file_path}"],
capture_output=True,
)
except FileNotFoundError as exc:
raise GitOperationError(ErrorMsg.TOOL_NOT_FOUND(tool="git")) from exc
if result.returncode != 0:
raise GitOperationError(result.stderr.decode("utf-8", errors="replace").strip())
return result.stdout.decode("utf-8", errors="surrogateescape")
Expand Down
1 change: 1 addition & 0 deletions pr_split/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class ErrorMsg(StrEnum):
)
DIRTY_WORKTREE = "Working tree has uncommitted changes; commit or stash first"
GH_AUTH_FAILED = "GitHub CLI authentication failed; run 'gh auth login'"
TOOL_NOT_FOUND = "'{tool}' is not installed or not on PATH"
CYCLE_DETECTED = "Dependency cycle detected in split plan"
COVERAGE_GAP = "Hunk {file}[{index}] not assigned to any group"
COVERAGE_OVERLAP = "Hunk {file}[{index}] assigned to multiple groups: {groups}"
Expand Down
44 changes: 26 additions & 18 deletions pr_split/git_ops/branches.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

import os
import re
import shutil
import subprocess
import time

from loguru import logger

from .. import logs
from ..constants import PLAN_DIR
from ..exceptions import GitOperationError
from ..exceptions import ErrorMsg, GitOperationError


def _git_env() -> dict[str, str]:
Expand All @@ -18,29 +19,36 @@ def _git_env() -> dict[str, str]:
return {**os.environ, "LC_ALL": "C", "LANGUAGE": "C"}


def run_git(*args: str) -> str:
result = subprocess.run(
["git", *args],
capture_output=True,
text=True,
env=_git_env(),
)
def _git(args: tuple[str, ...], cwd: str | None = None) -> str:
try:
result = subprocess.run(
["git", *args],
capture_output=True,
text=True,
cwd=cwd,
env=_git_env(),
)
except FileNotFoundError as exc:
raise GitOperationError(ErrorMsg.TOOL_NOT_FOUND(tool="git")) from exc
if result.returncode != 0:
raise GitOperationError(result.stderr.strip())
return result.stdout.strip()


def run_git(*args: str) -> str:
return _git(args)


def run_git_in_dir(cwd: str, *args: str) -> str:
result = subprocess.run(
["git", *args],
capture_output=True,
text=True,
cwd=cwd,
env=_git_env(),
)
if result.returncode != 0:
raise GitOperationError(result.stderr.strip())
return result.stdout.strip()
return _git(args, cwd=cwd)


def require_tools(*tools: str) -> str | None:
"""Return the first of ``tools`` that is not on PATH, or None."""
for tool in tools:
if shutil.which(tool) is None:
return tool
return None


def commit_exists(ref: str) -> bool:
Expand Down
15 changes: 10 additions & 5 deletions pr_split/git_ops/prs.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,16 @@


def _run_gh(*args: str) -> str:
result = subprocess.run(
["gh", *args],
capture_output=True,
text=True,
)
try:
result = subprocess.run(
["gh", *args],
capture_output=True,
text=True,
)
except FileNotFoundError as exc:
# Callers only expect GitOperationError; a missing binary must not
# escape as a raw traceback (or make check_gh_auth raise).
raise GitOperationError(ErrorMsg.TOOL_NOT_FOUND(tool="gh")) from exc
if result.returncode != 0:
raise GitOperationError(result.stderr.strip())
return result.stdout.strip()
Expand Down
8 changes: 8 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,11 @@ def _no_live_base_merge_lookup(monkeypatch: pytest.MonkeyPatch) -> None:
"""status and merge ask GitHub whether the plan's base branch has merged;
keep that off the network in tests. Tests that exercise it patch it again."""
monkeypatch.setattr("pr_split.cli._base_has_merged", lambda base: False)


@pytest.fixture(autouse=True)
def _cli_tools_present(monkeypatch: pytest.MonkeyPatch) -> None:
"""The CLI checks that git and gh are on PATH before doing anything; the
tests mock every git/gh call, so they must not depend on the machine
having gh installed. Tests of the missing-tool path patch it again."""
monkeypatch.setattr("pr_split.cli.require_tools", lambda *tools: None)
Loading
Loading