From b67c70fa2f45b93af15e04cff3995219591eeda9 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Tue, 1 Sep 2026 11:11:07 -0400 Subject: [PATCH] A component can name the files its build reads elsewhere Freshness compared the commits under a component's own cwd, which is right about where its files are and wrong about what its build reads. tdep-survey's two clients each source one shared scripts/android-sdk-env.sh: a fix committed there changed what every build does, moved no component's subtree head, rebuilt nothing, and left every card correctly reporting "current" while answering a narrower question than the reader was asking. alsoWatch names the rest. The paths join the component's own in the same subtree-head and dirty comparisons -- git takes several pathspecs, so it stays one call each -- and it is part of the acceptance gate like any other declared field. Declared rather than inferred: which files a build reads is not knowable from here, and both wrong guesses are expensive. Raised by the tdep-survey session, which traced why its dioxus client could never become stale on the serving host. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 15 ++++++++ README.md | 30 +++++++++++++++ server/src/build_state.rs | 81 ++++++++++++++++++++++++++++++++++++--- server/src/config.rs | 42 ++++++++++++++++++++ server/src/git.rs | 54 +++++++++++++++++++------- server/src/registry.rs | 4 ++ server/src/service.rs | 1 + 7 files changed, 209 insertions(+), 18 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3e022d0..1dcae1a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -240,6 +240,21 @@ mutable at runtime from the phone. somebody asked. Stop and Uninstall do go through the script, and do strand the phone; the app confirms them rather than hiding them. +- **A component's freshness covers its own directory plus whatever it + declared with `alsoWatch`** (`Component::watched_paths`, handed to + `git::subtree_head` and `subtree_dirty` as pathspecs -- git takes + several, so it stays one call). Scoping to `cwd` is right about where a + component's files are and wrong about what its build reads: tdep-survey's + two clients each source one shared `scripts/android-sdk-env.sh`, so a fix + committed there changed what every build does, moved no component's + subtree head, rebuilt nothing, and left every card correctly reporting + "current" -- the narrower-question shape, where nothing is wrong and + nothing says so. Declared rather than inferred because which files a + build reads is not knowable from here, and both wrong guesses are + expensive: too wide rebuilds everything on every commit, too narrow is + that silence. It is part of the acceptance gate like every other + declared field -- `same_declaration` destructures exhaustively, which is + what forced the decision when the field was added. - **A stripped copy is only serveable if it is signed by the key the build was.** `strip.rs` re-signs with `~/.android/debug.keystore`, and nothing about a debug keystore says which builds it made -- it is per diff --git a/README.md b/README.md index 620ee20..31899fe 100644 --- a/README.md +++ b/README.md @@ -261,6 +261,36 @@ either against source, which is what makes it cheap and needs no knowledge of what the build reads. The consequence is that it only means anything once `olderThan` has itself been built at least once. +### Files a build reads from somewhere else + +A component is considered out of date when the commits under its own +directory move past the ones it was built from — `cwd`, or the project +root for a component that names none. That is right about where a +component's *files* are and wrong about what its build *reads*: a project +whose clients each source one shared script, which locates their SDK and +their signing key, has a file that changes what every build does and moves +no component's directory. A commit to it rebuilds nothing, and every card +correctly reports "current" while answering a narrower question than you +were asking. + +A component can name the rest of what it reads: + +```ron +Apk( + name: "app", + cwd: "app-dioxus", + build: "./build-android-arm.sh", + alsoWatch: ["scripts"], +), +``` + +Those paths join its own in the same comparison — a commit touching any of +them is a commit to this component, and an uncommitted change in one makes +its freshness unknown, exactly as one in its own directory does. Declared +rather than worked out, because which files a build reads is not something +this server can know: guess too wide and every commit rebuilds everything, +too narrow and you are back to the silence above. + Paths are relative to `projectPath` (absolute ones are also accepted), and `build` is argv, run without a shell — written either as a line you would type or as explicit arguments; the string form splits on whitespace and diff --git a/server/src/build_state.rs b/server/src/build_state.rs index 7e59c6a..ca4b4cb 100644 --- a/server/src/build_state.rs +++ b/server/src/build_state.rs @@ -556,10 +556,11 @@ impl BuildState { let Some(built) = built else { return Freshness::Unknown; }; - if crate::git::subtree_dirty(&self.project_path, component.cwd()) != Some(false) { + if crate::git::subtree_dirty(&self.project_path, &component.watched_paths()) != Some(false) + { return Freshness::Unknown; } - match crate::git::subtree_head(&self.project_path, component.cwd()) { + match crate::git::subtree_head(&self.project_path, &component.watched_paths()) { Some(current) if current == built => Freshness::Current, Some(_) => Freshness::Behind, None => Freshness::Unknown, @@ -611,7 +612,7 @@ impl BuildState { // tell; neither is evidence of staleness, and announcing one would // make every project built before this existed demand a rebuild. if let Some(built) = built_from.get(component.name()) - && crate::git::subtree_head(&self.project_path, component.cwd()) + && crate::git::subtree_head(&self.project_path, &component.watched_paths()) .is_some_and(|current| ¤t != built) { return true; @@ -850,7 +851,8 @@ impl BuildState { // // A checkout that cannot be read records nothing, which reads // as unknown rather than as current -- see `component_is_stale`. - if let Some(sha) = crate::git::subtree_head(&self.project_path, component.cwd()) { + if let Some(sha) = crate::git::subtree_head(&self.project_path, &component.watched_paths()) + { self.inner .lock() .unwrap() @@ -1324,6 +1326,7 @@ mod tests { build: crate::config::Command::from_line("true"), cwd: Some(PathBuf::from("server")), stale_when: None, + also_watch: Vec::new(), service: None, built_from: None, }, @@ -1332,6 +1335,7 @@ mod tests { build: crate::config::Command::from_line("true"), cwd: Some(PathBuf::from("app")), stale_when: None, + also_watch: Vec::new(), strip: false, package: None, built_from: None, @@ -1388,6 +1392,7 @@ mod tests { build: crate::config::Command::from_line("touch built-marker"), cwd: None, stale_when: None, + also_watch: Vec::new(), strip: false, package: None, built_from: None, @@ -1441,6 +1446,7 @@ mod tests { build: crate::config::Command::from_line("touch backend-built"), cwd: Some(PathBuf::from("server")), stale_when: None, + also_watch: Vec::new(), service: None, built_from: None, }, @@ -1449,6 +1455,7 @@ mod tests { build: crate::config::Command::from_line("touch app-built"), cwd: Some(PathBuf::from("app")), stale_when: None, + also_watch: Vec::new(), strip: false, package: None, built_from: None, @@ -1522,6 +1529,7 @@ mod tests { }), cwd: Some(PathBuf::from(name)), stale_when: None, + also_watch: Vec::new(), strip: false, package: None, built_from: None, @@ -1592,6 +1600,7 @@ mod tests { }), cwd: Some(PathBuf::from(name)), stale_when: None, + also_watch: Vec::new(), strip: false, package: None, built_from: None, @@ -1661,6 +1670,7 @@ mod tests { build: crate::config::Command::from_line("touch a-built"), cwd: Some(PathBuf::from("a")), stale_when: None, + also_watch: Vec::new(), strip: false, package: None, built_from: None, @@ -1670,6 +1680,7 @@ mod tests { build: crate::config::Command::from_line("touch b-built"), cwd: Some(PathBuf::from("b")), stale_when: None, + also_watch: Vec::new(), strip: false, package: None, built_from: None, @@ -1767,7 +1778,7 @@ mod tests { // Built now, from what is checked out now. for name in ["backend", "app"] { let cwd = PathBuf::from(if name == "backend" { "server" } else { "app" }); - let sha = crate::git::subtree_head(root, Some(&cwd)).expect("a commit"); + let sha = crate::git::subtree_head(root, std::slice::from_ref(&cwd)).expect("a commit"); state .inner .lock() @@ -1789,6 +1800,65 @@ mod tests { ); } + /// The other half of that scoping: a component's build reads more + /// than its own directory, and what it names with `alsoWatch` counts + /// as its own. + /// + /// The case this is for had every piece defensible and the whole + /// silent: two clients sourcing one shared script that locates their + /// SDK and mints their signing key, a fix committed to that script, + /// and no component's subtree head moved -- so nothing rebuilt, and + /// every card correctly reported "current" while answering a + /// narrower question than the reader was asking. The component that + /// says nothing must still be left alone, which is the second + /// assertion here. + #[test] + fn a_commit_to_a_watched_path_makes_only_the_components_that_watch_it_stale() { + let dir = tempfile::tempdir().expect("tempdir"); + let root = dir.path(); + let mut components = two_component_checkout(root); + let Component::Apk { also_watch, .. } = &mut components[1] else { + panic!("the app is the second component"); + }; + *also_watch = vec![PathBuf::from("scripts")]; + let state = state_for(root, components); + + std::fs::create_dir_all(root.join("scripts")).expect("mkdir"); + std::fs::write(root.join("scripts/env.sh"), "one").expect("write"); + run(root, &["git", "add", "-A"]); + run(root, &["git", "commit", "-qm", "scripts"]); + + for name in ["backend", "app"] { + let component = state + .components + .iter() + .find(|component| component.name() == name) + .expect("component"); + let sha = crate::git::subtree_head(root, &component.watched_paths()).expect("a commit"); + state + .inner + .lock() + .unwrap() + .built_from + .insert(name.to_string(), sha); + } + assert!(!stale(&state, "backend"), "just built"); + assert!(!stale(&state, "app"), "just built"); + + // A change to neither component's own directory. + std::fs::write(root.join("scripts/env.sh"), "two").expect("write"); + run(root, &["git", "commit", "-qam", "shared script"]); + + assert!( + stale(&state, "app"), + "the app watches scripts/, so a commit there is a commit to it", + ); + assert!( + !stale(&state, "backend"), + "the server said nothing about scripts/, so nothing about it changed", + ); + } + /// Nothing recorded is "we have never built this", not "this is out of /// date" -- otherwise every project that existed before this feature /// would demand a rebuild the moment it arrived. @@ -1822,6 +1892,7 @@ mod tests { build: crate::config::Command::from_line("true"), cwd: None, stale_when: None, + also_watch: Vec::new(), strip: false, package: None, built_from: Some("0000000000000000000000000000000000000000".to_string()), diff --git a/server/src/config.rs b/server/src/config.rs index f7744be..35e9d74 100644 --- a/server/src/config.rs +++ b/server/src/config.rs @@ -237,6 +237,8 @@ pub enum Component { cwd: Option, #[serde(default, skip_serializing_if = "Option::is_none")] stale_when: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + also_watch: Vec, /// Serve a debug-symbol-stripped copy (see `crate::strip`). /// /// Declared rather than detected: whether the symbols are worth the @@ -279,6 +281,8 @@ pub enum Component { cwd: Option, #[serde(default, skip_serializing_if = "Option::is_none")] stale_when: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + also_watch: Vec, /// How this server's service is driven: with the project's own /// script, or by dev-updater's built-in one. See [`Service`]. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -345,6 +349,38 @@ impl Component { } } + /// Every path in the checkout whose commits count as this + /// component's: its own directory, plus anything it declared with + /// `alsoWatch`. Relative to the project root, which is what git wants + /// as a pathspec. + /// + /// The list exists because scoping a component to its `cwd` alone is + /// right about where its *files* are and wrong about what its build + /// *reads*. A project whose clients each source one shared script -- + /// which locates their SDK, their JDK, their signing key -- has a file + /// that changes what every build does and moves no component's + /// subtree head, so a commit to it left every card correctly + /// reporting "current" while answering a narrower question than the + /// reader was asking. Nothing rebuilt, and nothing said why. + /// + /// Declared rather than inferred: which files a build actually reads + /// is not something this server can know, and a guess here is the + /// expensive kind -- too wide rebuilds everything on every commit, too + /// narrow is the silence above. + pub fn watched_paths(&self) -> Vec { + let also_watch = match self { + Self::Apk { also_watch, .. } | Self::Server { also_watch, .. } => also_watch, + }; + // The component's own directory first, so a caller reading the + // list sees the same thing `dir` would answer. `.` for a + // component at the root, which is the pathspec for the whole + // checkout and what a component declaring no `cwd` has always + // meant. + std::iter::once(self.cwd().unwrap_or(Path::new(".")).to_path_buf()) + .chain(also_watch.iter().cloned()) + .collect() + } + /// Whether these two say the same thing about what to *do*, ignoring /// what this server measured for itself. /// @@ -365,6 +401,7 @@ impl Component { build, cwd, stale_when, + also_watch, strip, package: _, built_from: _, @@ -374,6 +411,7 @@ impl Component { build: other_build, cwd: other_cwd, stale_when: other_stale_when, + also_watch: other_also_watch, strip: other_strip, package: _, built_from: _, @@ -383,6 +421,7 @@ impl Component { && build == other_build && cwd == other_cwd && stale_when == other_stale_when + && also_watch == other_also_watch && strip == other_strip } ( @@ -391,6 +430,7 @@ impl Component { build, cwd, stale_when, + also_watch, service, built_from: _, }, @@ -399,6 +439,7 @@ impl Component { build: other_build, cwd: other_cwd, stale_when: other_stale_when, + also_watch: other_also_watch, service: other_service, built_from: _, }, @@ -407,6 +448,7 @@ impl Component { && build == other_build && cwd == other_cwd && stale_when == other_stale_when + && also_watch == other_also_watch && service == other_service } _ => false, diff --git a/server/src/git.rs b/server/src/git.rs index 58fe7c5..320c714 100644 --- a/server/src/git.rs +++ b/server/src/git.rs @@ -79,15 +79,35 @@ pub fn status(project: &Path) -> Option { /// submodule case correctly and without a special case: a submodule whose /// working tree has moved shows as a modified path, so whichever component /// contains it goes unknown and the others do not. -pub fn subtree_dirty(project: &Path, within: Option<&Path>) -> Option { - let mut args = vec!["status", "--porcelain", "--"]; - let within = within.map(|path| path.to_string_lossy().into_owned()); - args.push(within.as_deref().unwrap_or(".")); +pub fn subtree_dirty(project: &Path, within: &[PathBuf]) -> Option { + let mut args = vec![ + "status".to_string(), + "--porcelain".to_string(), + "--".to_string(), + ]; + args.extend(pathspecs(within)); + let args: Vec<&str> = args.iter().map(String::as_str).collect(); git(project, &args).ok().map(|out| !out.trim().is_empty()) } -/// The newest commit touching `within` (a path relative to `project`), or -/// the whole checkout when `within` is `None`. +/// The paths to hand git, which is every one it was given or `.` for a +/// caller that named none. One function because the empty case has to +/// mean the same thing to both of the reads above and below -- an empty +/// pathspec list is git's "everything", and getting that wrong in one of +/// them would make a component's dirtiness and its commit disagree about +/// which files they are talking about. +fn pathspecs(within: &[PathBuf]) -> Vec { + if within.is_empty() { + return vec![".".to_string()]; + } + within + .iter() + .map(|path| path.to_string_lossy().into_owned()) + .collect() +} + +/// The newest commit touching any of `within` (paths relative to +/// `project`), or the whole checkout when the list is empty. /// /// This is what a build is recorded against, and the scoping is the point. /// One checkout routinely produces several things -- a backend under @@ -97,15 +117,23 @@ pub fn subtree_dirty(project: &Path, within: Option<&Path>) -> Option { /// touched the component's own directory makes "out of date" mean what a /// person reading the card expects it to. /// +/// Several paths rather than one because a component's build reads more +/// than its own directory: `alsoWatch` names the shared script both of a +/// project's clients source, and a commit to it has to count as a commit +/// to each of them. Git takes the pathspecs directly, so "the newest +/// commit touching any of these" costs the same one call. +/// /// `None` when the checkout cannot be read, which is "we cannot tell" /// rather than an answer; see `BuildState::freshness`. -pub fn subtree_head(project: &Path, within: Option<&Path>) -> Option { - let mut args = vec!["log", "-1", "--format=%H"]; - let within = within.map(|path| path.to_string_lossy().into_owned()); - if let Some(path) = within.as_deref() { - args.push("--"); - args.push(path); - } +pub fn subtree_head(project: &Path, within: &[PathBuf]) -> Option { + let mut args = vec![ + "log".to_string(), + "-1".to_string(), + "--format=%H".to_string(), + "--".to_string(), + ]; + args.extend(pathspecs(within)); + let args: Vec<&str> = args.iter().map(String::as_str).collect(); let sha = git(project, &args).ok()?; (!sha.is_empty()).then_some(sha) } diff --git a/server/src/registry.rs b/server/src/registry.rs index d8f2e98..d0f5bf5 100644 --- a/server/src/registry.rs +++ b/server/src/registry.rs @@ -574,6 +574,7 @@ impl AppState { build: crate::config::Command::default(), cwd: None, stale_when: None, + also_watch: Vec::new(), strip: false, package, built_from: None, @@ -744,6 +745,7 @@ fn reconcile_self(config: &mut Config, self_project: &Path) -> bool { build: crate::config::Command::default(), cwd: None, stale_when: None, + also_watch: Vec::new(), strip: false, package: None, built_from: None, @@ -914,6 +916,7 @@ mod tests { build: crate::config::Command::from_line(command), cwd: None, stale_when: None, + also_watch: Vec::new(), strip: false, package: None, built_from: None, @@ -994,6 +997,7 @@ mod tests { build: crate::config::Command::default(), cwd: cwd.map(PathBuf::from), stale_when: None, + also_watch: Vec::new(), strip: false, package: None, built_from: None, diff --git a/server/src/service.rs b/server/src/service.rs index f4fedc7..20540c3 100644 --- a/server/src/service.rs +++ b/server/src/service.rs @@ -398,6 +398,7 @@ mod tests { build: Command::default(), cwd: None, stale_when: None, + also_watch: Vec::new(), service, built_from: None, }