From 2dbad1e5d09ed0f030919545d3e3081737332d0f Mon Sep 17 00:00:00 2001 From: tomasarrachea Date: Fri, 28 Aug 2026 18:31:56 -0300 Subject: [PATCH 1/5] fix: derive partial status from the install intent instead of the full channel --- docs/src/design/spec.md | 2 +- src/commands/list.rs | 8 ++++---- src/commands/show.rs | 6 +----- src/manifest/v3/channel.rs | 8 -------- src/state/installation.rs | 13 +++++++++++++ tests/operations.rs | 19 +++++++++++++++---- 6 files changed, 34 insertions(+), 22 deletions(-) diff --git a/docs/src/design/spec.md b/docs/src/design/spec.md index b3efe4a3..9936c224 100644 --- a/docs/src/design/spec.md +++ b/docs/src/design/spec.md @@ -558,7 +558,7 @@ the superset is a warning, and the component in the active view wins. ### 8.6 Partial status is derived -Local state does not record a "partial" flag. When the upstream manifest is available, an installation is displayed as partial if its installed component set is a proper subset of the channel's complete set. When upstream is unavailable, partial status is not displayed. +Local state does not record a "partial" flag. When the upstream manifest is available, an installation is displayed as partial if upstream resolves its recorded intent to components it does not hold — that is, the channel has grown within what the user asked for, not merely grown at all. An intent upstream can no longer resolve makes no claim, and when upstream is unavailable, partial status is not displayed. Components with no physical output (`command` with zero artifacts) count as installed for membership purposes and are exempt from physical verification (§9.6). diff --git a/src/commands/list.rs b/src/commands/list.rs index 9a51ec33..d81adac9 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -11,11 +11,11 @@ pub fn list(config: &Config, state: &LocalState) -> anyhow::Result<()> { let channel_name = &channel.name; // Partial status is *derived*, never recorded (spec section 8.6): an installation is - // partial exactly when it holds fewer components than the channel offers. A stored - // flag would be a second answer to a question the component set already answers, and - // the two would drift. + // partial exactly when upstream resolves its recorded intent to components it does + // not hold. A stored flag would be a second answer to a question the component set + // already answers, and the two would drift. let installed_indicator = match state.get(&channel.name) { - Some(installation) if installation.as_channel().is_partially_installed(channel) => { + Some(installation) if installation.is_partially_installed(channel) => { format!(" {}", "(partially installed)".yellow()) }, Some(_) => format!(" {}", "(installed)".green()), diff --git a/src/commands/show.rs b/src/commands/show.rs index 5d8a90a8..3fa74b56 100644 --- a/src/commands/show.rs +++ b/src/commands/show.rs @@ -117,11 +117,7 @@ impl ShowCommand { if let Some(manifest) = upstream { match manifest.get_channel_by_name(name) { - Some(channel) - if installation - .as_channel() - .is_partially_installed(channel) => - { + Some(channel) if installation.is_partially_installed(channel) => { line.push_str(&format!( " {}", "(partially installed)".yellow() diff --git a/src/manifest/v3/channel.rs b/src/manifest/v3/channel.rs index 8713f67b..af0edba1 100644 --- a/src/manifest/v3/channel.rs +++ b/src/manifest/v3/channel.rs @@ -64,14 +64,6 @@ impl Channel { self.components.iter_mut().find(|c| c.name == name) } - /// Whether this channel holds fewer components than the `upstream` one it is compared against. - /// - /// Derived, never recorded (spec section 8.6): a stored "partial" flag would be a second answer - /// to a question the component set already answers, and two answers can disagree. - pub fn is_partially_installed(&self, upstream: &Channel) -> bool { - self.components.len() < upstream.components.len() - } - pub fn get_channel_dir(&self, config: &Config) -> PathBuf { let installed_toolchains_dir = config.midenup_home.join("toolchains"); installed_toolchains_dir.join(format!("{}", self.name)) diff --git a/src/state/installation.rs b/src/state/installation.rs index 0fd254fd..fd279f94 100644 --- a/src/state/installation.rs +++ b/src/state/installation.rs @@ -120,4 +120,17 @@ impl Installation { pub fn as_channel(&self) -> crate::manifest::Channel { crate::manifest::Channel::new(self.channel.clone(), self.components.clone()) } + + /// Whether `upstream` resolves this record's intent to components it does not hold. + /// + /// Derived, never recorded (spec section 8.6). An intent upstream can no longer resolve makes + /// no claim: the installation holds everything it was asked for. + pub fn is_partially_installed(&self, upstream: &crate::manifest::Channel) -> bool { + let Ok(expected) = crate::resolve::resolve(upstream, &self.intent) else { + return false; + }; + let installed: std::collections::BTreeSet<&str> = + self.components.iter().map(|c| c.name.as_ref()).collect(); + expected.iter().any(|component| !installed.contains(component.name.as_ref())) + } } diff --git a/tests/operations.rs b/tests/operations.rs index 11b1b506..0776b8bb 100644 --- a/tests/operations.rs +++ b/tests/operations.rs @@ -587,9 +587,9 @@ fn integration_an_alias_conflict_inside_the_active_view_is_an_error() { ); } -/// Spec section 8.6: local state carries no partial flag. The display derives it by comparing what -/// is installed against the complete upstream channel -- and when upstream is unavailable, simply -/// does not show it rather than guessing. +/// Spec section 8.6: local state carries no partial flag. The display derives it by resolving the +/// recorded intent against upstream -- and when upstream is unavailable, simply does not show it +/// rather than guessing. #[test] fn integration_partial_status_is_derived_from_upstream_not_stored() { let _guard = common::harness::mutating_test_guard(); @@ -612,7 +612,18 @@ fn integration_partial_status_is_derived_from_upstream_not_stored() { let raw = std::fs::read_to_string(midenup::paths::state_path(&env.midenup_home)).unwrap(); assert!(!raw.contains("partial"), "state must not persist a partial flag: {raw}"); - let shown = midenup(&manifest, &["show", "list"]); + // `extra` belongs to no profile, so a minimal install that never asked for it is complete. + let complete = midenup(&manifest, &["show", "list"]); + assert!( + !String::from_utf8_lossy(&complete.stdout).contains("partially installed"), + "holding everything the intent resolves to is not partial: {}", + String::from_utf8_lossy(&complete.stdout) + ); + + // The minimal profile grows upstream, so the same intent now resolves to more than is held. + let grown = + fixture.manifest("grown.json", &[("vm", &["minimal"], &[]), ("extra", &["minimal"], &[])]); + let shown = midenup(&grown, &["show", "list"]); assert!( String::from_utf8_lossy(&shown.stdout).contains("partially installed"), "with upstream available it must be derived and shown: {}", From 4de43c71654627f2d6cc36eb198f88dee299c074 Mon Sep 17 00:00:00 2001 From: tomasarrachea Date: Tue, 1 Sep 2026 18:37:03 -0300 Subject: [PATCH 2/5] feat: detect if toolchain needs updating --- docs/src/design/spec.md | 6 +-- src/commands/list.rs | 14 ++++--- src/commands/show.rs | 39 +++++++++++++----- src/commands/update.rs | 29 +++++++++++++ src/state/installation.rs | 13 ------ tests/operations.rs | 86 ++++++++++++++++++++++++++++++++++----- 6 files changed, 143 insertions(+), 44 deletions(-) diff --git a/docs/src/design/spec.md b/docs/src/design/spec.md index a5702f4c..f8eb6965 100644 --- a/docs/src/design/spec.md +++ b/docs/src/design/spec.md @@ -556,9 +556,9 @@ The active view is a scoping and discovery mechanism, not a security boundary. **Alias conflicts are scoped to the view.** In v1, `Channel::get_aliases` would hard-error on any duplicate alias across the whole channel. Under a superset that accretes components from multiple projects, two components that no project ever activates together could collide and break *every* command. Therefore: a conflict *within the active view* is an error; a conflict that exists only in the superset is a warning, and the component in the active view wins. -### 8.6 Partial status is derived +### 8.6 Update status is derived -Local state does not record a "partial" flag. When the upstream manifest is available, an installation is displayed as partial if upstream resolves its recorded intent to components it does not hold — that is, the channel has grown within what the user asked for, not merely grown at all. An intent upstream can no longer resolve makes no claim, and when upstream is unavailable, partial status is not displayed. +Local state does not record an "update available" flag. When the upstream manifest is available, an installation is displayed as having an update if an update would change it or react to it: its recorded intent resolves to a different component set than it holds, the upstream definition of a component it holds has changed (§11.1's change classes, anything other than "nothing at all"), or its intent no longer resolves at all — the situation an update warns about (§11.3). The lookup follows migration lineage the way `update` does (§11.4): a superseded channel shows the update marker, not `(unavailable upstream)`. When upstream is unavailable, update status is not displayed. Components with no physical output (`command` with zero artifacts) count as installed for membership purposes and are exempt from physical verification (§9.6). @@ -569,7 +569,7 @@ upstream is unavailable the annotation is omitted entirely rather than derived l guess would be exactly the derivation networks exist to eliminate, and a stale one would tell a user they are on mainnet when they are not. The other markers are unchanged: `(needs reinstallation)` is derived from local state alone - a migrated record with no publication - and is always shown, while -`(partially installed)` and `(unavailable upstream)` need upstream and are omitted with it. +`(update available)` and `(unavailable upstream)` need upstream and are omitted with it. --- diff --git a/src/commands/list.rs b/src/commands/list.rs index 97fbb9c2..601866b9 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -11,13 +11,15 @@ pub fn list(config: &Config, state: &LocalState) -> anyhow::Result<()> { .map(|channel| { let channel_name = &channel.name; - // Partial status is *derived*, never recorded (spec section 8.6): an installation is - // partial exactly when upstream resolves its recorded intent to components it does - // not hold. A stored flag would be a second answer to a question the component set - // already answers, and the two would drift. + // Update status is *derived*, never recorded (spec section 8.6): an installation has + // an update exactly when re-resolving its recorded intent against upstream would + // change it. A stored flag would be a second answer to a question the manifest and + // the component set already answer, and the two would drift. let installed_indicator = match state.get(&channel.name) { - Some(installation) if installation.is_partially_installed(channel) => { - format!(" {}", "(partially installed)".yellow()) + Some(installation) + if super::update::needs_update(config, installation, channel) => + { + format!(" {}", "(update available)".yellow()) }, Some(_) => format!(" {}", "(installed)".green()), None => String::new(), diff --git a/src/commands/show.rs b/src/commands/show.rs index 54487a7b..534260c6 100644 --- a/src/commands/show.rs +++ b/src/commands/show.rs @@ -3,6 +3,7 @@ use colored::Colorize; use super::Flags; use crate::{ + channel::UpstreamMatch, config::Config, report, state::LocalState, @@ -65,8 +66,8 @@ impl ShowCommand { Self::List { .. } => { // Installed toolchains are recorded locally, so this works with no network at all. // Upstream only adds *markers* -- which networks name a channel, which - // installations are partial or no longer published -- so when it is unavailable - // they are simply omitted rather than guessed at. + // installations have updates or are no longer published -- so when it is + // unavailable they are simply omitted rather than guessed at. let upstream = config.upstream_manifest().ok(); // The upstream lookup may have emitted a report to stderr, so restore stdout's // color policy immediately before rendering the result. @@ -141,17 +142,33 @@ impl ShowCommand { write!(&mut line, " -- run `midenup install {name}`").unwrap(); } - if let Some(manifest) = upstream { - match manifest.get_channel_by_name(name) { - Some(channel) if installation.is_partially_installed(channel) => { - if use_color { - write!(&mut line, " {}", "(partially installed)".yellow()) - .unwrap(); - } else { - line.push_str(" (partially installed)"); + if upstream.is_some() { + // The lookup follows migration lineage the way `update` does, so a + // superseded channel shows as updatable rather than gone. + match installation.as_channel().find_upstream_counterpart(config) { + Some(counterpart) => { + let has_update = match counterpart.upstream_match { + // Being superseded is the update: running it migrates. + UpstreamMatch::Migrated { .. } => true, + UpstreamMatch::UpstreamCounterpart => { + super::update::needs_update( + config, + installation, + &counterpart.channel, + ) + }, + }; + if has_update { + let marker = format!( + "(update available) -- run `midenup update {name}`" + ); + if use_color { + write!(&mut line, " {}", marker.yellow()).unwrap(); + } else { + write!(&mut line, " {marker}").unwrap(); + } } }, - Some(_) => {}, // Retained, not deleted: the user may still want `var/` and an // explicit uninstall (spec section 12.3). None if use_color => { diff --git a/src/commands/update.rs b/src/commands/update.rs index 63f469df..22fe1fd5 100644 --- a/src/commands/update.rs +++ b/src/commands/update.rs @@ -409,6 +409,35 @@ fn work_for( Ok(Work::Nothing) } +/// Whether an update against `upstream` would change this installation: its recorded intent +/// resolves to a different component set, or a component it holds changed upstream (spec 8.6). +pub fn needs_update(config: &Config, installation: &Installation, upstream: &Channel) -> bool { + match crate::resolve::resolve(upstream, &installation.intent) { + Ok(resolved) => { + let installed_names: std::collections::BTreeSet<&str> = installation + .components + .iter() + .map(|component| component.name.as_ref()) + .collect(); + let resolved_names: std::collections::BTreeSet<&str> = + resolved.iter().map(|component| component.name.as_ref()).collect(); + if installed_names != resolved_names { + return true; + } + }, + // An intent upstream can no longer resolve -- an explicit root removed, say -- is exactly + // what an update warns about (spec section 11.3), so it needs the user's attention. + Err(_) => return true, + } + + installation.components.iter().any(|installed| { + upstream.get_component(&installed.name).is_some_and(|new| { + classify(installed, new, config.target(), &config.working_directory) + != ChangeClass::None + }) + }) +} + /// Commits selection and metadata changes that no installed file reflects. /// /// Each component's recorded *authority* is preserved rather than taken from upstream. Reaching diff --git a/src/state/installation.rs b/src/state/installation.rs index fd279f94..0fd254fd 100644 --- a/src/state/installation.rs +++ b/src/state/installation.rs @@ -120,17 +120,4 @@ impl Installation { pub fn as_channel(&self) -> crate::manifest::Channel { crate::manifest::Channel::new(self.channel.clone(), self.components.clone()) } - - /// Whether `upstream` resolves this record's intent to components it does not hold. - /// - /// Derived, never recorded (spec section 8.6). An intent upstream can no longer resolve makes - /// no claim: the installation holds everything it was asked for. - pub fn is_partially_installed(&self, upstream: &crate::manifest::Channel) -> bool { - let Ok(expected) = crate::resolve::resolve(upstream, &self.intent) else { - return false; - }; - let installed: std::collections::BTreeSet<&str> = - self.components.iter().map(|c| c.name.as_ref()).collect(); - expected.iter().any(|component| !installed.contains(component.name.as_ref())) - } } diff --git a/tests/operations.rs b/tests/operations.rs index 0776b8bb..9b741679 100644 --- a/tests/operations.rs +++ b/tests/operations.rs @@ -153,6 +153,28 @@ impl Fixture { self.write(file, components.iter().map(|spec| self.component(*spec)).collect()) } + /// A manifest whose only channel supersedes `0.15.0` via `migrates_from`, so an installation + /// of 0.15.0 has no same-version counterpart upstream. + fn manifest_migrated(&self, file: &str, components: &[Spec<'_>]) -> String { + let manifest = serde_json::json!({ + "manifest_version": "3.0.0", + "date": 1735689600, + "networks": {"mainnet": "0.16.0"}, + "channels": [{ + "name": "0.16.0", + "migrates_from": "0.15.0", + "components": components + .iter() + .map(|spec| self.component(*spec)) + .collect::>() + }] + }); + + let path = self.dir.join(file); + std::fs::write(&path, serde_json::to_string_pretty(&manifest).unwrap()).unwrap(); + format!("file://{}", path.display()) + } + fn write(&self, file: &str, components: Vec) -> String { let manifest = serde_json::json!({ "manifest_version": "3.0.0", @@ -587,13 +609,13 @@ fn integration_an_alias_conflict_inside_the_active_view_is_an_error() { ); } -/// Spec section 8.6: local state carries no partial flag. The display derives it by resolving the -/// recorded intent against upstream -- and when upstream is unavailable, simply does not show it -/// rather than guessing. +/// Spec section 8.6: local state carries no update-available flag. The display derives it by +/// re-resolving the recorded intent and diffing component definitions against upstream -- and when +/// upstream is unavailable, simply does not show it rather than guessing. #[test] -fn integration_partial_status_is_derived_from_upstream_not_stored() { +fn integration_update_status_is_derived_from_upstream_not_stored() { let _guard = common::harness::mutating_test_guard(); - // Not named "partial": the temp directory path ends up inside state.json, in artifact URIs. + // Not named "update": the temp directory path ends up inside state.json, in artifact URIs. let env = environment_setup("derived_status"); let fixture = Fixture::new(env.tmp_dir.path()); let manifest = @@ -610,22 +632,36 @@ fn integration_partial_status_is_derived_from_upstream_not_stored() { assert!(installed.status.success(), "{}", String::from_utf8_lossy(&installed.stderr)); let raw = std::fs::read_to_string(midenup::paths::state_path(&env.midenup_home)).unwrap(); - assert!(!raw.contains("partial"), "state must not persist a partial flag: {raw}"); + assert!(!raw.contains("update"), "state must not persist an update flag: {raw}"); - // `extra` belongs to no profile, so a minimal install that never asked for it is complete. + // `extra` belongs to no profile, so a minimal install that never asked for it is up to date. let complete = midenup(&manifest, &["show", "list"]); assert!( - !String::from_utf8_lossy(&complete.stdout).contains("partially installed"), - "holding everything the intent resolves to is not partial: {}", + !String::from_utf8_lossy(&complete.stdout).contains("update available"), + "an installation an update would not change has no update: {}", String::from_utf8_lossy(&complete.stdout) ); + // A definition change to a held component -- an alias, the canonical metadata-only change -- + // is an update even though the component set is unchanged. + let aliased = fixture.manifest_with_vm_alias( + "aliased.json", + &[("vm", &["minimal"], &[]), ("extra", &[], &[])], + "vm-alias", + ); + let changed = midenup(&aliased, &["show", "list"]); + assert!( + String::from_utf8_lossy(&changed.stdout).contains("update available"), + "a changed component definition upstream must be shown: {}", + String::from_utf8_lossy(&changed.stdout) + ); + // The minimal profile grows upstream, so the same intent now resolves to more than is held. let grown = fixture.manifest("grown.json", &[("vm", &["minimal"], &[]), ("extra", &["minimal"], &[])]); let shown = midenup(&grown, &["show", "list"]); assert!( - String::from_utf8_lossy(&shown.stdout).contains("partially installed"), + String::from_utf8_lossy(&shown.stdout).contains("update available"), "with upstream available it must be derived and shown: {}", String::from_utf8_lossy(&shown.stdout) ); @@ -635,6 +671,31 @@ fn integration_partial_status_is_derived_from_upstream_not_stored() { String::from_utf8_lossy(&shown.stdout) ); + // A superseded channel: the update *is* the migration, so it shows as updatable rather than + // as unavailable upstream. + let migrated = fixture.manifest_migrated("migrated.json", &[("vm", &["minimal"], &[])]); + let superseded = midenup(&migrated, &["show", "list"]); + assert!( + String::from_utf8_lossy(&superseded.stdout).contains("update available"), + "a superseded channel must show as updatable: {}", + String::from_utf8_lossy(&superseded.stdout) + ); + + // An explicit root removed upstream makes the intent unresolvable, which an update would warn + // about (spec section 11.3) -- so it needs the user's attention too. + let rooted = midenup( + &manifest, + &["install", "0.15.0", "--profile", "minimal", "--component", "extra"], + ); + assert!(rooted.status.success(), "{}", String::from_utf8_lossy(&rooted.stderr)); + let shrunk = fixture.manifest("shrunk.json", &[("vm", &["minimal"], &[])]); + let dangling = midenup(&shrunk, &["show", "list"]); + assert!( + String::from_utf8_lossy(&dangling.stdout).contains("update available"), + "an unresolvable intent must be shown: {}", + String::from_utf8_lossy(&dangling.stdout) + ); + // With upstream unavailable it is not shown -- and never guessed at. The cached manifest would // answer, so it goes too. std::fs::remove_file(midenup::paths::manifest_cache(&env.midenup_home)).unwrap(); @@ -646,6 +707,9 @@ fn integration_partial_status_is_derived_from_upstream_not_stored() { stdout.contains("0.15.0"), "the installed channel must still be listed: {stdout}" ); - assert!(!stdout.contains("partial"), "but its partial status must not be: {stdout}"); + assert!( + !stdout.contains("update available"), + "but its update status must not be: {stdout}" + ); assert!(!stdout.contains("mainnet"), "nor the networks naming it: {stdout}"); } From 0d8d6a391b2dc020f9f2f945d6a2042a724df8ee Mon Sep 17 00:00:00 2001 From: tomasarrachea Date: Wed, 2 Sep 2026 12:44:49 -0300 Subject: [PATCH 3/5] feat: derive update-available marker from manifest --- docs/src/design/spec.md | 4 +- src/commands/list.rs | 13 ++- src/commands/show.rs | 69 ++++++++------- src/commands/update.rs | 92 +++++++++++++++++--- src/manifest/v3/channel.rs | 22 ++--- tests/operations.rs | 168 +++++++++++++++++++++++++------------ 6 files changed, 256 insertions(+), 112 deletions(-) diff --git a/docs/src/design/spec.md b/docs/src/design/spec.md index f8eb6965..c8b71c3d 100644 --- a/docs/src/design/spec.md +++ b/docs/src/design/spec.md @@ -558,7 +558,9 @@ the superset is a warning, and the component in the active view wins. ### 8.6 Update status is derived -Local state does not record an "update available" flag. When the upstream manifest is available, an installation is displayed as having an update if an update would change it or react to it: its recorded intent resolves to a different component set than it holds, the upstream definition of a component it holds has changed (§11.1's change classes, anything other than "nothing at all"), or its intent no longer resolves at all — the situation an update warns about (§11.3). The lookup follows migration lineage the way `update` does (§11.4): a superseded channel shows the update marker, not `(unavailable upstream)`. When upstream is unavailable, update status is not displayed. +Local state does not record an "update available" flag. When the upstream manifest is available, an installation is displayed as having an update if the manifest's content changed for it. + +The comparison is manifest content alone. The pins install records on an authority — a branch's latest commit, a path's modification time — exist only locally and move on their own, so they are excluded: drift behind a pin is reconciled by `update` itself, which is what pinning is for, and a listing performs no source or network I/O beyond the manifest fetch. Components with no physical output (`command` with zero artifacts) count as installed for membership purposes and are exempt from physical verification (§9.6). diff --git a/src/commands/list.rs b/src/commands/list.rs index 601866b9..0c8564c6 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -16,12 +16,19 @@ pub fn list(config: &Config, state: &LocalState) -> anyhow::Result<()> { // change it. A stored flag would be a second answer to a question the manifest and // the component set already answer, and the two would drift. let installed_indicator = match state.get(&channel.name) { - Some(installation) - if super::update::needs_update(config, installation, channel) => - { + Some(installation) if super::update::needs_update(installation, channel) => { format!(" {}", "(update available)".yellow()) }, Some(_) => format!(" {}", "(installed)".green()), + // A channel that supersedes an installed one (spec section 11.4): updating the + // predecessor migrates to it. + None if channel + .migrates_from + .as_ref() + .is_some_and(|old| state.get(old).is_some()) => + { + format!(" {}", "(update available)".yellow()) + }, None => String::new(), }; diff --git a/src/commands/show.rs b/src/commands/show.rs index 534260c6..9846a03e 100644 --- a/src/commands/show.rs +++ b/src/commands/show.rs @@ -10,6 +10,17 @@ use crate::{ toolchain::{Toolchain, ToolchainJustification}, }; +/// Appends a status marker to a listing row: yellow as one span when color is on, plain otherwise. +fn push_marker(line: &mut String, text: &str, use_color: bool) { + use core::fmt::Write; + + if use_color { + write!(line, " {}", text.yellow()).unwrap(); + } else { + write!(line, " {text}").unwrap(); + } +} + #[derive(Debug, Subcommand)] pub enum ShowCommand { /// Show the active toolchain. @@ -113,17 +124,12 @@ impl ShowCommand { if linked != name { continue; } - if let Some(marker) = crate::networks::drift( + if let Some(notice) = crate::networks::drift( network, linked, manifest.network_version(network), ) { - if use_color { - write!(&mut line, " {}", marker.yellow()).unwrap(); - } else { - line.push(' '); - line.push_str(&marker); - } + push_marker(&mut line, ¬ice, use_color); } } } @@ -133,49 +139,42 @@ impl ShowCommand { // point: the user's toolchain still works, but only after it is installed // properly, and they should not have to infer that from a failure. if !installation.is_managed() { - if use_color { - write!(&mut line, " {}", "(needs reinstallation)".yellow()) - .unwrap(); - } else { - line.push_str(" (needs reinstallation)"); - } - write!(&mut line, " -- run `midenup install {name}`").unwrap(); + push_marker( + &mut line, + &format!("(needs reinstallation) -- run `midenup install {name}`"), + use_color, + ); } if upstream.is_some() { // The lookup follows migration lineage the way `update` does, so a - // superseded channel shows as updatable rather than gone. - match installation.as_channel().find_upstream_counterpart(config) { - Some(counterpart) => { - let has_update = match counterpart.upstream_match { + // superseded channel shows as updatable rather than gone. Verbatim + // manifest content: a listing pins nothing and reaches for no source. + match installation.as_channel().upstream_counterpart_raw(config) { + // An unmanaged record already carries its one instruction, the + // reinstall above; a second directive would contradict it. + Some(_) if !installation.is_managed() => {}, + Some((channel, upstream_match)) => { + let has_update = match upstream_match { // Being superseded is the update: running it migrates. UpstreamMatch::Migrated { .. } => true, UpstreamMatch::UpstreamCounterpart => { - super::update::needs_update( - config, - installation, - &counterpart.channel, - ) + super::update::needs_update(installation, channel) }, }; if has_update { - let marker = format!( - "(update available) -- run `midenup update {name}`" + push_marker( + &mut line, + &format!( + "(update available) -- run `midenup update {name}`" + ), + use_color, ); - if use_color { - write!(&mut line, " {}", marker.yellow()).unwrap(); - } else { - write!(&mut line, " {marker}").unwrap(); - } } }, // Retained, not deleted: the user may still want `var/` and an // explicit uninstall (spec section 12.3). - None if use_color => { - write!(&mut line, " {}", "(unavailable upstream)".yellow()) - .unwrap() - }, - None => line.push_str(" (unavailable upstream)"), + None => push_marker(&mut line, "(unavailable upstream)", use_color), } } diff --git a/src/commands/update.rs b/src/commands/update.rs index 22fe1fd5..f611efb0 100644 --- a/src/commands/update.rs +++ b/src/commands/update.rs @@ -20,7 +20,7 @@ use crate::{ manifest::Component, options::{InstallationOptions, IntentUpdate, PathUpdate, UpdateOptions}, state::{Installation, LocalState}, - version::Authority, + version::{Authority, GitTarget}, }; /// Updates installed toolchains. @@ -409,9 +409,8 @@ fn work_for( Ok(Work::Nothing) } -/// Whether an update against `upstream` would change this installation: its recorded intent -/// resolves to a different component set, or a component it holds changed upstream (spec 8.6). -pub fn needs_update(config: &Config, installation: &Installation, upstream: &Channel) -> bool { +/// Whether the manifest's content changed for this installation. +pub fn needs_update(installation: &Installation, upstream: &Channel) -> bool { match crate::resolve::resolve(upstream, &installation.intent) { Ok(resolved) => { let installed_names: std::collections::BTreeSet<&str> = installation @@ -425,19 +424,41 @@ pub fn needs_update(config: &Config, installation: &Installation, upstream: &Cha return true; } }, - // An intent upstream can no longer resolve -- an explicit root removed, say -- is exactly - // what an update warns about (spec section 11.3), so it needs the user's attention. Err(_) => return true, } installation.components.iter().any(|installed| { - upstream.get_component(&installed.name).is_some_and(|new| { - classify(installed, new, config.target(), &config.working_directory) - != ChangeClass::None - }) + upstream + .get_component(&installed.name) + .is_some_and(|new| definition_changed(installed, new)) }) } +/// Whether two definitions of one component differ as manifest content. +/// +/// The pins install records on an authority -- a branch's commit, a path's modification time -- +/// exist only locally and move on their own, so they are normalized away: drift behind them is +/// reconciled by the update itself, which is what pins for ([`classify`]); a listing must not. +fn definition_changed(installed: &Component, upstream: &Component) -> bool { + let normalized = |component: &Component| { + let mut component = component.clone(); + match &mut component.version { + Authority::Path { last_modification, .. } => *last_modification = None, + Authority::Git { + target: GitTarget::Branch { latest_revision, .. }, + .. + } => *latest_revision = None, + Authority::Git { .. } | Authority::Registry { .. } => (), + } + serde_json::to_value(component).ok() + }; + + match (normalized(installed), normalized(upstream)) { + (Some(old), Some(new)) => old != new, + _ => true, + } +} + /// Commits selection and metadata changes that no installed file reflects. /// /// Each component's recorded *authority* is preserved rather than taken from upstream. Reaching @@ -701,6 +722,57 @@ mod tests { assert_eq!(classify_pair(base(), base()), ChangeClass::None); } + /// The pins install records -- a branch's commit, a path's modification time -- exist only + /// locally and move on their own, so a listing must not read them as a manifest change. + #[test] + fn an_authority_pin_is_not_a_definition_change() { + let on_branch = |latest_revision: Option<&str>| { + let mut component = base(); + component.version = Authority::Git { + repository_url: "https://example.invalid/repo".to_string(), + subpath: None, + target: GitTarget::Branch { + name: "main".to_string(), + latest_revision: latest_revision.map(str::to_string), + }, + }; + component + }; + assert!(!definition_changed(&on_branch(Some("abc123")), &on_branch(None))); + + let at_path = |last_modification: Option| { + let mut component = base(); + component.version = Authority::Path { path: "vm".into(), last_modification }; + component + }; + let pinned = at_path(Some(std::time::SystemTime::UNIX_EPOCH)); + assert!(!definition_changed(&pinned, &at_path(None))); + } + + /// Normalizing the pin must not mask a real change riding alongside it. + #[test] + fn a_change_next_to_a_pin_is_still_a_definition_change() { + let mut installed = base(); + installed.version = Authority::Git { + repository_url: "https://example.invalid/repo".to_string(), + subpath: None, + target: GitTarget::Branch { + name: "main".to_string(), + latest_revision: Some("abc123".to_string()), + }, + }; + let mut upstream = base(); + upstream.version = Authority::Git { + repository_url: "https://example.invalid/repo".to_string(), + subpath: None, + target: GitTarget::Branch { + name: "next".to_string(), + latest_revision: None, + }, + }; + assert!(definition_changed(&installed, &upstream)); + } + /// An artifact is a file in the publication, so a component whose artifact URI moves to a new /// release must be reinstalled even though nothing else about it changed. #[test] diff --git a/src/manifest/v3/channel.rs b/src/manifest/v3/channel.rs index af0edba1..f1ded2d1 100644 --- a/src/manifest/v3/channel.rs +++ b/src/manifest/v3/channel.rs @@ -158,27 +158,29 @@ impl Channel { /// channel>`. A same-version match wins: a channel that still exists upstream is not migrated /// away from, whatever some other channel claims to supersede. pub fn find_upstream_counterpart(&self, config: &Config) -> Option { + let (channel, upstream_match) = self.upstream_counterpart_raw(config)?; + Some(UpstreamChannel::new(channel.clone(), upstream_match, config)) + } + + /// As [`Self::find_upstream_counterpart`], but returning the manifest's definitions verbatim: + /// no sync, so no source or network I/O beyond the manifest itself. + pub fn upstream_counterpart_raw<'a>( + &self, + config: &'a Config, + ) -> Option<(&'a Channel, UpstreamMatch)> { let upstream_manifest = config.upstream_manifest().ok()?; if let Some(same_version) = upstream_manifest.get_channels().find(|upstream| upstream.name == self.name) { - return Some(UpstreamChannel::new( - same_version.clone(), - UpstreamMatch::UpstreamCounterpart, - config, - )); + return Some((same_version, UpstreamMatch::UpstreamCounterpart)); } let successor = upstream_manifest .get_channels() .find(|upstream| upstream.migrates_from.as_ref() == Some(&self.name))?; - Some(UpstreamChannel::new( - successor.clone(), - UpstreamMatch::Migrated { old_channel: self.name.clone() }, - config, - )) + Some((successor, UpstreamMatch::Migrated { old_channel: self.name.clone() })) } // Syncs the channel to the latest changes diff --git a/tests/operations.rs b/tests/operations.rs index 9b741679..8da38702 100644 --- a/tests/operations.rs +++ b/tests/operations.rs @@ -609,57 +609,106 @@ fn integration_an_alias_conflict_inside_the_active_view_is_an_error() { ); } -/// Spec section 8.6: local state carries no update-available flag. The display derives it by -/// re-resolving the recorded intent and diffing component definitions against upstream -- and when -/// upstream is unavailable, simply does not show it rather than guessing. +/// Runs the midenup binary against `env` and `manifest_uri`. +fn midenup_run(env: &TestEnvironment, manifest_uri: &str, args: &[&str]) -> std::process::Output { + midenup_command(env!("CARGO_BIN_EXE_midenup"), env, manifest_uri) + .args(args) + .output() + .expect("failed to run midenup") +} + +/// Spec section 8.6: local state carries no update-available flag, and with no manifest at all the +/// status is omitted. #[test] -fn integration_update_status_is_derived_from_upstream_not_stored() { +fn integration_update_status_is_not_stored() { let _guard = common::harness::mutating_test_guard(); // Not named "update": the temp directory path ends up inside state.json, in artifact URIs. - let env = environment_setup("derived_status"); + let env = environment_setup("derived_clean"); let fixture = Fixture::new(env.tmp_dir.path()); let manifest = fixture.manifest("manifest.json", &[("vm", &["minimal"], &[]), ("extra", &[], &[])]); - let midenup = |manifest_uri: &str, args: &[&str]| { - midenup_command(env!("CARGO_BIN_EXE_midenup"), &env, manifest_uri) - .args(args) - .output() - .expect("failed to run midenup") - }; - - let installed = midenup(&manifest, &["install", "0.15.0", "--profile", "minimal"]); + let installed = midenup_run(&env, &manifest, &["install", "0.15.0", "--profile", "minimal"]); assert!(installed.status.success(), "{}", String::from_utf8_lossy(&installed.stderr)); let raw = std::fs::read_to_string(midenup::paths::state_path(&env.midenup_home)).unwrap(); assert!(!raw.contains("update"), "state must not persist an update flag: {raw}"); // `extra` belongs to no profile, so a minimal install that never asked for it is up to date. - let complete = midenup(&manifest, &["show", "list"]); + let complete = midenup_run(&env, &manifest, &["show", "list"]); assert!( !String::from_utf8_lossy(&complete.stdout).contains("update available"), "an installation an update would not change has no update: {}", String::from_utf8_lossy(&complete.stdout) ); - // A definition change to a held component -- an alias, the canonical metadata-only change -- - // is an update even though the component set is unchanged. + // With upstream unavailable it is not shown. The cached manifest would answer, so it goes too. + std::fs::remove_file(midenup::paths::manifest_cache(&env.midenup_home)).unwrap(); + let offline = midenup_run(&env, "https://127.0.0.1:1/nope.json", &["show", "list"]); + assert!(offline.status.success(), "listing what is installed must work offline"); + + let stdout = String::from_utf8_lossy(&offline.stdout); + assert!( + stdout.contains("0.15.0"), + "the installed channel must still be listed: {stdout}" + ); + assert!( + !stdout.contains("update available"), + "but its update status must not be: {stdout}" + ); + assert!(!stdout.contains("mainnet"), "nor the networks naming it: {stdout}"); +} + +/// A definition change to a held component -- an alias, the canonical metadata-only change -- is +/// an update even though the component set is unchanged, in `show list` and `list` alike. +#[test] +fn integration_update_status_shows_definition_changes() { + let _guard = common::harness::mutating_test_guard(); + let env = environment_setup("derived_defchange"); + let fixture = Fixture::new(env.tmp_dir.path()); + let manifest = + fixture.manifest("manifest.json", &[("vm", &["minimal"], &[]), ("extra", &[], &[])]); + + let installed = midenup_run(&env, &manifest, &["install", "0.15.0", "--profile", "minimal"]); + assert!(installed.status.success(), "{}", String::from_utf8_lossy(&installed.stderr)); + + let unchanged = midenup_run(&env, &manifest, &["list"]); + assert!( + String::from_utf8_lossy(&unchanged.stdout).contains("(installed)"), + "an unchanged installation lists as installed: {}", + String::from_utf8_lossy(&unchanged.stdout) + ); + let aliased = fixture.manifest_with_vm_alias( "aliased.json", &[("vm", &["minimal"], &[]), ("extra", &[], &[])], "vm-alias", ); - let changed = midenup(&aliased, &["show", "list"]); - assert!( - String::from_utf8_lossy(&changed.stdout).contains("update available"), - "a changed component definition upstream must be shown: {}", - String::from_utf8_lossy(&changed.stdout) - ); + for command in [["show", "list"].as_slice(), ["list"].as_slice()] { + let shown = midenup_run(&env, &aliased, command); + assert!( + String::from_utf8_lossy(&shown.stdout).contains("update available"), + "a changed component definition upstream must be shown by {command:?}: {}", + String::from_utf8_lossy(&shown.stdout) + ); + } +} + +/// The minimal profile grows upstream, so the same intent resolves to more than is held. +#[test] +fn integration_update_status_shows_profile_growth() { + let _guard = common::harness::mutating_test_guard(); + let env = environment_setup("derived_growth"); + let fixture = Fixture::new(env.tmp_dir.path()); + let manifest = + fixture.manifest("manifest.json", &[("vm", &["minimal"], &[]), ("extra", &[], &[])]); + + let installed = midenup_run(&env, &manifest, &["install", "0.15.0", "--profile", "minimal"]); + assert!(installed.status.success(), "{}", String::from_utf8_lossy(&installed.stderr)); - // The minimal profile grows upstream, so the same intent now resolves to more than is held. let grown = fixture.manifest("grown.json", &[("vm", &["minimal"], &[]), ("extra", &["minimal"], &[])]); - let shown = midenup(&grown, &["show", "list"]); + let shown = midenup_run(&env, &grown, &["show", "list"]); assert!( String::from_utf8_lossy(&shown.stdout).contains("update available"), "with upstream available it must be derived and shown: {}", @@ -670,46 +719,59 @@ fn integration_update_status_is_derived_from_upstream_not_stored() { "and so must the networks naming the channel: {}", String::from_utf8_lossy(&shown.stdout) ); +} + +/// A superseded channel: the update *is* the migration, so both listings show it as updatable +/// rather than as unavailable or absent. +#[test] +fn integration_update_status_follows_migration_lineage() { + let _guard = common::harness::mutating_test_guard(); + let env = environment_setup("derived_lineage"); + let fixture = Fixture::new(env.tmp_dir.path()); + let manifest = fixture.manifest("manifest.json", &[("vm", &["minimal"], &[])]); + + let installed = midenup_run(&env, &manifest, &["install", "0.15.0", "--profile", "minimal"]); + assert!(installed.status.success(), "{}", String::from_utf8_lossy(&installed.stderr)); - // A superseded channel: the update *is* the migration, so it shows as updatable rather than - // as unavailable upstream. let migrated = fixture.manifest_migrated("migrated.json", &[("vm", &["minimal"], &[])]); - let superseded = midenup(&migrated, &["show", "list"]); + let shown = midenup_run(&env, &migrated, &["show", "list"]); assert!( - String::from_utf8_lossy(&superseded.stdout).contains("update available"), + String::from_utf8_lossy(&shown.stdout).contains("update available"), "a superseded channel must show as updatable: {}", - String::from_utf8_lossy(&superseded.stdout) + String::from_utf8_lossy(&shown.stdout) ); - // An explicit root removed upstream makes the intent unresolvable, which an update would warn - // about (spec section 11.3) -- so it needs the user's attention too. - let rooted = midenup( - &manifest, - &["install", "0.15.0", "--profile", "minimal", "--component", "extra"], - ); - assert!(rooted.status.success(), "{}", String::from_utf8_lossy(&rooted.stderr)); - let shrunk = fixture.manifest("shrunk.json", &[("vm", &["minimal"], &[])]); - let dangling = midenup(&shrunk, &["show", "list"]); + // `list` shows the successor channel, marked because updating the predecessor lands on it. + let listed = midenup_run(&env, &migrated, &["list"]); + let stdout = String::from_utf8_lossy(&listed.stdout); assert!( - String::from_utf8_lossy(&dangling.stdout).contains("update available"), - "an unresolvable intent must be shown: {}", - String::from_utf8_lossy(&dangling.stdout) + stdout.contains("0.16.0") && stdout.contains("update available"), + "the successor must be listed as the pending update: {stdout}" ); +} - // With upstream unavailable it is not shown -- and never guessed at. The cached manifest would - // answer, so it goes too. - std::fs::remove_file(midenup::paths::manifest_cache(&env.midenup_home)).unwrap(); - let offline = midenup("https://127.0.0.1:1/nope.json", &["show", "list"]); - assert!(offline.status.success(), "listing what is installed must work offline"); +/// An explicit root removed upstream makes the intent unresolvable, which still shows as an +/// update: running it reports the missing root (spec section 11.3) rather than dropping it. +#[test] +fn integration_update_status_shows_an_unresolvable_intent() { + let _guard = common::harness::mutating_test_guard(); + let env = environment_setup("derived_dangling"); + let fixture = Fixture::new(env.tmp_dir.path()); + let manifest = + fixture.manifest("manifest.json", &[("vm", &["minimal"], &[]), ("extra", &[], &[])]); - let stdout = String::from_utf8_lossy(&offline.stdout); - assert!( - stdout.contains("0.15.0"), - "the installed channel must still be listed: {stdout}" + let installed = midenup_run( + &env, + &manifest, + &["install", "0.15.0", "--profile", "minimal", "--component", "extra"], ); + assert!(installed.status.success(), "{}", String::from_utf8_lossy(&installed.stderr)); + + let shrunk = fixture.manifest("shrunk.json", &[("vm", &["minimal"], &[])]); + let shown = midenup_run(&env, &shrunk, &["show", "list"]); + let stdout = String::from_utf8_lossy(&shown.stdout); assert!( - !stdout.contains("update available"), - "but its update status must not be: {stdout}" + stdout.contains("update available"), + "an unresolvable intent must be shown as an update: {stdout}" ); - assert!(!stdout.contains("mainnet"), "nor the networks naming it: {stdout}"); } From e0963c8900921e8a38b89638f9f4c157057124a1 Mon Sep 17 00:00:00 2001 From: tomasarrachea Date: Fri, 4 Sep 2026 16:06:33 -0300 Subject: [PATCH 4/5] fix: compare relative path authorities in their installed absolute form --- src/commands/list.rs | 4 +++- src/commands/show.rs | 6 +++++- src/commands/update.rs | 37 ++++++++++++++++++++++++++++++------- 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/commands/list.rs b/src/commands/list.rs index 0c8564c6..1ca88ea4 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -16,7 +16,9 @@ pub fn list(config: &Config, state: &LocalState) -> anyhow::Result<()> { // change it. A stored flag would be a second answer to a question the manifest and // the component set already answer, and the two would drift. let installed_indicator = match state.get(&channel.name) { - Some(installation) if super::update::needs_update(installation, channel) => { + Some(installation) + if super::update::needs_update(config, installation, channel) => + { format!(" {}", "(update available)".yellow()) }, Some(_) => format!(" {}", "(installed)".green()), diff --git a/src/commands/show.rs b/src/commands/show.rs index 9846a03e..12ac0699 100644 --- a/src/commands/show.rs +++ b/src/commands/show.rs @@ -159,7 +159,11 @@ impl ShowCommand { // Being superseded is the update: running it migrates. UpstreamMatch::Migrated { .. } => true, UpstreamMatch::UpstreamCounterpart => { - super::update::needs_update(installation, channel) + super::update::needs_update( + config, + installation, + channel, + ) }, }; if has_update { diff --git a/src/commands/update.rs b/src/commands/update.rs index f611efb0..443f3af4 100644 --- a/src/commands/update.rs +++ b/src/commands/update.rs @@ -410,7 +410,7 @@ fn work_for( } /// Whether the manifest's content changed for this installation. -pub fn needs_update(installation: &Installation, upstream: &Channel) -> bool { +pub fn needs_update(config: &Config, installation: &Installation, upstream: &Channel) -> bool { match crate::resolve::resolve(upstream, &installation.intent) { Ok(resolved) => { let installed_names: std::collections::BTreeSet<&str> = installation @@ -430,7 +430,7 @@ pub fn needs_update(installation: &Installation, upstream: &Channel) -> bool { installation.components.iter().any(|installed| { upstream .get_component(&installed.name) - .is_some_and(|new| definition_changed(installed, new)) + .is_some_and(|new| definition_changed(installed, new, &config.working_directory)) }) } @@ -439,11 +439,19 @@ pub fn needs_update(installation: &Installation, upstream: &Channel) -> bool { /// The pins install records on an authority -- a branch's commit, a path's modification time -- /// exist only locally and move on their own, so they are normalized away: drift behind them is /// reconciled by the update itself, which is what pins for ([`classify`]); a listing must not. -fn definition_changed(installed: &Component, upstream: &Component) -> bool { +/// +/// A relative path is stored absolute, joined onto `cwd` at install time, so the upstream form is +/// joined the same way before comparing. +fn definition_changed(installed: &Component, upstream: &Component, cwd: &Path) -> bool { let normalized = |component: &Component| { let mut component = component.clone(); match &mut component.version { - Authority::Path { last_modification, .. } => *last_modification = None, + Authority::Path { path, last_modification } => { + if path.is_relative() { + *path = cwd.join(&*path); + } + *last_modification = None; + }, Authority::Git { target: GitTarget::Branch { latest_revision, .. }, .. @@ -738,7 +746,7 @@ mod tests { }; component }; - assert!(!definition_changed(&on_branch(Some("abc123")), &on_branch(None))); + assert!(!definition_changed(&on_branch(Some("abc123")), &on_branch(None), cwd())); let at_path = |last_modification: Option| { let mut component = base(); @@ -746,7 +754,22 @@ mod tests { component }; let pinned = at_path(Some(std::time::SystemTime::UNIX_EPOCH)); - assert!(!definition_changed(&pinned, &at_path(None))); + assert!(!definition_changed(&pinned, &at_path(None), cwd())); + } + + /// Install stores a relative path joined onto the working directory; the manifest still says + /// the relative form, and the two are the same definition. + #[test] + fn a_relative_path_matches_its_installed_absolute_form() { + let cwd = Path::new("/work"); + let at_path = |path: &str, last_modification: Option| { + let mut component = base(); + component.version = Authority::Path { path: path.into(), last_modification }; + component + }; + let installed = at_path("/work/vm", Some(std::time::SystemTime::UNIX_EPOCH)); + assert!(!definition_changed(&installed, &at_path("vm", None), cwd)); + assert!(definition_changed(&installed, &at_path("other", None), cwd)); } /// Normalizing the pin must not mask a real change riding alongside it. @@ -770,7 +793,7 @@ mod tests { latest_revision: None, }, }; - assert!(definition_changed(&installed, &upstream)); + assert!(definition_changed(&installed, &upstream, cwd())); } /// An artifact is a file in the publication, so a component whose artifact URI moves to a new From 395c8ad7a7d358aa9cfa6ccef618784504fc2064 Mon Sep 17 00:00:00 2001 From: tomasarrachea Date: Fri, 4 Sep 2026 16:06:33 -0300 Subject: [PATCH 5/5] fix: mark a successor channel as an update only when the updater would migrate to it --- src/commands/list.rs | 14 +++++++++++--- tests/operations.rs | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/commands/list.rs b/src/commands/list.rs index 1ca88ea4..d281f29b 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -1,6 +1,6 @@ use colored::Colorize; -use crate::{config::Config, state::LocalState}; +use crate::{channel::UpstreamMatch, config::Config, state::LocalState}; /// List all the available [[Channels]] presents in the upstream manifest. pub fn list(config: &Config, state: &LocalState) -> anyhow::Result<()> { @@ -23,11 +23,19 @@ pub fn list(config: &Config, state: &LocalState) -> anyhow::Result<()> { }, Some(_) => format!(" {}", "(installed)".green()), // A channel that supersedes an installed one (spec section 11.4): updating the - // predecessor migrates to it. + // predecessor migrates to it. The counterpart lookup is the updater's, so a + // predecessor still published upstream is not shown as migrating away. None if channel .migrates_from .as_ref() - .is_some_and(|old| state.get(old).is_some()) => + .and_then(|old| state.get(old)) + .is_some_and(|predecessor| { + matches!( + predecessor.as_channel().upstream_counterpart_raw(config), + Some((successor, UpstreamMatch::Migrated { .. })) + if successor.name == channel.name + ) + }) => { format!(" {}", "(update available)".yellow()) }, diff --git a/tests/operations.rs b/tests/operations.rs index 8da38702..5e417348 100644 --- a/tests/operations.rs +++ b/tests/operations.rs @@ -175,6 +175,24 @@ impl Fixture { format!("file://{}", path.display()) } + /// A manifest publishing both `0.15.0` and a `0.16.0` that declares `migrates_from: 0.15.0`. + fn manifest_with_successor(&self, file: &str, components: &[Spec<'_>]) -> String { + let components: Vec<_> = components.iter().map(|spec| self.component(*spec)).collect(); + let manifest = serde_json::json!({ + "manifest_version": "3.0.0", + "date": 1735689600, + "networks": {"mainnet": "0.15.0"}, + "channels": [ + {"name": "0.15.0", "components": components}, + {"name": "0.16.0", "migrates_from": "0.15.0", "components": components}, + ] + }); + + let path = self.dir.join(file); + std::fs::write(&path, serde_json::to_string_pretty(&manifest).unwrap()).unwrap(); + format!("file://{}", path.display()) + } + fn write(&self, file: &str, components: Vec) -> String { let manifest = serde_json::json!({ "manifest_version": "3.0.0", @@ -750,6 +768,27 @@ fn integration_update_status_follows_migration_lineage() { ); } +/// A same-version match wins over `migrates_from` in the updater, so while the installed channel is +/// still published its successor is not a pending update. +#[test] +fn integration_update_status_prefers_a_still_published_predecessor() { + let _guard = common::harness::mutating_test_guard(); + let env = environment_setup("derived_same_version"); + let fixture = Fixture::new(env.tmp_dir.path()); + let manifest = fixture.manifest("manifest.json", &[("vm", &["minimal"], &[])]); + + let installed = midenup_run(&env, &manifest, &["install", "0.15.0", "--profile", "minimal"]); + assert!(installed.status.success(), "{}", String::from_utf8_lossy(&installed.stderr)); + + let both = fixture.manifest_with_successor("both.json", &[("vm", &["minimal"], &[])]); + let listed = midenup_run(&env, &both, &["list"]); + let stdout = String::from_utf8_lossy(&listed.stdout); + assert!( + stdout.contains("0.15.0 (installed)") && !stdout.contains("update available"), + "a published predecessor is up to date and its successor is not pending: {stdout}" + ); +} + /// An explicit root removed upstream makes the intent unresolvable, which still shows as an /// update: running it reports the missing root (spec section 11.3) rather than dropping it. #[test]