From 72c48c077ed73924efba0bfd233a41f34f0fc462 Mon Sep 17 00:00:00 2001 From: rami3l Date: Sun, 8 Oct 2023 19:27:15 +0800 Subject: [PATCH 1/6] Clean up some `manifestation` logic --- src/dist/manifestation.rs | 44 +++++++++++++-------------------------- 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 8b66ca47b0..c6ecf742ed 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -130,25 +130,19 @@ impl Manifestation { } // Validate that the requested components are available - match update.unavailable_components(new_manifest, toolchain_str) { - Ok(_) => {} - Err(e) => { - if force_update { - if let Ok(RustupError::RequestedComponentsUnavailable { components, .. }) = - e.downcast::() - { - for component in &components { - (download_cfg.notify_handler)( - Notification::ForcingUnavailableComponent( - component.name(new_manifest).as_str(), - ), - ); - } - update.drop_components_to_install(&components); - } - } else { - return Err(e); + if let Err(e) = update.unavailable_components(new_manifest, toolchain_str) { + if !force_update { + return Err(e); + } + if let Ok(RustupError::RequestedComponentsUnavailable { components, .. }) = + e.downcast::() + { + for component in &components { + (download_cfg.notify_handler)(Notification::ForcingUnavailableComponent( + &component.name(new_manifest), + )); } + update.drop_components_to_install(&components); } } @@ -678,18 +672,8 @@ impl Update { } fn drop_components_to_install(&mut self, to_drop: &[Component]) { - let components: Vec<_> = self - .components_to_install - .drain(..) - .filter(|c| !to_drop.contains(c)) - .collect(); - self.components_to_install.extend(components); - let final_components: Vec<_> = self - .final_component_list - .drain(..) - .filter(|c| !to_drop.contains(c)) - .collect(); - self.final_component_list = final_components; + self.components_to_install.retain(|c| !to_drop.contains(c)); + self.final_component_list.retain(|c| !to_drop.contains(c)); } /// Map components to urls and hashes From fe9c89cc500da6d41da7276584c9806c77b4ee3b Mon Sep 17 00:00:00 2001 From: rami3l Date: Sun, 8 Oct 2023 19:27:16 +0800 Subject: [PATCH 2/6] Delete suggestions of removing the relevant component from `component_unavailable_msg` After digging into the codebase, I realized that the message generated by `component_unavailable_msg` is used only when: - Some components are missing from the toolchain so that the installation can no longer proceed; - We are not installing these components as a part of a toolchain-wide operation (e.g. updating the existing `nightly`), which is covered by `components_missing_msg` by catching the `RustupError::RequestedComponentsUnavailable` and re-throwing it as a `DistError::ToolchainComponentsMissing` (see for more info). Thus, I decided to remove the `rustup component remove` suggestion altogether. --- src/dist/manifestation/tests.rs | 88 +++++++++++++++++++++++++-------- src/errors.rs | 43 +++------------- 2 files changed, 76 insertions(+), 55 deletions(-) diff --git a/src/dist/manifestation/tests.rs b/src/dist/manifestation/tests.rs index 7dd9180525..cfe81562cf 100644 --- a/src/dist/manifestation/tests.rs +++ b/src/dist/manifestation/tests.rs @@ -772,8 +772,17 @@ fn unavailable_component() { ) .unwrap_err(); match err.downcast::() { - Ok(e @ RustupError::RequestedComponentsUnavailable { .. }) => { - assert!(e.to_string().contains("rustup component remove --toolchain nightly --target x86_64-apple-darwin bonus")); + Ok(RustupError::RequestedComponentsUnavailable { + components, + manifest, + toolchain, + }) => { + assert_eq!(toolchain, "nightly"); + let descriptions = components + .iter() + .map(|c| c.description(&manifest)) + .collect::>(); + assert_eq!(descriptions, ["'bonus' for target 'x86_64-apple-darwin'"]) } _ => panic!(), } @@ -833,8 +842,17 @@ fn unavailable_component_from_profile() { ) .unwrap_err(); match err.downcast::() { - Ok(e @ RustupError::RequestedComponentsUnavailable { .. }) => { - assert!(e.to_string().contains("rustup component remove --toolchain nightly --target x86_64-apple-darwin rustc")); + Ok(RustupError::RequestedComponentsUnavailable { + components, + manifest, + toolchain, + }) => { + assert_eq!(toolchain, "nightly"); + let descriptions = components + .iter() + .map(|c| c.description(&manifest)) + .collect::>(); + assert_eq!(descriptions, ["'rustc' for target 'x86_64-apple-darwin'"]) } _ => panic!(), } @@ -913,8 +931,17 @@ fn removed_component() { ) .unwrap_err(); match err.downcast::() { - Ok(e @ RustupError::RequestedComponentsUnavailable { .. }) => { - assert!(e.to_string().contains("rustup component remove --toolchain nightly --target x86_64-apple-darwin bonus")); + Ok(RustupError::RequestedComponentsUnavailable { + components, + manifest, + toolchain, + }) => { + assert_eq!(toolchain, "nightly"); + let descriptions = components + .iter() + .map(|c| c.description(&manifest)) + .collect::>(); + assert_eq!(descriptions, ["'bonus' for target 'x86_64-apple-darwin'"]) } _ => panic!(), } @@ -992,13 +1019,24 @@ fn unavailable_components_is_target() { ) .unwrap_err(); match err.downcast::() { - Ok(e @ RustupError::RequestedComponentsUnavailable { .. }) => { - let err_str = e.to_string(); - assert!(err_str - .contains("rustup target remove --toolchain nightly i686-apple-darwin")); - assert!(err_str.contains( - "rustup target remove --toolchain nightly i686-unknown-linux-gnu" - )); + Ok(RustupError::RequestedComponentsUnavailable { + components, + manifest, + toolchain, + }) => { + assert_eq!(toolchain, "nightly"); + let descriptions = components + .iter() + .map(|c| c.description(&manifest)) + .collect::>(); + assert_eq!( + descriptions, + [ + "'rust-std' for target 'x86_64-apple-darwin'", + "'rust-std' for target 'i686-apple-darwin'", + "'rust-std' for target 'i686-unknown-linux-gnu'" + ] + ); } _ => panic!(), } @@ -1071,13 +1109,23 @@ fn unavailable_components_with_same_target() { ) .unwrap_err(); match err.downcast::() { - Ok(e @ RustupError::RequestedComponentsUnavailable { .. }) => { - let err_str = e.to_string(); - assert!(err_str - .contains("rustup target remove --toolchain nightly x86_64-apple-darwin")); - assert!(err_str.contains( - "rustup component remove --toolchain nightly --target x86_64-apple-darwin rustc" - )); + Ok(RustupError::RequestedComponentsUnavailable { + components, + manifest, + toolchain, + }) => { + assert_eq!(toolchain, "nightly"); + let descriptions = components + .iter() + .map(|c| c.description(&manifest)) + .collect::>(); + assert_eq!( + descriptions, + [ + "'rustc' for target 'x86_64-apple-darwin'", + "'rust-std' for target 'x86_64-apple-darwin'" + ] + ); } _ => panic!(), } diff --git a/src/errors.rs b/src/errors.rs index e6014cdb3f..03511f8771 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -147,29 +147,6 @@ fn suggest_message(suggestion: &Option) -> String { } } -fn remove_component_msg(cs: &Component, manifest: &Manifest, toolchain: &str) -> String { - if cs.short_name_in_manifest() == "rust-std" { - // We special-case rust-std as it's the stdlib so really you want to do - // rustup target remove - format!( - " rustup target remove --toolchain {} {}", - toolchain, - cs.target.as_deref().unwrap_or(toolchain) - ) - } else { - format!( - " rustup component remove --toolchain {}{} {}", - toolchain, - if let Some(target) = cs.target.as_ref() { - format!(" --target {target}") - } else { - String::default() - }, - cs.short_name(manifest) - ) - } -} - fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: &str) -> String { assert!(!cs.is_empty()); @@ -188,11 +165,6 @@ fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: & "Sometimes not all components are available in any given nightly. " ); } - let _ = write!( - buf, - "If you don't need the component, you can remove it with:\n\n{}", - remove_component_msg(&cs[0], manifest, toolchain) - ); } else { // More than one component @@ -212,16 +184,17 @@ fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: & .join(", ") }; - let remove_msg = cs - .iter() - .map(|c| remove_component_msg(c, manifest, toolchain)) - .collect::>() - .join("\n"); let _ = write!( buf, - "some components unavailable for download for channel '{toolchain}': {cs_str}\n\ - If you don't need the components, you can remove them with:\n\n{remove_msg}\n\n{TOOLSTATE_MSG}", + "some components unavailable for download for channel '{toolchain}': {cs_str}", ); + + if toolchain.starts_with("nightly") { + let _ = write!( + buf, + "Sometimes not all components are available in any given nightly. " + ); + } } String::from_utf8(buf).unwrap() From 50b0abac6a5f61b6650148d2e38ee2664b51d327 Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 10 Oct 2023 18:09:01 +0800 Subject: [PATCH 3/6] Refactor `components_*_msg` --- src/dist/dist.rs | 64 ++++++++++++++++++++++------------------- src/errors.rs | 75 ++++++++++++++++++++++++------------------------ 2 files changed, 72 insertions(+), 67 deletions(-) diff --git a/src/dist/dist.rs b/src/dist/dist.rs index ceceae331e..b63a3d0ba7 100644 --- a/src/dist/dist.rs +++ b/src/dist/dist.rs @@ -44,45 +44,49 @@ static TOOLCHAIN_CHANNELS: &[&str] = &[ ]; fn components_missing_msg(cs: &[Component], manifest: &ManifestV2, toolchain: &str) -> String { - assert!(!cs.is_empty()); let mut buf = vec![]; let suggestion = format!(" rustup toolchain add {toolchain} --profile minimal"); let nightly_tips = "Sometimes not all components are available in any given nightly. "; - if cs.len() == 1 { - let _ = writeln!( - buf, - "component {} is unavailable for download for channel '{}'", - &cs[0].description(manifest), - toolchain, - ); + match cs { + [] => panic!("`components_missing_msg` should not be called with an empty collection of unavailable components"), + [c] => { + let _ = writeln!( + buf, + "component {} is unavailable for download for channel '{}'", + c.description(manifest), + toolchain, + ); + + if toolchain.starts_with("nightly") { + let _ = write!(buf, "{nightly_tips}"); + } - if toolchain.starts_with("nightly") { - let _ = write!(buf, "{nightly_tips}"); + let _ = write!( + buf, + "If you don't need the component, you could try a minimal installation with:\n\n{suggestion}" + ); } + cs => { + let cs_str = cs + .iter() + .map(|c| c.description(manifest)) + .collect::>() + .join(", "); + let _ = write!( + buf, + "some components unavailable for download for channel '{toolchain}': {cs_str}" + ); - let _ = write!( - buf, - "If you don't need the component, you could try a minimal installation with:\n\n{suggestion}" - ); - } else { - let cs_str = cs - .iter() - .map(|c| c.description(manifest)) - .collect::>() - .join(", "); - let _ = write!( - buf, - "some components unavailable for download for channel '{toolchain}': {cs_str}" - ); + if toolchain.starts_with("nightly") { + let _ = write!(buf, "{nightly_tips}"); + } - if toolchain.starts_with("nightly") { - let _ = write!(buf, "{nightly_tips}"); + let _ = write!( + buf, + "If you don't need the components, you could try a minimal installation with:\n\n{suggestion}" + ); } - let _ = write!( - buf, - "If you don't need the components, you could try a minimal installation with:\n\n{suggestion}" - ); } String::from_utf8(buf).unwrap() diff --git a/src/errors.rs b/src/errors.rs index 03511f8771..df4df3f428 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -148,52 +148,53 @@ fn suggest_message(suggestion: &Option) -> String { } fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: &str) -> String { - assert!(!cs.is_empty()); - let mut buf = vec![]; - - if cs.len() == 1 { - let _ = writeln!( - buf, - "component {} is unavailable for download for channel '{}'", - &cs[0].description(manifest), - toolchain, - ); - if toolchain.starts_with("nightly") { - let _ = write!( + match cs { + [] => panic!("`component_unavailable_msg` should not be called with an empty collection of unavailable components"), + [c] => { + let _ = writeln!( buf, - "Sometimes not all components are available in any given nightly. " + "component {} is unavailable for download for channel '{}'", + c.description(manifest), + toolchain, ); - } - } else { - // More than one component - - let same_target = cs - .iter() - .all(|c| c.target == cs[0].target || c.target.is_none()); - let cs_str = if same_target { - cs.iter() - .map(|c| format!("'{}'", c.short_name(manifest))) - .collect::>() - .join(", ") - } else { - cs.iter() - .map(|c| c.description(manifest)) - .collect::>() - .join(", ") - }; + if toolchain.starts_with("nightly") { + let _ = write!( + buf, + "Sometimes not all components are available in any given nightly. " + ); + } + } + cs => { + // More than one component + let same_target = cs + .iter() + .all(|c| c.target == cs[0].target || c.target.is_none()); - let _ = write!( - buf, - "some components unavailable for download for channel '{toolchain}': {cs_str}", - ); + let cs_str = if same_target { + cs.iter() + .map(|c| format!("'{}'", c.short_name(manifest))) + .collect::>() + .join(", ") + } else { + cs.iter() + .map(|c| c.description(manifest)) + .collect::>() + .join(", ") + }; - if toolchain.starts_with("nightly") { let _ = write!( buf, - "Sometimes not all components are available in any given nightly. " + "some components unavailable for download for channel '{toolchain}': {cs_str}" ); + + if toolchain.starts_with("nightly") { + let _ = write!( + buf, + "Sometimes not all components are available in any given nightly. " + ); + } } } From a37e99b71712b412badc6f33b54ce3ee21e6b0e5 Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 10 Oct 2023 18:09:22 +0800 Subject: [PATCH 4/6] Add `Panics` sections to docstrings --- src/dist/dist.rs | 7 +++++++ src/errors.rs | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/src/dist/dist.rs b/src/dist/dist.rs index b63a3d0ba7..e531f382e2 100644 --- a/src/dist/dist.rs +++ b/src/dist/dist.rs @@ -43,6 +43,13 @@ static TOOLCHAIN_CHANNELS: &[&str] = &[ r"\d{1}\.\d{1,3}(?:\.\d{1,2})?", ]; +/// Returns a error message indicating that certain [`Component`]s are missing in a toolchain distribution. +/// +/// This message is currently used exclusively in toolchain-wide operations, +/// otherwise [`component_unavailable_msg`](../../errors/fn.component_unavailable_msg.html) will be used. +/// +/// # Panics +/// This function will panic when the collection of unavailable components `cs` is empty. fn components_missing_msg(cs: &[Component], manifest: &ManifestV2, toolchain: &str) -> String { let mut buf = vec![]; let suggestion = format!(" rustup toolchain add {toolchain} --profile minimal"); diff --git a/src/errors.rs b/src/errors.rs index df4df3f428..d6d21388f2 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -147,6 +147,13 @@ fn suggest_message(suggestion: &Option) -> String { } } +/// Returns a error message indicating that certain [`Component`]s are unavailable. +/// +/// See also [`component_missing_msg`](../dist/dist/fn.components_missing_msg.html) +/// which generates error messages for component unavailability toolchain-wide operations. +/// +/// # Panics +/// This function will panic when the collection of unavailable components `cs` is empty. fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: &str) -> String { let mut buf = vec![]; match cs { From 1df4b8a97cd47ac359f34d53305b855c447d4b35 Mon Sep 17 00:00:00 2001 From: rami3l Date: Sun, 8 Oct 2023 19:27:16 +0800 Subject: [PATCH 5/6] Add test to ensure resolution of #3418 --- tests/suite/cli_v2.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/suite/cli_v2.rs b/tests/suite/cli_v2.rs index f94f3da91a..15673ab53a 100644 --- a/tests/suite/cli_v2.rs +++ b/tests/suite/cli_v2.rs @@ -1034,6 +1034,27 @@ fn update_unavailable_std() { }); } +#[test] +fn add_missing_component() { + setup(&|config| { + make_component_unavailable(config, "rls-preview", &this_host_triple()); + config.expect_ok(&["rustup", "toolchain", "add", "nightly"]); + config.expect_err( + &["rustup", "component", "add", "rls-preview"], + for_host!( + "component 'rls' for target '{0}' is unavailable for download for channel 'nightly'\n\ + Sometimes not all components are available in any given nightly." + ), + ); + // Make sure the following pattern does not match, + // thus addressing https://github.com/rust-lang/rustup/issues/3418. + config.expect_not_stderr_err( + &["rustup", "component", "add", "rls-preview"], + "If you don't need the component, you can remove it with:", + ); + }); +} + #[test] fn add_missing_component_toolchain() { setup(&|config| { From aee34d929ffd1061b371e9b51598bbc8b440020f Mon Sep 17 00:00:00 2001 From: rami3l Date: Sun, 8 Oct 2023 19:27:16 +0800 Subject: [PATCH 6/6] Move `TOOLSTATE_MSG` to `dist` to serve toolchain-wide operations --- src/dist/dist.rs | 12 ++++++++++-- src/errors.rs | 8 -------- tests/suite/cli_v2.rs | 13 ++++++++++++- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/dist/dist.rs b/src/dist/dist.rs index e531f382e2..7e397e8a13 100644 --- a/src/dist/dist.rs +++ b/src/dist/dist.rs @@ -43,6 +43,14 @@ static TOOLCHAIN_CHANNELS: &[&str] = &[ r"\d{1}\.\d{1,3}(?:\.\d{1,2})?", ]; +const TOOLSTATE_MSG: &str = + "If you require these components, please install and use the latest successful build version,\n\ + which you can find at .\n\nAfter determining \ + the correct date, install it with a command such as:\n\n \ + rustup toolchain install nightly-2018-12-27\n\n\ + Then you can use the toolchain with commands such as:\n\n \ + cargo +nightly-2018-12-27 build"; + /// Returns a error message indicating that certain [`Component`]s are missing in a toolchain distribution. /// /// This message is currently used exclusively in toolchain-wide operations, @@ -71,7 +79,7 @@ fn components_missing_msg(cs: &[Component], manifest: &ManifestV2, toolchain: &s let _ = write!( buf, - "If you don't need the component, you could try a minimal installation with:\n\n{suggestion}" + "If you don't need the component, you could try a minimal installation with:\n\n{suggestion}\n\n{TOOLSTATE_MSG}" ); } cs => { @@ -91,7 +99,7 @@ fn components_missing_msg(cs: &[Component], manifest: &ManifestV2, toolchain: &s let _ = write!( buf, - "If you don't need the components, you could try a minimal installation with:\n\n{suggestion}" + "If you don't need the components, you could try a minimal installation with:\n\n{suggestion}\n\n{TOOLSTATE_MSG}" ); } } diff --git a/src/errors.rs b/src/errors.rs index d6d21388f2..a320ee4d5f 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -19,14 +19,6 @@ use crate::{ toolchain::names::{PathBasedToolchainName, ToolchainName}, }; -const TOOLSTATE_MSG: &str = - "If you require these components, please install and use the latest successful build version,\n\ - which you can find at .\n\nAfter determining \ - the correct date, install it with a command such as:\n\n \ - rustup toolchain install nightly-2018-12-27\n\n\ - Then you can use the toolchain with commands such as:\n\n \ - cargo +nightly-2018-12-27 build"; - /// A type erasing thunk for the retry crate to permit use with anyhow. See #[derive(Debug, ThisError)] #[error(transparent)] diff --git a/tests/suite/cli_v2.rs b/tests/suite/cli_v2.rs index 15673ab53a..563e6609fc 100644 --- a/tests/suite/cli_v2.rs +++ b/tests/suite/cli_v2.rs @@ -1065,7 +1065,18 @@ fn add_missing_component_toolchain() { r"component 'rust-std' for target '{0}' is unavailable for download for channel 'nightly' Sometimes not all components are available in any given nightly. If you don't need the component, you could try a minimal installation with: - rustup toolchain add nightly --profile minimal" + rustup toolchain add nightly --profile minimal + +If you require these components, please install and use the latest successful build version, +which you can find at . + +After determining the correct date, install it with a command such as: + + rustup toolchain install nightly-2018-12-27 + +Then you can use the toolchain with commands such as: + + cargo +nightly-2018-12-27 build" ), ); });