Keep the verified download when the install step fails
A download that passed its SHA-256 check was deleted whether or not the install succeeded, so every retry downloaded it again (usually after the password prompt was dismissed). It now stays until the install succeeds, is hashed again before it is reused, and shows as "Retry install" (or "Retry update to <version>") with "Download again" in the menu. A dismissed system prompt now says "Install cancelled" rather than "Download cancelled". A download checked by size only is still deleted. Ported onto main from the original PR: the check thread, rewritten for ArtCraft's release feed, reports kept downloads after it marks the apps loaded; the Retry button uses the restyled widgets::button; the backend tests build on main's Shared and ask no real service; the grid test scrolls the card into view. Finished files kept after a failed install are now cleaned up after 7 days instead of 30 (paused downloads keep 30). Fixes #5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
7a193042fd
commit
5f74027176
|
|
@ -37,7 +37,7 @@ pub enum Command {
|
|||
Pause {
|
||||
app: String,
|
||||
},
|
||||
/// Stop a running download, or throw away a paused one.
|
||||
/// Stop a running download, or throw away a paused one or one kept after a failed install.
|
||||
Cancel {
|
||||
app: String,
|
||||
},
|
||||
|
|
@ -96,6 +96,9 @@ pub struct PausedDownload {
|
|||
pub const PAUSED: &str = "paused";
|
||||
/// The `JobFinished` error a cancelled job ends with (not a failure).
|
||||
pub const CANCELLED: &str = "cancelled";
|
||||
/// The `JobFinished` error of an install whose system prompt (password, Windows Installer) the
|
||||
/// user dismissed after the download finished (not a failure).
|
||||
pub const INSTALL_CANCELLED: &str = "install cancelled";
|
||||
|
||||
/// Which service an app's release information came from (the footer names them).
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
|
||||
|
|
@ -171,6 +174,12 @@ pub enum Event {
|
|||
app: String,
|
||||
download: Option<PausedDownload>,
|
||||
},
|
||||
/// Whether a finished download is waiting because its install step failed; `Install` checks
|
||||
/// it again and goes straight to installing.
|
||||
ReadyToInstall {
|
||||
app: String,
|
||||
ready: bool,
|
||||
},
|
||||
/// A job ended: `Ok` carries a short success message, `Err` what went wrong.
|
||||
JobFinished {
|
||||
app: String,
|
||||
|
|
|
|||
|
|
@ -169,8 +169,9 @@ impl NativeBackend {
|
|||
let platform = self.platform;
|
||||
let options = self.options();
|
||||
std::thread::spawn(move || {
|
||||
// Leftovers from interrupted installs or older versions of the launcher.
|
||||
http::clean_downloads(&paths.downloads(), Duration::from_secs(30 * 86_400));
|
||||
// Leftovers from interrupted installs or older versions of the launcher. A paused
|
||||
// download stays a month; a finished one kept after a failed install, a week.
|
||||
http::clean_downloads(&paths.downloads(), Duration::from_secs(30 * 86_400), Duration::from_secs(7 * 86_400));
|
||||
let snapshot = crate::detect::Snapshot::take();
|
||||
let manifest = shared.manifest.lock().unwrap_or_else(PoisonError::into_inner).clone();
|
||||
for app in &SUITE {
|
||||
|
|
@ -213,6 +214,7 @@ impl NativeBackend {
|
|||
}
|
||||
let shared = self.shared.clone();
|
||||
let paths = self.paths.clone();
|
||||
let platform = self.platform;
|
||||
let pre = self.options().prereleases;
|
||||
std::thread::spawn(move || {
|
||||
shared.emit(Event::CheckStarted);
|
||||
|
|
@ -268,6 +270,11 @@ impl NativeBackend {
|
|||
shared.loaded.store(true, Ordering::SeqCst);
|
||||
}
|
||||
}
|
||||
// A newer release changes which file an install would use.
|
||||
let options = shared.options.lock().unwrap_or_else(PoisonError::into_inner).clone();
|
||||
for app in &SUITE {
|
||||
report_partial(app, &shared, &paths, platform, &options);
|
||||
}
|
||||
let sources = sources.into_inner().unwrap_or_else(PoisonError::into_inner);
|
||||
let source_used = if sources.contains(&Source::Cache) {
|
||||
Source::Cache
|
||||
|
|
@ -463,20 +470,28 @@ 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.
|
||||
/// Tells the UI whether `app` has a paused download waiting, or a finished one to install.
|
||||
fn report_partial(app: &CraftApp, shared: &Shared, paths: &Paths, platform: Option<Platform>, options: &BackendOptions) {
|
||||
let download = partial_for(app, shared, paths, platform, options).map(|(asset, done)| PausedDownload { done, total: asset.size });
|
||||
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 });
|
||||
let ready = planned.is_some_and(|(dest, size)| http::finished(&dest, size));
|
||||
shared.emit(Event::ReadyToInstall { app: app.id.to_string(), ready });
|
||||
}
|
||||
|
||||
/// The planned download of `app` and how many bytes of it are already on disk.
|
||||
fn partial_for(app: &CraftApp, shared: &Shared, paths: &Paths, platform: Option<Platform>, options: &BackendOptions) -> Option<(Asset, u64)> {
|
||||
/// Where the planned download of `app` is kept, and its size.
|
||||
fn planned_download(
|
||||
app: &CraftApp,
|
||||
shared: &Shared,
|
||||
paths: &Paths,
|
||||
platform: Option<Platform>,
|
||||
options: &BackendOptions,
|
||||
) -> Option<(std::path::PathBuf, u64)> {
|
||||
let release = shared.latest.lock().unwrap_or_else(PoisonError::into_inner).get(app.id).cloned()?;
|
||||
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 (asset, _) = planned_asset(app, &release, platform?, options, existing.as_ref(), record.as_ref())?;
|
||||
let done = http::partial_bytes(&install::download_path(paths, &asset), asset.size)?;
|
||||
Some((asset, done))
|
||||
artcraft_launcher_core::release::valid_file_name(&asset.name).then(|| (install::download_path(paths, &asset), asset.size))
|
||||
}
|
||||
|
||||
/// How much the feed is trusted in one check, and how freely GitHub may be asked.
|
||||
|
|
@ -686,14 +701,15 @@ impl Backend for NativeBackend {
|
|||
let running = self.shared.jobs.lock().unwrap_or_else(PoisonError::into_inner).get(&app).cloned();
|
||||
match running {
|
||||
Some(stop) => stop.cancel(),
|
||||
// A paused download: throw its partial file away.
|
||||
// A paused download, or one kept after a failed install: throw the file away.
|
||||
None => {
|
||||
if let Some(def) = CraftApp::by_id(&app)
|
||||
&& let Some((asset, _)) = partial_for(def, &self.shared, &self.paths, self.platform, &self.options())
|
||||
&& let Some((dest, size)) = planned_download(def, &self.shared, &self.paths, self.platform, &self.options())
|
||||
{
|
||||
http::discard_partial(&install::download_path(&self.paths, &asset), asset.size);
|
||||
http::discard(&dest, size);
|
||||
}
|
||||
self.shared.emit(Event::Paused { app, download: None });
|
||||
self.shared.emit(Event::Paused { app: app.clone(), download: None });
|
||||
self.shared.emit(Event::ReadyToInstall { app, ready: false });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -724,6 +740,11 @@ impl Backend for NativeBackend {
|
|||
*o = options;
|
||||
changed
|
||||
};
|
||||
// Another package format means another file to download.
|
||||
let options = self.options();
|
||||
for app in &SUITE {
|
||||
report_partial(app, &self.shared, &self.paths, self.platform, &options);
|
||||
}
|
||||
if changed_pre {
|
||||
self.check(true);
|
||||
}
|
||||
|
|
@ -945,4 +966,114 @@ mod tests {
|
|||
let (_, _, used) = latest_for(photo, Some(&feed), LOADED, &github, &fresh_shared, false, false, NOW + 2 * 86_400);
|
||||
assert_eq!(used, Some(Source::GitHubApi));
|
||||
}
|
||||
|
||||
const APP: &str = "photocraft";
|
||||
|
||||
/// A PhotoCraft release with one file per platform and format, so whichever this machine
|
||||
/// installs is there.
|
||||
fn release(version: &str) -> Release {
|
||||
let suffixes = ["-portable.zip", ".tar.gz", ".msi", ".dmg", ".AppImage", ".deb", ".rpm", ".flatpak"];
|
||||
let tokens = ["windows-x64", "windows-x86", "windows-arm64", "macos-universal", "linux-x86_64", "linux-aarch64", "freebsd-x86_64"];
|
||||
let assets = tokens
|
||||
.iter()
|
||||
.flat_map(|t| suffixes.iter().map(move |s| format!("{APP}-{version}-{t}{s}")))
|
||||
.map(|name| Asset { url: format!("https://example.invalid/{name}"), name, size: 3, sha256: None })
|
||||
.collect();
|
||||
Release { assets, ..Release::default() }
|
||||
}
|
||||
|
||||
/// A backend that knows `release("0.5.0")` and keeps its files under `root`.
|
||||
fn backend(root: &std::path::Path, platform: Platform, source: Option<Arc<dyn ReleaseSource>>) -> NativeBackend {
|
||||
let shared = shared();
|
||||
shared.latest.lock().unwrap().insert(APP.to_string(), release("0.5.0"));
|
||||
let shared = Arc::new(shared);
|
||||
NativeBackend { shared, paths: Paths::under(root), http: None, source, platform: Some(platform), self_install: None }
|
||||
}
|
||||
|
||||
/// 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();
|
||||
report_partial(app, &backend.shared, &backend.paths, backend.platform, &BackendOptions::default());
|
||||
last_ready(backend.poll())
|
||||
}
|
||||
|
||||
fn last_ready(events: Vec<Event>) -> Option<bool> {
|
||||
events.into_iter().rev().find_map(|e| match e {
|
||||
Event::ReadyToInstall { app, ready } if app == APP => Some(ready),
|
||||
_ => None,
|
||||
})
|
||||
}
|
||||
|
||||
/// Puts the planned download on disk, as a failed install leaves it.
|
||||
fn keep_file(backend: &NativeBackend) -> std::path::PathBuf {
|
||||
let app = CraftApp::by_id(APP).unwrap();
|
||||
let (dest, size) = planned_download(app, &backend.shared, &backend.paths, backend.platform, &BackendOptions::default()).unwrap();
|
||||
std::fs::create_dir_all(backend.paths.downloads()).unwrap();
|
||||
std::fs::write(&dest, vec![0u8; size as usize]).unwrap();
|
||||
dest
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_kept_download_is_reported_and_can_be_thrown_away() {
|
||||
let Some(platform) = native_platform() else { return };
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mut backend = backend(dir.path(), platform, None);
|
||||
assert_eq!(ready(&mut backend), Some(false));
|
||||
let dest = keep_file(&backend);
|
||||
assert_eq!(ready(&mut backend), Some(true));
|
||||
|
||||
backend.send(Command::Cancel { app: APP.to_string() });
|
||||
assert!(!dest.exists());
|
||||
assert_eq!(last_ready(backend.poll()), Some(false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn changing_the_settings_reports_kept_downloads_again() {
|
||||
let Some(platform) = native_platform() else { return };
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mut backend = backend(dir.path(), platform, None);
|
||||
keep_file(&backend);
|
||||
assert_eq!(ready(&mut backend), Some(true));
|
||||
// The planned file changed under the UI; only a new report can tell it.
|
||||
backend.shared.latest.lock().unwrap().insert(APP.to_string(), release("0.6.0"));
|
||||
backend.send(Command::Configure(BackendOptions::default()));
|
||||
assert_eq!(last_ready(backend.poll()), Some(false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_check_that_finds_a_newer_release_ends_the_retry_offer() {
|
||||
struct Newer;
|
||||
impl ReleaseSource for Newer {
|
||||
fn latest(&self, repo: &str, _prereleases: bool) -> crate::source::Answer {
|
||||
let found = CraftApp::by_id(APP).is_some_and(|a| a.repo == repo);
|
||||
let latest = if found { LatestRelease::Found(release("0.6.0")) } else { LatestRelease::None };
|
||||
crate::source::Answer { latest, source: Source::GitHubApi }
|
||||
}
|
||||
fn cached(&self, _repo: &str, _prereleases: bool) -> Option<LatestRelease> {
|
||||
None
|
||||
}
|
||||
fn rate_limited_until(&self) -> Option<i64> {
|
||||
None
|
||||
}
|
||||
fn save(&self) {}
|
||||
}
|
||||
let Some(platform) = native_platform() else { return };
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mut backend = backend(dir.path(), platform, Some(Arc::new(Newer)));
|
||||
// A check needs a client; with pre-releases on it asks only `Newer`, never ArtCraft's API.
|
||||
backend.http = Some(Http::new().unwrap());
|
||||
backend.shared.options.lock().unwrap().prereleases = true;
|
||||
keep_file(&backend);
|
||||
assert_eq!(ready(&mut backend), Some(true));
|
||||
|
||||
backend.send(Command::CheckForUpdates { force: true });
|
||||
let mut events = Vec::new();
|
||||
let deadline = Instant::now() + Duration::from_secs(20);
|
||||
while !events.iter().any(|e| matches!(e, Event::CheckFinished { .. })) {
|
||||
assert!(Instant::now() < deadline, "the check never finished");
|
||||
std::thread::sleep(Duration::from_millis(10));
|
||||
events.extend(backend.poll());
|
||||
}
|
||||
assert_eq!(last_ready(events), Some(false));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -186,8 +186,9 @@ impl Http {
|
|||
std::fs::create_dir_all(dir).map_err(|e| format!("couldn't create {} ({e})", dir.display()))?;
|
||||
}
|
||||
let verified = if sha256.is_some() { Verified::Checksum } else { Verified::SizeOnly };
|
||||
// A finished copy from an earlier attempt that never got installed.
|
||||
if dest.is_file() && matches_published(dest, size, sha256) {
|
||||
// A finished copy from an earlier attempt that never got installed. Marking it as just
|
||||
// used, before checking it, keeps `clean_downloads` away from it while it's installed.
|
||||
if dest.is_file() && touch(dest) && matches_published(dest, size, sha256) {
|
||||
progress(Transfer::Bytes { done: size, total: size });
|
||||
return Ok(verified);
|
||||
}
|
||||
|
|
@ -402,10 +403,22 @@ pub fn partial_bytes(dest: &Path, size: u64) -> Option<u64> {
|
|||
std::fs::metadata(part_path(dest, size)).ok().map(|m| m.len()).filter(|n| *n > 0)
|
||||
}
|
||||
|
||||
/// Removes leftovers from the download folder: files in an old or unknown naming scheme, and
|
||||
/// paused downloads untouched for `max_age` (they're for releases long since replaced). Files
|
||||
/// written within the last minute are always kept, so a running download is never touched.
|
||||
pub fn clean_downloads(dir: &Path, max_age: Duration) {
|
||||
/// Sets a file's modification time to now.
|
||||
fn touch(path: &Path) -> bool {
|
||||
std::fs::File::options().write(true).open(path).and_then(|f| f.set_modified(std::time::SystemTime::now())).is_ok()
|
||||
}
|
||||
|
||||
/// Whether a finished download is waiting at `dest`. Judged by size alone: `download` checks it
|
||||
/// against the published checksum before anything uses it.
|
||||
pub fn finished(dest: &Path, size: u64) -> bool {
|
||||
std::fs::metadata(dest).is_ok_and(|m| m.is_file() && m.len() == size)
|
||||
}
|
||||
|
||||
/// Removes leftovers from the download folder: partial files in an old naming scheme, paused
|
||||
/// downloads untouched for `partial_max_age`, and finished ones (kept after a failed install)
|
||||
/// untouched for `finished_max_age` (they're for releases long since replaced, or given up on).
|
||||
/// Files written within the last minute are always kept, so a running download is never touched.
|
||||
pub fn clean_downloads(dir: &Path, partial_max_age: Duration, finished_max_age: Duration) {
|
||||
let Ok(entries) = std::fs::read_dir(dir) else { return };
|
||||
let now = std::time::SystemTime::now();
|
||||
for entry in entries.flatten() {
|
||||
|
|
@ -424,16 +437,19 @@ pub fn clean_downloads(dir: &Path, max_age: Duration) {
|
|||
.strip_suffix(".part")
|
||||
.and_then(|stem| stem.rsplit_once('.'))
|
||||
.is_some_and(|(_, size)| !size.is_empty() && size.bytes().all(|b| b.is_ascii_digit()));
|
||||
if !current_part || age > max_age {
|
||||
let legacy_part = name.ends_with(".part") && !current_part;
|
||||
let max_age = if name.ends_with(".part") { partial_max_age } else { finished_max_age };
|
||||
if legacy_part || age > max_age {
|
||||
log::info!("removing stale download {}", path.display());
|
||||
let _ = std::fs::remove_file(&path);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Throws away a paused download.
|
||||
pub fn discard_partial(dest: &Path, size: u64) {
|
||||
/// Throws away a paused download, and a finished one kept for another try at installing.
|
||||
pub fn discard(dest: &Path, size: u64) {
|
||||
let _ = std::fs::remove_file(part_path(dest, size));
|
||||
let _ = std::fs::remove_file(dest);
|
||||
}
|
||||
|
||||
enum AttemptError {
|
||||
|
|
@ -485,9 +501,12 @@ fn host(url: &str) -> &str {
|
|||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
pub(crate) mod tests {
|
||||
use super::*;
|
||||
|
||||
const DAYS_30: Duration = Duration::from_secs(30 * 86_400);
|
||||
const DAYS_7: Duration = Duration::from_secs(7 * 86_400);
|
||||
|
||||
#[test]
|
||||
fn cleaning_keeps_current_partial_files_and_removes_leftovers() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
|
@ -503,13 +522,61 @@ mod tests {
|
|||
let finished = make("photocraft-0.5.0-macos-universal.dmg");
|
||||
let fresh = dir.path().join("x.part");
|
||||
std::fs::write(&fresh, b"x").unwrap();
|
||||
clean_downloads(dir.path(), Duration::from_secs(30 * 86_400));
|
||||
assert!(keep.exists() && fresh.exists());
|
||||
assert!(!legacy.exists() && !finished.exists());
|
||||
clean_downloads(dir.path(), Duration::from_secs(60));
|
||||
clean_downloads(dir.path(), DAYS_30, DAYS_7);
|
||||
assert!(keep.exists() && fresh.exists() && finished.exists());
|
||||
assert!(!legacy.exists());
|
||||
// A finished file goes sooner than a paused one.
|
||||
clean_downloads(dir.path(), DAYS_30, Duration::from_secs(60));
|
||||
assert!(keep.exists() && !finished.exists());
|
||||
clean_downloads(dir.path(), Duration::from_secs(60), DAYS_7);
|
||||
assert!(!keep.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_kept_file_that_still_matches_is_not_fetched_again() {
|
||||
let (body, sha) = body();
|
||||
let len = body.len() as u64;
|
||||
let (url, requests) = serve(body.clone(), vec![Reply::Full, Reply::Full]);
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let dest = dir.path().join("file.bin");
|
||||
let http = Http::new().unwrap();
|
||||
http.download(&url, &dest, len, Some(&sha), &Stop::default(), &mut |_| {}).unwrap();
|
||||
assert!(finished(&dest, len) && !finished(&dest, 0));
|
||||
// Old enough to be cleaned up, were it not about to be used.
|
||||
let old = std::time::SystemTime::now() - Duration::from_secs(40 * 86_400);
|
||||
std::fs::File::options().write(true).open(&dest).unwrap().set_modified(old).unwrap();
|
||||
let mut last = (0, 0);
|
||||
let again = http.download(&url, &dest, len, Some(&sha), &Stop::default(), &mut |t| {
|
||||
if let Transfer::Bytes { done, total } = t {
|
||||
last = (done, total);
|
||||
}
|
||||
});
|
||||
assert_eq!(again, Ok(Verified::Checksum));
|
||||
assert_eq!(last, (len, len));
|
||||
assert_eq!(requests.lock().unwrap().len(), 1);
|
||||
// A cleanup while the installer runs leaves the reused file alone.
|
||||
clean_downloads(dir.path(), DAYS_30, DAYS_7);
|
||||
assert_eq!(std::fs::read(&dest).unwrap(), body);
|
||||
discard(&dest, len);
|
||||
assert!(!finished(&dest, len));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_kept_file_that_no_longer_matches_is_fetched_again() {
|
||||
let (body, sha) = body();
|
||||
let len = body.len() as u64;
|
||||
let (url, requests) = serve(body.clone(), vec![Reply::Full, Reply::Full]);
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let dest = dir.path().join("file.bin");
|
||||
let http = Http::new().unwrap();
|
||||
http.download(&url, &dest, len, Some(&sha), &Stop::default(), &mut |_| {}).unwrap();
|
||||
// The same size, so only the checksum can tell.
|
||||
std::fs::write(&dest, vec![b'x'; body.len()]).unwrap();
|
||||
http.download(&url, &dest, len, Some(&sha), &Stop::default(), &mut |_| {}).unwrap();
|
||||
assert_eq!(std::fs::read(&dest).unwrap(), body);
|
||||
assert_eq!(requests.lock().unwrap().len(), 2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reads_content_ranges() {
|
||||
assert_eq!(parse_content_range("bytes 100-199/1000"), Some((100, Some(1000))));
|
||||
|
|
@ -524,7 +591,7 @@ mod tests {
|
|||
|
||||
/// How the fake server answers one request.
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
enum Reply {
|
||||
pub(crate) enum Reply {
|
||||
/// The whole file.
|
||||
Full,
|
||||
/// Promise the whole file, send `n` bytes, hang up.
|
||||
|
|
@ -539,7 +606,7 @@ mod tests {
|
|||
|
||||
/// Serves `body` over plain HTTP following `script`, one reply per connection; records the
|
||||
/// `Range` header each request carried.
|
||||
fn serve(body: Vec<u8>, script: Vec<Reply>) -> (String, Arc<Mutex<Vec<Option<String>>>>) {
|
||||
pub(crate) fn serve(body: Vec<u8>, script: Vec<Reply>) -> (String, Arc<Mutex<Vec<Option<String>>>>) {
|
||||
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
|
||||
let url = format!("http://{}/file.bin", listener.local_addr().unwrap());
|
||||
let ranges = Arc::new(Mutex::new(Vec::new()));
|
||||
|
|
@ -585,7 +652,7 @@ mod tests {
|
|||
(url, ranges)
|
||||
}
|
||||
|
||||
fn body() -> (Vec<u8>, String) {
|
||||
pub(crate) fn body() -> (Vec<u8>, String) {
|
||||
let body: Vec<u8> = (0..700_000u32).map(|i| (i % 251) as u8).collect();
|
||||
let sha = hex(&Sha256::digest(&body));
|
||||
(body, sha)
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@
|
|||
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use artcraft_launcher_core::protocol::JobPhase;
|
||||
use artcraft_launcher_core::protocol::{CANCELLED, INSTALL_CANCELLED, JobPhase};
|
||||
use artcraft_launcher_core::release::{self, Asset, Release};
|
||||
use artcraft_launcher_core::{CraftApp, InstallSource, Installed, PackageKind};
|
||||
|
||||
|
|
@ -67,9 +67,18 @@ pub fn install(
|
|||
PackageKind::TarGz => freebsd::install_tree(app, &file, job.paths).map(|(bin, files, root)| Some(record(&version, kind, bin, files, root))),
|
||||
other => Err(format!("{} isn't supported on this system", other.label())),
|
||||
};
|
||||
// The download has served its purpose either way; a failed install starts fresh next time.
|
||||
let _ = std::fs::remove_file(&file);
|
||||
result.map(|r| (r, verified))
|
||||
if !keep_download(result.is_ok(), verified) {
|
||||
let _ = std::fs::remove_file(&file);
|
||||
}
|
||||
// The system's own prompt was dismissed: the download itself wasn't cancelled.
|
||||
result.map(|r| (r, verified)).map_err(|e| if e == CANCELLED { INSTALL_CANCELLED.to_string() } else { e })
|
||||
}
|
||||
|
||||
/// Whether the download stays on disk once the install step has ended. A failed install keeps it:
|
||||
/// the next try checks it against its published checksum again and skips the transfer. Without a
|
||||
/// checksum there's nothing to check it against later, so it goes.
|
||||
fn keep_download(installed: bool, verified: Verified) -> bool {
|
||||
!installed && verified == Verified::Checksum
|
||||
}
|
||||
|
||||
/// Where `asset` is downloaded to (and its partial file kept while unfinished).
|
||||
|
|
@ -710,6 +719,51 @@ mod tests {
|
|||
assert_eq!(std::fs::read_to_string(dest.join("v")).unwrap(), "new");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn only_a_failed_install_of_a_checksummed_download_keeps_it() {
|
||||
for (installed, verified, keep) in [
|
||||
(false, Verified::Checksum, true),
|
||||
(false, Verified::SizeOnly, false),
|
||||
(true, Verified::Checksum, false),
|
||||
(true, Verified::SizeOnly, false),
|
||||
] {
|
||||
assert_eq!(keep_download(installed, verified), keep, "installed: {installed}, {verified:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_failed_install_keeps_the_download_for_the_next_try() {
|
||||
use crate::http::tests::{Reply, body, serve};
|
||||
let app = CraftApp::by_id("wordcraft").unwrap();
|
||||
let (body, sha) = body();
|
||||
let (url, requests) = serve(body.clone(), vec![Reply::Full, Reply::Full]);
|
||||
let asset = Asset { name: "wordcraft-0.3.0-test.pkg".into(), size: body.len() as u64, url, sha256: Some(sha) };
|
||||
let release = Release { assets: vec![asset.clone()], ..Release::default() };
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let paths = Paths::under(dir.path());
|
||||
let http = Http::new().unwrap();
|
||||
// A format no build of the launcher installs here stands in for any failing install step.
|
||||
let kind = if cfg!(windows) { PackageKind::Dmg } else { PackageKind::Msi };
|
||||
let mut phases = Vec::new();
|
||||
for _ in 0..2 {
|
||||
let mut phase = |p: JobPhase| phases.push(p);
|
||||
let mut job = Job { http: &http, paths: &paths, stop: &Stop::default(), phase: &mut phase };
|
||||
let err = install(app, &release, &asset, kind, None, &mut job).unwrap_err();
|
||||
assert!(err.contains("isn't supported"), "{err}");
|
||||
assert_eq!(std::fs::read(download_path(&paths, &asset)).unwrap(), body);
|
||||
}
|
||||
assert_eq!(requests.lock().unwrap().len(), 1, "the second try reuses the kept file");
|
||||
assert_eq!(phases.iter().filter(|p| **p == JobPhase::Installing).count(), 2);
|
||||
|
||||
// Checked by size only: not kept.
|
||||
let unchecked = Asset { name: "wordcraft-0.3.1-test.pkg".into(), sha256: None, ..asset };
|
||||
let mut phase = |_: JobPhase| {};
|
||||
let mut job = Job { http: &http, paths: &paths, stop: &Stop::default(), phase: &mut phase };
|
||||
install(app, &release, &unchecked, kind, None, &mut job).unwrap_err();
|
||||
assert!(!download_path(&paths, &unchecked).exists());
|
||||
assert_eq!(requests.lock().unwrap().len(), 2);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn tarball(path: &Path, entries: &[&str]) {
|
||||
let f = std::fs::File::create(path).unwrap();
|
||||
|
|
|
|||
|
|
@ -2,7 +2,9 @@
|
|||
|
||||
use std::time::Duration;
|
||||
|
||||
use artcraft_launcher_core::protocol::{Backend, BackendOptions, CANCELLED, Capabilities, Command, Event, LatestRelease, PAUSED, Source};
|
||||
use artcraft_launcher_core::protocol::{
|
||||
Backend, BackendOptions, CANCELLED, Capabilities, Command, Event, INSTALL_CANCELLED, LatestRelease, PAUSED, Source,
|
||||
};
|
||||
use artcraft_launcher_core::release::Release;
|
||||
use artcraft_launcher_core::status::AppStatus;
|
||||
use artcraft_launcher_core::{Platform, SUITE};
|
||||
|
|
@ -267,6 +269,12 @@ impl LauncherUi {
|
|||
}
|
||||
false
|
||||
}
|
||||
Event::ReadyToInstall { app, ready } => {
|
||||
if let Some(e) = self.entry_mut(&app) {
|
||||
e.ready = ready;
|
||||
}
|
||||
false
|
||||
}
|
||||
Event::JobFinished { app, result } => {
|
||||
if app == artcraft_launcher_core::catalog::LAUNCHER_ID {
|
||||
self.launcher_job = None;
|
||||
|
|
@ -274,11 +282,16 @@ impl LauncherUi {
|
|||
if let Some(e) = self.entry_mut(&app) {
|
||||
e.job = None;
|
||||
}
|
||||
let kept = self.entry(&app).is_some_and(|e| e.ready);
|
||||
match result {
|
||||
Ok(msg) => self.toast(ctx, msg, false),
|
||||
// The paused card says it all.
|
||||
Err(e) if e == PAUSED => {}
|
||||
Err(e) if e == CANCELLED => self.toast(ctx, "Download cancelled", false),
|
||||
Err(e) if e == INSTALL_CANCELLED && kept => {
|
||||
self.toast(ctx, "Install cancelled. The download is kept for the next try.", false);
|
||||
}
|
||||
Err(e) if e == INSTALL_CANCELLED => self.toast(ctx, "Install cancelled", false),
|
||||
Err(e) => self.toast(ctx, e, true),
|
||||
}
|
||||
true
|
||||
|
|
|
|||
|
|
@ -18,11 +18,13 @@ pub struct AppEntry {
|
|||
pub origin: Option<artcraft_launcher_core::protocol::Origin>,
|
||||
/// A download waiting to be resumed.
|
||||
pub paused: Option<PausedDownload>,
|
||||
/// A finished download waiting for another try at installing it.
|
||||
pub ready: bool,
|
||||
}
|
||||
|
||||
impl AppEntry {
|
||||
pub fn new(app: &'static CraftApp) -> Self {
|
||||
Self { app, latest: None, installed: None, detected: false, job: None, origin: None, paused: None }
|
||||
Self { app, latest: None, installed: None, detected: false, job: None, origin: None, paused: None, ready: false }
|
||||
}
|
||||
|
||||
pub fn release(&self) -> Option<&Release> {
|
||||
|
|
|
|||
|
|
@ -16,6 +16,8 @@ use crate::widgets::{self, Cell, Kind};
|
|||
/// What the main button of an app does.
|
||||
enum Primary {
|
||||
Install,
|
||||
/// The download is already here; only the install step is left. Carries the button's label.
|
||||
Retry(String),
|
||||
Update(String),
|
||||
Open,
|
||||
Download(String),
|
||||
|
|
@ -582,6 +584,8 @@ impl LauncherUi {
|
|||
};
|
||||
}
|
||||
match status {
|
||||
AppStatus::Available if entry.ready => Primary::Retry("Retry install".into()),
|
||||
AppStatus::UpdateAvailable if entry.ready => Primary::Retry(format!("Retry update to {}", entry.latest_label().unwrap_or_default())),
|
||||
AppStatus::Available => Primary::Install,
|
||||
AppStatus::UpdateAvailable => Primary::Update(entry.latest_label().unwrap_or_default()),
|
||||
AppStatus::UpToDate | AppStatus::Installed => Primary::Open,
|
||||
|
|
@ -674,6 +678,12 @@ impl LauncherUi {
|
|||
self.send(Command::Install { app: id.clone() });
|
||||
}
|
||||
}
|
||||
Primary::Retry(label) => {
|
||||
let tip = "The download is still here. It's checked again, then installed.";
|
||||
if widgets::button(ui, &label, Kind::Primary, main_w).on_hover_text(tip).clicked() {
|
||||
self.send(Command::Install { app: id.clone() });
|
||||
}
|
||||
}
|
||||
Primary::Update(v) => {
|
||||
if widgets::button(ui, &format!("Update to {v}"), Kind::Primary, main_w).clicked() {
|
||||
self.send(Command::Install { app: id.clone() });
|
||||
|
|
@ -709,6 +719,14 @@ impl LauncherUi {
|
|||
self.dialog = Some(Dialog::Notes(id.clone()));
|
||||
ui.close();
|
||||
}
|
||||
// Only where Install would download: an up-to-date app would be reinstalled.
|
||||
let can_download_again =
|
||||
manage && entry.ready && entry.paused.is_none() && matches!(status, AppStatus::Available | AppStatus::UpdateAvailable);
|
||||
if can_download_again && ui.button("Download again").clicked() {
|
||||
self.send(Command::Cancel { app: id.clone() });
|
||||
self.send(Command::Install { app: id.clone() });
|
||||
ui.close();
|
||||
}
|
||||
if manage && status.is_installed() {
|
||||
if ui.button("Open").clicked() {
|
||||
self.send(Command::Launch { app: id.clone() });
|
||||
|
|
|
|||
|
|
@ -6,7 +6,9 @@
|
|||
use std::cell::{Cell, RefCell};
|
||||
use std::rc::Rc;
|
||||
|
||||
use artcraft_launcher_core::protocol::{Backend, Capabilities, Command, Event, JobPhase, LatestRelease, Origin, PausedDownload, Source};
|
||||
use artcraft_launcher_core::protocol::{
|
||||
Backend, CANCELLED, Capabilities, Command, Event, INSTALL_CANCELLED, JobPhase, LatestRelease, Origin, PausedDownload, Source,
|
||||
};
|
||||
use artcraft_launcher_core::release::{Asset, Release};
|
||||
use artcraft_launcher_core::status::InstallSource;
|
||||
use artcraft_launcher_core::{Arch, Installed, Os, Platform, SUITE, version};
|
||||
|
|
@ -101,6 +103,7 @@ fn events() -> Vec<Event> {
|
|||
ev.push(Event::Progress { app: "filmcraft".into(), phase: JobPhase::Downloading { done: 48 << 20, total: 131 << 20 } });
|
||||
ev.push(Event::Paused { app: "deckcraft".into(), download: Some(PausedDownload { done: 30 << 20, total: 120 << 20 }) });
|
||||
ev.push(Event::Progress { app: "gridcraft".into(), phase: JobPhase::Reconnecting { done: 10 << 20, total: 90 << 20 } });
|
||||
ev.push(Event::ReadyToInstall { app: "cadcraft".into(), ready: true });
|
||||
ev.push(Event::CheckFinished { at: NOW - 180, source: Source::GitHubApi, rate_limited_until: None });
|
||||
ev
|
||||
}
|
||||
|
|
@ -110,9 +113,19 @@ fn harness(view: ViewMode, theme: egui::Theme, manage: bool) -> (Harness<'static
|
|||
}
|
||||
|
||||
fn harness_at(view: ViewMode, theme: egui::Theme, manage: bool, scale: f32) -> (Harness<'static, Option<LauncherUi>>, Rc<RefCell<Vec<Command>>>) {
|
||||
harness_with(events(), view, theme, manage, scale)
|
||||
}
|
||||
|
||||
fn harness_with(
|
||||
events: Vec<Event>,
|
||||
view: ViewMode,
|
||||
theme: egui::Theme,
|
||||
manage: bool,
|
||||
scale: f32,
|
||||
) -> (Harness<'static, Option<LauncherUi>>, Rc<RefCell<Vec<Command>>>) {
|
||||
let sent = Rc::new(RefCell::new(Vec::new()));
|
||||
let sent2 = sent.clone();
|
||||
let mut backend = Some(Fake { events: events(), sent: sent2, manage, clock: Rc::new(Cell::new(NOW)) });
|
||||
let mut backend = Some(Fake { events, sent: sent2, manage, clock: Rc::new(Cell::new(NOW)) });
|
||||
let settings = Settings { view, notify: false, ..Settings::default() };
|
||||
// No `.wgpu()`: it gives every test its own GPU device, and those segfault on parallel test threads (see above).
|
||||
// The default renderer creates one only when a test renders.
|
||||
|
|
@ -222,6 +235,76 @@ fn downloads_can_be_paused_resumed_and_discarded() {
|
|||
assert!(sent.borrow().iter().any(|c| matches!(c, Command::Pause { .. })));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_kept_download_offers_retry_install_and_download_again() {
|
||||
let (mut h, sent) = harness(ViewMode::Grid, egui::Theme::Dark, true);
|
||||
h.run_steps(4);
|
||||
if let Some(l) = h.state_mut().as_mut() {
|
||||
l.settings.filter = artcraft_launcher_ui_egui::settings::Filter::NotInstalled;
|
||||
}
|
||||
h.run_steps(6);
|
||||
// Below the fold in the restyled grid: bring the card on screen first.
|
||||
h.get_by_label("Retry install").scroll_to_me();
|
||||
h.run_steps(4);
|
||||
snapshot(&mut h, "grid-retry-install-dark");
|
||||
sent.borrow_mut().clear();
|
||||
let retry = h.get_by_label("Retry install");
|
||||
let row = retry.rect();
|
||||
retry.click();
|
||||
h.run_steps(2);
|
||||
assert_eq!(*sent.borrow(), [Command::Install { app: "cadcraft".into() }]);
|
||||
sent.borrow_mut().clear();
|
||||
// The … menu beside it.
|
||||
let beside = |r: egui::Rect| row.y_range().contains(r.center().y) && (row.right()..row.right() + 60.0).contains(&r.left());
|
||||
h.get_all_by_label("…").find(|n| beside(n.rect())).unwrap().click();
|
||||
h.run_steps(3);
|
||||
snapshot(&mut h, "grid-retry-install-menu-dark");
|
||||
h.get_by_label("Download again").click();
|
||||
h.run_steps(2);
|
||||
assert_eq!(*sent.borrow(), [Command::Cancel { app: "cadcraft".into() }, Command::Install { app: "cadcraft".into() }]);
|
||||
// Apps without a kept download don't offer it.
|
||||
assert_eq!(h.query_all_by_label("Retry install").count(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_dismissed_install_prompt_is_not_called_a_cancelled_download() {
|
||||
let finished = |app: &str, why: &str| Event::JobFinished { app: app.into(), result: Err(why.into()) };
|
||||
// The password prompt was dismissed with the download kept (cadcraft) and without (pdfcraft);
|
||||
// a download was cancelled (vectorcraft).
|
||||
let mut ev = events();
|
||||
ev.push(finished("cadcraft", INSTALL_CANCELLED));
|
||||
ev.push(finished("pdfcraft", INSTALL_CANCELLED));
|
||||
ev.push(finished("vectorcraft", CANCELLED));
|
||||
let (mut h, _) = harness_with(ev, ViewMode::Grid, egui::Theme::Dark, true, 1.0);
|
||||
h.run_steps(6);
|
||||
snapshot(&mut h, "grid-install-cancelled-dark");
|
||||
for toast in ["Install cancelled. The download is kept for the next try.", "Install cancelled", "Download cancelled"] {
|
||||
assert_eq!(h.query_all_by_label(toast).count(), 1, "{toast}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_kept_update_names_its_version_and_an_up_to_date_app_is_not_downloaded_again() {
|
||||
let mut ev = events();
|
||||
for app in ["effectcraft", "wordcraft"] {
|
||||
ev.push(Event::ReadyToInstall { app: app.into(), ready: true });
|
||||
}
|
||||
let (mut h, sent) = harness_with(ev, ViewMode::Table, egui::Theme::Dark, true, 1.0);
|
||||
h.run_steps(8);
|
||||
snapshot(&mut h, "table-retry-update-dark");
|
||||
sent.borrow_mut().clear();
|
||||
assert!(h.query_by_label("Update to 0.6.0").is_none());
|
||||
h.get_by_label("Retry update to 0.6.0").click();
|
||||
h.run_steps(2);
|
||||
assert_eq!(*sent.borrow(), [Command::Install { app: "effectcraft".into() }]);
|
||||
// WordCraft is up to date: its menu offers Reinstall, which uses the kept file.
|
||||
let row = h.get_all_by_label("WordCraft").next().unwrap().rect().center().y;
|
||||
h.get_all_by_label("…").find(|n| (n.rect().center().y - row).abs() < 25.0).unwrap().click();
|
||||
h.run_steps(3);
|
||||
h.get_by_label("Reinstall");
|
||||
assert!(h.query_by_label("Download again").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn table_view_and_light_theme() {
|
||||
let (mut h, sent) = harness(ViewMode::Table, egui::Theme::Light, true);
|
||||
|
|
|
|||
Loading…
Reference in New Issue