fix(build): stop watching .git/index for the build-version rerun trigger (#3432)
Drops `.git/index` from the build-version rerun watch: git rewrites its stat cache on a plain `git status`, so background git activity forced a full lib plus binary recompile on builds with no source change. Adds `logs/HEAD` alongside `HEAD` so the embedded `AOE_BUILD_VERSION` still refreshes on commit, pull, merge, rebase, and reset, which `HEAD` alone misses because it holds `ref: refs/heads/<branch>` and is not rewritten when the branch advances. Without it the #1754 respawn gate would match a worker still running the previous binary. Includes a test pinning both halves of the trigger contract: a commit must disturb a watched path, a plain `git status` must not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -35,9 +35,12 @@ fn emit_build_version() {
|
||||
use std::process::Command;
|
||||
|
||||
// Re-run when the committed revision changes or an override toggles.
|
||||
// HEAD moves on checkout/commit; index moves on stage. Resolve the real
|
||||
// paths via `git rev-parse --git-path` rather than hardcoding `.git/HEAD`:
|
||||
// in a git worktree `.git` is a file pointing at
|
||||
// HEAD moves on checkout; logs/HEAD moves on every commit, pull, merge,
|
||||
// rebase, or reset. `index` is not watched: git rewrites it on a plain
|
||||
// `git status`, which forced a full recompile on builds with no source
|
||||
// change (see `git_watch_paths`'s doc comment). Resolve the real paths
|
||||
// via `git rev-parse --git-path` rather than hardcoding
|
||||
// `.git/HEAD`: in a git worktree `.git` is a file pointing at
|
||||
// `<main>/.git/worktrees/<name>/`, so the literal `.git/HEAD` path does not
|
||||
// exist. Cargo treats a missing `rerun-if-changed` input as perpetually
|
||||
// stale, which reran this script (and recompiled the lib + binary that read
|
||||
|
||||
+16
-3
@@ -3,8 +3,21 @@
|
||||
// dependencies on the rest of the crate: build scripts compile in isolation.
|
||||
|
||||
/// The git files cargo should watch so `AOE_BUILD_VERSION` is recomputed when
|
||||
/// the checkout's revision or staged state changes: `HEAD` moves on
|
||||
/// checkout/commit, `index` moves on stage.
|
||||
/// the checkout's revision changes: `HEAD` for a checkout or a detached-HEAD
|
||||
/// move, and the per-worktree reflog `logs/HEAD` for everything that advances
|
||||
/// the branch `HEAD` points at (commit, pull, merge, rebase, reset), which
|
||||
/// leaves the `HEAD` file itself untouched.
|
||||
///
|
||||
/// `index` is deliberately not watched. It looks like the natural trigger for
|
||||
/// the dirty-flag suffix, but git rewrites its stat cache on a plain
|
||||
/// `git status` (racily-clean revalidation), not just on real staging, so a
|
||||
/// shell prompt or editor git integration running in the background made
|
||||
/// cargo recompile the lib + binary for a build with zero source changes.
|
||||
/// The cost is that the dirty suffix now lags: staging or editing files no
|
||||
/// longer refreshes it on its own, so a binary rebuilt after an edit can
|
||||
/// still report the flag computed at the last revision change. That was
|
||||
/// already true for unstaged edits before, and the flag is documented as
|
||||
/// coarse (see `emit_build_version` in `build.rs`).
|
||||
///
|
||||
/// Paths are resolved for the repository rooted at `dir` via
|
||||
/// `git rev-parse --git-path`, which is correct for both a normal checkout
|
||||
@@ -20,7 +33,7 @@
|
||||
/// checkout (e.g. a source tarball), leaving the build version pinned to
|
||||
/// `CARGO_PKG_VERSION` with no spurious rerun trigger.
|
||||
pub fn git_watch_paths(dir: &std::path::Path) -> Vec<String> {
|
||||
["HEAD", "index"]
|
||||
["HEAD", "logs/HEAD"]
|
||||
.iter()
|
||||
.filter_map(|file| git_path(dir, file))
|
||||
.filter(|path| watched_path_exists(dir, path))
|
||||
|
||||
@@ -1,17 +1,19 @@
|
||||
//! Regression test for issue #1962: inside a git worktree the build script
|
||||
//! must watch the *real* per-worktree `HEAD`/`index`, not the literal
|
||||
//! `.git/HEAD` (which does not exist there). A missing `rerun-if-changed`
|
||||
//! input makes cargo treat the build script as perpetually stale, recompiling
|
||||
//! the lib + binary on every build.
|
||||
//! must watch the *real* per-worktree `HEAD`, not the literal `.git/HEAD`
|
||||
//! (which does not exist there). A missing `rerun-if-changed` input makes
|
||||
//! cargo treat the build script as perpetually stale, recompiling the lib +
|
||||
//! binary on every build.
|
||||
//!
|
||||
//! The test drives the actual watch-path logic used by `build.rs`
|
||||
//! (`build_git_watch.rs`, shared via `include!`) against a temporary git
|
||||
//! worktree and asserts every watched path exists on disk.
|
||||
//! worktree and asserts every watched path exists on disk. It also pins the
|
||||
//! two halves of the trigger contract: a revision change must disturb a
|
||||
//! watched file, and a plain `git status` (which rewrites `index`) must not.
|
||||
|
||||
#[path = "../build_git_watch.rs"]
|
||||
mod build_git_watch;
|
||||
|
||||
use std::path::Path;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::process::Command;
|
||||
|
||||
fn git(dir: &Path, args: &[&str]) -> std::process::Output {
|
||||
@@ -88,10 +90,20 @@ fn watch_paths_resolve_in_a_git_worktree() {
|
||||
);
|
||||
|
||||
let paths = build_git_watch::git_watch_paths(&worktree);
|
||||
let wt_git_dir = String::from_utf8(git(&worktree, &["rev-parse", "--absolute-git-dir"]).stdout)
|
||||
.expect("utf8 git dir");
|
||||
let wt_git_dir = wt_git_dir.trim();
|
||||
for expected in ["HEAD", "logs/HEAD"] {
|
||||
let want = format!("{wt_git_dir}/{expected}");
|
||||
assert!(
|
||||
paths.iter().any(|p| p == &want),
|
||||
"expected the per-worktree {expected} in {paths:?}"
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
paths.len(),
|
||||
2,
|
||||
"expected HEAD and index watch paths, got {paths:?}"
|
||||
"expected only the per-worktree HEAD and logs/HEAD, got {paths:?}"
|
||||
);
|
||||
for watched in &paths {
|
||||
assert!(
|
||||
@@ -147,3 +159,79 @@ fn git_watch_paths_empty_when_git_cannot_resolve() {
|
||||
let no_repo = tmp.path().join("nonexistent");
|
||||
assert!(build_git_watch::git_watch_paths(&no_repo).is_empty());
|
||||
}
|
||||
|
||||
/// The watch set has to fire on a revision change and stay quiet through the
|
||||
/// `git status` churn that made watching `index` unusable. Assertions compare
|
||||
/// file *contents* rather than mtimes so the test does not depend on
|
||||
/// filesystem timestamp granularity; a rewrite is what moves the mtime cargo
|
||||
/// actually compares.
|
||||
#[test]
|
||||
fn watched_paths_move_on_a_commit_but_not_on_a_plain_status() {
|
||||
if !git_available() {
|
||||
eprintln!("skipping: git not available");
|
||||
return;
|
||||
}
|
||||
|
||||
let tmp = tempfile::tempdir().expect("tempdir");
|
||||
let repo = tmp.path().join("repo");
|
||||
std::fs::create_dir(&repo).expect("create repo dir");
|
||||
assert!(git(&repo, &["init", "-q"]).status.success());
|
||||
git(&repo, &["config", "user.email", "test@example.com"]);
|
||||
git(&repo, &["config", "user.name", "Test"]);
|
||||
std::fs::write(repo.join("tracked.txt"), "one\n").expect("write tracked file");
|
||||
assert!(git(&repo, &["add", "tracked.txt"]).status.success());
|
||||
assert!(git(&repo, &["commit", "-qm", "init"]).status.success());
|
||||
|
||||
let snapshot = |repo: &Path| -> Vec<(String, Vec<u8>)> {
|
||||
build_git_watch::git_watch_paths(repo)
|
||||
.into_iter()
|
||||
.map(|p| {
|
||||
let resolved = if Path::new(&p).is_absolute() {
|
||||
PathBuf::from(&p)
|
||||
} else {
|
||||
repo.join(&p)
|
||||
};
|
||||
let bytes = std::fs::read(&resolved).unwrap_or_else(|e| panic!("read {p}: {e}"));
|
||||
(p, bytes)
|
||||
})
|
||||
.collect()
|
||||
};
|
||||
let before = snapshot(&repo);
|
||||
assert!(!before.is_empty(), "expected at least one watched path");
|
||||
|
||||
// Stat-only churn plus a prompt-style `git status`: git revalidates the
|
||||
// racily-clean entry and rewrites `index`. Nothing cargo watches may move.
|
||||
let index = repo.join(".git/index");
|
||||
let index_before = std::fs::read(&index).expect("read index");
|
||||
std::fs::File::options()
|
||||
.write(true)
|
||||
.open(repo.join("tracked.txt"))
|
||||
.expect("open tracked file")
|
||||
.set_times(
|
||||
std::fs::FileTimes::new()
|
||||
.set_modified(std::time::UNIX_EPOCH + std::time::Duration::from_secs(1)),
|
||||
)
|
||||
.expect("backdate tracked file");
|
||||
assert!(git(&repo, &["status", "--porcelain"]).status.success());
|
||||
assert_ne!(
|
||||
index_before,
|
||||
std::fs::read(&index).expect("read index"),
|
||||
"expected `git status` to rewrite the index stat cache"
|
||||
);
|
||||
assert_eq!(
|
||||
before,
|
||||
snapshot(&repo),
|
||||
"a plain `git status` must not disturb a watched path"
|
||||
);
|
||||
|
||||
// A commit advances the branch ref; `HEAD` itself never changes, so the
|
||||
// reflog is what keeps the embedded version from going stale.
|
||||
assert!(git(&repo, &["commit", "-qm", "second", "--allow-empty"])
|
||||
.status
|
||||
.success());
|
||||
assert_ne!(
|
||||
before,
|
||||
snapshot(&repo),
|
||||
"a commit must disturb a watched path so the build version recomputes"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user