From 56014f722df776fc4ea1a5e0ca393a98cd8bfe63 Mon Sep 17 00:00:00 2001 From: Dany Marcoux Date: Tue, 16 Jun 2020 23:48:42 +0200 Subject: [PATCH 1/3] Describe how to remove components when update fails --- src/errors.rs | 39 +++++++++-- tests/dist.rs | 175 ++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 205 insertions(+), 9 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index 480a8533b2..f263f83144 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -394,6 +394,19 @@ fn valid_profile_names() -> String { .join(", ") } +fn remove_component_msg(cs: &Component, manifest: &Manifest, toolchain: &str) -> String { + 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()); @@ -411,6 +424,12 @@ fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: & "" } ); + + let _ = write!( + buf, + "If you don't need the component, you can remove it with:\n{}", + remove_component_msg(&cs[0], manifest, toolchain) + ); } else { let same_target = cs .iter() @@ -421,10 +440,16 @@ fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: & .map(|c| format!("'{}'", c.short_name(manifest))) .collect::>() .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 {}: {}\n{}", - toolchain, cs_str, TOOLSTATE_MSG, + "some components unavailable for download for channel {}: {} + If you don't need the components, you can remove them with:\n{}\n{}", + toolchain, cs_str, remove_msg, TOOLSTATE_MSG, ); } else { let cs_str = cs @@ -432,10 +457,16 @@ fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: & .map(|c| c.description(manifest)) .collect::>() .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 {}: {}\n{}", - toolchain, cs_str, TOOLSTATE_MSG, + "some components unavailable for download for channel {}: {} + If you don't need the components, you can remove them with:\n{}\n{}", + toolchain, cs_str, remove_msg, TOOLSTATE_MSG, ); } } diff --git a/tests/dist.rs b/tests/dist.rs index d3d93c4add..35e4d36d63 100644 --- a/tests/dist.rs +++ b/tests/dist.rs @@ -764,7 +764,9 @@ fn unavailable_component() { ) .unwrap_err(); match *err.kind() { - ErrorKind::RequestedComponentsUnavailable(..) => {} + ErrorKind::RequestedComponentsUnavailable(..) => { + assert!(err.to_string().contains("rustup component remove --toolchain nightly --target x86_64-apple-darwin bonus")); + } _ => panic!(), } }, @@ -823,7 +825,9 @@ fn unavailable_component_from_profile() { ) .unwrap_err(); match *err.kind() { - ErrorKind::RequestedComponentsUnavailable(..) => {} + ErrorKind::RequestedComponentsUnavailable(..) => { + assert!(err.to_string().contains("rustup component remove --toolchain nightly --target x86_64-apple-darwin rustc")); + } _ => panic!(), } @@ -889,7 +893,161 @@ fn removed_component() { // Update without bonus, should fail with RequestedComponentsUnavailable change_channel_date(url, "nightly", "2016-02-02"); - assert!(update_from_dist( + let err = update_from_dist( + url, + toolchain, + prefix, + &[], + &[], + &download_cfg, + temp_cfg, + false, + ) + .unwrap_err(); + match *err.kind() { + ErrorKind::RequestedComponentsUnavailable(..) => { + assert!(err.to_string().contains("rustup component remove --toolchain nightly --target x86_64-apple-darwin bonus")); + } + _ => panic!(), + } + }, + ); +} + +#[test] +fn unavailable_components_with_different_target() { + // On day 2 the rust-std component is no longer available + let edit = &|date: &str, chan: &mut MockChannel| { + // Mark the rust-std package as unavailable in 2016-02-02 + if date == "2016-02-02" { + let pkg = chan + .packages + .iter_mut() + .find(|p| p.name == "rust-std") + .unwrap(); + + for target in &mut pkg.targets { + target.available = false; + } + } + }; + + setup( + Some(edit), + false, + &|url, toolchain, prefix, download_cfg, temp_cfg| { + let adds = [ + Component::new( + "rust-std".to_string(), + Some(TargetTriple::new("i686-apple-darwin")), + false, + ), + Component::new( + "rust-std".to_string(), + Some(TargetTriple::new("i686-unknown-linux-gnu")), + false, + ), + ]; + + // Update with rust-std + change_channel_date(url, "nightly", "2016-02-01"); + update_from_dist( + url, + toolchain, + prefix, + &adds, + &[], + &download_cfg, + temp_cfg, + false, + ) + .unwrap(); + + assert!(utils::path_exists( + &prefix.path().join("lib/i686-apple-darwin/libstd.rlib") + )); + assert!(utils::path_exists( + &prefix.path().join("lib/i686-unknown-linux-gnu/libstd.rlib") + )); + + // Update without rust-std + change_channel_date(url, "nightly", "2016-02-02"); + let err = update_from_dist( + url, + toolchain, + prefix, + &[], + &[], + &download_cfg, + temp_cfg, + false, + ) + .unwrap_err(); + match *err.kind() { + ErrorKind::RequestedComponentsUnavailable(..) => { + assert!(err.to_string().contains("rustup component remove --toolchain nightly --target i686-apple-darwin rust-std")); + assert!(err.to_string().contains("rustup component remove --toolchain nightly --target i686-unknown-linux-gnu rust-std")); + } + _ => panic!(), + } + }, + ); +} + +#[test] +fn unavailable_components_with_same_target() { + // On day 2, the rust-std and rustc components are no longer available + let edit = &|date: &str, chan: &mut MockChannel| { + // Mark the rust-std package as unavailable in 2016-02-02 + if date == "2016-02-02" { + let pkg = chan + .packages + .iter_mut() + .find(|p| p.name == "rust-std") + .unwrap(); + + for target in &mut pkg.targets { + target.available = false; + } + } + + // Mark the rustc package as unavailable in 2016-02-02 + if date == "2016-02-02" { + let pkg = chan + .packages + .iter_mut() + .find(|p| p.name == "rustc") + .unwrap(); + + for target in &mut pkg.targets { + target.available = false; + } + } + }; + + setup( + Some(edit), + false, + &|url, toolchain, prefix, download_cfg, temp_cfg| { + // Update with rust-std and rustc + change_channel_date(url, "nightly", "2016-02-01"); + update_from_dist( + url, + toolchain, + prefix, + &[], + &[], + &download_cfg, + temp_cfg, + false, + ) + .unwrap(); + assert!(utils::path_exists(&prefix.path().join("bin/rustc"))); + assert!(utils::path_exists(&prefix.path().join("lib/libstd.rlib"))); + + // Update without rust-std and rustc + change_channel_date(url, "nightly", "2016-02-02"); + let err = update_from_dist( url, toolchain, prefix, @@ -897,9 +1055,16 @@ fn removed_component() { &[], &download_cfg, temp_cfg, - false + false, ) - .is_err()); + .unwrap_err(); + match *err.kind() { + ErrorKind::RequestedComponentsUnavailable(..) => { + assert!(err.to_string().contains("rustup component remove --toolchain nightly --target x86_64-apple-darwin rust-std")); + assert!(err.to_string().contains("rustup component remove --toolchain nightly --target x86_64-apple-darwin rustc")); + } + _ => panic!(), + } }, ); } From 531fc863fefa233ab691b728c09e5990cb0ed9a4 Mon Sep 17 00:00:00 2001 From: Daniel Silverstone Date: Tue, 17 Nov 2020 10:31:10 +0000 Subject: [PATCH 2/3] dist: Add Deref for TargetTriple Signed-off-by: Daniel Silverstone --- src/dist/dist.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/dist/dist.rs b/src/dist/dist.rs index f1fc3f84a6..9a90a6bbcb 100644 --- a/src/dist/dist.rs +++ b/src/dist/dist.rs @@ -1,5 +1,6 @@ use std::env; use std::fmt; +use std::ops::Deref; use std::path::Path; use std::str::FromStr; @@ -178,6 +179,13 @@ impl FromStr for ParsedToolchainDesc { } } +impl Deref for TargetTriple { + type Target = str; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + impl TargetTriple { pub fn new(name: &str) -> Self { Self(name.to_string()) From df38b334c6e30da939fde3ff5536ffaae4be82a2 Mon Sep 17 00:00:00 2001 From: Daniel Silverstone Date: Tue, 17 Nov 2020 10:31:55 +0000 Subject: [PATCH 3/3] UX: instruct users to remove targets differently from components Signed-off-by: Daniel Silverstone --- src/errors.rs | 34 ++++++++++++++++++++++------------ tests/dist.rs | 16 +++++++++++----- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index f263f83144..9fd6050608 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -395,16 +395,26 @@ fn valid_profile_names() -> String { } fn remove_component_msg(cs: &Component, manifest: &Manifest, toolchain: &str) -> String { - format!( - "rustup component remove --toolchain {}{} {}", - toolchain, - if let Some(target) = cs.target.as_ref() { - format!(" --target {}", target) - } else { - String::default() - }, - cs.short_name(manifest) - ) + 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 { @@ -427,7 +437,7 @@ fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: & let _ = write!( buf, - "If you don't need the component, you can remove it with:\n{}", + "If you don't need the component, you can remove it with:\n\n{}", remove_component_msg(&cs[0], manifest, toolchain) ); } else { @@ -448,7 +458,7 @@ fn component_unavailable_msg(cs: &[Component], manifest: &Manifest, toolchain: & let _ = write!( buf, "some components unavailable for download for channel {}: {} - If you don't need the components, you can remove them with:\n{}\n{}", + If you don't need the components, you can remove them with:\n\n{}\n\n{}", toolchain, cs_str, remove_msg, TOOLSTATE_MSG, ); } else { diff --git a/tests/dist.rs b/tests/dist.rs index 35e4d36d63..5e4b58afb6 100644 --- a/tests/dist.rs +++ b/tests/dist.rs @@ -915,7 +915,7 @@ fn removed_component() { } #[test] -fn unavailable_components_with_different_target() { +fn unavailable_components_is_target() { // On day 2 the rust-std component is no longer available let edit = &|date: &str, chan: &mut MockChannel| { // Mark the rust-std package as unavailable in 2016-02-02 @@ -985,8 +985,12 @@ fn unavailable_components_with_different_target() { .unwrap_err(); match *err.kind() { ErrorKind::RequestedComponentsUnavailable(..) => { - assert!(err.to_string().contains("rustup component remove --toolchain nightly --target i686-apple-darwin rust-std")); - assert!(err.to_string().contains("rustup component remove --toolchain nightly --target i686-unknown-linux-gnu rust-std")); + let err_str = err.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" + )); } _ => panic!(), } @@ -1060,8 +1064,10 @@ fn unavailable_components_with_same_target() { .unwrap_err(); match *err.kind() { ErrorKind::RequestedComponentsUnavailable(..) => { - assert!(err.to_string().contains("rustup component remove --toolchain nightly --target x86_64-apple-darwin rust-std")); - assert!(err.to_string().contains("rustup component remove --toolchain nightly --target x86_64-apple-darwin rustc")); + let err_str = err.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")); } _ => panic!(), }