From c04cb27c21adb0c50d20ffe1f898afd6e9d2507a Mon Sep 17 00:00:00 2001 From: Nic-dorman Date: Tue, 3 Nov 2026 13:03:59 +0000 Subject: [PATCH 1/2] build: only watch git paths that exist, so the crate isn't rebuilt on every cargo run build.rs declared rerun-if-changed for .git/HEAD, .git/refs/heads and .git/packed-refs. Cargo treats a path that doesn't exist as changed on every build, so whenever one was missing the script reran and ant-node was recompiled from scratch on each cargo invocation. That happens in a git worktree (.git is a file) and in CI checkouts: on PR #269 every `cargo test --test X` spent ~2m20s recompiling ant-node for a test binary the previous step had already built, and the per-target steps in today's ci.yml pay the same cost (2-5 min each for sub-second tests). The script now asks git for the real git dir and common dir, and only watches the paths that exist. ANT_GIT_COMMIT is still refreshed when HEAD or a branch ref moves. Closes V2-1828 Co-Authored-By: Claude Opus 5.5 --- build.rs | 52 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/build.rs b/build.rs index d25d45a8..48c7b8af 100644 --- a/build.rs +++ b/build.rs @@ -1,23 +1,51 @@ //! Build script for ant-node. +use std::path::{Path, PathBuf}; use std::process::Command; -fn main() { - // Rerun if the git HEAD pointer changes (branch switch, detached HEAD, etc.) - println!("cargo:rerun-if-changed=.git/HEAD"); - // Rerun if the current branch tip moves (new commits) - println!("cargo:rerun-if-changed=.git/refs/heads"); - // Rerun if refs are packed (e.g. after `git gc`) - println!("cargo:rerun-if-changed=.git/packed-refs"); - println!("cargo:rerun-if-changed=build.rs"); - - let commit = Command::new("git") - .args(["rev-parse", "--short", "HEAD"]) +/// Runs `git` with `args` and returns its trimmed stdout, or `None` when git is +/// unavailable or the command fails (for example, outside a git checkout). +fn git(args: &[&str]) -> Option { + Command::new("git") + .args(args) .output() .ok() .filter(|o| o.status.success()) .and_then(|o| String::from_utf8(o.stdout).ok()) - .map_or_else(|| "unknown".to_string(), |s| s.trim().to_string()); + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) +} + +/// Asks Cargo to rerun this script when `path` changes, but only if it exists. +/// +/// Cargo treats a `rerun-if-changed` path that does not exist as changed on +/// every build, which reruns this script and recompiles the whole crate each +/// time. Hard-coded `.git/...` paths hit that in a git worktree (where `.git` +/// is a file) and in CI checkouts that have no `packed-refs`, so every +/// `cargo test --test X` rebuilt ant-node from scratch. +fn rerun_if_exists(path: &Path) { + if path.exists() { + println!("cargo:rerun-if-changed={}", path.display()); + } +} + +fn main() { + println!("cargo:rerun-if-changed=build.rs"); + + // HEAD lives in the per-worktree git dir; branch refs live in the common + // dir that all worktrees share. In a plain clone both are `.git`. + if let Some(git_dir) = git(&["rev-parse", "--git-dir"]) { + // Rerun if the HEAD pointer changes (branch switch, detached HEAD, etc.) + rerun_if_exists(&PathBuf::from(git_dir).join("HEAD")); + } + if let Some(common_dir) = git(&["rev-parse", "--git-common-dir"]) { + let common = PathBuf::from(common_dir); + // Rerun if the current branch tip moves (new commits) + rerun_if_exists(&common.join("refs").join("heads")); + // Rerun if refs are packed (e.g. after `git gc`) + rerun_if_exists(&common.join("packed-refs")); + } + let commit = git(&["rev-parse", "--short", "HEAD"]).unwrap_or_else(|| "unknown".to_string()); println!("cargo:rustc-env=ANT_GIT_COMMIT={commit}"); } From 8acb6c7f3f09234cc72eb2365d3dbe07fea35b6d Mon Sep 17 00:00:00 2001 From: Nic-dorman Date: Tue, 3 Nov 2026 13:40:53 +0000 Subject: [PATCH 2/2] build: watch refs/heads' nearest existing ancestor when it's missing; add regression tests Review on #270: with every ref packed and no refs/heads directory, the previous commit watched nothing for branch refs. The next commit creates refs/heads/ without touching HEAD or packed-refs, so Cargo kept a stale ANT_GIT_COMMIT. The watch-set logic moves to build_support/git_watch.rs, shared by build.rs and a new test target through #[path]. When refs/heads is missing it now watches the nearest existing ancestor inside the common dir (normally refs/), which Cargo scans recursively and so sees the loose ref appear. Every emitted path still exists. tests/build_script_git_watch.rs drives real repositories: - a branch commit changes a watched path - refs packed, refs/heads removed: the next commit leaves HEAD and packed-refs untouched but still changes a watched path (fails against the previous version of the logic) - in a worktree every watched path exists and a commit is seen ci.yml runs the new target alongside the other named ones. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 2 + build.rs | 39 +++---- build_support/git_watch.rs | 53 +++++++++ tests/build_script_git_watch.rs | 195 ++++++++++++++++++++++++++++++++ 4 files changed, 263 insertions(+), 26 deletions(-) create mode 100644 build_support/git_watch.rs create mode 100644 tests/build_script_git_watch.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b1377682..18048126 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -121,6 +121,8 @@ jobs: run: cargo test --test poc_audit_handler_live --features test-utils - name: Run bootstrap-stall PoC regression marker run: cargo test --test poc_bootstrap_stall --features test-utils + - name: Run build-script git-watch regression tests + run: cargo test --test build_script_git_watch --features test-utils filesystems: name: Storage on ${{ matrix.fs }} diff --git a/build.rs b/build.rs index 48c7b8af..1943f6bb 100644 --- a/build.rs +++ b/build.rs @@ -1,8 +1,11 @@ //! Build script for ant-node. -use std::path::{Path, PathBuf}; +use std::path::Path; use std::process::Command; +#[path = "build_support/git_watch.rs"] +mod git_watch; + /// Runs `git` with `args` and returns its trimmed stdout, or `None` when git is /// unavailable or the command fails (for example, outside a git checkout). fn git(args: &[&str]) -> Option { @@ -16,34 +19,18 @@ fn git(args: &[&str]) -> Option { .filter(|s| !s.is_empty()) } -/// Asks Cargo to rerun this script when `path` changes, but only if it exists. -/// -/// Cargo treats a `rerun-if-changed` path that does not exist as changed on -/// every build, which reruns this script and recompiles the whole crate each -/// time. Hard-coded `.git/...` paths hit that in a git worktree (where `.git` -/// is a file) and in CI checkouts that have no `packed-refs`, so every -/// `cargo test --test X` rebuilt ant-node from scratch. -fn rerun_if_exists(path: &Path) { - if path.exists() { - println!("cargo:rerun-if-changed={}", path.display()); - } -} - fn main() { println!("cargo:rerun-if-changed=build.rs"); + println!("cargo:rerun-if-changed=build_support/git_watch.rs"); - // HEAD lives in the per-worktree git dir; branch refs live in the common - // dir that all worktrees share. In a plain clone both are `.git`. - if let Some(git_dir) = git(&["rev-parse", "--git-dir"]) { - // Rerun if the HEAD pointer changes (branch switch, detached HEAD, etc.) - rerun_if_exists(&PathBuf::from(git_dir).join("HEAD")); - } - if let Some(common_dir) = git(&["rev-parse", "--git-common-dir"]) { - let common = PathBuf::from(common_dir); - // Rerun if the current branch tip moves (new commits) - rerun_if_exists(&common.join("refs").join("heads")); - // Rerun if refs are packed (e.g. after `git gc`) - rerun_if_exists(&common.join("packed-refs")); + // Watch only git paths that exist; see `git_watch::watch_paths` for why. + if let (Some(git_dir), Some(common_dir)) = ( + git(&["rev-parse", "--git-dir"]), + git(&["rev-parse", "--git-common-dir"]), + ) { + for path in git_watch::watch_paths(Path::new(&git_dir), Path::new(&common_dir)) { + println!("cargo:rerun-if-changed={}", path.display()); + } } let commit = git(&["rev-parse", "--short", "HEAD"]).unwrap_or_else(|| "unknown".to_string()); diff --git a/build_support/git_watch.rs b/build_support/git_watch.rs new file mode 100644 index 00000000..55a1be02 --- /dev/null +++ b/build_support/git_watch.rs @@ -0,0 +1,53 @@ +//! Which git paths the build script asks Cargo to watch, so that `ANT_GIT_COMMIT` +//! is refreshed whenever `HEAD` resolves to a different commit. +//! +//! Shared by `build.rs` and its regression test (`tests/build_script_git_watch.rs`) +//! through `#[path]`, because a build script can't have tests of its own. + +use std::path::{Path, PathBuf}; + +/// The paths to pass to `cargo:rerun-if-changed`, given the per-worktree git +/// dir (`git rev-parse --git-dir`) and the common dir that all worktrees share +/// (`git rev-parse --git-common-dir`). In a plain clone both are `.git`. +/// +/// Every returned path exists. Cargo treats a `rerun-if-changed` path that does +/// not exist as changed on every build, which reruns the build script and +/// recompiles the whole crate each time; hard-coded `.git/...` paths hit that +/// in a git worktree (where `.git` is a file) and in CI checkouts. +/// +/// What is watched, and why: +/// * `HEAD` in the git dir. It changes on checkout or branch switch, and on +/// every commit when `HEAD` is detached. +/// * `refs/heads` in the common dir, which Cargo scans recursively. A commit on +/// a branch rewrites the loose ref file `refs/heads/`. When every ref +/// is packed the directory can be absent, and the next commit then *creates* +/// `refs/heads/` without touching `HEAD` or `packed-refs`. So when +/// `refs/heads` is missing, its nearest existing ancestor inside the common +/// dir (normally `refs`) is watched instead, which sees that file appear. +/// * `packed-refs` in the common dir, when present. It changes when refs are +/// packed or a packed ref is updated in place. +#[must_use] +pub fn watch_paths(git_dir: &Path, common_dir: &Path) -> Vec { + let mut paths = Vec::new(); + + let head = git_dir.join("HEAD"); + if head.exists() { + paths.push(head); + } + + let heads = common_dir.join("refs").join("heads"); + if let Some(existing) = heads + .ancestors() + .take_while(|p| p.starts_with(common_dir)) + .find(|p| p.exists()) + { + paths.push(existing.to_path_buf()); + } + + let packed = common_dir.join("packed-refs"); + if packed.exists() { + paths.push(packed); + } + + paths +} diff --git a/tests/build_script_git_watch.rs b/tests/build_script_git_watch.rs new file mode 100644 index 00000000..2f42ab0f --- /dev/null +++ b/tests/build_script_git_watch.rs @@ -0,0 +1,195 @@ +//! Regression tests for the git paths `build.rs` asks Cargo to watch (V2-1828). +//! +//! Cargo reruns a build script when a `rerun-if-changed` path is newer than the +//! script's last run (scanning directories recursively), and treats a missing +//! path as changed on every build. The watch set therefore has to contain only +//! existing paths, and some watched path has to change whenever `HEAD` comes to +//! resolve to a different commit, or `ANT_GIT_COMMIT` goes stale. +//! +//! These drive real git repositories in a temporary directory, so they need +//! `git` on the PATH, as the build script itself does. + +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +#[path = "../build_support/git_watch.rs"] +mod git_watch; + +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::thread::sleep; +use std::time::{Duration, SystemTime}; + +/// Runs git in `dir` with a fixed identity and no signing, panicking on failure. +fn git(dir: &Path, args: &[&str]) -> String { + let out = Command::new("git") + .current_dir(dir) + .args([ + "-c", + "user.name=ant-node test", + "-c", + "user.email=test@example.invalid", + "-c", + "commit.gpgsign=false", + ]) + .args(args) + .output() + .expect("git is installed"); + assert!( + out.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8(out.stdout).unwrap().trim().to_string() +} + +/// A repository in a temporary directory with one commit on `main`. +fn repo() -> (tempfile::TempDir, PathBuf) { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path().join("repo"); + fs::create_dir(&dir).unwrap(); + git(&dir, &["init", "-q", "-b", "main"]); + git(&dir, &["commit", "-q", "--allow-empty", "-m", "one"]); + (tmp, dir) +} + +/// The watch set as `build.rs` computes it when run from `dir`. +fn watch_paths_for(dir: &Path) -> Vec { + let resolve = |p: String| { + let p = PathBuf::from(p); + if p.is_absolute() { + p + } else { + dir.join(p) + } + }; + let git_dir = resolve(git(dir, &["rev-parse", "--git-dir"])); + let common_dir = resolve(git(dir, &["rev-parse", "--git-common-dir"])); + git_watch::watch_paths(&git_dir, &common_dir) +} + +/// The newest modification time of `path`, scanning directories recursively +/// as Cargo does for a `rerun-if-changed` directory. +fn newest_mtime(path: &Path) -> SystemTime { + let meta = fs::metadata(path).unwrap(); + let mut newest = meta.modified().unwrap(); + if meta.is_dir() { + for entry in fs::read_dir(path).unwrap() { + newest = newest.max(newest_mtime(&entry.unwrap().path())); + } + } + newest +} + +fn newest_of(paths: &[PathBuf]) -> SystemTime { + paths + .iter() + .map(|p| newest_mtime(p)) + .max() + .expect("the watch set is not empty") +} + +/// Waits out coarse filesystem timestamps, so a later write gets a later mtime. +fn tick() { + sleep(Duration::from_millis(1100)); +} + +fn assert_all_exist(paths: &[PathBuf]) { + assert!(!paths.is_empty(), "nothing is watched"); + for p in paths { + assert!( + p.exists(), + "watch set contains a missing path: {}", + p.display() + ); + } +} + +#[test] +fn a_commit_on_a_branch_changes_a_watched_path() { + let (_tmp, dir) = repo(); + let paths = watch_paths_for(&dir); + assert_all_exist(&paths); + let before = newest_of(&paths); + + tick(); + git(&dir, &["commit", "-q", "--allow-empty", "-m", "two"]); + + assert!( + newest_of(&paths) > before, + "a commit changed no watched path" + ); +} + +/// The case the first version of the fix missed: with every ref packed and no +/// `refs/heads` directory, the next commit creates `refs/heads/main` and leaves +/// both `HEAD` and `packed-refs` untouched. +#[test] +fn with_refs_packed_and_no_refs_heads_the_next_commit_is_still_seen() { + let (_tmp, dir) = repo(); + git(&dir, &["pack-refs", "--all", "--prune"]); + let heads = dir.join(".git").join("refs").join("heads"); + if heads.exists() { + // pack-refs empties it; git recreates it on the next commit. + fs::remove_dir(&heads).unwrap(); + } + assert!(!heads.exists()); + + let paths = watch_paths_for(&dir); + assert_all_exist(&paths); + assert!( + !paths.contains(&heads), + "the missing refs/heads must not be emitted" + ); + let head = dir.join(".git").join("HEAD"); + let packed = dir.join(".git").join("packed-refs"); + let head_before = newest_mtime(&head); + let packed_before = newest_mtime(&packed); + let before = newest_of(&paths); + + tick(); + git(&dir, &["commit", "-q", "--allow-empty", "-m", "two"]); + + assert!(heads.join("main").exists(), "git recreated the loose ref"); + // This is what made the old watch set (HEAD, packed-refs) miss the commit. + assert_eq!(newest_mtime(&head), head_before, "HEAD was not rewritten"); + assert_eq!( + newest_mtime(&packed), + packed_before, + "packed-refs was not rewritten" + ); + assert!( + newest_of(&paths) > before, + "the commit changed no watched path, so ANT_GIT_COMMIT would go stale" + ); +} + +#[test] +fn in_a_worktree_every_watched_path_exists_and_a_commit_is_seen() { + let (tmp, dir) = repo(); + let wt = tmp.path().join("wt"); + git( + &dir, + &[ + "worktree", + "add", + "-q", + "-b", + "feature", + wt.to_str().unwrap(), + ], + ); + assert!(wt.join(".git").is_file(), "a worktree's .git is a file"); + + let paths = watch_paths_for(&wt); + assert_all_exist(&paths); + let before = newest_of(&paths); + + tick(); + git(&wt, &["commit", "-q", "--allow-empty", "-m", "two"]); + + assert!( + newest_of(&paths) > before, + "a worktree commit changed no watched path" + ); +}