From 4c183a0b0483a64aaee17804e04e2265260d8159 Mon Sep 17 00:00:00 2001 From: ychampion Date: Mon, 13 Jul 2026 13:05:14 +0000 Subject: [PATCH 1/3] refactor: avoid duplicated manifest path checks --- src/dist/manifestation.rs | 5 ++--- src/dist/prefix.rs | 5 +++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 469d830910..9a733f32df 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -29,14 +29,13 @@ use crate::{ config::Config, download::{DownloadCfg, DownloadStatus, File}, manifest::{Component, CompressionKind, HashedBinary, Manifest}, - prefix::InstallPrefix, + prefix::{DIST_MANIFEST, InstallPrefix}, temp, }, errors::RustupError, utils, }; -pub(crate) const DIST_MANIFEST: &str = "multirust-channel-manifest.toml"; pub(crate) const CONFIG_FILE: &str = "multirust-config.toml"; #[derive(Debug)] @@ -397,7 +396,7 @@ impl Manifestation { pub fn load_manifest(&self) -> Result> { let prefix = self.installation.prefix(); let old_manifest_path = prefix.manifest_file(DIST_MANIFEST); - if utils::path_exists(&old_manifest_path) { + if prefix.has_manifest() { let manifest_str = utils::read_file("installed manifest", &old_manifest_path)?; Ok(Some(Manifest::parse(&manifest_str).with_context(|| { RustupError::ParsingFile { diff --git a/src/dist/prefix.rs b/src/dist/prefix.rs index 9b71118017..3878db2e8a 100644 --- a/src/dist/prefix.rs +++ b/src/dist/prefix.rs @@ -11,6 +11,7 @@ const REL_MANIFEST_DIR: &str = match std::path::MAIN_SEPARATOR { }; static V1_COMMON_COMPONENT_LIST: &[&str] = &["cargo", "rustc", "rust-docs"]; +pub(crate) const DIST_MANIFEST: &str = "multirust-channel-manifest.toml"; #[derive(Clone, Debug)] pub struct InstallPrefix { @@ -37,6 +38,10 @@ impl InstallPrefix { path } + pub(crate) fn has_manifest(&self) -> bool { + utils::path_exists(self.manifest_file(DIST_MANIFEST)) + } + pub(crate) fn rel_manifest_file(&self, name: &str) -> PathBuf { let mut path = PathBuf::from(REL_MANIFEST_DIR); path.push(name); From c015a928e3455e4a8d0784703318fd486c88bcf3 Mon Sep 17 00:00:00 2001 From: ychampion Date: Mon, 13 Jul 2026 13:05:14 +0000 Subject: [PATCH 2/3] fix: repair toolchains without an installed manifest --- src/dist/mod.rs | 7 ++--- src/toolchain/distributable.rs | 12 ++++--- tests/suite/cli_rustup.rs | 57 +++++++++++++++++++++++++++++----- 3 files changed, 59 insertions(+), 17 deletions(-) diff --git a/src/dist/mod.rs b/src/dist/mod.rs index b44ddd8d45..b91aeddb3a 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -1125,10 +1125,9 @@ async fn try_update_from_dist_( } else { download .dl_v2_manifest( - // Even if manifest has not changed, we must continue to install requested components. - // So if components or targets is not empty, we skip passing `update_hash` so that - // we essentially degenerate to `rustup component add` / `rustup target add` - if components.is_empty() && targets.is_empty() { + // Skip the update hash when the installed manifest is missing or when components + // or targets were requested, since either case still requires the channel manifest. + if prefix.has_manifest() && components.is_empty() && targets.is_empty() { Some(update_hash) } else { None diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index a76776fa07..ab3bbb9b27 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -439,12 +439,14 @@ impl<'a> DistributableToolchain<'a> { } pub async fn fetch_dist_manifest(&self) -> anyhow::Result> { + let prefix = InstallPrefix::from(self.toolchain.path()); + let update_hash = if prefix.has_manifest() { + Some(self.toolchain.cfg.get_hash_file(&self.desc, false)?) + } else { + None + }; DownloadCfg::new(self.toolchain.cfg) - .dl_v2_manifest( - Some(&self.toolchain.cfg.get_hash_file(&self.desc, false)?), - &self.desc, - self.toolchain.cfg, - ) + .dl_v2_manifest(update_hash.as_deref(), &self.desc, self.toolchain.cfg) .await } diff --git a/tests/suite/cli_rustup.rs b/tests/suite/cli_rustup.rs index 1001c0a253..07d183e596 100644 --- a/tests/suite/cli_rustup.rs +++ b/tests/suite/cli_rustup.rs @@ -4192,6 +4192,16 @@ installed targets: .is_ok(); } +fn nightly_manifest_path(cx: &CliTestContext) -> PathBuf { + cx.config + .rustupdir + .join("toolchains") + .join(format!("nightly-{}", this_host_tuple())) + .join("lib") + .join("rustlib") + .join("multirust-channel-manifest.toml") +} + #[tokio::test] async fn missing_manifest_shows_reinstall_help() { let cx = CliTestContext::new(Scenario::SimpleV2).await; @@ -4201,14 +4211,7 @@ async fn missing_manifest_shows_reinstall_help() { .await .is_ok(); - let manifest_path = cx - .config - .rustupdir - .join("toolchains") - .join(format!("nightly-{}", this_host_tuple())) - .join("lib") - .join("rustlib") - .join("multirust-channel-manifest.toml"); + let manifest_path = nightly_manifest_path(&cx); fs::remove_file(&manifest_path).unwrap(); @@ -4223,3 +4226,41 @@ help: try reinstalling or updating the toolchain "#]]) .is_err(); } + +#[tokio::test] +async fn reinstall_restores_missing_manifest() { + let cx = CliTestContext::new(Scenario::SimpleV2).await; + + cx.config + .expect(["rustup", "toolchain", "install", "nightly"]) + .await + .is_ok(); + + let manifest_path = nightly_manifest_path(&cx); + + fs::remove_file(&manifest_path).unwrap(); + + cx.config + .expect(["rustup", "toolchain", "install", "nightly"]) + .await + .is_ok(); + + assert!(manifest_path.exists()); +} + +#[tokio::test] +async fn update_all_restores_missing_manifest() { + let cx = CliTestContext::new(Scenario::SimpleV2).await; + + cx.config + .expect(["rustup", "toolchain", "install", "nightly"]) + .await + .is_ok(); + + let manifest_path = nightly_manifest_path(&cx); + fs::remove_file(&manifest_path).unwrap(); + + cx.config.expect(["rustup", "update"]).await.is_ok(); + + assert!(manifest_path.exists()); +} From 0b0f43757798d57bee9fcfe2220a82969fb2ec59 Mon Sep 17 00:00:00 2001 From: ychampion Date: Mon, 13 Jul 2026 13:45:25 +0000 Subject: [PATCH 3/3] refactor: make installed manifest access explicit Tested: cargo test --features test --test test_bonanza missing_manifest Tested: cargo check --all --all-targets --features test --- src/dist/manifestation.rs | 3 +-- src/dist/mod.rs | 2 +- src/dist/prefix.rs | 5 +++-- src/toolchain/distributable.rs | 2 +- tests/suite/cli_rustup.rs | 20 ++++++++++---------- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 9a733f32df..9bb2467e52 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -395,8 +395,7 @@ impl Manifestation { #[tracing::instrument(level = "trace")] pub fn load_manifest(&self) -> Result> { let prefix = self.installation.prefix(); - let old_manifest_path = prefix.manifest_file(DIST_MANIFEST); - if prefix.has_manifest() { + if let Some(old_manifest_path) = prefix.dist_manifest() { let manifest_str = utils::read_file("installed manifest", &old_manifest_path)?; Ok(Some(Manifest::parse(&manifest_str).with_context(|| { RustupError::ParsingFile { diff --git a/src/dist/mod.rs b/src/dist/mod.rs index b91aeddb3a..176f472c87 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -1127,7 +1127,7 @@ async fn try_update_from_dist_( .dl_v2_manifest( // Skip the update hash when the installed manifest is missing or when components // or targets were requested, since either case still requires the channel manifest. - if prefix.has_manifest() && components.is_empty() && targets.is_empty() { + if prefix.dist_manifest().is_some() && components.is_empty() && targets.is_empty() { Some(update_hash) } else { None diff --git a/src/dist/prefix.rs b/src/dist/prefix.rs index 3878db2e8a..c0217bfb86 100644 --- a/src/dist/prefix.rs +++ b/src/dist/prefix.rs @@ -38,8 +38,9 @@ impl InstallPrefix { path } - pub(crate) fn has_manifest(&self) -> bool { - utils::path_exists(self.manifest_file(DIST_MANIFEST)) + pub(crate) fn dist_manifest(&self) -> Option { + let path = self.manifest_file(DIST_MANIFEST); + utils::path_exists(&path).then_some(path) } pub(crate) fn rel_manifest_file(&self, name: &str) -> PathBuf { diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index ab3bbb9b27..96dfe265d2 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -440,7 +440,7 @@ impl<'a> DistributableToolchain<'a> { pub async fn fetch_dist_manifest(&self) -> anyhow::Result> { let prefix = InstallPrefix::from(self.toolchain.path()); - let update_hash = if prefix.has_manifest() { + let update_hash = if prefix.dist_manifest().is_some() { Some(self.toolchain.cfg.get_hash_file(&self.desc, false)?) } else { None diff --git a/tests/suite/cli_rustup.rs b/tests/suite/cli_rustup.rs index 07d183e596..ee69fdb81d 100644 --- a/tests/suite/cli_rustup.rs +++ b/tests/suite/cli_rustup.rs @@ -4192,16 +4192,6 @@ installed targets: .is_ok(); } -fn nightly_manifest_path(cx: &CliTestContext) -> PathBuf { - cx.config - .rustupdir - .join("toolchains") - .join(format!("nightly-{}", this_host_tuple())) - .join("lib") - .join("rustlib") - .join("multirust-channel-manifest.toml") -} - #[tokio::test] async fn missing_manifest_shows_reinstall_help() { let cx = CliTestContext::new(Scenario::SimpleV2).await; @@ -4264,3 +4254,13 @@ async fn update_all_restores_missing_manifest() { assert!(manifest_path.exists()); } + +fn nightly_manifest_path(cx: &CliTestContext) -> PathBuf { + cx.config + .rustupdir + .join("toolchains") + .join(format!("nightly-{}", this_host_tuple())) + .join("lib") + .join("rustlib") + .join("multirust-channel-manifest.toml") +}