From 04f5a5e9628c6581ad0a6470a4dfef983926101d Mon Sep 17 00:00:00 2001 From: ongchi Date: Sun, 24 Dec 2023 19:29:29 +0800 Subject: [PATCH 1/3] Replace Component::new_with_target by Component::try_new --- src/cli/rustup_mode.rs | 6 ++---- src/dist/manifest.rs | 45 +++++++++++++++++++++++++++++------------- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 8281e77ebe..31aa1ab6ac 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -1364,8 +1364,7 @@ fn component_add(cfg: &Cfg, m: &ArgMatches) -> Result { let target = get_target(m, &distributable); for component in m.get_many::("component").unwrap() { - let new_component = Component::new_with_target(component, false) - .unwrap_or_else(|| Component::new(component.to_string(), target.clone(), true)); + let new_component = Component::try_new(component, &distributable, target.as_ref())?; distributable.add_component(new_component)?; } @@ -1385,8 +1384,7 @@ fn component_remove(cfg: &Cfg, m: &ArgMatches) -> Result { let target = get_target(m, &distributable); for component in m.get_many::("component").unwrap() { - let new_component = Component::new_with_target(component, false) - .unwrap_or_else(|| Component::new(component.to_string(), target.clone(), true)); + let new_component = Component::try_new(component, &distributable, target.as_ref())?; distributable.remove_component(new_component)?; } diff --git a/src/dist/manifest.rs b/src/dist/manifest.rs index c46dd8d2a4..551d9d3599 100644 --- a/src/dist/manifest.rs +++ b/src/dist/manifest.rs @@ -18,8 +18,9 @@ use std::str::FromStr; use anyhow::{anyhow, bail, Context, Result}; -use crate::dist::dist::{PartialTargetTriple, Profile, TargetTriple}; +use crate::dist::dist::{Profile, TargetTriple}; use crate::errors::*; +use crate::toolchain::distributable::DistributableToolchain; use crate::utils::toml_utils::*; use super::{config::Config, dist::ToolchainDesc}; @@ -588,21 +589,37 @@ impl Component { } } - pub(crate) fn new_with_target(pkg_with_target: &str, is_extension: bool) -> Option { - for (pos, _) in pkg_with_target.match_indices('-') { - let pkg = &pkg_with_target[0..pos]; - let target = &pkg_with_target[pos + 1..]; - if let Some(partial) = PartialTargetTriple::new(target) { - if let Ok(triple) = TargetTriple::try_from(partial) { - return Some(Self { - pkg: pkg.to_string(), - target: Some(triple), - is_extension, - }); - } + pub(crate) fn try_new( + name: &str, + distributable: &DistributableToolchain<'_>, + fallback_target: Option<&TargetTriple>, + ) -> Result { + let manifestation = distributable.get_manifestation()?; + let config = manifestation.read_config()?.unwrap_or_default(); + let manifest = distributable.get_manifest()?; + let manifest_components = manifest.query_components(distributable.desc(), &config)?; + + for component_status in manifest_components { + let short_name = component_status.component.short_name_in_manifest(); + let target = component_status.component.target.as_ref(); + + if name.starts_with(short_name) + && target.is_some() + && name == format!("{}-{}", short_name, target.unwrap()) + { + return Ok(Component::new( + short_name.to_string(), + target.cloned(), + false, + )); } } - None + + Ok(Component::new( + name.to_string(), + fallback_target.cloned(), + true, + )) } pub(crate) fn wildcard(&self) -> Self { From 17202391a02a4816ac02a3ccf83f4231b0da2e9c Mon Sep 17 00:00:00 2001 From: ongchi Date: Sun, 24 Dec 2023 21:18:03 +0800 Subject: [PATCH 2/3] Add tests for add/remove components by name with target triple --- tests/suite/cli_rustup.rs | 100 +++++++++++++++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 2 deletions(-) diff --git a/tests/suite/cli_rustup.rs b/tests/suite/cli_rustup.rs index 9a6adde7a4..ced16aeb86 100644 --- a/tests/suite/cli_rustup.rs +++ b/tests/suite/cli_rustup.rs @@ -1366,6 +1366,63 @@ fn add_component() { }); } +#[test] +fn add_component_by_target_triple() { + test(&|config| { + config.with_scenario(Scenario::SimpleV2, &|config| { + config.expect_ok(&["rustup", "default", "stable"]); + config.expect_ok(&[ + "rustup", + "component", + "add", + &format!("rust-std-{}", clitools::CROSS_ARCH1), + ]); + let path = format!( + "toolchains/stable-{}/lib/rustlib/{}/lib/libstd.rlib", + this_host_triple(), + clitools::CROSS_ARCH1 + ); + assert!(config.rustupdir.has(path)); + }) + }); +} + +#[test] +fn fail_invalid_component_name() { + test(&|config| { + config.with_scenario(Scenario::SimpleV2, &|config| { + config.expect_ok(&["rustup", "default", "stable"]); + config.expect_err( + &[ + "rustup", + "component", + "add", + &format!("dummy-{}", clitools::CROSS_ARCH1), + ], + &format!("error: toolchain 'stable-{}' does not contain component 'dummy-{}' for target '{}'",this_host_triple(), clitools::CROSS_ARCH1, this_host_triple()), + ); + }) + }); +} + +#[test] +fn fail_invalid_component_target() { + test(&|config| { + config.with_scenario(Scenario::SimpleV2, &|config| { + config.expect_ok(&["rustup", "default", "stable"]); + config.expect_err( + &[ + "rustup", + "component", + "add", + "rust-std-invalid-target", + ], + &format!("error: toolchain 'stable-{}' does not contain component 'rust-std-invalid-target' for target '{}'",this_host_triple(), this_host_triple()), + ); + }) + }); +} + #[test] fn remove_component() { test(&|config| { @@ -1383,22 +1440,61 @@ fn remove_component() { }); } +#[test] +fn remove_component_by_target_triple() { + let component_with_triple = format!("rust-std-{}", clitools::CROSS_ARCH1); + test(&|config| { + config.with_scenario(Scenario::SimpleV2, &|config| { + config.expect_ok(&["rustup", "default", "stable"]); + config.expect_ok(&["rustup", "component", "add", &component_with_triple]); + let path = PathBuf::from(format!( + "toolchains/stable-{}/lib/rustlib/{}/lib/libstd.rlib", + this_host_triple(), + clitools::CROSS_ARCH1 + )); + assert!(config.rustupdir.has(&path)); + config.expect_ok(&["rustup", "component", "remove", &component_with_triple]); + assert!(!config.rustupdir.has(path.parent().unwrap())); + }) + }); +} + #[test] fn add_remove_multiple_components() { let files = [ "lib/rustlib/src/rust-src/foo.rs".to_owned(), format!("lib/rustlib/{}/analysis/libfoo.json", this_host_triple()), + format!("lib/rustlib/{}/lib/libstd.rlib", clitools::CROSS_ARCH1), + format!("lib/rustlib/{}/lib/libstd.rlib", clitools::CROSS_ARCH2), ]; + let component_with_triple1 = format!("rust-std-{}", clitools::CROSS_ARCH1); + let component_with_triple2 = format!("rust-std-{}", clitools::CROSS_ARCH2); test(&|config| { config.with_scenario(Scenario::SimpleV2, &|config| { config.expect_ok(&["rustup", "default", "nightly"]); - config.expect_ok(&["rustup", "component", "add", "rust-src", "rust-analysis"]); + config.expect_ok(&[ + "rustup", + "component", + "add", + "rust-src", + "rust-analysis", + &component_with_triple1, + &component_with_triple2, + ]); for file in &files { let path = format!("toolchains/nightly-{}/{}", this_host_triple(), file); assert!(config.rustupdir.has(&path)); } - config.expect_ok(&["rustup", "component", "remove", "rust-src", "rust-analysis"]); + config.expect_ok(&[ + "rustup", + "component", + "remove", + "rust-src", + "rust-analysis", + &component_with_triple1, + &component_with_triple2, + ]); for file in &files { let path = PathBuf::from(format!( "toolchains/nightly-{}/{}", From cc15f593dbe8753845c8c602de0016008c98e3bc Mon Sep 17 00:00:00 2001 From: ongchi Date: Sun, 24 Dec 2023 21:56:30 +0800 Subject: [PATCH 3/3] Remove TryFrom for TargetTriple --- src/dist/dist.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/dist/dist.rs b/src/dist/dist.rs index aca41172d9..d00acfb51b 100644 --- a/src/dist/dist.rs +++ b/src/dist/dist.rs @@ -468,22 +468,6 @@ impl TargetTriple { } } -impl std::convert::TryFrom for TargetTriple { - type Error = &'static str; - fn try_from(value: PartialTargetTriple) -> std::result::Result { - if value.arch.is_some() && value.os.is_some() && value.env.is_some() { - Ok(Self(format!( - "{}-{}-{}", - value.arch.unwrap(), - value.os.unwrap(), - value.env.unwrap() - ))) - } else { - Err("Incomplete / bad target triple") - } - } -} - impl FromStr for PartialToolchainDesc { type Err = anyhow::Error; fn from_str(name: &str) -> Result {