Skip to content

Commit 853dc90

Browse files
committed
Merge branch 'fix/pre-commit-activation' into dev
Fills the onboarding gap between the committed `.pre-commit-config.yaml` and contributors actually having hooks active on their commits. - `./binder setup` installs pre-commit hooks (previously it only ran doctor). Handles the core.hooksPath trap gracefully. - Vault corpus-guard moved from a bare `.git/hooks/pre-commit` symlink into the framework (as a local always_run hook in `.pre-commit-config.yaml`). Same protection, framework-delivered. - Deleted orphan `book/.pre-commit-config.yaml` — had broken paths that caused "can't open file" errors when `pre-commit install` was run from the `book/` directory. Every hook in it had a functional equivalent in the root config. - `book/docs/CONTRIBUTING.md` step 3 is now "Set Up the Development Environment" directing new contributors to `./book/binder setup`. Result: a fresh clone + `./binder setup` now activates the ~60 framework hooks (EPUB hygiene, BibTeX tidy, figure div syntax, vault corpus-guard, etc.) that were previously dormant for anyone without manual activation.
2 parents e23949a + 9e194e6 commit 853dc90

4 files changed

Lines changed: 160 additions & 216 deletions

File tree

.pre-commit-config.yaml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,28 @@ repos:
4646
args: ["--skip", "*.json,*.bib,*.js,*.tex,*.pdf,_site,_book,node_modules,.venv,htmlcov", "--ignore-words", ".codespell-ignore-words.txt"]
4747
exclude: "^(_site/|_book/|htmlcov/|.*\\.js$|.*\\.pdf$)"
4848

49+
# Vault corpus guard — refuses direct edits to the generated
50+
# `interviews/vault/corpus.json` unless the commit carries the
51+
# `Vault-Override: corpus-json-hand-edit` trailer. The underlying
52+
# script previously ran as a bare `.git/hooks/pre-commit` symlink,
53+
# which meant (a) contributors never got the protection unless they
54+
# manually copied the script, and (b) installing the pre-commit
55+
# framework silently disabled the guard. Wiring it into the framework
56+
# puts both layers on the same rails: every contributor who runs
57+
# `pre-commit install` (or `./binder setup`) gets corpus-guard +
58+
# EPUB hygiene + every other framework hook in one shot.
59+
- repo: local
60+
hooks:
61+
- id: vault-corpus-guard
62+
name: "Vault: Block direct edits to corpus.json"
63+
entry: python3 interviews/vault-cli/scripts/pre_commit_corpus_guard.py
64+
language: system
65+
pass_filenames: false
66+
# The script consults `git diff --cached` itself and short-
67+
# circuits on commits that don't touch the guarded path, so it
68+
# is safe to run on every commit.
69+
always_run: true
70+
4971
# Block subsite-mirror drift on the shared assets we cannot symlink.
5072
# Quarto's resource-copy step preserves symlinks instead of dereferencing
5173
# them, so we keep real-file copies of certain shared assets per

book/.pre-commit-config.yaml

Lines changed: 0 additions & 205 deletions
This file was deleted.

book/cli/commands/maintenance.py

Lines changed: 119 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,24 +156,54 @@ def show_about(self) -> bool:
156156
return True
157157

158158
def setup_environment(self) -> bool:
159-
"""Setup development environment (simplified version)."""
159+
"""Setup development environment.
160+
161+
Two-step onboarding:
162+
1. Install pre-commit hooks (activates the framework so every
163+
commit runs the hygiene / corpus-guard / formatting hooks
164+
declared in .pre-commit-config.yaml).
165+
2. Run the doctor health check to report state.
166+
167+
Step 1 is the load-bearing bit: without it, the .pre-commit-
168+
config.yaml file is committed but never runs on contributor
169+
commits. Pre-commit-framework's `install` command writes a
170+
shim to .git/hooks/pre-commit that dispatches to the
171+
framework; if there was already a different hook there
172+
(e.g. a hand-installed script) pre-commit moves it aside
173+
with a `.legacy` suffix, so running this twice is safe.
174+
"""
160175
console.print("[bold blue]🔧 MLSysBook Environment Setup[/bold blue]")
161176
console.print("[dim]Setting up your development environment...[/dim]\n")
162177

163-
# Run doctor command for comprehensive check
164-
console.print("[blue]🏥 Running health check first...[/blue]")
178+
# --- Step 1: install pre-commit hooks ---------------------------
179+
console.print("[blue]🪝 Installing pre-commit hooks...[/blue]")
180+
hooks_ok = self._install_pre_commit_hooks()
181+
if hooks_ok:
182+
console.print(
183+
"[green] ✅ Pre-commit framework active[/green] "
184+
"[dim](hooks in .pre-commit-config.yaml now run on every commit)[/dim]\n"
185+
)
186+
else:
187+
console.print(
188+
"[yellow] ⚠️ Could not install pre-commit hooks automatically[/yellow]\n"
189+
"[dim] Install manually with: pip install pre-commit && pre-commit install[/dim]\n"
190+
)
191+
192+
# --- Step 2: run doctor health check ---------------------------
193+
console.print("[blue]🏥 Running health check...[/blue]")
165194

166195
# Import and run doctor (avoiding circular imports)
167196
from .doctor import DoctorCommand
168197
doctor = DoctorCommand(self.config_manager, self.chapter_discovery)
169198
health_ok = doctor.run_health_check()
170199

171-
if health_ok:
200+
overall_ok = hooks_ok and health_ok
201+
if overall_ok:
172202
console.print("\n[green]✅ Environment setup complete![/green]")
173203
console.print("[dim]💡 Your system is healthy and ready for development[/dim]")
174204
else:
175205
console.print("\n[yellow]⚠️ Environment setup completed with issues[/yellow]")
176-
console.print("[dim]💡 Please review the health check results above[/dim]")
206+
console.print("[dim]💡 Please review the output above[/dim]")
177207

178208
# Show next steps
179209
next_steps = Panel(
@@ -187,7 +217,90 @@ def setup_environment(self) -> bool:
187217
)
188218
console.print(next_steps)
189219

190-
return health_ok
220+
return overall_ok
221+
222+
def _install_pre_commit_hooks(self) -> bool:
223+
"""Run `pre-commit install` from the repo root.
224+
225+
Returns True on success, False on failure. Does not raise — a
226+
missing `pre-commit` binary or a non-git working tree both
227+
result in a clean False return so the caller can surface a
228+
helpful install hint rather than crashing mid-setup.
229+
"""
230+
import shutil
231+
import subprocess
232+
233+
repo_root = self.config_manager.book_dir.parent
234+
if not (repo_root / ".pre-commit-config.yaml").is_file():
235+
console.print(
236+
f"[yellow] No .pre-commit-config.yaml at {repo_root}; skipping.[/yellow]"
237+
)
238+
return False
239+
240+
if not shutil.which("pre-commit"):
241+
console.print(
242+
"[yellow] `pre-commit` not on PATH.[/yellow] "
243+
"[dim]Install it with `pip install pre-commit`.[/dim]"
244+
)
245+
return False
246+
247+
# pre-commit refuses to install when `core.hooksPath` is
248+
# explicitly set (even to the default), to avoid surprising a
249+
# contributor who configured a custom hooks directory. That
250+
# turns into a confusing error message for someone running
251+
# setup — detect the case and surface a one-liner fix.
252+
try:
253+
probe = subprocess.run(
254+
["git", "config", "--get", "core.hooksPath"],
255+
cwd=repo_root, capture_output=True, text=True, timeout=5,
256+
)
257+
if probe.returncode == 0 and probe.stdout.strip():
258+
console.print(
259+
"[yellow] core.hooksPath is set in this clone "
260+
f"({probe.stdout.strip()}).[/yellow]"
261+
)
262+
console.print(
263+
"[dim] pre-commit refuses to install over an explicit "
264+
"hooksPath. Unset it with:[/dim]"
265+
)
266+
console.print(
267+
" [cyan]git config --unset core.hooksPath[/cyan]"
268+
)
269+
console.print(
270+
"[dim] Then re-run `./binder setup`.[/dim]"
271+
)
272+
return False
273+
except (subprocess.TimeoutExpired, OSError):
274+
# Probe failure is non-fatal — fall through to the install
275+
# attempt, which will surface any real error itself.
276+
pass
277+
278+
try:
279+
result = subprocess.run(
280+
["pre-commit", "install"],
281+
cwd=repo_root,
282+
capture_output=True,
283+
text=True,
284+
timeout=30,
285+
)
286+
except (subprocess.TimeoutExpired, OSError) as e:
287+
console.print(f"[red] pre-commit install failed: {e}[/red]")
288+
return False
289+
290+
if result.returncode != 0:
291+
# Print the first line of stderr for diagnosis without flooding.
292+
msg = (result.stderr or result.stdout or "").strip().splitlines()
293+
if msg:
294+
console.print(f"[red] pre-commit install error: {msg[0]}[/red]")
295+
return False
296+
297+
# Capture the "pre-commit installed at .git/hooks/pre-commit" line
298+
# when present, so the user sees confirmation.
299+
for line in (result.stdout or "").splitlines():
300+
line = line.strip()
301+
if line:
302+
console.print(f"[dim] {line}[/dim]")
303+
return True
191304

192305
def run_namespace(self, args) -> bool:
193306
"""Handle `binder maintain ...` namespace commands."""

0 commit comments

Comments
 (0)