From 7fd60c58706d2ed7db8add8747fbb95668fac3de Mon Sep 17 00:00:00 2001 From: Jeremy Date: Thu, 29 Oct 2026 13:29:47 +0000 Subject: [PATCH] craft-update: a build from source is never offered the release it already contains Reported by FredApps: PhotoCraft built from main as 0.17.0-integration.20261029.f5b3f123 offered "Update and Restart" to 0.17.0, a downgrade. Semver puts a pre-release before its release, and the update check and the self-update guard compared with plain semver. New version::offers_update: a pre-release label the suite publishes (alpha, beta, rc, nightly) sorts as before; any other label marks a build from source, which is only offered a strictly higher X.Y.Z. check_for_update (through a new testable offered_release) and the self-update guard both use it. Co-Authored-By: Claude Opus 5.5 --- crates/update/src/lib.rs | 49 +++++++++++++++++++++++------- crates/update/src/self_update.rs | 20 ++++++++++++- crates/update/src/version.rs | 51 ++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 12 deletions(-) diff --git a/crates/update/src/lib.rs b/crates/update/src/lib.rs index d0fec496..d886a062 100644 --- a/crates/update/src/lib.rs +++ b/crates/update/src/lib.rs @@ -50,17 +50,7 @@ pub use dialog::{StartupPrompt, StartupPromptResponse}; pub fn check_for_update(app: &App, http: &Http, current_version: &str) -> Result, String> { let url = format!("https://api.github.com/repos/{}/releases?per_page=30", app.repo); match http.get_api(&url, None)? { - crate::http::Fetched::Body { text, .. } => { - let latest = crate::release::parse_github_release_list(&text, app.repo, false)?; - if let Some(release) = latest - && let Some(rel_v) = &release.version - && let Some(cur_v) = crate::version::parse(current_version) - && crate::version::is_newer(rel_v, &cur_v) - { - return Ok(Some(release)); - } - Ok(None) - } + crate::http::Fetched::Body { text, .. } => offered_release(&text, app.repo, current_version), crate::http::Fetched::RateLimited { .. } => Err("GitHub's request limit was reached".into()), crate::http::Fetched::NotModified | crate::http::Fetched::NotFound => Ok(None), } @@ -71,5 +61,42 @@ pub fn now() -> i64 { std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map_or(0, |d| i64::try_from(d.as_secs()).unwrap_or(i64::MAX)) } +/// The release in GitHub's release list `json` to offer a copy running `current_version`, if any: +/// the latest published release, when [`version::offers_update`] says so. +fn offered_release(json: &str, repo: &str, current_version: &str) -> Result, String> { + let latest = crate::release::parse_github_release_list(json, repo, false)?; + Ok(latest.filter(|release| { + let running = crate::version::parse(current_version); + release.version.as_ref().zip(running.as_ref()).is_some_and(|(rel_v, cur_v)| crate::version::offers_update(rel_v, cur_v)) + })) +} + #[cfg(test)] pub(crate) const TEST_AGENT: &str = "ArtCraft-Launcher-Test/0.0"; + +#[cfg(test)] +mod offer_tests { + use super::offered_release; + + const REPO: &str = "storytold/photocraft"; + const LIST: &str = r#"[{"tag_name":"v0.17.0","prerelease":false,"draft":false,"assets":[]}, + {"tag_name":"v0.16.4","prerelease":false,"draft":false,"assets":[]}]"#; + + fn offered(running: &str) -> Option { + offered_release(LIST, REPO, running).unwrap().map(|release| release.tag) + } + + #[test] + fn a_build_from_main_is_not_offered_the_release_it_already_contains() { + assert_eq!(offered("0.17.0-integration.20261029.f5b3f123"), None); + assert_eq!(offered("0.16.5-dev"), Some("v0.17.0".into())); + } + + #[test] + fn releases_and_release_candidates_are_offered_as_before() { + assert_eq!(offered("0.17.0-rc.2"), Some("v0.17.0".into())); + assert_eq!(offered("0.16.4"), Some("v0.17.0".into())); + assert_eq!(offered("0.17.0"), None); + assert_eq!(offered("not a version"), None); + } +} diff --git a/crates/update/src/self_update.rs b/crates/update/src/self_update.rs index f22dfb35..1793a5a5 100644 --- a/crates/update/src/self_update.rs +++ b/crates/update/src/self_update.rs @@ -305,8 +305,9 @@ pub fn update( ) -> Result<(), String> { let cur_v = running_version.and_then(|v| current_version(app, v)); if let (Some(rel_v), Some(cur_v)) = (release.version.as_ref(), cur_v.as_ref()) - && !version::is_newer(rel_v, cur_v) + && !version::offers_update(rel_v, cur_v) { + // A build from source is only replaced by a strictly higher release (see `offers_update`). return Err(format!("{} {} is not newer than currently running version {}", app.name, release.version_label(), cur_v)); } let (asset, _) = pick_asset(release, &[app.id], platform, &[install.kind()]) @@ -1738,6 +1739,23 @@ mod tests { assert!(result.unwrap_err().contains("is not newer than currently running version")); } + #[test] + fn a_build_from_main_refuses_to_install_the_release_it_already_contains() { + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(dir.path()); + let http = Http::new(crate::TEST_AGENT).unwrap(); + let mut phase = |_| (); + let last = || Ok(()); + let stop = Stop::default(); + let release = Release { version: version::parse("0.2.0"), tag: "v0.2.0".into(), ..Release::default() }; + let install = Install::AppImage(dir.path().join("launcher.AppImage")); + use crate::{Arch, Os}; + let platform = Platform { os: Os::Linux, arch: Arch::X64 }; + let running = Some("0.2.0-integration.20261029.f5b3f123"); + let result = update(&crate::suite::LAUNCHER, &install, &release, platform, &http, &paths, &stop, &mut phase, &last, running); + assert!(result.unwrap_err().contains("is not newer than currently running version")); + } + #[cfg(target_os = "linux")] #[test] fn parses_flatpak_metadata_for_restart() { diff --git a/crates/update/src/version.rs b/crates/update/src/version.rs index 0beceef1..1915f7b3 100644 --- a/crates/update/src/version.rs +++ b/crates/update/src/version.rs @@ -112,6 +112,28 @@ pub fn is_newer(latest: &Version, installed: &Version) -> bool { compare(latest, installed) == Ordering::Greater } +/// The pre-release labels the suite publishes, which sort against releases as semver says +/// (`0.17.0-rc.2` is before `0.17.0`). +const RELEASE_CHANNELS: [&str; 4] = ["alpha", "beta", "rc", "nightly"]; + +/// Whether `v` is a build made from source rather than a published release: it has a pre-release +/// label the suite never publishes (`0.17.0-integration.20261029.f5b3f123`, `0.18.0-dev`). +pub fn is_local_build(v: &Version) -> bool { + let label = v.pre.as_str().split('.').next().unwrap_or_default(); + !v.pre.is_empty() && !RELEASE_CHANNELS.iter().any(|channel| label.eq_ignore_ascii_case(channel)) +} + +/// Whether a copy running `running` should be offered `latest`. As [`is_newer`], except for a +/// local build: semver puts `0.17.0-integration…` before `0.17.0`, but a build from `main` already +/// holds 0.17.0's changes and more, so replacing it with 0.17.0 is a downgrade. A local build is +/// only offered a strictly higher `X.Y.Z`. +pub fn offers_update(latest: &Version, running: &Version) -> bool { + if is_local_build(running) { + return (latest.major, latest.minor, latest.patch) > (running.major, running.minor, running.patch); + } + is_newer(latest, running) +} + /// Platforms that only record `X.Y.Z` (the MSI ProductVersion, `CFBundleShortVersionString`) /// drop a pre-release suffix. A numeric-only installed version therefore counts as current when it /// matches a pre-release's numbers, rather than nagging forever. @@ -133,6 +155,35 @@ mod tests { parse(s).unwrap() } + #[test] + fn a_build_from_main_is_never_offered_its_own_release() { + let running = v("0.17.0-integration.20261029.f5b3f123"); + assert!(is_local_build(&running)); + assert!(!offers_update(&v("0.17.0"), &running)); + assert!(!offers_update(&v("0.16.4"), &running)); + assert!(!offers_update(&v("0.17.0-rc.3"), &running)); + assert!(offers_update(&v("0.17.1"), &running)); + assert!(offers_update(&v("0.18.0"), &running)); + assert!(offers_update(&v("1.0.0"), &v("0.18.0-dev"))); + assert!(!offers_update(&v("0.18.0"), &v("0.18.0-DEV.4"))); + } + + #[test] + fn published_pre_releases_and_releases_order_as_before() { + for label in ["0.17.0-rc.2", "0.17.0-beta.1", "0.17.0-alpha", "0.17.0-nightly.20261029", "0.17.0-RC.1"] { + let running = v(label); + assert!(!is_local_build(&running), "{label}"); + assert!(offers_update(&v("0.17.0"), &running), "{label}"); + } + assert!(offers_update(&v("0.17.0-rc.3"), &v("0.17.0-rc.2"))); + assert!(!offers_update(&v("0.17.0-rc.2"), &v("0.17.0-rc.2"))); + assert!(!is_local_build(&v("0.17.0"))); + assert!(offers_update(&v("0.17.1"), &v("0.17.0"))); + assert!(!offers_update(&v("0.17.0"), &v("0.17.0"))); + assert!(!offers_update(&v("0.16.9"), &v("0.17.0"))); + assert!(!offers_update(&v("0.17.0+build.5"), &v("0.17.0"))); + } + #[test] fn reads_tags_and_platform_spellings() { assert_eq!(v("v0.5.0"), Version::new(0, 5, 0));