From 2a8f8334d01c1589df9f157df0bd4c0975d71217 Mon Sep 17 00:00:00 2001 From: rami3l Date: Mon, 8 Jul 2024 16:58:01 +0800 Subject: [PATCH 1/2] feat(cli): improve warning when removing the last/host target for a toolchain --- src/cli/rustup_mode.rs | 4 ++-- tests/suite/cli_v2.rs | 12 +++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index c04c841b29..3b947a21bf 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -1165,7 +1165,7 @@ async fn target_remove( let target = TargetTriple::new(target); let default_target = cfg.get_default_host_triple()?; if target == default_target { - warn!("after removing the default host target, proc-macros and build scripts might no longer build"); + warn!("removing the default host target; proc-macros and build scripts might no longer build"); } // Whether we have at most 1 component target that is not `None` (wildcard). let has_at_most_one_target = distributable @@ -1179,7 +1179,7 @@ async fn target_remove( .at_most_one() .is_ok(); if has_at_most_one_target { - warn!("after removing the last target, no build targets will be available"); + warn!("removing the last target; no build targets will be available"); } let new_component = Component::new("rust-std".to_string(), Some(target), false); distributable.remove_component(new_component).await?; diff --git a/tests/suite/cli_v2.rs b/tests/suite/cli_v2.rs index 57e9424abf..396a5084dc 100644 --- a/tests/suite/cli_v2.rs +++ b/tests/suite/cli_v2.rs @@ -1208,10 +1208,12 @@ async fn remove_target_host() { cx.config .expect_ok(&["rustup", "target", "add", clitools::CROSS_ARCH1]) .await; - cx.config.expect_stderr_ok( - &["rustup", "target", "remove", &host], - "after removing the default host target, proc-macros and build scripts might no longer build", - ).await; + cx.config + .expect_stderr_ok( + &["rustup", "target", "remove", &host], + "removing the default host target; proc-macros and build scripts might no longer build", + ) + .await; let path = format!("toolchains/nightly-{host}/lib/rustlib/{host}/lib/libstd.rlib"); assert!(!cx.config.rustupdir.has(path)); let path = format!("toolchains/nightly-{host}/lib/rustlib/{host}/lib"); @@ -1228,7 +1230,7 @@ async fn remove_target_last() { cx.config .expect_stderr_ok( &["rustup", "target", "remove", &host], - "after removing the last target, no build targets will be available", + "removing the last target; no build targets will be available", ) .await; } From 6e41282331e8089dc539dc9ec725e2f6bab39d8b Mon Sep 17 00:00:00 2001 From: rami3l Date: Sun, 7 Jul 2024 20:29:16 +0800 Subject: [PATCH 2/2] feat(cli): warn when removing the default/active toolchain --- src/cli/rustup_mode.rs | 17 +++++++++++++++++ src/toolchain/names.rs | 9 +++++++++ tests/suite/cli_v2.rs | 25 +++++++++++++++++++++++-- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 3b947a21bf..1157f0b0c1 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -1281,8 +1281,25 @@ async fn toolchain_link( } fn toolchain_remove(cfg: &mut Cfg<'_>, opts: UninstallOpts) -> Result { + let default_toolchain = cfg.get_default().ok().flatten(); + let active_toolchain = cfg.find_active_toolchain().ok().flatten().map(|(it, _)| it); + for toolchain_name in &opts.toolchain { let toolchain_name = toolchain_name.resolve(&cfg.get_default_host_triple()?)?; + + if active_toolchain + .as_ref() + .is_some_and(|n| n == &toolchain_name) + { + warn!("removing the active toolchain; a toolchain override will be required for running Rust tools"); + } + if default_toolchain + .as_ref() + .is_some_and(|n| n == &toolchain_name) + { + warn!("removing the default toolchain; proc-macros and build scripts might no longer build"); + } + Toolchain::ensure_removed(cfg, (&toolchain_name).into())?; } Ok(utils::ExitCode(0)) diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index f4cefc875e..5ce4ba482a 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -359,6 +359,15 @@ from_variant!( LocalToolchainName::Path ); +impl PartialEq for LocalToolchainName { + fn eq(&self, other: &ToolchainName) -> bool { + match self { + LocalToolchainName::Named(n) => n == other, + _ => false, + } + } +} + impl Display for LocalToolchainName { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { diff --git a/tests/suite/cli_v2.rs b/tests/suite/cli_v2.rs index 396a5084dc..a4bd1fa330 100644 --- a/tests/suite/cli_v2.rs +++ b/tests/suite/cli_v2.rs @@ -212,11 +212,14 @@ async fn list_toolchains_with_none() { } #[tokio::test] -async fn remove_toolchain() { +async fn remove_toolchain_default() { let mut cx = CliTestContext::new(Scenario::SimpleV2).await; cx.config.expect_ok(&["rustup", "update", "nightly"]).await; cx.config - .expect_ok(&["rustup", "toolchain", "remove", "nightly"]) + .expect_stderr_ok( + &["rustup", "toolchain", "remove", "nightly"], + "removing the default toolchain; proc-macros and build scripts might no longer build", + ) .await; cx.config.expect_ok(&["rustup", "toolchain", "list"]).await; cx.config @@ -224,6 +227,24 @@ async fn remove_toolchain() { .await; } +#[tokio::test] +async fn remove_toolchain_active() { + let mut cx = CliTestContext::new(Scenario::SimpleV2).await; + cx.config.expect_ok(&["rustup", "default", "nightly"]).await; + cx.config + .expect_ok(&["rustup", "override", "set", "stable"]) + .await; + cx.config + .expect_stderr_ok( + &["rustup", "toolchain", "remove", "stable"], + "removing the active toolchain; a toolchain override will be required for running Rust tools", + ) + .await; + cx.config + .expect_stdout_ok(&["rustup", "toolchain", "list"], "nightly") + .await; +} + // Issue #2873 #[tokio::test] async fn remove_toolchain_ignore_trailing_slash() {