fix(code-health): hooks prefer the repo virtualenv; scripts/check refuses Python < 3.11
unsupportedpastels M5: both hooks ran whatever python3 came first on PATH, but scripts/check imports ruff_runner, which imports tomllib (3.11+, the project minimum). With Ubuntu 22.04's python3 3.10 every hooked commit or push died with "ModuleNotFoundError: No module named 'tomllib'". The hooks now use the repo's virtualenv interpreter when there is one (.venv/ or venv/, bin/python or Scripts/python.exe; this worktree first, then the main checkout) and fall back to python3, then python. scripts/check checks sys.version_info before importing the engine and exits 2 with a one-line message naming the virtualenv instead of a traceback. Verified with a real CPython 3.10.21: exit 2 and the message, where 96bb97a crashed in ruff_runner.
This commit is contained in:
@@ -28,6 +28,13 @@ from contextlib import contextmanager
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
|
||||
# Before the engine imports: they need 3.11 (tomllib), and an older python3 first on PATH would
|
||||
# otherwise die in a traceback on every hooked commit or push.
|
||||
if sys.version_info < (3, 11):
|
||||
print(f"scripts/check needs Python 3.11+, this is {'.'.join(map(str, sys.version_info[:3]))};"
|
||||
" run it with the repo's virtualenv (.venv/bin/python scripts/check)", file=sys.stderr)
|
||||
raise SystemExit(2)
|
||||
|
||||
REPO = Path(__file__).resolve().parent.parent
|
||||
sys.path.insert(0, str(REPO))
|
||||
|
||||
@@ -40,8 +47,17 @@ _HOOK_HEAD = f"""#!/usr/bin/env bash
|
||||
repo="$(git rev-parse --show-toplevel)"
|
||||
# Hooks are shared by every worktree; a branch that predates scripts/check is not checked.
|
||||
[ -f "$repo/scripts/check" ] || exit 0
|
||||
py=python3
|
||||
command -v python3 >/dev/null 2>&1 || py=python
|
||||
# The repo's virtualenv first (this worktree's, then the main checkout's): a bare python3 on
|
||||
# PATH may predate the 3.11 that scripts/check needs.
|
||||
common="$(cd "$(git rev-parse --git-common-dir)/.." && pwd)"
|
||||
py=
|
||||
for root in "$repo" "$common"; do
|
||||
for rel in .venv/bin/python venv/bin/python .venv/Scripts/python.exe venv/Scripts/python.exe; do
|
||||
if [ -z "$py" ] && [ -x "$root/$rel" ]; then py="$root/$rel"; fi
|
||||
done
|
||||
done
|
||||
[ -n "$py" ] || py="$(command -v python3 || command -v python)" || {{
|
||||
echo "scripts/check hook: no python3 or python on PATH" >&2; exit 1; }}
|
||||
"""
|
||||
# pre-push reads "<local ref> <local sha> <remote ref> <remote sha>" per pushed ref; an all-zero
|
||||
# local sha is a branch deletion. Each pushed commit is judged as CI will judge the PR.
|
||||
|
||||
@@ -81,20 +81,22 @@ def _init(path: Path) -> Path:
|
||||
|
||||
|
||||
def _add_engine(repo: Path) -> list[str]:
|
||||
"""The checker and everything its jobs read; returns the top-level paths to stage."""
|
||||
for rel in _ENGINE:
|
||||
(repo / rel).parent.mkdir(parents=True, exist_ok=True)
|
||||
shutil.copy2(REPO / rel, repo / rel)
|
||||
shutil.copytree(REPO / "scripts/code_health", repo / "scripts/code_health",
|
||||
ignore=shutil.ignore_patterns("__pycache__"), dirs_exist_ok=True)
|
||||
return ["scripts"]
|
||||
_write(repo, {"tests/README.md": "os-marker-fakes scans tests/\n"})
|
||||
return ["scripts", "tests"]
|
||||
|
||||
|
||||
def _engine_repo(tmp_path: Path) -> Path:
|
||||
"""A repo whose first commit already carries the checker, with one clean module."""
|
||||
repo = _init(tmp_path / "repo")
|
||||
_write(repo, {"pkg/a.py": "def a():\n return 1\n"})
|
||||
_add_engine(repo)
|
||||
_git(repo, "add", "--all", "--", ".gitignore", "pyproject.toml", "pkg", "scripts")
|
||||
staged = _add_engine(repo)
|
||||
_git(repo, "add", "--all", "--", ".gitignore", "pyproject.toml", "pkg", *staged)
|
||||
_git(repo, "commit", "-q", "-m", "base")
|
||||
return repo
|
||||
|
||||
@@ -186,6 +188,43 @@ def test_json_stdout_is_json_on_every_path(tmp_path, capsys, case, switch, files
|
||||
assert "code health:" in err # the human explanation moves to stderr
|
||||
|
||||
|
||||
# --- M5: interpreter choice and the version guard ---------------------------------------------
|
||||
|
||||
|
||||
def test_hooks_prefer_the_repo_virtualenv_over_path_python(tmp_path):
|
||||
repo = _engine_repo(tmp_path)
|
||||
assert _check(repo, "--install-hook", "pre-commit").returncode == 0
|
||||
fake = tmp_path / "fakebin"
|
||||
fake.mkdir()
|
||||
for name in ("python3", "python"):
|
||||
(fake / name).write_text("#!/bin/sh\necho 'wrong interpreter' >&2\nexit 97\n", encoding="utf-8")
|
||||
(fake / name).chmod(0o755)
|
||||
env = _env()
|
||||
env["PATH"] = f"{fake}{os.pathsep}{env['PATH']}"
|
||||
(repo / ".venv").symlink_to(Path(sys.prefix), target_is_directory=True)
|
||||
_write(repo, {"pkg/b.py": "def b():\n return 2\n"})
|
||||
_git(repo, "add", "--", "pkg/b.py")
|
||||
proc = _sh(repo, "git", "commit", "-q", "-m", "venv", env=env)
|
||||
assert proc.returncode == 0, proc.stdout + proc.stderr
|
||||
# control: without a virtualenv the hook falls back to PATH's python3
|
||||
(repo / ".venv").unlink()
|
||||
_write(repo, {"pkg/b.py": "def b():\n return 3\n"})
|
||||
_git(repo, "add", "--", "pkg/b.py")
|
||||
proc = _sh(repo, "git", "commit", "-q", "-m", "path", env=env)
|
||||
assert proc.returncode != 0 and "wrong interpreter" in proc.stderr
|
||||
|
||||
|
||||
def test_check_refuses_old_python_before_importing_anything():
|
||||
# The guard reads sys.version_info; a 3.10 interpreter is emulated by overriding it before
|
||||
# scripts/check runs (tomllib, imported via ruff_runner, would otherwise crash it).
|
||||
code = ("import runpy, sys; sys.version_info = (3, 10, 12, 'final', 0); "
|
||||
f"sys.argv = ['scripts/check', '--only', 'shebangs']; "
|
||||
f"runpy.run_path({str(REPO / 'scripts/check')!r}, run_name='__main__')")
|
||||
proc = _sh(REPO, sys.executable, "-c", code)
|
||||
assert proc.returncode == 2, proc.stdout + proc.stderr
|
||||
assert "3.11" in proc.stderr and "Traceback" not in proc.stderr
|
||||
|
||||
|
||||
# --- m3: the ENFORCEMENT switch ---------------------------------------------------------------
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user