Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cli): warn when removing the default/active toolchain #3924

Merged
merged 2 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions src/cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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?;
Expand Down Expand Up @@ -1281,8 +1281,25 @@ async fn toolchain_link(
}

fn toolchain_remove(cfg: &mut Cfg<'_>, opts: UninstallOpts) -> Result<utils::ExitCode> {
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))
Expand Down
9 changes: 9 additions & 0 deletions src/toolchain/names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,15 @@ from_variant!(
LocalToolchainName::Path
);

impl PartialEq<ToolchainName> 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 {
Expand Down
37 changes: 30 additions & 7 deletions tests/suite/cli_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,18 +212,39 @@ 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
.expect_stdout_ok(&["rustup", "toolchain", "list"], "no installed toolchains")
.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() {
Expand Down Expand Up @@ -1208,10 +1229,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");
Expand All @@ -1228,7 +1251,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;
}
Expand Down
Loading