Hold every cargo build to the git hooks

A hook that lost its executable bit, or a global core.hooksPath, never
touched a file Cargo watches, so build.rs could not notice either and
only warned when it did. The check now lives in scripts/hook-check, a
workspace member whose build script reruns on every build, so only that
empty crate recompiles. A build whose hooks cannot be put in effect
fails with the installer's instructions; CODETRIAL_SKIP_HOOKS=1 opts
out, and CI sets it. cargo test and commands naming their targets leave
the check out.

The installer now asks git once and returns early when every hook is in
effect, and indent.sh covers the new crate.

Author: jserv

.github/workflows/check.yml

@@ -68,6 +68,10 @@ env:
   # and absent is what it was here, so the file that describes every other
   # check was the one file nothing checked.
   ACTIONLINT_VERSION: 1.7.12
+  # scripts/hook-check would install git hooks into each runner's checkout and
+  # fail the build over them. Nobody commits here, and the commit-log step
+  # replays every message the hooks would have judged.
+  CODETRIAL_SKIP_HOOKS: 1
 
 jobs:
   audit:
@@ -746,7 +750,7 @@ jobs:
         shell: bash
       - name: Build the self-contained executable
         if: runner.os != 'Linux'
-        run: cargo build --release --locked --target ${{ matrix.target }}
+        run: cargo build --release --locked -p codetrial --target ${{ matrix.target }}
       # GitHub retired the Ubuntu 20.04 runner, but its glibc 2.31 remains the
       # useful Linux release baseline. Build inside Bullseye, which carries the
       # same glibc, rather than on the runner. This is deliberately not musl:
@@ -792,7 +796,7 @@ jobs:
             --env CXX=/clang/bin/clang++ \
             --env GITHUB_SHA \
             ${{ matrix.image }} \
-            cargo build --release --locked --target ${{ matrix.target }}
+            cargo build --release --locked -p codetrial --target ${{ matrix.target }}
       # A successful link does not prove the Objective-C categories survived.
       # The selector reference remains even when its implementation is absent,
       # so match a method name entry rather than any occurrence of the selector.

Cargo.lock

@@ -85,6 +85,14 @@ tokio = { version = "1.54.1", features = ["test-util"] }
 name = "audio-latency"
 test = true
 
+# scripts/hook-check holds every `cargo build` to the git hooks being in
+# effect; its build.rs says why it is a member of its own. `vendor/` holds path
+# dependencies, not members.
+[workspace]
+members = ["scripts/hook-check"]
+default-members = [".", "scripts/hook-check"]
+exclude = ["vendor"]
+
 # Release archives distribute the executable, not a debugging target. Cargo
 # already omits debug information in this profile; omit linker symbols too.
 #

Cargo.toml

@@ -1,5 +1,5 @@
-//! Makes `rust-embed` see a changed asset tree, records the commit being
-//! built, and installs the git hooks.
+//! Makes `rust-embed` see a changed asset tree and records the commit being
+//! built.
 //!
 //! The derive expands to one `include_bytes!` per file, and rustc records
 //! those in dep-info, so *editing* an embedded asset already rebuilds. Files
@@ -19,7 +19,6 @@ use std::process::Command;
 fn main() {
     println!("cargo:rerun-if-changed=web");
     set_build_commit();
-    install_git_hooks();
 }
 
 /// The released binary must identify the source it was built from even when
@@ -102,99 +101,3 @@ fn watch_git_head(root: &std::path::Path) {
 fn git_path(root: &std::path::Path, name: &str) -> Option<std::path::PathBuf> {
     git_output(root, &["rev-parse", "--git-path", name]).map(|path| root.join(path.trim()))
 }
-
-/// Installs a wrapper for each `scripts/git-*.sh` in the hooks directory of a
-/// checkout, unless they are already in effect.
-///
-/// A hook nobody installed is a hook nobody runs, and the first person to find
-/// that out is whoever reads a commit message the rules would have caught. A
-/// build is the one step every contributor takes, so it is where installation
-/// belongs; `make hooks` stays for anyone who wants it on its own. `cargo
-/// check`, clippy and rust-analyzer share this script's run, so whichever comes
-/// first in a fresh clone installs.
-///
-/// Never fails the build. A checkout can be read-only, a hooks directory can
-/// belong to somebody else's tooling, and a Windows machine may have no `sh`.
-/// None of that is a reason to refuse to compile, so a failure is a warning
-/// and the build goes on.
-fn install_git_hooks() {
-    let root = std::env::var_os("CARGO_MANIFEST_DIR").map(std::path::PathBuf::from);
-    let Some(root) = root else { return };
-
-    // A worktree has a `.git` file rather than a directory, and both mean the
-    // same thing here: this is a checkout, so it has hooks worth installing. A
-    // packaged crate has neither.
-    if !root.join(".git").exists() {
-        return;
-    }
-
-    // Watched so that a changed installer, or a deleted hook, reruns this.
-    // Without the second Cargo would keep the build script fresh and a removed
-    // hook would stay removed until something else forced a rerun. The first
-    // build after a fresh clone pays one extra rebuild, because installing the
-    // hooks is itself a change to the directory being watched.
-    //
-    // Not the whole of scripts/, though adding a hook there is the one change
-    // this does not notice. That directory holds every gate script in the
-    // repository and each edit to one would rebuild the binary; `make hooks`
-    // covers the rare case at no standing cost.
-    //
-    // A checkout without the installer has nothing to run, and a watch on a
-    // path that does not exist would rerun this script on every build.
-    if !root.join("scripts/install-git-hooks.sh").is_file() {
-        return;
-    }
-    println!("cargo:rerun-if-changed=scripts/install-git-hooks.sh");
-    run_hook_installer(&root);
-
-    // Asked of git rather than assumed to be `.git/hooks`: a linked worktree
-    // keeps its hooks with the common directory, and core.hooksPath moves them
-    // anywhere. After the install rather than before, because a clone made with
-    // an empty template has no hooks directory until the installer makes one,
-    // and a directory that was not there to watch is never watched.
-    if let Some(hooks) = git_path(&root, "hooks")
-        && hooks.is_dir()
-    {
-        println!("cargo:rerun-if-changed={}", hooks.display());
-    }
-}
-
-fn run_hook_installer(root: &std::path::Path) {
-    let installer = root.join("scripts/install-git-hooks.sh");
-    let run = |args: &[&str]| {
-        Command::new("sh")
-            .arg(&installer)
-            .args(args)
-            .current_dir(root)
-            .output()
-    };
-
-    // Installed already, which is every build after the first: the check writes
-    // nothing, so it leaves the watched directory alone and says nothing. The
-    // installer stays the one judge of what installed means.
-    //
-    // No `sh` at all is the other quiet case, in practice a Git for Windows
-    // install that did not put its shell on PATH. The release job builds a
-    // Windows target on every push, and a warning nobody can act on teaches
-    // people to skim them.
-    match run(&["--check"]) {
-        Ok(output) if output.status.success() => return,
-        Err(error) if error.kind() == std::io::ErrorKind::NotFound => return,
-        _ => {}
-    }
-
-    // Cargo swallows a build script's stdout, which is right for the OK and
-    // HOOK lines. The installer writes what needs a person to stderr instead: a
-    // hook it refused to overwrite, a core.hooksPath shared with other
-    // repositories, and how to fix each. That reaches the terminal line for
-    // line or nothing does.
-    let reason = match run(&[]) {
-        Ok(output) if output.status.success() => return,
-        Ok(output) => String::from_utf8_lossy(&output.stderr).into_owned(),
-        Err(error) => error.to_string(),
-    };
-    for line in reason.lines().filter(|line| !line.trim().is_empty()) {
-        println!("cargo::warning=git hooks: {line}");
-    }
-    println!("cargo::warning=git hooks are not all in effect; run make hooks");
-}

build.rs

@@ -246,18 +246,27 @@ repository is reported as `SHARED` and nothing is written there. A checkout
 from before the scripts existed has nothing to run and commits as before.
 `./scripts/install-git-hooks.sh --check` runs the verification alone.
 
-`build.rs` runs that check whenever the build script reruns and the install
-only when it fails. The script runs on the first `cargo build`, `cargo check`
-or rust-analyzer pass in a clone or worktree, and again when the installer or
-the hooks directory changes, so a deleted hook comes back on the next build.
-A refusal reaches the terminal as a Cargo warning and never fails the build.
-
-The hooks run the `scripts/` of the checkout they fire in, as `build.rs` does
-on a build, so review a branch before committing on it as you would before
-building it. No script of ours runs while a merge is concluded: by the time
-its commit-msg or prepare-commit-msg fires the incoming tree is written, and
-running it would make `git pull` execute what it pulled, so the wrapper skips
-it, and there is no pre-merge-commit hook. CI checks a pull request's content.
+Every `cargo build`, `cargo check`, `cargo run` and rust-analyzer pass in a
+clone or worktree runs the install through `scripts/hook-check`, a
+workspace member whose build script reruns on every build, so a deleted or
+non-executable hook and a `core.hooksPath` set in a global config are all
+caught on the next one. Anything the install cannot settle, a `FOREIGN` hook, a
+`SHARED` hooks directory or no `sh` to run it with, fails the build with the
+installer's own instructions. `CODETRIAL_SKIP_HOOKS=1` skips the check: CI sets
+it, since it replays every commit message anyway, and so can a checkout that
+cannot take hooks. It is also what keeps `make uninstall-hooks` in effect, since
+the next build otherwise puts the hooks back. `cargo test` leaves the check
+out, since it would link an empty test binary for it each time, and so does a
+command that names its targets: `-p codetrial`, `--bin`, `--test` or
+`cargo install --path .`.
+
+The hooks run the `scripts/` of the checkout they fire in, as
+`scripts/hook-check` does on a build, so review a branch before committing on it
+as you would before building it. No script of ours runs while a merge is
+concluded: by the time its commit-msg or prepare-commit-msg fires the incoming
+tree is written, and running it would make `git pull` execute what it pulled, so
+the wrapper skips it, and there is no pre-merge-commit hook. CI checks a pull
+request's content.
 
 The pre-commit hook runs ESLint, `ruff check` and `shellcheck`, then
 `scripts/indent.sh --staged`, over a checkout of the index, so an unstaged edit

docs/development.md

@@ -0,0 +1,11 @@
+[package]
+name = "hook-check"
+version = "0.0.0"
+edition = "2024"
+publish = false
+
+# Nothing to compile but the build script, which is the whole of this crate.
+[lib]
+path = "lib.rs"
+test = false
+doctest = false

scripts/hook-check/Cargo.toml

@@ -0,0 +1,66 @@
+//! Holds every `cargo build` of a checkout to its git hooks being in effect,
+//! installing them when they are not and failing the build when that does not
+//! work.
+//!
+//! A crate of its own, which the server does not depend on, so that this script
+//! can rerun on every build: Cargo notices a change by a file's mtime, and
+//! neither a hook losing its executable bit nor a `core.hooksPath` set in a
+//! global config touches one. Rerunning a build script recompiles its package,
+//! which for this empty one costs a fraction of a second and for the server
+//! would cost a full rebuild.
+
+use std::path::PathBuf;
+use std::process::{Command, exit};
+
+fn main() {
+    // A path that never exists is stale on every build, which is the point.
+    println!("cargo:rerun-if-changed=.rerun-on-every-build");
+
+    // For CI, which commits nothing and replays every message anyway, and for a
+    // checkout that cannot take hooks at all, such as a read-only mount. Only
+    // `1`, as documented: a `0` or `false` meant to keep the check would
+    // otherwise turn it off.
+    if std::env::var_os("CODETRIAL_SKIP_HOOKS").is_some_and(|value| value == "1") {
+        return;
+    }
+    let Some(root) = checkout() else { return };
+
+    // The installer is the one judge of what in effect means: present, current,
+    // ours and executable, wherever core.hooksPath points today. It rewrites
+    // only what is missing or stale, so a healthy checkout costs a read. Cargo
+    // swallows a build script's stdout, which is right for the OK lines, and
+    // shows its stderr when the script fails: there the installer says what it
+    // refused, a hook of the user's or a shared hooks directory, and the fix.
+    let installer = root.join("scripts/install-git-hooks.sh");
+    let reason = match Command::new("sh")
+        .arg(&installer)
+        .current_dir(&root)
+        .output()
+    {
+        Ok(output) if output.status.success() => return,
+        Ok(output) => String::from_utf8_lossy(&output.stderr).into_owned(),
+        Err(error) => format!("cannot run {}: {error}\n", installer.display()),
+    };
+    eprint!("{reason}");
+    eprintln!("git hooks are not all in effect; settle the above and build again.");
+    eprintln!("Where hooks cannot be installed, CODETRIAL_SKIP_HOOKS=1 skips this.");
+    exit(1);
+}
+
+/// The checkout this crate sits in, two levels up, when git agrees it is the
+/// top of a work tree. git skips a stray or half-copied `.git` and keeps
+/// searching upward, and a tree unpacked inside some other repository must not
+/// write hooks into that one, so an empty prefix is the test. A source archive
+/// has no repository, and git refuses. So does a git that is missing, refused
+/// by safe.directory or facing a broken `.git`, and that is a skip too: hooks
+/// guard commits, and a git that cannot run here makes none.
+fn checkout() -> Option<PathBuf> {
+    let manifest = PathBuf::from(std::env::var_os("CARGO_MANIFEST_DIR")?);
+    let root = manifest.parent()?.parent()?.to_path_buf();
+    let output = Command::new("git")
+        .args(["rev-parse", "--show-prefix"])
+        .current_dir(&root)
+        .output()
+        .ok()?;
+    (output.status.success() && output.stdout.trim_ascii().is_empty()).then_some(root)
+}

scripts/hook-check/build.rs

@@ -0,0 +1 @@
+//! Empty on purpose: build.rs beside this file is the check.

scripts/hook-check/lib.rs

@@ -108,7 +108,7 @@ list()
     done
 }
 
-rust=$(list 'build.rs' 'src/*.rs' 'tests/*.rs')
+rust=$(list 'build.rs' 'src/*.rs' 'tests/*.rs' 'scripts/*.rs')
 shell=$(list '*.sh')
 
 # tests/ as well as scripts/: the Python under tests/ drives the recording and
@@ -249,15 +249,17 @@ if [ "$mode" = check ]; then
     work=$(mktemp -d "$notes/tree.XXXXXX") || exit 2
 
     # One tar pipe rather than a mkdir and a cp per file: 81 files was 162
-    # process spawns and most of this gate's wall clock. Cargo.toml because
-    # `cargo fmt` reads the manifest to find the crate. A setting the tree does
-    # not have is left out, or tar would refuse the whole copy over it.
+    # process spawns and most of this gate's wall clock. Every manifest, because
+    # `cargo fmt` reads them to find the workspace and each member's files. A
+    # setting the tree does not have is left out, or tar would refuse the whole
+    # copy over it.
     copy=$files
     for setting in $configs; do
         [ -f "$setting" ] && copy="$copy
 $setting"
     done
-    printf '%s\n' "$copy" Cargo.toml \
+    manifests=$(list 'Cargo.toml' '*/Cargo.toml')
+    printf '%s\n' "$copy" "$manifests" \
         | tar -cf - -T - \
         | (cd "$work" && tar -xf -) || exit 2
 fi

scripts/indent.sh

@@ -6,8 +6,8 @@
 # invoked it instead of naming one by path.
 #
 # Exit 0 means every hook is in effect, not merely that this script ran. --check
-# asks the same question without writing anything, and build.rs asks it on every
-# build-script rerun.
+# asks the same question without writing anything, and scripts/hook-check asks
+# it on every build.
 
 set -u
 
@@ -35,8 +35,18 @@ abs()
         *) printf '%s/%s\n' "$ROOT" "$1" ;;
     esac
 }
-hooks=$(abs "$(git rev-parse --git-path hooks)") || exit 1
-common=$(abs "$(git rev-parse --git-common-dir)") || exit 1
+
+# One git for both answers, split without another process: hook-check runs this
+# on every build.
+paths=$(git rev-parse --git-path hooks --git-common-dir) || exit 1
+{
+    IFS= read -r hooks
+    IFS= read -r common
+} << EOF
+$paths
+EOF
+hooks=$(abs "$hooks")
+common=$(abs "$common")
 
 # The wrapper says it is ours on its second line, so a later install can tell it
 # from a hook somebody wrote and rewrite it when its text changes, rather than
@@ -129,7 +139,7 @@ foreign()
 # wrapper that lost it reads as installed while nothing runs.
 #
 # Progress goes to stdout and anything a person has to act on to stderr, so
-# build.rs can pass the second on as it stands and drop the first.
+# scripts/hook-check can pass the second on as it stands and drop the first.
 verify()
 {
     status=0
@@ -180,6 +190,13 @@ if [ "$mode" = uninstall ]; then
     exit 0
 fi
 
+# Every hook in effect already, which is every build after the first: nothing to
+# write, and one pass of verify says so.
+if report=$(verify 2> /dev/null); then
+    printf '%s\n' "$report"
+    exit 0
+fi
+
 # core.hooksPath can send every repository's hooks to one shared directory,
 # usually from a global config. Writing there would install these wrappers for
 # repositories that have no scripts/ to run, so this only reports.