max / makenotwork
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
3 files changed,
+191 insertions,
-25 deletions
| @@ -17,18 +17,7 @@ | |||
| 17 | 17 | /// without `.git`), which the health body maps to `null` rather than to a | |
| 18 | 18 | /// misleading empty string. | |
| 19 | 19 | fn stamp_git_hash() { | |
| 20 | - | let hash = Command::new("git") | |
| 21 | - | .args(["rev-parse", "--short", "HEAD"]) | |
| 22 | - | .output() | |
| 23 | - | .ok() | |
| 24 | - | .filter(|o| o.status.success()) | |
| 25 | - | .and_then(|o| String::from_utf8(o.stdout).ok()) | |
| 26 | - | .map(|s| s.trim().to_string()) | |
| 27 | - | .unwrap_or_default(); | |
| 28 | - | ||
| 29 | - | println!("cargo::rustc-env=GIT_HASH={hash}"); | |
| 30 | - | // Only re-run when HEAD moves. | |
| 31 | - | println!("cargo::rerun-if-changed=.git/HEAD"); | |
| 20 | + | println!("cargo::rustc-env=GIT_HASH={}", git_hash()); | |
| 32 | 21 | } | |
| 33 | 22 | ||
| 34 | 23 | /// Content-hash the served static files and emit the template partials that | |
| @@ -197,3 +186,85 @@ | |||
| 197 | 186 | ), | |
| 198 | 187 | } | |
| 199 | 188 | } | |
| 189 | + | ||
| 190 | + | /// The short commit sha to stamp into `GIT_HASH`, and the watches that decide | |
| 191 | + | /// when this build script has to run again. | |
| 192 | + | /// | |
| 193 | + | /// The watches are the whole point. Cargo treats a `rerun-if-changed` path that | |
| 194 | + | /// does not exist as *changed*, so a watch on a path that can never exist makes | |
| 195 | + | /// this script re-run on every single cargo invocation, and re-running it | |
| 196 | + | /// recompiles the crate. This file used to watch `.git/HEAD`, which resolves | |
| 197 | + | /// against the package root: there is no `.git` in `server/` or in | |
| 198 | + | /// `multithreaded/` because the repository root is `MNW/`. So the watch never | |
| 199 | + | /// resolved, and the crate recompiled every time. | |
| 200 | + | /// | |
| 201 | + | /// It cost 347s per Sando pipeline (294s in the server, 53s in multithreaded, | |
| 202 | + | /// measured off `/srv/sando/logs/0.11.20/cargo_test.log`), which is 35% of the | |
| 203 | + | /// `cargo_test` gate, and it cost the same on every local `cargo build`, | |
| 204 | + | /// `cargo test` and `cargo clippy`. The two crates carrying a `build.rs` were | |
| 205 | + | /// the only two in the pipeline that recompiled on a second cargo invocation; | |
| 206 | + | /// the ones without one were already free. | |
| 207 | + | /// | |
| 208 | + | /// Two rules follow, and both matter: | |
| 209 | + | /// - resolve the paths through git rather than guessing them, and | |
| 210 | + | /// - emit a watch ONLY for a path that exists, or the bug comes straight back. | |
| 211 | + | fn git_hash() -> String { | |
| 212 | + | // An explicit hash wins and skips git entirely, for a build system that | |
| 213 | + | // already knows the sha. Nothing sets this today: Sando would have to set it | |
| 214 | + | // on EVERY cargo invocation it makes, because `GIT_HASH` is a `rustc-env` | |
| 215 | + | // and therefore part of the crate fingerprint, so a value present for the | |
| 216 | + | // release build and absent for `cargo_test` would force the recompile this | |
| 217 | + | // function exists to remove. | |
| 218 | + | println!("cargo::rerun-if-env-changed=MNW_GIT_HASH"); | |
| 219 | + | if let Ok(h) = std::env::var("MNW_GIT_HASH") { | |
| 220 | + | let h = h.trim().to_string(); | |
| 221 | + | if !h.is_empty() { | |
| 222 | + | return h; | |
| 223 | + | } | |
| 224 | + | } | |
| 225 | + | ||
| 226 | + | // Watch what git actually rewrites when the checkout moves. `.git/HEAD` | |
| 227 | + | // alone is not enough even when resolved: committing on a branch rewrites | |
| 228 | + | // that branch's ref, not HEAD, so a watch on HEAD by itself would go stale | |
| 229 | + | // in the ordinary case of a commit. | |
| 230 | + | watch_if_exists(git_path("HEAD").as_deref()); | |
| 231 | + | if let Some(r) = git_output(&["symbolic-ref", "-q", "HEAD"]) { | |
| 232 | + | watch_if_exists(git_path(&r).as_deref()); | |
| 233 | + | } | |
| 234 | + | // A ref that has been packed has no loose file, so this is the fallback | |
| 235 | + | // that keeps the watch honest after a `git gc`. | |
| 236 | + | watch_if_exists(git_path("packed-refs").as_deref()); | |
| 237 | + | ||
| 238 | + | git_output(&["rev-parse", "--short", "HEAD"]).unwrap_or_default() | |
| 239 | + | } | |
| 240 | + | ||
| 241 | + | /// Run a git command in the package directory and return its trimmed stdout. | |
| 242 | + | fn git_output(args: &[&str]) -> Option<String> { | |
| 243 | + | Command::new("git") | |
| 244 | + | .args(args) | |
| 245 | + | .output() | |
| 246 | + | .ok() | |
| 247 | + | .filter(|o| o.status.success()) | |
| 248 | + | .and_then(|o| String::from_utf8(o.stdout).ok()) | |
| 249 | + | .map(|s| s.trim().to_string()) | |
| 250 | + | .filter(|s| !s.is_empty()) | |
| 251 | + | } | |
| 252 | + | ||
| 253 | + | /// Resolve a name inside the git directory to a path, honouring worktrees and a | |
| 254 | + | /// `.git` file that points elsewhere. `None` when this is not a checkout at all, | |
| 255 | + | /// which is the case in a vendored or packaged build. | |
| 256 | + | fn git_path(name: &str) -> Option<String> { | |
| 257 | + | git_output(&["rev-parse", "--git-path", name]) | |
| 258 | + | } | |
| 259 | + | ||
| 260 | + | /// Emit a watch for a path, but only when it exists. | |
| 261 | + | /// | |
| 262 | + | /// The guard is the fix. A missing path reads as changed to cargo, so emitting | |
| 263 | + | /// one unconditionally is what caused the recompile-every-time bug. | |
| 264 | + | fn watch_if_exists(path: Option<&str>) { | |
| 265 | + | if let Some(p) = path | |
| 266 | + | && Path::new(p).exists() | |
| 267 | + | { | |
| 268 | + | println!("cargo::rerun-if-changed={p}"); | |
| 269 | + | } | |
| 270 | + | } |
| @@ -6,18 +6,7 @@ | |||
| 6 | 6 | ||
| 7 | 7 | fn main() { | |
| 8 | 8 | // Set GIT_HASH env var for compile-time inclusion via option_env!() | |
| 9 | - | let hash = Command::new("git") | |
| 10 | - | .args(["rev-parse", "--short", "HEAD"]) | |
| 11 | - | .output() | |
| 12 | - | .ok() | |
| 13 | - | .filter(|o| o.status.success()) | |
| 14 | - | .and_then(|o| String::from_utf8(o.stdout).ok()) | |
| 15 | - | .map(|s| s.trim().to_string()) | |
| 16 | - | .unwrap_or_default(); | |
| 17 | - | ||
| 18 | - | println!("cargo::rustc-env=GIT_HASH={hash}"); | |
| 19 | - | // Only re-run when HEAD changes | |
| 20 | - | println!("cargo::rerun-if-changed=.git/HEAD"); | |
| 9 | + | println!("cargo::rustc-env=GIT_HASH={}", git_hash()); | |
| 21 | 10 | ||
| 22 | 11 | // Compile the TypeScript frontend to static/dist/ (best-effort, see fn). | |
| 23 | 12 | build_frontend(); | |
| @@ -153,11 +142,24 @@ | |||
| 153 | 142 | "static/idiomorph-ext.min.js", | |
| 154 | 143 | "static/upload.js", | |
| 155 | 144 | "static/passkey.js", | |
| 156 | - | "static/insertions.js", | |
| 145 | + | // `static/insertions.js` was here until 2026-08-20. The file went away | |
| 146 | + | // in bd448cbe and the entry did not, so it was a watch on a path that | |
| 147 | + | // could not exist -- which cargo reads as changed, re-running this | |
| 148 | + | // script and recompiling the crate on every invocation. Same bug as the | |
| 149 | + | // old `.git/HEAD` watch; see `git_hash`. Anything added here must exist. | |
| 157 | 150 | ]; | |
| 158 | 151 | ||
| 159 | 152 | let mut hasher = DefaultHasher::new(); | |
| 160 | 153 | for path in &static_files { | |
| 154 | + | // Every path in the list above is expected to exist. Assert it rather | |
| 155 | + | // than watching a phantom: a deleted file that keeps its entry costs a | |
| 156 | + | // full recompile per cargo invocation and is invisible otherwise. | |
| 157 | + | assert!( | |
| 158 | + | Path::new(path).exists(), | |
| 159 | + | "build.rs watches {path}, which does not exist. Remove the entry, or \ | |
| 160 | + | restore the file: a missing watch path recompiles this crate on \ | |
| 161 | + | every cargo invocation.", | |
| 162 | + | ); | |
| 161 | 163 | println!("cargo::rerun-if-changed={path}"); | |
| 162 | 164 | if let Ok(content) = fs::read(path) { | |
| 163 | 165 | content.hash(&mut hasher); | |
| @@ -638,3 +640,85 @@ | |||
| 638 | 640 | ("tab", "cursor"), | |
| 639 | 641 | ("table-row", "display"), | |
| 640 | 642 | ]; | |
| 643 | + | ||
| 644 | + | /// The short commit sha to stamp into `GIT_HASH`, and the watches that decide | |
| 645 | + | /// when this build script has to run again. | |
| 646 | + | /// | |
| 647 | + | /// The watches are the whole point. Cargo treats a `rerun-if-changed` path that | |
| 648 | + | /// does not exist as *changed*, so a watch on a path that can never exist makes | |
| 649 | + | /// this script re-run on every single cargo invocation, and re-running it | |
| 650 | + | /// recompiles the crate. This file used to watch `.git/HEAD`, which resolves | |
| 651 | + | /// against the package root: there is no `.git` in `server/` or in | |
| 652 | + | /// `multithreaded/` because the repository root is `MNW/`. So the watch never | |
| 653 | + | /// resolved, and the crate recompiled every time. | |
| 654 | + | /// | |
| 655 | + | /// It cost 347s per Sando pipeline (294s in the server, 53s in multithreaded, | |
| 656 | + | /// measured off `/srv/sando/logs/0.11.20/cargo_test.log`), which is 35% of the | |
| 657 | + | /// `cargo_test` gate, and it cost the same on every local `cargo build`, | |
| 658 | + | /// `cargo test` and `cargo clippy`. The two crates carrying a `build.rs` were | |
| 659 | + | /// the only two in the pipeline that recompiled on a second cargo invocation; | |
| 660 | + | /// the ones without one were already free. | |
| 661 | + | /// | |
| 662 | + | /// Two rules follow, and both matter: | |
| 663 | + | /// - resolve the paths through git rather than guessing them, and | |
| 664 | + | /// - emit a watch ONLY for a path that exists, or the bug comes straight back. | |
| 665 | + | fn git_hash() -> String { | |
| 666 | + | // An explicit hash wins and skips git entirely, for a build system that | |
| 667 | + | // already knows the sha. Nothing sets this today: Sando would have to set it | |
| 668 | + | // on EVERY cargo invocation it makes, because `GIT_HASH` is a `rustc-env` | |
| 669 | + | // and therefore part of the crate fingerprint, so a value present for the | |
| 670 | + | // release build and absent for `cargo_test` would force the recompile this | |
| 671 | + | // function exists to remove. | |
| 672 | + | println!("cargo::rerun-if-env-changed=MNW_GIT_HASH"); | |
| 673 | + | if let Ok(h) = std::env::var("MNW_GIT_HASH") { | |
| 674 | + | let h = h.trim().to_string(); | |
| 675 | + | if !h.is_empty() { | |
| 676 | + | return h; | |
| 677 | + | } | |
| 678 | + | } | |
| 679 | + | ||
| 680 | + | // Watch what git actually rewrites when the checkout moves. `.git/HEAD` | |
| 681 | + | // alone is not enough even when resolved: committing on a branch rewrites | |
| 682 | + | // that branch's ref, not HEAD, so a watch on HEAD by itself would go stale | |
| 683 | + | // in the ordinary case of a commit. | |
| 684 | + | watch_if_exists(git_path("HEAD").as_deref()); | |
| 685 | + | if let Some(r) = git_output(&["symbolic-ref", "-q", "HEAD"]) { | |
| 686 | + | watch_if_exists(git_path(&r).as_deref()); | |
| 687 | + | } | |
| 688 | + | // A ref that has been packed has no loose file, so this is the fallback | |
| 689 | + | // that keeps the watch honest after a `git gc`. | |
| 690 | + | watch_if_exists(git_path("packed-refs").as_deref()); | |
| 691 | + | ||
| 692 | + | git_output(&["rev-parse", "--short", "HEAD"]).unwrap_or_default() | |
| 693 | + | } | |
| 694 | + | ||
| 695 | + | /// Run a git command in the package directory and return its trimmed stdout. | |
| 696 | + | fn git_output(args: &[&str]) -> Option<String> { | |
| 697 | + | Command::new("git") | |
| 698 | + | .args(args) | |
| 699 | + | .output() | |
| 700 | + | .ok() | |
| 701 | + | .filter(|o| o.status.success()) | |
| 702 | + | .and_then(|o| String::from_utf8(o.stdout).ok()) | |
| 703 | + | .map(|s| s.trim().to_string()) | |
| 704 | + | .filter(|s| !s.is_empty()) | |
| 705 | + | } | |
| 706 | + | ||
| 707 | + | /// Resolve a name inside the git directory to a path, honouring worktrees and a | |
| 708 | + | /// `.git` file that points elsewhere. `None` when this is not a checkout at all, | |
| 709 | + | /// which is the case in a vendored or packaged build. | |
| 710 | + | fn git_path(name: &str) -> Option<String> { | |
| 711 | + | git_output(&["rev-parse", "--git-path", name]) | |
| 712 | + | } | |
| 713 | + | ||
| 714 | + | /// Emit a watch for a path, but only when it exists. | |
| 715 | + | /// | |
| 716 | + | /// The guard is the fix. A missing path reads as changed to cargo, so emitting | |
| 717 | + | /// one unconditionally is what caused the recompile-every-time bug. | |
| 718 | + | fn watch_if_exists(path: Option<&str>) { | |
| 719 | + | if let Some(p) = path | |
| 720 | + | && Path::new(p).exists() | |
| 721 | + | { | |
| 722 | + | println!("cargo::rerun-if-changed={p}"); | |
| 723 | + | } | |
| 724 | + | } |
| @@ -382,6 +382,17 @@ | |||
| 382 | 382 | ||
| 383 | 383 | // Full run: the test binaries are already built above, so cargo's | |
| 384 | 384 | // up-to-date check skips compilation and this just runs the tests. | |
| 385 | + | // | |
| 386 | + | // That claim is only true when the crate's build script is up to date | |
| 387 | + | // too, and for a long time it was not. server and multithreaded both | |
| 388 | + | // watched `.git/HEAD`, a path that does not exist at either package | |
| 389 | + | // root, and cargo reads a missing watch as changed: the build script | |
| 390 | + | // re-ran and the crate recompiled here, every time. It cost 347s a | |
| 391 | + | // pipeline, 35% of this gate, while this comment said it cost nothing. | |
| 392 | + | // Fixed 2026-08-20 in both build scripts; see `git_hash` in either. | |
| 393 | + | // | |
| 394 | + | // If this gate's duration ever climbs back toward the pre-pass's, look | |
| 395 | + | // for a new phantom watch before looking anywhere else. | |
| 385 | 396 | let mut child = match cargo_test_command(ctx, &dir, target, &features, &[]).spawn() { | |
| 386 | 397 | Ok(c) => c, | |
| 387 | 398 | Err(e) => { |