diff --git a/todo_hooks/gh.py b/todo_hooks/gh.py index ca604fd..a5349eb 100644 --- a/todo_hooks/gh.py +++ b/todo_hooks/gh.py @@ -17,6 +17,10 @@ class GhError(Exception): """gh missing, offline, failing or answering nonsense.""" +class GhNotFound(GhError): + """GitHub does not know the commit (HTTP 404/422), e.g. an unpushed sha.""" + + @dataclass(frozen=True) class PullRequest: """A pull request GitHub links to a commit.""" @@ -44,7 +48,10 @@ def run_gh(args: list[str]) -> str: msg = "gh timed out" raise GhError(msg) from err if done.returncode != 0: - raise GhError(done.stderr.strip() or f"gh exit {done.returncode}") + message = done.stderr.strip() or f"gh exit {done.returncode}" + if "(HTTP 404)" in message or "(HTTP 422)" in message: + raise GhNotFound(message) + raise GhError(message) return done.stdout diff --git a/todo_hooks/import_pr.py b/todo_hooks/import_pr.py index d83791d..f8bb704 100644 --- a/todo_hooks/import_pr.py +++ b/todo_hooks/import_pr.py @@ -21,6 +21,7 @@ if TYPE_CHECKING: from todo_hooks.context import Context MAX_COMMITS = 30 +_PAIR_FIELDS = 2 USAGE = ( "usage: python3 -m todo_hooks import-pr " "post-merge | post-rewrite [rebase] | --range A..B" @@ -38,11 +39,15 @@ def commits_post_merge(root: Path) -> list[str]: def commits_post_rewrite(root: Path, command: str, pairs: str) -> list[str]: - """New shas from git's ``old new`` stdin lines; only for a rebase.""" + """Rebase only: ORIG_HEAD..HEAD plus the new shas of stdin ``old new`` lines.""" if command != "rebase": return [] - shas = [row.split()[1] for row in pairs.splitlines() if len(row.split()) >= 2] # noqa: PLR2004 - return shas[:MAX_COMMITS] or commits_post_merge(root) + shas = commits_post_merge(root) + for row in pairs.splitlines(): + fields = row.split() + if len(fields) >= _PAIR_FIELDS and fields[1] not in shas: + shas.append(fields[1]) + return shas[:MAX_COMMITS] def read_stdin(timeout: float = 1.0) -> str: @@ -74,6 +79,15 @@ def _add(client: TaskClient, repo: str, number: int, item: TodoItem) -> bool: return True +def _prs_of( + slug: tuple[str, str], sha: str, runner: Callable[[list[str]], str] | None +) -> list[gh.PullRequest]: + try: + return gh.merged_prs(slug[0], slug[1], sha, runner) + except gh.GhNotFound: + return [] + + def import_for_commits( ctx: Context, shas: list[str], runner: Callable[[list[str]], str] | None ) -> int: @@ -89,7 +103,7 @@ def import_for_commits( client: TaskClient | None = None try: for sha in shas: - for pr in gh.merged_prs(slug[0], slug[1], sha, runner): + for pr in _prs_of(slug, sha, runner): items = parse_todo_items(pr.body) if not pr.merged or pr.number in seen or not items: continue diff --git a/todo_hooks/systemd/todo-drift.service b/todo_hooks/systemd/todo-drift.service index a9ba629..0c3d67b 100644 --- a/todo_hooks/systemd/todo-drift.service +++ b/todo_hooks/systemd/todo-drift.service @@ -6,4 +6,4 @@ Type=oneshot ExecStart=%h/src/utils/scripts/todo_hooks.sh sweep Nice=19 IOSchedulingClass=idle -TimeoutStartSec=30min +TimeoutStartSec=infinity diff --git a/todo_hooks/tests/test_import_pr_commits.py b/todo_hooks/tests/test_import_pr_commits.py index 50a1da2..abec077 100644 --- a/todo_hooks/tests/test_import_pr_commits.py +++ b/todo_hooks/tests/test_import_pr_commits.py @@ -4,7 +4,7 @@ from __future__ import annotations from typing import TYPE_CHECKING -from todo_hooks import import_pr +from todo_hooks import gh, import_pr from todo_hooks.tests._fakes import FakeGh, line, make_pr from todo_hooks.tests._pr_helpers import pr_repo, search_client from todo_hooks.tests.conftest import git @@ -160,3 +160,45 @@ def test_nothing_new_to_import_is_silent( ctx, out = make_ctx(search_client(existing), root) assert import_pr.import_for_commits(ctx, [sha], FakeGh({sha: [make_pr(7)]})) == 0 assert out == [] + + +def test_rewrite_includes_upstream_commits_after_pull_rebase(tmp_path: Path) -> None: + upstream = tmp_path / "up.git" + git(tmp_path, "init", "-q", "--bare", "-b", "main", str(upstream)) + mine, other = tmp_path / "mine", tmp_path / "other" + git(tmp_path, "clone", "-q", str(upstream), str(mine)) + (mine / "a").write_text("a") + git(mine, "add", "a") + git(mine, "commit", "-q", "-m", "base") + git(mine, "push", "-q", "origin", "HEAD:main") + git(tmp_path, "clone", "-q", str(upstream), str(other)) + (other / "b").write_text("b") + git(other, "add", "b") + git(other, "commit", "-q", "-m", "arrives via pull") + git(other, "push", "-q", "origin", "HEAD:main") + arrived = git(other, "rev-parse", "HEAD") + (mine / "c").write_text("c") + git(mine, "add", "c") + git(mine, "commit", "-q", "-m", "local only") + git(mine, "pull", "-q", "--rebase", "origin", "main") + replayed = git(mine, "rev-parse", "HEAD") + shas = import_pr.commits_post_rewrite(mine, "rebase", f"old {replayed}\n") + assert arrived in shas + assert shas.count(replayed) == 1 + + +def test_unpushed_commit_does_not_stop_the_scan( + make_repo: Callable[..., Path], + make_ctx: Callable[..., tuple[Context, list[str]]], +) -> None: + root = pr_repo(make_repo, 1) + fake = FakeGh({"known": [make_pr(7)]}) + + def runner(args: list[str]) -> str: + if "unpushed" in args[1]: + raise gh.GhNotFound("No commit found (HTTP 422)") + return fake(args) + + client = search_client() + ctx, _ = make_ctx(client, root) + assert import_pr.import_for_commits(ctx, ["unpushed", "known"], runner) == 3 diff --git a/todo_hooks/tests/test_pr_parse_gh.py b/todo_hooks/tests/test_pr_parse_gh.py index 09701fe..a56be95 100644 --- a/todo_hooks/tests/test_pr_parse_gh.py +++ b/todo_hooks/tests/test_pr_parse_gh.py @@ -71,3 +71,14 @@ def test_merged_prs_parses_fields() -> None: prs = gh.merged_prs("kuhyx", "demo", "abc", fake) assert [(p.number, p.merged) for p in prs] == [(5, True), (6, False)] assert fake.calls[0] == ["api", "repos/kuhyx/demo/commits/abc/pulls"] + + +def test_run_gh_unknown_commit_is_not_found( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + fake = tmp_path / "gh" + fake.write_text('#!/bin/sh\necho "gh: No commit found (HTTP 422)" >&2\nexit 1\n') + fake.chmod(0o755) + monkeypatch.setenv("PATH", f"{tmp_path}:/usr/bin") + with pytest.raises(gh.GhNotFound): + gh.run_gh(["x"])