fix(doctor): hold every Git read to one budget

loopx/doctor_git.py

@@ -5,11 +5,37 @@
 import os
 import re
 import subprocess
+from dataclasses import dataclass
 from enum import Enum
 from pathlib import Path
 from typing import Any
 from urllib.parse import urlsplit
 
+from .extensions.process_runtime import terminate_process_tree
+
+
+# `doctor` inspects a checkout the operator may not control, so every Git read
+# below runs under one budget. A repository blocked on a stale index lock, a
+# network mount or a very large history must not hold the diagnostic open.
+GIT_PROBE_TIMEOUT_SECONDS = 10.0
+
+
+@dataclass(frozen=True)
+class GitAnswer:
+    """One finished Git query: the only fields the callers below read."""
+
+    returncode: int
+    stdout: str
+    stderr: str
+
+
+class GitSourceState(str, Enum):
+    """What one checkout says about owning a Git identity of its own."""
+
+    WORKTREE = "worktree"
+    ABSENT = "absent"
+    UNCONFIRMED = "unconfirmed"
+
 
 class GitRevisionRelation(str, Enum):
     SAME = "same"
@@ -27,60 +53,126 @@ def _source_git_environment() -> dict[str, str]:
     return env
 
 
-def _is_git_worktree_root(source_root: Path) -> bool:
-    """Reject Git's upward discovery while accepting linked worktree gitfiles."""
-    if not (source_root / ".git").exists():
-        return False
+def _retire_owned_probe(process: subprocess.Popen[str]) -> None:
+    """Stop the query's own process group, then drop its pipes unread.
+
+    Git runs helpers -- fsmonitor hooks, pagers, credential helpers -- inside
+    the group it starts in, so killing the leader alone leaves them behind and
+    repeated diagnostics accumulate them. An inherited pipe also survives that
+    kill, so a drained ``communicate`` would wait on work nobody owns any more;
+    the pipes are closed instead.
+    """
+
+    try:
+        terminate_process_tree(process, grace_seconds=0.2)
+    except (OSError, RuntimeError):
+        # The answer is already unknown; an unconfirmed cleanup must not turn a
+        # read-only diagnostic into a traceback.
+        pass
+    finally:
+        for stream in (process.stdout, process.stderr):
+            if stream is not None:
+                stream.close()
+        try:
+            process.wait(timeout=1.0)
+        except subprocess.TimeoutExpired:
+            pass
+
+
+def _run_git(
+    source_root: Path, *arguments: str
+) -> GitAnswer | None:
+    """Answer one local Git query, or None when Git cannot answer in budget.
+
+    Every caller already treats a missing answer as "unknown" rather than as
+    evidence, so an exhausted budget degrades along that existing path instead
+    of raising out of a read-only diagnostic. The query is launched in a
+    session of its own so the budget can retire the helpers it started, not
+    just the Git process this call happens to hold. Cancelling the diagnostic
+    (a terminal Ctrl-C) reaches this process's group and not that session, so
+    an interrupted wait retires the same helpers and then keeps travelling.
+    """
+
+    argv = ["git", "-C", str(source_root), *arguments]
     try:
-        result = subprocess.run(
-            ["git", "-C", str(source_root), "rev-parse", "--show-toplevel"],
-            check=False, capture_output=True, text=True,
-            encoding="utf-8", errors="replace",
+        process = subprocess.Popen(
+            argv,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            text=True,
+            encoding="utf-8",
+            errors="replace",
             env=_source_git_environment(),
-        )
-        return (
-            result.returncode == 0
-            and bool(result.stdout.strip())
-            and Path(result.stdout.strip()).resolve() == source_root
+            start_new_session=os.name == "posix",
         )
     except OSError:
-        return False
+        return None
+    try:
+        stdout, stderr = process.communicate(timeout=GIT_PROBE_TIMEOUT_SECONDS)
+    except subprocess.TimeoutExpired:
+        _retire_owned_probe(process)
+        return None
+    except BaseException:
+        # The wait is over without Git answering: an operator cancelled the
+        # diagnostic. Retiring here is the only owner this session has, and the
+        # original cancellation still has to reach its own handler, so the
+        # timeout keeps answering "unknown" while Ctrl-C keeps being Ctrl-C.
+        _retire_owned_probe(process)
+        raise
+    return GitAnswer(
+        returncode=process.returncode, stdout=stdout, stderr=stderr
+    )
 
 
-def git_metadata_for_root(root: Path | None) -> dict[str, Any]:
+def _git_source_state(source_root: Path) -> GitSourceState:
+    """Reject Git's upward discovery while accepting linked worktree gitfiles.
+
+    A checkout that carries its own ``.git`` is a Git source even when this run
+    cannot read it. Callers that stand something else in for a missing answer
+    have to tell that case apart from "there is no Git here at all".
+    """
+
+    if not (source_root / ".git").exists():
+        return GitSourceState.ABSENT
+    result = _run_git(source_root, "rev-parse", "--show-toplevel")
+    if result is None or result.returncode != 0:
+        return GitSourceState.UNCONFIRMED
+    reported = result.stdout.strip()
+    if not reported:
+        return GitSourceState.UNCONFIRMED
+    if Path(reported).resolve() != source_root:
+        return GitSourceState.ABSENT
+    return GitSourceState.WORKTREE
+
+
+def _is_git_worktree_root(source_root: Path) -> bool:
+    return _git_source_state(source_root) is GitSourceState.WORKTREE
+
+
+def git_source_metadata(
+    root: Path | None,
+) -> tuple[dict[str, Any], GitSourceState]:
+    """Return the identity a root claims and how far this run could read it.
+
+    The pair matters because "this checkout has no Git identity" and "this
+    checkout has one that did not answer in time" call for opposite behaviour
+    from callers that have something else to fall back on.
+    """
+
+    unknown = {"root": None, "git_commit": None, "git_ref": None, "git_dirty": None}
     if root is None:
-        return {
-            "root": None,
-            "git_commit": None,
-            "git_ref": None,
-            "git_dirty": None,
-        }
+        return unknown, GitSourceState.ABSENT
     try:
         source_root = root.expanduser().resolve()
     except OSError:
         source_root = root.expanduser()
-    if not _is_git_worktree_root(source_root):
-        return {
-            "root": str(source_root),
-            "git_commit": None,
-            "git_ref": None,
-            "git_dirty": None,
-        }
+    state = _git_source_state(source_root)
+    if state is not GitSourceState.WORKTREE:
+        return {**unknown, "root": str(source_root)}, state
 
     def _run(args: list[str]) -> str | None:
-        try:
-            result = subprocess.run(
-                ["git", "-C", str(source_root), *args],
-                check=False,
-                capture_output=True,
-                text=True,
-                encoding="utf-8",
-                errors="replace",
-                env=_source_git_environment(),
-            )
-        except OSError:
-            return None
-        if result.returncode != 0:
+        result = _run_git(source_root, *args)
+        if result is None or result.returncode != 0:
             return None
         return result.stdout.strip()
 
@@ -90,12 +182,19 @@ def _run(args: list[str]) -> str | None:
     status = _run(["status", "--porcelain"])
     # Empty successful output proves clean; a failed status leaves it unknown.
     dirty = None if status is None else bool(status)
-    return {
-        "root": str(source_root),
-        "git_commit": commit,
-        "git_ref": branch or tag,
-        "git_dirty": dirty,
-    }
+    return (
+        {
+            "root": str(source_root),
+            "git_commit": commit,
+            "git_ref": branch or tag,
+            "git_dirty": dirty,
+        },
+        state,
+    )
+
+
+def git_metadata_for_root(root: Path | None) -> dict[str, Any]:
+    return git_source_metadata(root)[0]
 
 
 def git_revision_relation(
@@ -124,25 +223,10 @@ def git_revision_relation(
         return GitRevisionRelation.UNKNOWN
 
     def _is_ancestor(ancestor: str, descendant: str) -> bool | None:
-        try:
-            result = subprocess.run(
-                [
-                    "git",
-                    "-C",
-                    str(source_root),
-                    "merge-base",
-                    "--is-ancestor",
-                    ancestor,
-                    descendant,
-                ],
-                check=False,
-                capture_output=True,
-                text=True,
-                encoding="utf-8",
-                errors="replace",
-                env=_source_git_environment(),
-            )
-        except OSError:
+        result = _run_git(
+            source_root, "merge-base", "--is-ancestor", ancestor, descendant
+        )
+        if result is None:
             return None
         if result.returncode == 0:
             return True
@@ -208,36 +292,16 @@ def trusted_release_ref_for_root(
     if not _is_git_worktree_root(source_root):
         return None
 
-    try:
-        remotes = subprocess.run(
-            ["git", "-C", str(source_root), "remote"],
-            check=False,
-            capture_output=True,
-            text=True,
-            encoding="utf-8",
-            errors="replace",
-            env=_source_git_environment(),
-        )
-    except OSError:
-        return None
-    if remotes.returncode != 0:
+    remotes = _run_git(source_root, "remote")
+    if remotes is None or remotes.returncode != 0:
         return None
 
     for remote in remotes.stdout.splitlines():
         remote = remote.strip()
         if not remote:
             continue
-        try:
-            remote_url = subprocess.run(
-                ["git", "-C", str(source_root), "remote", "get-url", remote],
-                check=False,
-                capture_output=True,
-                text=True,
-                encoding="utf-8",
-                errors="replace",
-                env=_source_git_environment(),
-            )
-        except OSError:
+        remote_url = _run_git(source_root, "remote", "get-url", remote)
+        if remote_url is None:
             continue
         if (
             remote_url.returncode != 0
@@ -246,23 +310,12 @@ def trusted_release_ref_for_root(
         ):
             continue
         trusted_ref = f"refs/remotes/{remote}/{expected_ref}"
-        resolved = subprocess.run(
-            [
-                "git",
-                "-C",
-                str(source_root),
-                "rev-parse",
-                "--verify",
-                f"{trusted_ref}^{{commit}}",
-            ],
-            check=False,
-            capture_output=True,
-            text=True,
-            encoding="utf-8",
-            errors="replace",
-            env=_source_git_environment(),
+        resolved = _run_git(source_root, "rev-parse", "--verify", f"{trusted_ref}^{{commit}}")
+        commit = (
+            resolved.stdout.strip()
+            if resolved is not None and resolved.returncode == 0
+            else ""
         )
-        commit = resolved.stdout.strip() if resolved.returncode == 0 else ""
         if commit:
             return {
                 "label": f"{expected_repository}@{expected_ref}",

loopx/release_manifest.py

@@ -9,7 +9,7 @@
 from typing import Any
 
 from . import __version__
-from .doctor_git import git_metadata_for_root
+from .doctor_git import GitSourceState, git_source_metadata
 
 
 RELEASE_MANIFEST_SCHEMA_VERSION = "loopx_release_manifest_v0"
@@ -126,9 +126,13 @@ def _manifest_source_metadata(source_root: Path | None) -> dict[str, Any]:
 
 
 def _source_metadata(source_root: Path | None) -> dict[str, Any]:
-    git_metadata = git_metadata_for_root(source_root)
+    git_metadata, git_state = git_source_metadata(source_root)
     git_metadata.pop("root")
-    if any(git_metadata.get(key) is not None for key in ("git_commit", "git_ref", "git_dirty")):
+    if git_state is not GitSourceState.ABSENT:
+        # A checkout with a `.git` of its own is its own source of truth. When
+        # Git cannot answer this run, the `release.json` next to it describes an
+        # earlier install, so it must not be written back as this one: an
+        # unanswered read stays unknown instead of claiming the old commit.
         return {
             **git_metadata,
             "kind": None,

tests/test_distribution_runtime_identity.py

@@ -63,6 +63,10 @@ def test_real_pip_compiled_and_uncompiled_wheel_have_the_same_identity(tmp_path:
         "loopx/__init__.py": Path(manifest.__file__).with_name("__init__.py").read_text(),
         "loopx/release_manifest.py": Path(manifest.__file__).read_text(),
         "loopx/doctor_git.py": Path(manifest.__file__).with_name("doctor_git.py").read_text(),
+        "loopx/extensions/__init__.py": "",
+        "loopx/extensions/process_runtime.py": Path(manifest.__file__).with_name(
+            "extensions"
+        ).joinpath("process_runtime.py").read_text(),
         f"{metadata}/METADATA": f"Metadata-Version: 2.1\nName: loopx\nVersion: {version}\n",
         f"{metadata}/WHEEL": "Wheel-Version: 1.0\nGenerator: test\nRoot-Is-Purelib: true\nTag: py3-none-any\n",
     }