toolchain: precheck passes under the pre-commit hook from a worktree

.github/scripts/check_coverage_map_health.py

@@ -1,11 +1,10 @@
 """Fail loudly if the committed coverage map is stale or under-covers. Used by coverage-health.yml."""
 import datetime
-import subprocess
 import sys
 from pathlib import Path
 
 sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "toolchain"))
-from mfc.test.coverage import COVERAGE_MAP_PATH, load_map, map_health  # noqa: E402
+from mfc.test.coverage import COVERAGE_MAP_PATH, _git, load_map, map_health  # noqa: E402
 from mfc.test.cases import list_cases  # noqa: E402  (returns the current test list)
 
 MAX_AGE_DAYS = 10
@@ -40,7 +39,7 @@ def verified_sha(cwd=None):
     caller must read that as undeterminable and fall back to the wall-clock age rule, not
     as a failure -- an absent ref is not evidence of a broken refresh.
     """
-    rev = subprocess.run(["git", "rev-parse", "--verify", "--quiet", f"{VERIFIED_REF}^{{commit}}"], capture_output=True, text=True, check=False, cwd=cwd)
+    rev = _git(["rev-parse", "--verify", "--quiet", f"{VERIFIED_REF}^{{commit}}"], cwd)
     return rev.stdout.strip() or None
 
 
@@ -53,10 +52,10 @@ def verified_after_last_change(git_sha, cwd=None):
     """
     if not git_sha:
         return None
-    last = subprocess.run(["git", "log", "-1", "--format=%H", "--", *COVERAGE_RELEVANT_PATHS], capture_output=True, text=True, check=False, cwd=cwd)
+    last = _git(["log", "-1", "--format=%H", "--", *COVERAGE_RELEVANT_PATHS], cwd)
     if last.returncode != 0 or not last.stdout.strip():
         return None  # shallow clone or no such commit -> fall back to the age rule
-    ancestor = subprocess.run(["git", "merge-base", "--is-ancestor", last.stdout.strip(), git_sha], capture_output=True, check=False, cwd=cwd)
+    ancestor = _git(["merge-base", "--is-ancestor", last.stdout.strip(), git_sha], cwd)
     return {0: True, 1: False}.get(ancestor.returncode)  # anything else -> None (unknown sha, shallow history)
 
 

toolchain/mfc/test/coverage.py

@@ -216,8 +216,14 @@ def select_tests(cases, coverage_map, changed_files):
     return to_run, skipped, f"selected {len(to_run)}/{len(cases)} by coverage overlap"
 
 
+def _env_without_git():
+    # Git exports GIT_DIR and GIT_INDEX_FILE to hooks, and neither cwd nor `git -C` overrides them:
+    # under the pre-commit hook every call below would otherwise act on the committing repository.
+    return {k: v for k, v in os.environ.items() if not k.startswith("GIT_")}
+
+
 def _git(args, cwd, timeout=60):
-    return subprocess.run(["git", *args], capture_output=True, text=True, cwd=cwd, timeout=timeout, check=False)
+    return subprocess.run(["git", *args], capture_output=True, text=True, cwd=cwd, timeout=timeout, check=False, env=_env_without_git())
 
 
 def _merge_base(cwd, branch):

toolchain/mfc/test/test_coverage_unit.py

@@ -6,7 +6,7 @@
 from pathlib import Path
 from unittest.mock import patch
 
-from mfc.test.coverage import canonicalize_param_paths, entries_equal, format_summary, get_changed_files, is_always_run_all, load_map, map_health, param_hash, save_map, select_tests
+from mfc.test.coverage import _env_without_git, canonicalize_param_paths, entries_equal, format_summary, get_changed_files, is_always_run_all, load_map, map_health, param_hash, save_map, select_tests
 
 
 def test_param_hash_is_order_independent():
@@ -462,17 +462,6 @@ def test_health_fails_immediately_when_no_refresh_ran_since_last_source_change()
 CHANGED_SCRIPT = Path(__file__).resolve().parents[3] / ".github" / "scripts" / "coverage_map_changed.py"
 
 
-def _env_without_git():
-    """The environment minus every GIT_* variable.
-
-    `git -C <dir>` changes directory but does NOT override an inherited GIT_DIR or
-    GIT_INDEX_FILE. Git exports both when it runs a hook, and MFC's pre-commit hook runs
-    precheck, which runs this suite -- so without this scrub the commits below are made
-    against the real repository instead of the throwaway one.
-    """
-    return {k: v for k, v in os.environ.items() if not k.startswith("GIT_")}
-
-
 def _repo_with_committed_map(d, entries):
     """A throwaway git repo whose HEAD holds `entries` as the coverage map."""
     repo = Path(d)