Pull, Update, Remove -- and a card that says when its build is behind
Iris's call: three buttons on a project's row, where Pull takes the
commits and stops and Update goes the whole way -- pull, build what that
brought in, install every APK it produced. Splitting them is the same
reasoning that stopped a checkout building: bringing a checkout up to
date is cheap and something you do while looking, and the expensive half
belongs to the button that says it does it.
Update is composed from the two things the app already does rather than
given a route of its own, so each half reports where it already did. It
skips a component whose build just failed, and hands the installer one
APK at a time: an install intent is modal, so two fired together means
the second replaces the first and a component is silently never
installed. The rest wait in ComponentState.ReadyToInstall, offered again
by continuePendingInstalls -- the same machinery as a download waiting
for a wrongly-signed copy to be removed. Deriving "is the installer
free?" from the component states could not work, because handing a file
over clears the state it would have read; caught by pressing Update on a
project that builds two APKs.
The project-wide Rebuild goes with it, and ProjectState collapses to one
Working(what, status): Pull, Update and moving the checkout differ in
nothing but the label, and the old one said "Pulling and building" for
all three.
Then the two things behind "nothing was flagged as out of date, and Pull
is disabled" after switching ai-app onto another branch:
- hasWorkWaiting is now one predicate for both the "Up to date" heading
and whether Update can be pressed, and it counts a component whose
build no longer matches the checkout. Without that a branch switch
left the card filed under "Up to date" with only a small note in one
component's row.
- A moved checkout re-asks its remote (RemoteChecks::recheck, on top of
a new Checks::forget). The cached answer was about the branch just
left and was reported with checkPending false, so a branch one commit
behind its upstream read as one with nothing to pull. Measured: git
said [behind 1] while the card said newCommits false.
A component's install button says Install rather than Build where nothing
is built yet, since the button beside it now says Build.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
9efe08e4ce
commit
af605211e2
7 files changed
+515
-162
No files matched your search
@@ -756,6 +756,18 @@ impl BuildState {
|
||||
);
|
||||
}
|
||||
|
||||
/// Takes the commits and stops there.
|
||||
///
|
||||
/// The Pull button, as against Update. Splitting them is what lets
|
||||
/// "bring the checkout up to date" be a cheap thing somebody can do
|
||||
/// while looking, with building and installing behind the button that
|
||||
/// says it does those -- and it is the same reasoning that stopped a
|
||||
/// checkout building. What it leaves behind is a component the card
|
||||
/// reports as out of date, with its own Build beside it.
|
||||
pub fn pull_only(self: &Arc<Self>, force: bool) {
|
||||
self.after_moving(move |this| this.pull(force), None);
|
||||
}
|
||||
|
||||
/// Moves this checkout onto `target` -- a branch or a commit -- and
|
||||
/// builds nothing.
|
||||
///
|
||||
@@ -781,6 +793,14 @@ impl BuildState {
|
||||
this.begin_project("checking out");
|
||||
crate::git::checkout(&this.project_path, &target)?;
|
||||
tracing::info!("checked out {target} in {}", this.project_path.display());
|
||||
// The new-commits answer was about the branch just left,
|
||||
// and nothing downstream can tell that from an answer
|
||||
// about this one -- so it is dropped and asked again here,
|
||||
// where what invalidated it happened.
|
||||
this.shared.remote_checks.recheck(&crate::git::Checkout {
|
||||
path: this.project_path.clone(),
|
||||
ipv4: this.git_ipv4,
|
||||
});
|
||||
Ok(true)
|
||||
},
|
||||
None,
|
||||
@@ -2109,6 +2129,70 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// Moving the checkout re-asks the remote, because the last answer was
|
||||
/// about the branch just left.
|
||||
///
|
||||
/// The answer is cached per project, and the only things that used to
|
||||
/// start a check were the whole-list refresh and a card's own Refresh
|
||||
/// -- so after a move the card reported the previous branch's answer
|
||||
/// with nothing marking it as stale. Measured symptom: a branch one
|
||||
/// commit behind its upstream, with Pull disabled and no way to find
|
||||
/// out but pressing Refresh.
|
||||
#[tokio::test]
|
||||
async fn moving_the_checkout_asks_the_remote_again() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let root = dir.path();
|
||||
let work = root.join("work");
|
||||
std::fs::create_dir_all(&work).expect("mkdir");
|
||||
let components = two_component_checkout(&work);
|
||||
let origin = root.join("origin.git");
|
||||
run(root, &["git", "init", "-q", "--bare", "origin.git"]);
|
||||
run(
|
||||
&work,
|
||||
&[
|
||||
"git",
|
||||
"remote",
|
||||
"add",
|
||||
"origin",
|
||||
&origin.display().to_string(),
|
||||
],
|
||||
);
|
||||
run(&work, &["git", "push", "-q", "-u", "origin", "main"]);
|
||||
// A branch whose upstream has one commit this checkout does not,
|
||||
// which is exactly what Pull is for.
|
||||
run(&work, &["git", "checkout", "-qb", "topic"]);
|
||||
std::fs::write(work.join("app/main.kt"), "two").expect("write");
|
||||
run(&work, &["git", "commit", "-qam", "two"]);
|
||||
run(&work, &["git", "push", "-q", "-u", "origin", "topic"]);
|
||||
std::fs::write(work.join("app/main.kt"), "three").expect("write");
|
||||
run(&work, &["git", "commit", "-qam", "three"]);
|
||||
run(&work, &["git", "push", "-q", "origin", "topic"]);
|
||||
run(&work, &["git", "reset", "--hard", "-q", "HEAD~1"]);
|
||||
run(&work, &["git", "checkout", "-q", "main"]);
|
||||
|
||||
let state = state_for(&work, components);
|
||||
// What a check on `main` leaves behind: nothing to pull, said as
|
||||
// settled fact.
|
||||
state.shared.remote_checks.mark_current(&work);
|
||||
assert!(!state.shared.remote_checks.new_commits(&work));
|
||||
|
||||
state.move_checkout("topic".to_string());
|
||||
|
||||
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
|
||||
while !state.shared.remote_checks.new_commits(&work) {
|
||||
assert!(
|
||||
std::time::Instant::now() < deadline,
|
||||
"the card still reports the branch that was left: no commits to pull, on a \
|
||||
branch that is one behind",
|
||||
);
|
||||
tokio::time::sleep(std::time::Duration::from_millis(20)).await;
|
||||
}
|
||||
assert_eq!(
|
||||
crate::git::status(&work).expect("a checkout").branch,
|
||||
"topic",
|
||||
);
|
||||
}
|
||||
|
||||
/// A component with nothing built is not "current with the checkout",
|
||||
/// however still the commits are.
|
||||
///
|
||||
|
||||
@@ -173,6 +173,29 @@ impl<K: Eq + Hash + Clone + Send + 'static, A: Clone + Send + 'static> Checks<K,
|
||||
entry.error = None;
|
||||
}
|
||||
|
||||
/// Drops what is known about one key, for a change that makes the
|
||||
/// last answer describe something else.
|
||||
///
|
||||
/// Moving a checkout onto another branch is the case: "the remote has
|
||||
/// commits this branch does not" was asked of the branch that has just
|
||||
/// been left, and a display cannot tell that from an answer about the
|
||||
/// branch it is now on. Forgetting it makes the card say it does not
|
||||
/// know, which is true, where keeping it makes it say something false
|
||||
/// with the same confidence as everything else on the card.
|
||||
///
|
||||
/// Leaves `in_flight` alone for the same reason [`Self::update`] does:
|
||||
/// a worker already running still owns the right to write its answer.
|
||||
pub fn forget<Q>(&self, key: &Q)
|
||||
where
|
||||
K: Borrow<Q>,
|
||||
Q: Hash + Eq + ?Sized,
|
||||
{
|
||||
if let Some(entry) = self.0.lock().unwrap().get_mut(key) {
|
||||
entry.answer = None;
|
||||
entry.error = None;
|
||||
}
|
||||
}
|
||||
|
||||
/// Forgets everything whose key `keep` rejects, for something being
|
||||
/// removed.
|
||||
pub fn retain(&self, keep: impl FnMut(&K) -> bool) {
|
||||
|
||||
@@ -861,6 +861,20 @@ impl RemoteChecks {
|
||||
.update(project.to_path_buf(), |new_commits| *new_commits = false);
|
||||
}
|
||||
|
||||
/// Drops what was learned about one checkout and asks again, for a
|
||||
/// move that changed which branch the question is about.
|
||||
///
|
||||
/// Both halves matter. Without the forgetting, the card answers for
|
||||
/// the branch that was just left -- and says it with `checkPending`
|
||||
/// false, so nothing on screen marks it as an answer from before the
|
||||
/// move: a branch with commits waiting reads as one with none, and
|
||||
/// Pull is disabled about it. Without the asking, the card would sit
|
||||
/// on "still finding out" until somebody pressed Refresh.
|
||||
pub fn recheck(&self, checkout: &Checkout) {
|
||||
self.0.forget(&checkout.path);
|
||||
self.refresh(std::slice::from_ref(checkout));
|
||||
}
|
||||
|
||||
/// Asks every checkout that isn't already being asked. Returns at once.
|
||||
pub fn refresh(&self, projects: &[Checkout]) {
|
||||
for checkout in projects {
|
||||
|
||||
+22
-2
@@ -1237,6 +1237,15 @@ struct PurgeQuery {
|
||||
struct PullQuery {
|
||||
#[serde(default)]
|
||||
force: bool,
|
||||
/// Whether to build what the commits brought in.
|
||||
///
|
||||
/// The phone's two buttons: Pull takes the commits and stops, Update
|
||||
/// pulls and builds and then installs what that produced. Defaulted to
|
||||
/// building rather than not, because an app older than this parameter
|
||||
/// asks for the route that has always pulled *and* built -- omitting
|
||||
/// it has to keep meaning what it used to mean, and it is the frozen
|
||||
/// half of the contract that lets an old app fetch a new one.
|
||||
build: Option<bool>,
|
||||
}
|
||||
|
||||
/// Which build a download wants, and whose.
|
||||
@@ -1544,8 +1553,15 @@ async fn build_now(
|
||||
Ok(Json(build.status()))
|
||||
}
|
||||
|
||||
/// Fetches, fast-forwards, and builds -- reported through the same status
|
||||
/// the phone already polls for a build, so one progress path covers both.
|
||||
/// Fetches and fast-forwards, and builds unless asked not to -- reported
|
||||
/// through the same status the phone already polls for a build, so one
|
||||
/// progress path covers both.
|
||||
///
|
||||
/// `?build=false` is the Pull button, which takes the commits and stops;
|
||||
/// Update is this route with the build, followed by the phone installing
|
||||
/// what it produced. Same route rather than two, because everything
|
||||
/// except whether a build follows is identical -- the lock, the dirty
|
||||
/// check, the unrelated-histories report and the status the phone polls.
|
||||
///
|
||||
/// Pulling stays available while a project's build step is waiting to be
|
||||
/// accepted, and stops after the fast-forward: taking commits runs git,
|
||||
@@ -1562,6 +1578,10 @@ async fn build_pull(
|
||||
.build
|
||||
.as_ref()
|
||||
.ok_or_else(|| ApiError::NoBuildStep(entry.label.clone()))?;
|
||||
if !query.build.unwrap_or(true) {
|
||||
build.pull_only(query.force);
|
||||
return Ok(Json(build.status()));
|
||||
}
|
||||
// Asked after the fast-forward, not now: the declaration the gate
|
||||
// reads is the one the pull is about to rewrite, so the answer from
|
||||
// here is the answer for the commit being replaced.
|
||||
|
||||
Reference in new issue
Block a user