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 <noreply@anthropic.com>
This commit is contained in:
1 parent
f00c3b970f
commit
b67c70fa2f
7 files changed
+209
-18
No files matched your search
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()),
|
||||
|
||||
@@ -237,6 +237,8 @@ pub enum Component {
|
||||
cwd: Option<PathBuf>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
stale_when: Option<StaleRule>,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
also_watch: Vec<PathBuf>,
|
||||
/// 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<PathBuf>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
stale_when: Option<StaleRule>,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
also_watch: Vec<PathBuf>,
|
||||
/// 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<PathBuf> {
|
||||
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,
|
||||
|
||||
+41
-13
@@ -79,15 +79,35 @@ pub fn status(project: &Path) -> Option<GitStatus> {
|
||||
/// 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<bool> {
|
||||
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<bool> {
|
||||
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<String> {
|
||||
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<bool> {
|
||||
/// 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<String> {
|
||||
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<String> {
|
||||
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)
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -398,6 +398,7 @@ mod tests {
|
||||
build: Command::default(),
|
||||
cwd: None,
|
||||
stale_when: None,
|
||||
also_watch: Vec::new(),
|
||||
service,
|
||||
built_from: None,
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user