mirror of
https://github.com/storytold/craft-launcher.git
synced 2026-11-09 20:04:34 +00:00
fix: the backend says whether an update can replace the installed copy
The UI guessed from any download this platform takes, but an install keeps the way a copy was installed (an AppImage with an AppImage, an MSI with an MSI), so an AppImage copy with a .deb-only release still showed Update, was announced and started automatically, and then failed. The backend now sends Event::Installable, from the same plan an install makes, before each Latest and Detected it depends on and whenever the plan changes. The UI uses it for installed copies and keeps its own check where there is no answer (the web build). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
3583972e93
commit
4931dd717f
5 files changed
+104
-9
No files matched your search
@@ -237,6 +237,15 @@ pub enum Event {
|
||||
app: String,
|
||||
ready: bool,
|
||||
},
|
||||
/// Whether installing `app` now would find a file for it: its release has a package this
|
||||
/// computer can use and, for an installed copy, one that can replace it the way it was
|
||||
/// installed (an AppImage with an AppImage, an MSI with an MSI). Sent before the `Latest` or
|
||||
/// `Detected` it depends on, so no update is offered, announced or started automatically
|
||||
/// before this is known.
|
||||
Installable {
|
||||
app: String,
|
||||
installable: bool,
|
||||
},
|
||||
/// A new install stopped before downloading because `path` already exists and the launcher
|
||||
/// didn't put it there. `Command::Replace` installs over it.
|
||||
Occupied {
|
||||
|
||||
@@ -188,7 +188,8 @@ impl NativeBackend {
|
||||
/// Shows the saved answers from the last run straight away, before any network request: the
|
||||
/// release feed's where they're still fresh, GitHub's otherwise.
|
||||
fn replay_cache(&self) {
|
||||
let pre = self.options().prereleases;
|
||||
let options = self.options();
|
||||
let pre = options.prereleases;
|
||||
let feed = if pre { None } else { load_feed(&self.paths) };
|
||||
let now = crate::now();
|
||||
for app in &SUITE {
|
||||
@@ -201,6 +202,7 @@ impl NativeBackend {
|
||||
if let LatestRelease::Found(r) = &latest {
|
||||
self.shared.latest.lock().unwrap_or_else(PoisonError::into_inner).insert(app.id.to_string(), r.clone());
|
||||
}
|
||||
report_installable(app, &self.shared, self.platform, &options);
|
||||
self.shared.emit(Event::Latest { app: app.id.to_string(), latest, from });
|
||||
}
|
||||
}
|
||||
@@ -238,6 +240,7 @@ impl NativeBackend {
|
||||
None => map.remove(app.id),
|
||||
};
|
||||
}
|
||||
report_installable(app, &shared, platform, &options);
|
||||
shared.emit(Event::Detected { app: app.id.to_string(), installed: found });
|
||||
}
|
||||
// Downloads paused (or cut off by quitting) in an earlier run.
|
||||
@@ -283,7 +286,7 @@ impl NativeBackend {
|
||||
}
|
||||
let now = crate::now();
|
||||
let queue = Mutex::new(SUITE.iter().chain(std::iter::once(&LAUNCHER)).collect::<Vec<&CraftApp>>());
|
||||
let sources = sweep(&queue, &mirror, feed.as_ref(), fallback, &source, &shared, &paths, &options, force, now);
|
||||
let sources = sweep(&queue, &mirror, feed.as_ref(), fallback, &source, &shared, &paths, (platform, &options), force, now);
|
||||
source.save();
|
||||
{
|
||||
let answered = shared.answered.lock().unwrap_or_else(PoisonError::into_inner);
|
||||
@@ -397,6 +400,7 @@ impl NativeBackend {
|
||||
None => map.remove(app.id),
|
||||
};
|
||||
}
|
||||
report_installable(app, &shared, platform, &options);
|
||||
shared.emit(Event::Detected { app: app.id.to_string(), installed: found });
|
||||
drop(job);
|
||||
shared.emit(Event::JobFinished { app: app.id.to_string(), result });
|
||||
@@ -764,8 +768,21 @@ fn planned_asset(
|
||||
pick_asset(release, &stems, platform, &kinds).map(|(a, k)| (a.clone(), k))
|
||||
}
|
||||
|
||||
/// Tells the UI whether `app` has a paused download waiting, or a finished one to install.
|
||||
/// Tells the UI whether installing `app` would find a file for it now ([`Event::Installable`]):
|
||||
/// the same plan an install makes, for the copy that is installed. Nothing is said until its
|
||||
/// release is known.
|
||||
fn report_installable(app: &CraftApp, shared: &Shared, platform: Option<Platform>, options: &BackendOptions) {
|
||||
let Some(release) = shared.latest.lock().unwrap_or_else(PoisonError::into_inner).get(app.id).cloned() else { return };
|
||||
let existing = shared.installed.lock().unwrap_or_else(PoisonError::into_inner).get(app.id).cloned();
|
||||
let record = shared.manifest.lock().unwrap_or_else(PoisonError::into_inner).apps.get(app.id).cloned();
|
||||
let installable = platform.is_some_and(|p| planned_asset(app, &release, p, options, existing.as_ref(), record.as_ref()).is_some());
|
||||
shared.emit(Event::Installable { app: app.id.to_string(), installable });
|
||||
}
|
||||
|
||||
/// Tells the UI whether `app` has a paused download waiting, or a finished one to install, and
|
||||
/// whether an install would find a file at all (the package choices may have changed).
|
||||
fn report_partial(app: &CraftApp, shared: &Shared, paths: &Paths, platform: Option<Platform>, options: &BackendOptions) {
|
||||
report_installable(app, shared, platform, options);
|
||||
let planned = planned_download(app, shared, paths, platform, options);
|
||||
let download = planned.as_ref().and_then(|(dest, size)| Some(PausedDownload { done: http::partial_bytes(dest, *size)?, total: *size }));
|
||||
shared.emit(Event::Paused { app: app.id.to_string(), download });
|
||||
@@ -812,7 +829,7 @@ fn sweep(
|
||||
source: &Arc<dyn ReleaseSource>,
|
||||
shared: &Arc<Shared>,
|
||||
paths: &Paths,
|
||||
options: &BackendOptions,
|
||||
(platform, options): (Option<Platform>, &BackendOptions),
|
||||
force: bool,
|
||||
now: i64,
|
||||
) -> Vec<Source> {
|
||||
@@ -835,7 +852,7 @@ fn sweep(
|
||||
if let Some(used) = used {
|
||||
sources.lock().unwrap_or_else(PoisonError::into_inner).push(used);
|
||||
}
|
||||
check_app(app, latest, from, shared, paths);
|
||||
check_app(app, latest, from, shared, paths, (platform, options));
|
||||
}
|
||||
});
|
||||
}
|
||||
@@ -844,8 +861,16 @@ fn sweep(
|
||||
}
|
||||
|
||||
/// Publishes one app's answer to the shared state, or handles the launcher's own update check
|
||||
/// first (it doesn't appear in the grid, so it has no card of its own to publish to).
|
||||
fn check_app(app: &'static CraftApp, latest: Option<LatestRelease>, from: Origin, shared: &Arc<Shared>, paths: &Paths) {
|
||||
/// first (it doesn't appear in the grid, so it has no card of its own to publish to). `plan` is
|
||||
/// what an install would use, for [`report_installable`].
|
||||
fn check_app(
|
||||
app: &'static CraftApp,
|
||||
latest: Option<LatestRelease>,
|
||||
from: Origin,
|
||||
shared: &Arc<Shared>,
|
||||
paths: &Paths,
|
||||
(platform, options): (Option<Platform>, &BackendOptions),
|
||||
) {
|
||||
if app.id == LAUNCHER_ID {
|
||||
launcher_update(paths, latest, shared);
|
||||
return;
|
||||
@@ -857,6 +882,7 @@ fn check_app(app: &'static CraftApp, latest: Option<LatestRelease>, from: Origin
|
||||
if !matches!(latest, LatestRelease::Failed(_)) {
|
||||
shared.answered.lock().unwrap_or_else(PoisonError::into_inner).insert(app.id.to_string());
|
||||
}
|
||||
report_installable(app, shared, platform, options);
|
||||
shared.emit(Event::Latest { app: app.id.to_string(), latest, from });
|
||||
}
|
||||
|
||||
@@ -1817,6 +1843,34 @@ mod tests {
|
||||
NativeBackend { shared, paths: Paths::under(root), http: None, source, platform: Some(platform), pins: Some(root.join("versions.json")) }
|
||||
}
|
||||
|
||||
/// Each answer tells the UI whether an install would find a file before it says the release
|
||||
/// is out, so the UI never offers (or starts) an update it can't install. A release with
|
||||
/// nothing for this computer is not installable; a later answer with a file is.
|
||||
#[test]
|
||||
fn installability_is_told_before_the_release_it_depends_on() {
|
||||
let app = CraftApp::by_id(APP).unwrap();
|
||||
let platform = Platform { os: Os::MacOs, arch: Arch::Arm64 };
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mut backend = backend(dir.path(), platform, None);
|
||||
let order = |events: Vec<Event>| -> Vec<String> {
|
||||
events
|
||||
.into_iter()
|
||||
.filter_map(|e| match e {
|
||||
Event::Installable { app, installable } if app == APP => Some(format!("installable {installable}")),
|
||||
Event::Latest { app, .. } if app == APP => Some("latest".to_string()),
|
||||
_ => None,
|
||||
})
|
||||
.collect()
|
||||
};
|
||||
let options = BackendOptions::default();
|
||||
let mut windows_only = release("0.6.0");
|
||||
windows_only.assets.retain(|a| a.name.contains("windows"));
|
||||
check_app(app, Some(LatestRelease::Found(windows_only)), Origin::GitHub, &backend.shared, &backend.paths, (Some(platform), &options));
|
||||
let expected = planned_asset(app, &release("0.6.0"), platform, &options, None, None).is_some();
|
||||
check_app(app, Some(LatestRelease::Found(release("0.6.0"))), Origin::GitHub, &backend.shared, &backend.paths, (Some(platform), &options));
|
||||
assert_eq!(order(backend.poll()), ["installable false", "latest", &format!("installable {expected}"), "latest"]);
|
||||
}
|
||||
|
||||
/// Reports again, and returns what the UI was last told about a kept download.
|
||||
fn ready(backend: &mut NativeBackend) -> Option<bool> {
|
||||
let app = CraftApp::by_id(APP).unwrap();
|
||||
|
||||
@@ -599,6 +599,13 @@ impl LauncherUi {
|
||||
}
|
||||
false
|
||||
}
|
||||
// Can turn an update on or off, so updates are announced (or started) again.
|
||||
Event::Installable { app, installable } => {
|
||||
if let Some(e) = self.entry_mut(&app) {
|
||||
e.installable = Some(installable);
|
||||
}
|
||||
true
|
||||
}
|
||||
Event::Occupied { app, path } => {
|
||||
self.dialog = Some(Dialog::Replace { app, path });
|
||||
false
|
||||
|
||||
@@ -22,11 +22,15 @@ pub struct AppEntry {
|
||||
pub paused: Option<PausedDownload>,
|
||||
/// A finished download waiting for another try at installing it.
|
||||
pub ready: bool,
|
||||
/// The backend's word on whether an install would find a file that can replace the installed
|
||||
/// copy ([`Event::Installable`](artcraft_launcher_core::protocol::Event::Installable)); `None`
|
||||
/// where it gives none (the web build), and then [`Self::has_download`] decides.
|
||||
pub installable: Option<bool>,
|
||||
}
|
||||
|
||||
impl AppEntry {
|
||||
pub fn new(app: &'static CraftApp) -> Self {
|
||||
Self { app, latest: None, installed: None, detected: false, job: None, origin: None, paused: None, ready: false }
|
||||
Self { app, latest: None, installed: None, detected: false, job: None, origin: None, paused: None, ready: false, installable: None }
|
||||
}
|
||||
|
||||
pub fn release(&self) -> Option<&Release> {
|
||||
@@ -52,7 +56,15 @@ impl AppEntry {
|
||||
None => Latest::Pending,
|
||||
Some(LatestRelease::Failed(_)) => Latest::Failed,
|
||||
Some(LatestRelease::None) => Latest::NoRelease,
|
||||
Some(LatestRelease::Found(r)) => Latest::Found { version: r.version.as_ref(), has_asset: self.has_download(platform) },
|
||||
Some(LatestRelease::Found(r)) => {
|
||||
// Only the backend knows which package can replace an installed copy (an AppImage
|
||||
// with an AppImage); for an app not installed yet, any download this platform takes.
|
||||
let has_asset = match (&self.installed, self.installable) {
|
||||
(Some(_), Some(installable)) => installable,
|
||||
_ => self.has_download(platform),
|
||||
};
|
||||
Latest::Found { version: r.version.as_ref(), has_asset }
|
||||
}
|
||||
};
|
||||
status::status(self.installed.as_ref(), latest)
|
||||
}
|
||||
|
||||
@@ -1141,6 +1141,19 @@ fn a_newer_release_without_a_download_here_is_not_offered_as_an_update() {
|
||||
assert_eq!(launches_and_installs(&sent, "photocraft"), (1, 0), "{:?}", sent.borrow());
|
||||
}
|
||||
|
||||
#[test]
|
||||
/// The backend's answer outranks the UI's guess: a release with a Mac download that can't replace
|
||||
/// how PhotoCraft was installed (say an AppImage when only a .deb is out) is no update either.
|
||||
fn the_backends_installability_answer_decides_an_update() {
|
||||
let mut ev = events();
|
||||
ev.push(Event::Installable { app: "photocraft".into(), installable: false });
|
||||
let (mut h, _sent) = harness_with(ev, ViewMode::Grid, egui::Theme::Dark, true, 1.0);
|
||||
h.run_steps(8);
|
||||
assert!(h.query_by_label("Update to 0.5.0").is_none(), "the backend can't install it");
|
||||
assert!(h.query_by_label(KEEP_PHOTOCRAFT).is_none());
|
||||
h.get_by_label("Update 1 app");
|
||||
}
|
||||
|
||||
/// `events`, with PhotoCraft's 0.5.0 published for Windows only: nothing the test's Mac can install.
|
||||
fn no_photocraft_download_here(mut ev: Vec<Event>) -> Vec<Event> {
|
||||
for e in &mut ev {
|
||||
|
||||
Reference in new issue
Block a user