Block fixup commits and check hooks on each build

.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
 
   # The license and source policy in deny.toml, pinned like the tools above:
   # a release that reads that file differently should arrive as a bump.
@@ -749,7 +753,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:
@@ -801,7 +805,7 @@ jobs:
             --env CXX=/clang/bin/clang++ \
             --env GITHUB_SHA \
             ${{ matrix.image }} \
-            sh -c 'RUSTUP_TOOLCHAIN=${RUST_VERSION:?} exec cargo build --release --locked --target "$1"' \
+            sh -c 'RUSTUP_TOOLCHAIN=${RUST_VERSION:?} exec cargo build --release --locked -p codetrial --target "$1"' \
             sh ${{ matrix.target }}
       # A successful link does not prove the Objective-C categories survived.
       # The selector reference remains even when its implementation is absent,

CONTRIBUTING.md

@@ -97,8 +97,9 @@ own already in the way, or a `core.hooksPath` shared with other repositories, is
 left alone and reported with how to integrate it. Your own hook keeps running
 once moved to `.git/hooks/<name>.local`, after ours.
 `./scripts/install-git-hooks.sh --check` asks the same question without writing.
-`cargo build` runs the same installer whenever that check fails, and skips it
-once the hooks are in effect; what needs you shows up as a Cargo warning.
+`cargo build` and `cargo check` run the same installer on every build, and
+fail when the hooks cannot be put in effect, printing what needs you. Set
+`CODETRIAL_SKIP_HOOKS=1` for a checkout that cannot take hooks.
 
 Formatting is required of every contributor; only a pull request Dependabot
 opened and wrote has formatting drift reported rather than failed. The pre-commit
@@ -130,11 +131,13 @@ Refuse a permission listing that may be truncated
 Prove an empty test run is not a pass
 ```
 
-A commit needs a body: the hook and CI refuse a subject alone. Fixup, squash and
-merge commits are exempt, and so are Dependabot's. The body carries the premise
-and the trade, usually in a single paragraph. This tree keeps its detailed
-reasoning in the comment next to the code, so a body retelling the mechanics
-duplicates that comment and then goes stale on its own.
+A commit needs a body: the hook and CI refuse a subject alone. Merge commits are
+exempt, and so are Dependabot's. A `fixup!`, `squash!` or `amend!` subject is
+refused at commit, at push and in CI alike: fold the change into the commit it
+fixes, with `git rebase -i --autosquash` for an older one. The body carries the
+premise and the trade, usually in a single paragraph. This tree keeps its
+detailed reasoning in the comment next to the code, so a body retelling the
+mechanics duplicates that comment and then goes stale on its own.
 The hook rejects the shape that does it outright: a body line opening with
 `How:`, `Step:`, `Steps:`, `Changes:` or an `Implementation:` heading. Whether
 what is left says anything is the part only a reviewer can judge.

Cargo.toml

@@ -96,6 +96,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.
 #

build.rs

@@ -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");
-}

docs/development.md

@@ -252,18 +252,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

scripts/git-commit-msg.sh

@@ -20,7 +20,8 @@ if [ "${1:-}" = "--rules" ]; then
 2. Keep the subject within 50 characters, and say more than one word
 3. Capitalize the subject; no trailing period, no backticks
 4. Use the imperative mood: "Fix", not "Fixed" or "Fixes"
-5. No conventional-commit prefix: not "fix:", not "feat(web):"
+5. No conventional-commit prefix: not "fix:", not "feat(web):",
+   and no "fixup!", "squash!" or "amend!": fold those before committing
 6. Wrap the body at 72 characters
 7. Write a body saying what and why; the diff says how
 
@@ -62,10 +63,15 @@ error()
     failed=1
 }
 
-# A fixup lands in the commit it names, and a merge subject is git's wording,
-# not the author's. Judging either is judging a message nobody wrote.
+# An autosquash marker is a note to a rebase that has not happened yet, and the
+# subject behind it is a copy of another commit's: the change belongs in that
+# commit before this one exists. A merge subject is git's wording, not the
+# author's, and judging it is judging a message nobody wrote.
 case "$subject" in
-    "fixup! "* | "squash! "* | "amend! "*) exit 0 ;;
+    "fixup! "* | "squash! "* | "amend! "*)
+        error "fold this into the commit it fixes: git commit --amend for HEAD, git rebase -i --autosquash for an older one"
+        exit 1
+        ;;
     "Merge branch "* | "Merge branches "* | "Merge tag "* | "Merge commit "* | \
         "Merge pull request "* | "Merge remote-tracking branch "*) exit 0 ;;
 esac
@@ -96,9 +102,9 @@ case "$subject" in
     *.) error "subject must not end with a period" ;;
 esac
 
-# The subject completes "this commit will ...", so a past tense or a gerund in
-# the first word is the whole tell. Checking one word catches nearly all of it
-# and never argues with a legitimate "Fix the parser" style subject.
+# The subject completes "this commit will ...", so a past tense, a gerund or a
+# third person in the first word is the whole tell. Named rather than matched by
+# suffix, which would also refuse "Alias", "Seed" and "Bring".
 first_word=$(printf '%s' "$subject" | sed 's/[[:space:]].*//' | tr '[:upper:]' '[:lower:]')
 case "$first_word" in
     added | adds | adding | adjusted | adjusts | adjusting | allowed | allows | \
@@ -114,7 +120,13 @@ case "$first_word" in
         removes | removing | renamed | renames | renaming | replaced | replaces | \
         replacing | reverted | reverts | reverting | tested | tests | testing | \
         tidied | tidies | tidying | updated | updates | updating | used | uses | \
-        using)
+        using | wired | wires | wiring | lets | letting | built | builds | \
+        building | wrote | writes | writing | ran | runs | running | kept | \
+        keeps | keeping | sent | sends | sending | brought | brings | bringing | \
+        took | takes | taking | gave | gives | giving | got | gets | getting | \
+        began | begins | beginning | chose | chooses | choosing | met | meets | \
+        meeting | held | holds | holding | told | tells | telling | thought | \
+        thinks | thinking)
         error "use the imperative mood: \"$first_word\" describes what you did"
         ;;
 esac

scripts/hook-check/Cargo.toml

@@ -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/build.rs

@@ -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/lib.rs

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