From 8d6a1d3d40d408dd4e89e29c531110f02e0a7d39 Mon Sep 17 00:00:00 2001 From: rami3l Date: Mon, 15 Jan 2024 14:11:57 +0800 Subject: [PATCH 1/2] refactor(toolchain): extract `DistributableToolchain::components()` --- src/cli/common.rs | 7 +------ src/toolchain/distributable.rs | 7 +++++++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/cli/common.rs b/src/cli/common.rs index 9a85aca25b..20cbe7d1e1 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -448,12 +448,7 @@ pub(crate) fn list_components( pub(crate) fn list_installed_components(distributable: DistributableToolchain<'_>) -> Result<()> { let t = process().stdout(); - let manifestation = distributable.get_manifestation()?; - let config = manifestation.read_config()?.unwrap_or_default(); - let manifest = distributable.get_manifest()?; - let components = manifest.query_components(distributable.desc(), &config)?; - - for component in components { + for component in distributable.components()? { if component.installed { writeln!(t.lock(), "{}", component.name)?; } diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index d5aeadb8dc..8bc77901c2 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -115,6 +115,13 @@ impl<'a> DistributableToolchain<'a> { Ok(()) } + pub(crate) fn components(&self) -> anyhow::Result> { + let manifestation = self.get_manifestation()?; + let config = manifestation.read_config()?.unwrap_or_default(); + let manifest = self.get_manifest()?; + manifest.query_components(self.desc(), &config) + } + /// Are all the components installed in this distribution pub(crate) fn components_exist( &self, From 3810964e1483e75e35dd3d7731bfb22887709b43 Mon Sep 17 00:00:00 2001 From: rami3l Date: Mon, 15 Jan 2024 14:59:14 +0800 Subject: [PATCH 2/2] feat(cli): warn when removing the last/host target for a toolchain --- Cargo.lock | 12 +++++++++++- Cargo.toml | 1 + src/cli/rustup_mode.rs | 23 ++++++++++++++++++----- tests/suite/cli_v2.rs | 26 ++++++++++++++++++++++++-- 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1704235812..46c7d43ad2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1074,6 +1074,15 @@ dependencies = [ "either", ] +[[package]] +name = "itertools" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "25db6b064527c5d482d0423354fcd07a89a2dfe07b67892e62411946db7f07b0" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.10" @@ -1566,7 +1575,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5d2d8d10f3c6ded6da8b05b5fb3b8a5082514344d56c9f871412d29b4e075b4" dependencies = [ "anyhow", - "itertools", + "itertools 0.10.5", "proc-macro2", "quote", "syn 1.0.109", @@ -1887,6 +1896,7 @@ dependencies = [ "fs_at", "git-testament", "home", + "itertools 0.12.0", "libc", "once_cell", "opener", diff --git a/Cargo.toml b/Cargo.toml index 2a84c608d2..1d4cf9b4e0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,6 +57,7 @@ flate2 = "1" fs_at.workspace = true git-testament = "0.2" home = "0.5.4" +itertools = "0.12" libc = "0.2" once_cell.workspace = true opener = "0.6.0" diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 8281e77ebe..189899561c 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -10,6 +10,7 @@ use clap::{ Arg, ArgAction, ArgGroup, ArgMatches, Command, ValueEnum, }; use clap_complete::Shell; +use itertools::Itertools; use crate::{ cli::{ @@ -1334,11 +1335,23 @@ fn target_remove(cfg: &Cfg, m: &ArgMatches) -> Result { let distributable = DistributableToolchain::try_from(&toolchain)?; for target in m.get_many::("target").unwrap() { - let new_component = Component::new( - "rust-std".to_string(), - Some(TargetTriple::new(target)), - false, - ); + 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"); + } + let has_at_most_one_target = { + let components = distributable.components()?; + // Every component target that is not `None` (wildcard). + let targets = components + .iter() + .filter_map(|c| c.installed.then(|| c.component.target.clone()).flatten()); + targets.unique().at_most_one().is_ok() + }; + if has_at_most_one_target { + warn!("after 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)?; } diff --git a/tests/suite/cli_v2.rs b/tests/suite/cli_v2.rs index 563e6609fc..81ba41e75d 100644 --- a/tests/suite/cli_v2.rs +++ b/tests/suite/cli_v2.rs @@ -952,9 +952,31 @@ fn remove_target_again() { #[test] fn remove_target_host() { setup(&|config| { - let trip = this_host_triple(); + let host = this_host_triple(); config.expect_ok(&["rustup", "default", "nightly"]); - config.expect_ok(&["rustup", "target", "remove", &trip]); + config.expect_ok(&["rustup", "target", "add", clitools::CROSS_ARCH1]); + config.expect_stderr_ok( + &["rustup", "target", "remove", &host], + "after removing the default host target, proc-macros and build scripts might no longer build", + ); + let path = format!("toolchains/nightly-{host}/lib/rustlib/{host}/lib/libstd.rlib"); + assert!(!config.rustupdir.has(path)); + let path = format!("toolchains/nightly-{host}/lib/rustlib/{host}/lib"); + assert!(!config.rustupdir.has(path)); + let path = format!("toolchains/nightly-{host}/lib/rustlib/{host}"); + assert!(!config.rustupdir.has(path)); + }); +} + +#[test] +fn remove_target_last() { + setup(&|config| { + let host = this_host_triple(); + config.expect_ok(&["rustup", "default", "nightly"]); + config.expect_stderr_ok( + &["rustup", "target", "remove", &host], + "after removing the last target, no build targets will be available", + ); }); }