From e052330964512468cd4931c19b99dbe5057016e9 Mon Sep 17 00:00:00 2001 From: rami3l Date: Thu, 8 Aug 2024 18:30:26 +0800 Subject: [PATCH 01/10] refactor(config)!: rename `local_toolchain()` to `resolve_local_toolchain()` --- src/cli/proxy_mode.rs | 5 ++++- src/config.rs | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/cli/proxy_mode.rs b/src/cli/proxy_mode.rs index 753579aa0c..0b0399bb7e 100644 --- a/src/cli/proxy_mode.rs +++ b/src/cli/proxy_mode.rs @@ -32,6 +32,9 @@ pub async fn main(arg0: &str, current_dir: PathBuf, process: &Process) -> Result .collect(); let cfg = set_globals(current_dir, false, true, process)?; - let cmd = cfg.local_toolchain(toolchain).await?.command(arg0)?; + let cmd = cfg + .resolve_local_toolchain(toolchain)? + .await + .command(arg0)?; run_command_for_dir(cmd, arg0, &cmd_args) } diff --git a/src/config.rs b/src/config.rs index 6cc1165948..23ab0faa33 100644 --- a/src/config.rs +++ b/src/config.rs @@ -722,7 +722,7 @@ impl<'a> Cfg<'a> { }) } - pub(crate) async fn local_toolchain( + pub(crate) async fn resolve_local_toolchain( &self, name: Option, ) -> Result> { From 4356330a2c01bc05ee12cfa1c0f15c5479f24bc3 Mon Sep 17 00:00:00 2001 From: rami3l Date: Thu, 8 Aug 2024 19:17:07 +0800 Subject: [PATCH 02/10] feat(config)!: remove implicit installation from `resolve_local_toolchain()` --- src/config.rs | 9 +++++++-- tests/suite/cli_rustup.rs | 15 ++++++++++----- tests/suite/cli_v1.rs | 22 +++++++++------------- tests/suite/cli_v2.rs | 33 +++++++++++++-------------------- 4 files changed, 39 insertions(+), 40 deletions(-) diff --git a/src/config.rs b/src/config.rs index 23ab0faa33..1f48f01964 100644 --- a/src/config.rs +++ b/src/config.rs @@ -731,8 +731,13 @@ impl<'a> Cfg<'a> { .transpose()?; Ok(match local { - Some(tc) => Toolchain::from_local(tc, false, self).await?, - None => self.find_or_install_active_toolchain(false).await?.0, + Some(tc) => Toolchain::new(self, tc)?, + None => Toolchain::new( + self, + self.find_active_toolchain()? + .ok_or_else(|| no_toolchain_error(self.process))? + .0, + )?, }) } diff --git a/tests/suite/cli_rustup.rs b/tests/suite/cli_rustup.rs index b246fb9645..c1da5dbffb 100644 --- a/tests/suite/cli_rustup.rs +++ b/tests/suite/cli_rustup.rs @@ -2133,7 +2133,7 @@ async fn file_override_toml_format_install_both_toolchain_and_components() { cx.config.expect_ok(&["rustup", "default", "stable"]).await; } - let cx = cx.with_dist_dir(Scenario::ArchivesV2_2015_01_01); + let mut cx = cx.with_dist_dir(Scenario::ArchivesV2_2015_01_01); cx.config .expect_stdout_ok(&["rustc", "--version"], "hash-stable-1.1.0") .await; @@ -2153,6 +2153,9 @@ components = [ "rust-src" ] ) .unwrap(); + cx.config + .expect_ok(&["rustup", "toolchain", "install"]) + .await; cx.config .expect_stdout_ok(&["rustc", "--version"], "hash-nightly-1") .await; @@ -2233,7 +2236,7 @@ components = [ "rust-bongo" ] cx.config .expect_stderr_ok( - &["rustc", "--version"], + &["rustup", "toolchain", "install"], "warn: Force-skipping unavailable component 'rust-bongo", ) .await; @@ -2789,7 +2792,7 @@ async fn dont_warn_on_partial_build() { /// Checks that `rust-toolchain.toml` files are considered #[tokio::test] async fn rust_toolchain_toml() { - let cx = CliTestContext::new(Scenario::SimpleV2).await; + let mut cx = CliTestContext::new(Scenario::SimpleV2).await; cx.config .expect_err( &["rustc", "--version"], @@ -2800,7 +2803,9 @@ async fn rust_toolchain_toml() { let cwd = cx.config.current_dir(); let toolchain_file = cwd.join("rust-toolchain.toml"); raw::write_file(&toolchain_file, "[toolchain]\nchannel = \"nightly\"").unwrap(); - + cx.config + .expect_ok(&["rustup", "toolchain", "install"]) + .await; cx.config .expect_stdout_ok(&["rustc", "--version"], "hash-nightly-2") .await; @@ -2831,7 +2836,7 @@ async fn warn_on_duplicate_rust_toolchain_file() { cx.config .expect_stderr_ok( - &["rustc", "--version"], + &["rustup", "toolchain", "install"], &format!( "warn: both `{0}` and `{1}` exist. Using `{0}`", toolchain_file_1.canonicalize().unwrap().display(), diff --git a/tests/suite/cli_v1.rs b/tests/suite/cli_v1.rs index 78eb492545..4806754221 100644 --- a/tests/suite/cli_v1.rs +++ b/tests/suite/cli_v1.rs @@ -169,18 +169,6 @@ async fn remove_toolchain() { .await; } -#[tokio::test] -async fn remove_default_toolchain_autoinstalls() { - let mut cx = CliTestContext::new(Scenario::SimpleV1).await; - cx.config.expect_ok(&["rustup", "default", "nightly"]).await; - cx.config - .expect_ok(&["rustup", "toolchain", "remove", "nightly"]) - .await; - cx.config - .expect_stderr_ok(&["rustc", "--version"], "info: installing component") - .await; -} - #[tokio::test] async fn remove_override_toolchain_err_handling() { let mut cx = CliTestContext::new(Scenario::SimpleV1).await; @@ -194,7 +182,15 @@ async fn remove_override_toolchain_err_handling() { .expect_ok(&["rustup", "toolchain", "remove", "beta"]) .await; cx.config - .expect_stderr_ok(&["rustc", "--version"], "info: installing component") + .expect_err_ex( + &["rustc", "--version"], + "", + for_host!( + r"error: toolchain 'beta-{0}' is not installed +help: run `rustup toolchain install beta-{0}` to install it +" + ), + ) .await; } diff --git a/tests/suite/cli_v2.rs b/tests/suite/cli_v2.rs index 63c50b3348..995094d298 100644 --- a/tests/suite/cli_v2.rs +++ b/tests/suite/cli_v2.rs @@ -311,18 +311,6 @@ async fn add_remove_multiple_toolchains() { } } -#[tokio::test] -async fn remove_default_toolchain_autoinstalls() { - let mut cx = CliTestContext::new(Scenario::SimpleV2).await; - cx.config.expect_ok(&["rustup", "default", "nightly"]).await; - cx.config - .expect_ok(&["rustup", "toolchain", "remove", "nightly"]) - .await; - cx.config - .expect_stderr_ok(&["rustc", "--version"], "info: installing component") - .await; -} - #[tokio::test] async fn remove_override_toolchain_err_handling() { let mut cx = CliTestContext::new(Scenario::SimpleV2).await; @@ -336,7 +324,15 @@ async fn remove_override_toolchain_err_handling() { .expect_ok(&["rustup", "toolchain", "remove", "beta"]) .await; cx.config - .expect_stderr_ok(&["rustc", "--version"], "info: installing component") + .expect_err_ex( + &["rustc", "--version"], + "", + for_host!( + r"error: toolchain 'beta-{0}' is not installed +help: run `rustup toolchain install beta-{0}` to install it +" + ), + ) .await; } @@ -347,7 +343,10 @@ async fn file_override_toolchain_err_handling() { let toolchain_file = cwd.join("rust-toolchain"); rustup::utils::raw::write_file(&toolchain_file, "beta").unwrap(); cx.config - .expect_stderr_ok(&["rustc", "--version"], "info: installing component") + .expect_err( + &["rustc", "--version"], + for_host!("toolchain 'beta-{0}' is not installed"), + ) .await; } @@ -423,12 +422,6 @@ async fn bad_manifest() { &format!("error: could not parse manifest file: '{}'", path.display()), ) .await; - cx.config - .expect_err( - &["cargo", "--help"], - &format!("error: could not parse manifest file: '{}'", path.display()), - ) - .await; } #[tokio::test] From 42a5e003117a88323e1cdf8791563947d74ac34c Mon Sep 17 00:00:00 2001 From: rami3l Date: Thu, 8 Aug 2024 19:17:33 +0800 Subject: [PATCH 03/10] refactor(config)!: make `resolve_local_toolchain()` sync --- src/cli/proxy_mode.rs | 5 +---- src/config.rs | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/cli/proxy_mode.rs b/src/cli/proxy_mode.rs index 0b0399bb7e..de55f9a99c 100644 --- a/src/cli/proxy_mode.rs +++ b/src/cli/proxy_mode.rs @@ -32,9 +32,6 @@ pub async fn main(arg0: &str, current_dir: PathBuf, process: &Process) -> Result .collect(); let cfg = set_globals(current_dir, false, true, process)?; - let cmd = cfg - .resolve_local_toolchain(toolchain)? - .await - .command(arg0)?; + let cmd = cfg.resolve_local_toolchain(toolchain)?.command(arg0)?; run_command_for_dir(cmd, arg0, &cmd_args) } diff --git a/src/config.rs b/src/config.rs index 1f48f01964..66ff316b16 100644 --- a/src/config.rs +++ b/src/config.rs @@ -722,7 +722,7 @@ impl<'a> Cfg<'a> { }) } - pub(crate) async fn resolve_local_toolchain( + pub(crate) fn resolve_local_toolchain( &self, name: Option, ) -> Result> { From 18f4c6cfa0a9983327a526832dbb72458cf4888c Mon Sep 17 00:00:00 2001 From: rami3l Date: Thu, 8 Aug 2024 18:37:17 +0800 Subject: [PATCH 04/10] refactor(config): extract `toolchain` variable from `resolve_local_toolchain()` --- src/config.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/config.rs b/src/config.rs index 66ff316b16..9196c7e36b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -729,16 +729,15 @@ impl<'a> Cfg<'a> { let local = name .map(|name| name.resolve(&self.get_default_host_triple()?)) .transpose()?; - - Ok(match local { - Some(tc) => Toolchain::new(self, tc)?, - None => Toolchain::new( - self, + let toolchain = match local { + Some(tc) => tc, + None => { self.find_active_toolchain()? .ok_or_else(|| no_toolchain_error(self.process))? - .0, - )?, - }) + .0 + } + }; + Ok(Toolchain::new(self, toolchain)?) } #[tracing::instrument(level = "trace", skip_all)] From a73bbb07af84a8176bbd23e1e893f32fc5a753a7 Mon Sep 17 00:00:00 2001 From: rami3l Date: Thu, 8 Aug 2024 18:41:39 +0800 Subject: [PATCH 05/10] refactor(config): extract `local_toolchain()` from `resolve_local_toolchain()` --- src/config.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index 9196c7e36b..f61a3aa91b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -729,7 +729,11 @@ impl<'a> Cfg<'a> { let local = name .map(|name| name.resolve(&self.get_default_host_triple()?)) .transpose()?; - let toolchain = match local { + self.local_toolchain(local) + } + + fn local_toolchain(&self, name: Option) -> Result> { + let toolchain = match name { Some(tc) => tc, None => { self.find_active_toolchain()? From 045bbc89a159e13a0e074bee3b9dfcf1f95db42f Mon Sep 17 00:00:00 2001 From: rami3l Date: Fri, 9 Aug 2024 17:54:25 +0800 Subject: [PATCH 06/10] test(cli-rustup): remove `heal_damaged_toolchain()` --- tests/suite/cli_rustup.rs | 41 --------------------------------------- 1 file changed, 41 deletions(-) diff --git a/tests/suite/cli_rustup.rs b/tests/suite/cli_rustup.rs index c1da5dbffb..0ba9311e07 100644 --- a/tests/suite/cli_rustup.rs +++ b/tests/suite/cli_rustup.rs @@ -932,47 +932,6 @@ async fn list_default_and_override_toolchain() { .await; } -#[tokio::test] -async fn heal_damaged_toolchain() { - let mut cx = CliTestContext::new(Scenario::SimpleV2).await; - cx.config.expect_ok(&["rustup", "default", "nightly"]).await; - cx.config - .expect_not_stderr_ok(&["rustup", "which", "rustc"], "syncing channel updates") - .await; - let manifest_path = format!( - "toolchains/nightly-{}/lib/rustlib/multirust-channel-manifest.toml", - this_host_triple() - ); - - let mut rustc_path = cx.config.rustupdir.join( - [ - "toolchains", - &format!("nightly-{}", this_host_triple()), - "bin", - "rustc", - ] - .iter() - .collect::(), - ); - - if cfg!(windows) { - rustc_path.set_extension("exe"); - } - - fs::remove_file(cx.config.rustupdir.join(manifest_path)).unwrap(); - cx.config - .expect_ok_ex( - &["rustup", "which", "rustc"], - &format!("{}\n", rustc_path.to_str().unwrap()), - for_host!("info: syncing channel updates for 'nightly-{0}'\n"), - ) - .await; - cx.config.expect_ok(&["rustup", "default", "nightly"]).await; - cx.config - .expect_stderr_ok(&["rustup", "which", "rustc"], "syncing channel updates") - .await; -} - #[tokio::test] #[ignore = "FIXME: Windows shows UNC paths"] async fn show_toolchain_override() { From 349da9d08fd043663ff5ef85111e899429070250 Mon Sep 17 00:00:00 2001 From: rami3l Date: Thu, 8 Aug 2024 18:49:41 +0800 Subject: [PATCH 07/10] feat(config)!: remove implicit installation from `resolve_toolchain()` --- src/config.rs | 9 +++++---- tests/suite/cli_misc.rs | 4 ++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/config.rs b/src/config.rs index f61a3aa91b..0076dacd00 100644 --- a/src/config.rs +++ b/src/config.rs @@ -713,13 +713,14 @@ impl<'a> Cfg<'a> { &self, name: Option, ) -> Result> { - Ok(match name { + let toolchain = match name { Some(name) => { let desc = name.resolve(&self.get_default_host_triple()?)?; - Toolchain::new(self, desc.into())? + Some(desc.into()) } - None => self.find_or_install_active_toolchain(false).await?.0, - }) + None => None, + }; + self.local_toolchain(toolchain) } pub(crate) fn resolve_local_toolchain( diff --git a/tests/suite/cli_misc.rs b/tests/suite/cli_misc.rs index e8f3588d73..69a13983b1 100644 --- a/tests/suite/cli_misc.rs +++ b/tests/suite/cli_misc.rs @@ -1099,6 +1099,10 @@ async fn which_asking_uninstalled_toolchain() { #[tokio::test] async fn override_by_toolchain_on_the_command_line() { let mut cx = CliTestContext::new(Scenario::SimpleV2).await; + cx.config + .expect_ok(&["rustup", "toolchain", "install", "stable", "nightly"]) + .await; + #[cfg(windows)] cx.config .expect_stdout_ok( From b066dde7feea176d00819113b8b299a861671d7d Mon Sep 17 00:00:00 2001 From: rami3l Date: Thu, 8 Aug 2024 19:19:06 +0800 Subject: [PATCH 08/10] refactor(config)!: make `resolve_toolchain()` sync --- src/cli/rustup_mode.rs | 2 +- src/config.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 03a6470805..d80bc35e93 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -908,7 +908,7 @@ async fn which( binary: &str, toolchain: Option, ) -> Result { - let binary_path = cfg.resolve_toolchain(toolchain).await?.binary_file(binary); + let binary_path = cfg.resolve_toolchain(toolchain)?.binary_file(binary); utils::assert_is_file(&binary_path)?; diff --git a/src/config.rs b/src/config.rs index 0076dacd00..e0f7f9dcbd 100644 --- a/src/config.rs +++ b/src/config.rs @@ -709,7 +709,7 @@ impl<'a> Cfg<'a> { Ok(Some(Toolchain::new(self, name)?.rustc_version())) } - pub(crate) async fn resolve_toolchain( + pub(crate) fn resolve_toolchain( &self, name: Option, ) -> Result> { From e0b5727e95a908bca833dc385fb807a8a2711dd3 Mon Sep 17 00:00:00 2001 From: rami3l Date: Thu, 8 Aug 2024 18:56:22 +0800 Subject: [PATCH 09/10] feat(config)!: remove implicit installation from `toolchain_from_partial()` --- src/config.rs | 12 +++++------- tests/suite/cli_rustup.rs | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/config.rs b/src/config.rs index e0f7f9dcbd..ac3f30adc7 100644 --- a/src/config.rs +++ b/src/config.rs @@ -502,16 +502,14 @@ impl<'a> Cfg<'a> { &self, toolchain: Option, ) -> anyhow::Result> { - match toolchain { + let toolchain = match toolchain { Some(toolchain) => { let desc = toolchain.resolve(&self.get_default_host_triple()?)?; - Ok(Toolchain::new( - self, - LocalToolchainName::Named(ToolchainName::Official(desc)), - )?) + Some(LocalToolchainName::Named(ToolchainName::Official(desc))) } - None => Ok(self.find_or_install_active_toolchain(false).await?.0), - } + None => None, + }; + self.local_toolchain(toolchain) } pub(crate) fn find_active_toolchain( diff --git a/tests/suite/cli_rustup.rs b/tests/suite/cli_rustup.rs index 0ba9311e07..5eb5d3dd4b 100644 --- a/tests/suite/cli_rustup.rs +++ b/tests/suite/cli_rustup.rs @@ -2142,6 +2142,12 @@ components = [ "rust-src" ] ) .unwrap(); + cx.config + .expect_stderr_ok( + &["rustup", "toolchain", "install"], + "info: installing component 'rust-src'", + ) + .await; cx.config .expect_stdout_ok(&["rustup", "component", "list"], "rust-src (installed)") .await; @@ -2169,6 +2175,12 @@ targets = [ "arm-linux-androideabi" ] ) .unwrap(); + cx.config + .expect_stderr_ok( + &["rustup", "toolchain", "install"], + "info: installing component 'rust-std' for 'arm-linux-androideabi'", + ) + .await; cx.config .expect_stdout_ok( &["rustup", "component", "list"], @@ -2225,6 +2237,9 @@ channel = "nightly" "#, ) .unwrap(); + cx.config + .expect_ok(&["rustup", "toolchain", "install"]) + .await; cx.config .expect_not_stdout_ok( &["rustup", "component", "list"], From bfa64d3b1fa96c55fb99900a84f45ff90f364d6d Mon Sep 17 00:00:00 2001 From: rami3l Date: Thu, 8 Aug 2024 19:20:11 +0800 Subject: [PATCH 10/10] refactor(config)!: make `toolchain_from_partial()` sync --- src/cli/rustup_mode.rs | 16 ++++++++-------- src/config.rs | 2 +- src/toolchain/distributable.rs | 6 ++---- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index d80bc35e93..89fd348f23 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -1097,7 +1097,7 @@ async fn target_list( quiet: bool, ) -> Result { // downcasting required because the toolchain files can name any toolchain - let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; + let distributable = DistributableToolchain::from_partial(toolchain, cfg)?; common::list_items( distributable, |c| { @@ -1124,7 +1124,7 @@ async fn target_add( // isn't a feature yet. // list_components *and* add_component would both be inappropriate for // custom toolchains. - let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; + let distributable = DistributableToolchain::from_partial(toolchain, cfg)?; let components = distributable.components()?; if targets.contains(&"all".to_string()) { @@ -1168,7 +1168,7 @@ async fn target_remove( targets: Vec, toolchain: Option, ) -> Result { - let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; + let distributable = DistributableToolchain::from_partial(toolchain, cfg)?; for target in targets { let target = TargetTriple::new(target); @@ -1204,7 +1204,7 @@ async fn component_list( quiet: bool, ) -> Result { // downcasting required because the toolchain files can name any toolchain - let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; + let distributable = DistributableToolchain::from_partial(toolchain, cfg)?; common::list_items( distributable, |c| Some(&c.name), @@ -1220,7 +1220,7 @@ async fn component_add( toolchain: Option, target: Option, ) -> Result { - let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; + let distributable = DistributableToolchain::from_partial(toolchain, cfg)?; let target = get_target(target, &distributable); for component in &components { @@ -1246,7 +1246,7 @@ async fn component_remove( toolchain: Option, target: Option, ) -> Result { - let distributable = DistributableToolchain::from_partial(toolchain, cfg).await?; + let distributable = DistributableToolchain::from_partial(toolchain, cfg)?; let target = get_target(target, &distributable); for component in &components { @@ -1447,7 +1447,7 @@ async fn doc( mut topic: Option<&str>, doc_page: &DocPage, ) -> Result { - let toolchain = cfg.toolchain_from_partial(toolchain).await?; + let toolchain = cfg.toolchain_from_partial(toolchain)?; if let Ok(distributable) = DistributableToolchain::try_from(&toolchain) { if let [_] = distributable @@ -1508,7 +1508,7 @@ async fn man( command: &str, toolchain: Option, ) -> Result { - let toolchain = cfg.toolchain_from_partial(toolchain).await?; + let toolchain = cfg.toolchain_from_partial(toolchain)?; let path = toolchain.man_path(); utils::assert_is_directory(&path)?; diff --git a/src/config.rs b/src/config.rs index ac3f30adc7..52c26285fa 100644 --- a/src/config.rs +++ b/src/config.rs @@ -498,7 +498,7 @@ impl<'a> Cfg<'a> { .transpose()?) } - pub(crate) async fn toolchain_from_partial( + pub(crate) fn toolchain_from_partial( &self, toolchain: Option, ) -> anyhow::Result> { diff --git a/src/toolchain/distributable.rs b/src/toolchain/distributable.rs index 45cfa4da05..49a6cb6700 100644 --- a/src/toolchain/distributable.rs +++ b/src/toolchain/distributable.rs @@ -33,13 +33,11 @@ pub(crate) struct DistributableToolchain<'a> { } impl<'a> DistributableToolchain<'a> { - pub(crate) async fn from_partial( + pub(crate) fn from_partial( toolchain: Option, cfg: &'a Cfg<'a>, ) -> anyhow::Result { - Ok(Self::try_from( - &cfg.toolchain_from_partial(toolchain).await?, - )?) + Ok(Self::try_from(&cfg.toolchain_from_partial(toolchain)?)?) } pub(crate) fn new(cfg: &'a Cfg<'a>, desc: ToolchainDesc) -> Result {