From 235e5691820c309c13c9cc81cbb2c5f48a0a607c Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 07:00:26 +0200 Subject: [PATCH 01/17] Pass current_dir down from main() --- src/bin/rustup-init.rs | 8 ++++---- src/cli/common.rs | 19 +++++++++++-------- src/cli/proxy_mode.rs | 6 +++--- src/cli/rustup_mode.rs | 10 +++++----- src/cli/self_update.rs | 9 ++++++--- src/cli/setup_mode.rs | 6 ++++-- src/config.rs | 7 +++++-- src/test/mock/clitools.rs | 2 +- 8 files changed, 39 insertions(+), 28 deletions(-) diff --git a/src/bin/rustup-init.rs b/src/bin/rustup-init.rs index 7b13d00198..e27a3e4631 100644 --- a/src/bin/rustup-init.rs +++ b/src/bin/rustup-init.rs @@ -115,17 +115,17 @@ async fn run_rustup_inner() -> Result { // Before we do anything else, ensure we know where we are and who we // are because otherwise we cannot proceed usefully. - utils::current_dir()?; + let current_dir = utils::current_dir()?; utils::current_exe()?; match process().name().as_deref() { - Some("rustup") => rustup_mode::main().await, + Some("rustup") => rustup_mode::main(current_dir).await, Some(n) if n.starts_with("rustup-setup") || n.starts_with("rustup-init") => { // NB: The above check is only for the prefix of the file // name. Browsers rename duplicates to // e.g. rustup-setup(2), and this allows all variations // to work. - setup_mode::main().await + setup_mode::main(current_dir).await } Some(n) if n.starts_with("rustup-gc-") => { // This is the final uninstallation stage on windows where @@ -140,7 +140,7 @@ async fn run_rustup_inner() -> Result { } Some(n) => { is_proxyable_tools(n)?; - proxy_mode::main(n).await.map(ExitCode::from) + proxy_mode::main(n, current_dir).await.map(ExitCode::from) } None => { // Weird case. No arg0, or it's unparsable. diff --git a/src/cli/common.rs b/src/cli/common.rs index 45eae5760a..d65c28b4df 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -4,7 +4,7 @@ use std::cell::RefCell; use std::fmt::Display; use std::fs; use std::io::{BufRead, ErrorKind, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::sync::Arc; use std::{cmp, env}; @@ -168,19 +168,22 @@ impl NotifyOnConsole { } #[cfg_attr(feature = "otel", tracing::instrument)] -pub(crate) fn set_globals(verbose: bool, quiet: bool) -> Result { +pub(crate) fn set_globals(current_dir: PathBuf, verbose: bool, quiet: bool) -> Result { let download_tracker = DownloadTracker::new_with_display_progress(!quiet); let console_notifier = RefCell::new(NotifyOnConsole { verbose, ..Default::default() }); - Cfg::from_env(Arc::new(move |n: Notification<'_>| { - if download_tracker.lock().unwrap().handle_notification(&n) { - return; - } - console_notifier.borrow_mut().handle(n); - })) + Cfg::from_env( + current_dir, + Arc::new(move |n: Notification<'_>| { + if download_tracker.lock().unwrap().handle_notification(&n) { + return; + } + console_notifier.borrow_mut().handle(n); + }), + ) } pub(crate) fn show_channel_update( diff --git a/src/cli/proxy_mode.rs b/src/cli/proxy_mode.rs index 362de80d42..1ae30699db 100644 --- a/src/cli/proxy_mode.rs +++ b/src/cli/proxy_mode.rs @@ -1,4 +1,4 @@ -use std::ffi::OsString; +use std::{ffi::OsString, path::PathBuf}; use std::process::ExitStatus; use anyhow::Result; @@ -12,7 +12,7 @@ use crate::{ }; #[cfg_attr(feature = "otel", tracing::instrument)] -pub async fn main(arg0: &str) -> Result { +pub async fn main(arg0: &str, current_dir: PathBuf) -> Result { self_update::cleanup_self_updater()?; let _setup = job::setup(); @@ -35,7 +35,7 @@ pub async fn main(arg0: &str) -> Result { .skip(1 + toolchain.is_some() as usize) .collect(); - let cfg = set_globals(false, true)?; + let cfg = set_globals(current_dir, false, true)?; cfg.check_metadata_version()?; let toolchain = toolchain .map(|t| t.resolve(&cfg.get_default_host_triple()?)) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 6b760dc6a9..d207c0bb8a 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -524,7 +524,7 @@ enum SetSubcmd { } #[cfg_attr(feature = "otel", tracing::instrument(fields(args = format!("{:?}", process().args_os().collect::>()))))] -pub async fn main() -> Result { +pub async fn main(current_dir: PathBuf) -> Result { self_update::cleanup_self_updater()?; use clap::error::ErrorKind::*; @@ -539,8 +539,8 @@ pub async fn main() -> Result { info!("This is the version for the rustup toolchain manager, not the rustc compiler."); #[cfg_attr(feature = "otel", tracing::instrument)] - async fn rustc_version() -> std::result::Result> { - let cfg = &mut common::set_globals(false, true)?; + async fn rustc_version(current_dir: PathBuf) -> std::result::Result> { + let cfg = &mut common::set_globals(current_dir, false, true)?; if let Some(t) = process().args().find(|x| x.starts_with('+')) { debug!("Fetching rustc version from toolchain `{}`", t); @@ -552,7 +552,7 @@ pub async fn main() -> Result { Ok(toolchain.rustc_version()) } - match rustc_version().await { + match rustc_version(current_dir).await { Ok(version) => info!("The currently active `rustc` version is `{}`", version), Err(err) => debug!("Wanted to tell you the current rustc version, too, but ran into this error: {}", err), } @@ -577,7 +577,7 @@ pub async fn main() -> Result { Err(err) } }?; - let cfg = &mut common::set_globals(matches.verbose, matches.quiet)?; + let cfg = &mut common::set_globals(current_dir, matches.verbose, matches.quiet)?; if let Some(t) = &matches.plus_toolchain { cfg.set_toolchain_override(t); diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index cdcd6ac603..c873000481 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -365,6 +365,7 @@ fn canonical_cargo_home() -> Result> { /// `CARGO_HOME`/bin, hard-linking the various Rust tools to it, /// and adding `CARGO_HOME`/bin to PATH. pub(crate) async fn install( + current_dir: PathBuf, no_prompt: bool, verbose: bool, quiet: bool, @@ -458,7 +459,7 @@ pub(crate) async fn install( } } - let install_res = || async { + let install_res = move || async move { install_bins()?; #[cfg(unix)] @@ -476,6 +477,7 @@ pub(crate) async fn install( !opts.no_update_toolchain, opts.components, opts.targets, + current_dir, verbose, quiet, ) @@ -847,10 +849,11 @@ async fn maybe_install_rust( update_existing_toolchain: bool, components: &[&str], targets: &[&str], + current_dir: PathBuf, verbose: bool, quiet: bool, ) -> Result<()> { - let mut cfg = common::set_globals(verbose, quiet)?; + let mut cfg = common::set_globals(current_dir, verbose, quiet)?; let toolchain = _install_selection( &mut cfg, @@ -1329,7 +1332,7 @@ mod tests { currentprocess::with(tp.clone().into(), || -> Result<()> { // TODO: we could pass in a custom cfg to get notification // callbacks rather than output to the tp sink. - let mut cfg = common::set_globals(false, false).unwrap(); + let mut cfg = common::set_globals(tp.cwd.clone(), false, false).unwrap(); assert_eq!( "stable" .parse::() diff --git a/src/cli/setup_mode.rs b/src/cli/setup_mode.rs index 5dd32b49be..49dcbb3bb3 100644 --- a/src/cli/setup_mode.rs +++ b/src/cli/setup_mode.rs @@ -1,3 +1,5 @@ +use std::path::PathBuf; + use anyhow::Result; use clap::{builder::PossibleValuesParser, Parser}; @@ -74,7 +76,7 @@ struct RustupInit { } #[cfg_attr(feature = "otel", tracing::instrument)] -pub async fn main() -> Result { +pub async fn main(current_dir: PathBuf) -> Result { use clap::error::ErrorKind; let RustupInit { @@ -122,5 +124,5 @@ pub async fn main() -> Result { targets: &target.iter().map(|s| &**s).collect::>(), }; - self_update::install(no_prompt, verbose, quiet, opts).await + self_update::install(current_dir, no_prompt, verbose, quiet, opts).await } diff --git a/src/config.rs b/src/config.rs index 15a525f98b..26b76717f2 100644 --- a/src/config.rs +++ b/src/config.rs @@ -289,7 +289,10 @@ pub(crate) struct Cfg { } impl Cfg { - pub(crate) fn from_env(notify_handler: Arc)>) -> Result { + pub(crate) fn from_env( + current_dir: PathBuf, + notify_handler: Arc)>, + ) -> Result { // Set up the rustup home directory let rustup_dir = utils::rustup_home()?; @@ -367,7 +370,7 @@ impl Cfg { toolchain_override: None, env_override, dist_root_url: dist_root, - current_dir: utils::current_dir()?, + current_dir, }; // Run some basic checks against the constructed configuration diff --git a/src/test/mock/clitools.rs b/src/test/mock/clitools.rs index 4a47ecb5dd..572f4a1e0a 100644 --- a/src/test/mock/clitools.rs +++ b/src/test/mock/clitools.rs @@ -732,7 +732,7 @@ impl Config { .worker_threads(2) .max_blocking_threads(2); let process_res = - currentprocess::with_runtime(tp.clone().into(), builder, rustup_mode::main()); + currentprocess::with_runtime(tp.clone().into(), builder, rustup_mode::main(tp.cwd.clone())); // convert Err's into an ec let ec = match process_res { Ok(process_res) => process_res, From 0102b04bf073e66ec6d218de0285eda250a24474 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 07:05:05 +0200 Subject: [PATCH 02/17] Take explicit current_dir argument in to_absolute() --- src/cli/rustup_mode.rs | 2 +- src/utils/utils.rs | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index d207c0bb8a..6737789214 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -1265,7 +1265,7 @@ async fn toolchain_link( if true { InstallMethod::Link { - src: &utils::to_absolute(src)?, + src: &utils::to_absolute(src, &cfg.current_dir), dest, cfg, } diff --git a/src/utils/utils.rs b/src/utils/utils.rs index c93ee899d2..592e5cec8e 100644 --- a/src/utils/utils.rs +++ b/src/utils/utils.rs @@ -501,11 +501,10 @@ pub fn current_exe() -> Result { env::current_exe().context(RustupError::LocatingWorkingDir) } -pub(crate) fn to_absolute>(path: P) -> Result { - current_dir().map(|mut v| { - v.push(path); - v - }) +pub(crate) fn to_absolute>(path: P, cwd: &Path) -> PathBuf { + let mut new = PathBuf::from(cwd); + new.push(path); + new } pub(crate) fn home_dir() -> Option { From e4fc3b7cb5d9a1b8c0ebb4f283b6efd75e35c453 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 07:10:34 +0200 Subject: [PATCH 03/17] Inline utils::current_dir() --- src/bin/rustup-init.rs | 7 +++++-- src/currentprocess.rs | 2 +- src/dist/dist.rs | 2 +- src/errors.rs | 2 +- src/toolchain/names.rs | 6 +++--- src/utils/utils.rs | 6 ------ 6 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/bin/rustup-init.rs b/src/bin/rustup-init.rs index e27a3e4631..57b9057be5 100644 --- a/src/bin/rustup-init.rs +++ b/src/bin/rustup-init.rs @@ -13,7 +13,7 @@ #![recursion_limit = "1024"] -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, Context, Result}; use cfg_if::cfg_if; // Public macros require availability of the internal symbols use rs_tracing::{ @@ -29,6 +29,7 @@ use rustup::cli::self_update; use rustup::cli::setup_mode; use rustup::currentprocess::{process, with_runtime, Process}; use rustup::env_var::RUST_RECURSION_COUNT_MAX; +use rustup::errors::RustupError; use rustup::is_proxyable_tools; use rustup::utils::utils::{self, ExitCode}; @@ -115,7 +116,9 @@ async fn run_rustup_inner() -> Result { // Before we do anything else, ensure we know where we are and who we // are because otherwise we cannot proceed usefully. - let current_dir = utils::current_dir()?; + let current_dir = process() + .current_dir() + .context(RustupError::LocatingWorkingDir)?; utils::current_exe()?; match process().name().as_deref() { diff --git a/src/currentprocess.rs b/src/currentprocess.rs index ba3d982e3d..27660bc85f 100644 --- a/src/currentprocess.rs +++ b/src/currentprocess.rs @@ -106,7 +106,7 @@ impl Process { } } - pub(crate) fn current_dir(&self) -> io::Result { + pub fn current_dir(&self) -> io::Result { match self { Process::OSProcess(_) => env::current_dir(), #[cfg(feature = "test")] diff --git a/src/dist/dist.rs b/src/dist/dist.rs index e3e0711b95..7a2139fa00 100644 --- a/src/dist/dist.rs +++ b/src/dist/dist.rs @@ -96,7 +96,7 @@ fn components_missing_msg(cs: &[Component], manifest: &ManifestV2, toolchain: &s } #[derive(Debug, ThisError)] -pub(crate) enum DistError { +pub enum DistError { #[error("{}", components_missing_msg(.0, .1, .2))] ToolchainComponentsMissing(Vec, Box, String), #[error("no release found for '{0}'")] diff --git a/src/errors.rs b/src/errors.rs index dafc05b857..3ed45506c3 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -25,7 +25,7 @@ use crate::{ pub struct OperationError(pub anyhow::Error); #[derive(ThisError, Debug)] -pub(crate) enum RustupError { +pub enum RustupError { #[error("partially downloaded file may have been damaged and was removed, please try again")] BrokenPartialFile, #[error("component download failed for {0}")] diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index 3dece6e5fb..ad1147acc2 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -249,7 +249,7 @@ impl Display for MaybeOfficialToolchainName { /// like setting overrides, or that depend on configuration, like calculating /// the toolchain directory. #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] -pub(crate) enum ToolchainName { +pub enum ToolchainName { Custom(CustomToolchainName), Official(ToolchainDesc), } @@ -396,7 +396,7 @@ impl Display for LocalToolchainName { /// A custom toolchain name, but not an official toolchain name /// (e.g. my-custom-toolchain) #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] -pub(crate) struct CustomToolchainName(String); +pub struct CustomToolchainName(String); impl CustomToolchainName { fn validate(candidate: &str) -> Result { @@ -433,7 +433,7 @@ impl Display for CustomToolchainName { /// code execution in a rust dir, so as a partial mitigation is limited to /// absolute paths. #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] -pub(crate) struct PathBasedToolchainName(PathBuf, String); +pub struct PathBasedToolchainName(PathBuf, String); impl Display for PathBasedToolchainName { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { diff --git a/src/utils/utils.rs b/src/utils/utils.rs index 592e5cec8e..42beb3c458 100644 --- a/src/utils/utils.rs +++ b/src/utils/utils.rs @@ -491,12 +491,6 @@ pub(crate) fn make_executable(path: &Path) -> Result<()> { inner(path) } -pub fn current_dir() -> Result { - process() - .current_dir() - .context(RustupError::LocatingWorkingDir) -} - pub fn current_exe() -> Result { env::current_exe().context(RustupError::LocatingWorkingDir) } From ca03877ca084cc786543504ce1900116fba70041 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 09:56:06 +0200 Subject: [PATCH 04/17] Inline trivial single-use function --- src/utils/utils.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/utils/utils.rs b/src/utils/utils.rs index 42beb3c458..193e96ebfd 100644 --- a/src/utils/utils.rs +++ b/src/utils/utils.rs @@ -523,13 +523,11 @@ pub(crate) fn create_rustup_home() -> Result<()> { Ok(()) } -fn dot_dir(name: &str) -> Option { - home_dir().map(|p| p.join(name)) -} - fn rustup_home_in_user_dir() -> Result { // XXX: This error message seems wrong/bogus. - dot_dir(".rustup").ok_or_else(|| anyhow::anyhow!("couldn't find value of RUSTUP_HOME")) + home_dir() + .map(|p| p.join(".rustup")) + .ok_or_else(|| anyhow::anyhow!("couldn't find value of RUSTUP_HOME")) } pub(crate) fn rustup_home() -> Result { From e4d8269a9e52c85d7e5fcac1452beefa8ad08fdf Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 09:57:29 +0200 Subject: [PATCH 05/17] Inline trivial single-use function --- src/utils/utils.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/utils/utils.rs b/src/utils/utils.rs index 193e96ebfd..375dd763b2 100644 --- a/src/utils/utils.rs +++ b/src/utils/utils.rs @@ -517,17 +517,13 @@ pub(crate) fn create_rustup_home() -> Result<()> { return Ok(()); } - let home = rustup_home_in_user_dir()?; - fs::create_dir_all(home).context("unable to create ~/.rustup")?; - - Ok(()) -} - -fn rustup_home_in_user_dir() -> Result { // XXX: This error message seems wrong/bogus. - home_dir() + let home = home_dir() .map(|p| p.join(".rustup")) - .ok_or_else(|| anyhow::anyhow!("couldn't find value of RUSTUP_HOME")) + .ok_or_else(|| anyhow::anyhow!("couldn't find value of RUSTUP_HOME"))?; + + fs::create_dir_all(home).context("unable to create ~/.rustup")?; + Ok(()) } pub(crate) fn rustup_home() -> Result { From 0ec118cf032dddad48589c6bcb833a0da2ecc904 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 09:58:54 +0200 Subject: [PATCH 06/17] Improve error message for failing .rustup creation --- src/cli/proxy_mode.rs | 3 +-- src/cli/rustup_mode.rs | 4 +++- src/test/mock/clitools.rs | 7 +++++-- src/utils/utils.rs | 3 +-- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/cli/proxy_mode.rs b/src/cli/proxy_mode.rs index 1ae30699db..5f659a4eb7 100644 --- a/src/cli/proxy_mode.rs +++ b/src/cli/proxy_mode.rs @@ -1,5 +1,4 @@ -use std::{ffi::OsString, path::PathBuf}; -use std::process::ExitStatus; +use std::{ffi::OsString, path::PathBuf, process::ExitStatus}; use anyhow::Result; diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 6737789214..a500e5d4aa 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -539,7 +539,9 @@ pub async fn main(current_dir: PathBuf) -> Result { info!("This is the version for the rustup toolchain manager, not the rustc compiler."); #[cfg_attr(feature = "otel", tracing::instrument)] - async fn rustc_version(current_dir: PathBuf) -> std::result::Result> { + async fn rustc_version( + current_dir: PathBuf, + ) -> std::result::Result> { let cfg = &mut common::set_globals(current_dir, false, true)?; if let Some(t) = process().args().find(|x| x.starts_with('+')) { diff --git a/src/test/mock/clitools.rs b/src/test/mock/clitools.rs index 572f4a1e0a..efd093ffef 100644 --- a/src/test/mock/clitools.rs +++ b/src/test/mock/clitools.rs @@ -731,8 +731,11 @@ impl Config { .enable_all() .worker_threads(2) .max_blocking_threads(2); - let process_res = - currentprocess::with_runtime(tp.clone().into(), builder, rustup_mode::main(tp.cwd.clone())); + let process_res = currentprocess::with_runtime( + tp.clone().into(), + builder, + rustup_mode::main(tp.cwd.clone()), + ); // convert Err's into an ec let ec = match process_res { Ok(process_res) => process_res, diff --git a/src/utils/utils.rs b/src/utils/utils.rs index 375dd763b2..3aa546ed69 100644 --- a/src/utils/utils.rs +++ b/src/utils/utils.rs @@ -517,10 +517,9 @@ pub(crate) fn create_rustup_home() -> Result<()> { return Ok(()); } - // XXX: This error message seems wrong/bogus. let home = home_dir() .map(|p| p.join(".rustup")) - .ok_or_else(|| anyhow::anyhow!("couldn't find value of RUSTUP_HOME"))?; + .ok_or_else(|| anyhow::anyhow!("could not find home dir to put .rustup in"))?; fs::create_dir_all(home).context("unable to create ~/.rustup")?; Ok(()) From b4cf85913898bd026b944a195070213baa6f19f9 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 10:01:43 +0200 Subject: [PATCH 07/17] Inline single-use function --- src/cli/self_update.rs | 11 ++++++++++- src/utils/utils.rs | 16 ---------------- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index c873000481..a81158ef0e 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -469,7 +469,16 @@ pub(crate) async fn install( do_add_to_programs()?; do_add_to_path()?; } - utils::create_rustup_home()?; + + // If RUSTUP_HOME is not set, make sure it exists + if process().var_os("RUSTUP_HOME").is_none() { + let home = utils::home_dir() + .map(|p| p.join(".rustup")) + .ok_or_else(|| anyhow::anyhow!("could not find home dir to put .rustup in"))?; + + fs::create_dir_all(home).context("unable to create ~/.rustup")?; + } + maybe_install_rust( opts.default_toolchain, &opts.profile, diff --git a/src/utils/utils.rs b/src/utils/utils.rs index 3aa546ed69..a0b15c45de 100644 --- a/src/utils/utils.rs +++ b/src/utils/utils.rs @@ -509,22 +509,6 @@ pub(crate) fn cargo_home() -> Result { home::cargo_home_with_env(&home_process()).context("failed to determine cargo home") } -// Creates a ~/.rustup folder -pub(crate) fn create_rustup_home() -> Result<()> { - // If RUSTUP_HOME is set then don't make any assumptions about where it's - // ok to put ~/.rustup - if process().var_os("RUSTUP_HOME").is_some() { - return Ok(()); - } - - let home = home_dir() - .map(|p| p.join(".rustup")) - .ok_or_else(|| anyhow::anyhow!("could not find home dir to put .rustup in"))?; - - fs::create_dir_all(home).context("unable to create ~/.rustup")?; - Ok(()) -} - pub(crate) fn rustup_home() -> Result { home::rustup_home_with_env(&home_process()).context("failed to determine rustup home dir") } From 6cd2e2394eaee01e13dba3a0e043238865e38e4f Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 10:08:20 +0200 Subject: [PATCH 08/17] Move toolchain construction out of Cfg::create_command_for_toolchain() --- src/config.rs | 30 +----------------------------- src/toolchain/toolchain.rs | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 29 deletions(-) diff --git a/src/config.rs b/src/config.rs index 26b76717f2..5353666a23 100644 --- a/src/config.rs +++ b/src/config.rs @@ -945,35 +945,7 @@ impl Cfg { install_if_missing: bool, binary: &str, ) -> Result { - match toolchain_name { - LocalToolchainName::Named(ToolchainName::Official(desc)) => { - match DistributableToolchain::new(self, desc.clone()) { - Err(RustupError::ToolchainNotInstalled(_)) => { - if install_if_missing { - DistributableToolchain::install( - self, - desc, - &[], - &[], - self.get_profile()?, - true, - ) - .await?; - } - } - o => { - o?; - } - } - } - n => { - if !Toolchain::exists(self, n)? { - return Err(RustupError::ToolchainNotInstallable(n.to_string()).into()); - } - } - } - - let toolchain = Toolchain::new(self, toolchain_name.clone())?; + let toolchain = Toolchain::from_local(toolchain_name, install_if_missing, self).await?; // NB this can only fail in race conditions since we handle existence above // for dir. diff --git a/src/toolchain/toolchain.rs b/src/toolchain/toolchain.rs index a011226e5d..a765f1ddee 100644 --- a/src/toolchain/toolchain.rs +++ b/src/toolchain/toolchain.rs @@ -37,6 +37,42 @@ pub(crate) struct Toolchain<'a> { } impl<'a> Toolchain<'a> { + pub(crate) async fn from_local( + toolchain_name: &LocalToolchainName, + install_if_missing: bool, + cfg: &'a Cfg, + ) -> anyhow::Result> { + match toolchain_name { + LocalToolchainName::Named(ToolchainName::Official(desc)) => { + match DistributableToolchain::new(cfg, desc.clone()) { + Err(RustupError::ToolchainNotInstalled(_)) => { + if install_if_missing { + DistributableToolchain::install( + cfg, + desc, + &[], + &[], + cfg.get_profile()?, + true, + ) + .await?; + } + } + o => { + o?; + } + } + } + n => { + if !Self::exists(cfg, n)? { + return Err(RustupError::ToolchainNotInstallable(n.to_string()).into()); + } + } + } + + Ok(Self::new(cfg, toolchain_name.clone())?) + } + pub(crate) async fn from_partial( toolchain: Option, cfg: &'a Cfg, From 86b280a7495fc1d4abe89935556b408901c10166 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 10:15:47 +0200 Subject: [PATCH 09/17] Inline simple function Cfg::create_command_for_toolchain() --- src/cli/proxy_mode.rs | 9 ++++++++- src/cli/rustup_mode.rs | 8 +++++--- src/config.rs | 15 +-------------- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/cli/proxy_mode.rs b/src/cli/proxy_mode.rs index 5f659a4eb7..88ed5e4bca 100644 --- a/src/cli/proxy_mode.rs +++ b/src/cli/proxy_mode.rs @@ -2,6 +2,7 @@ use std::{ffi::OsString, path::PathBuf, process::ExitStatus}; use anyhow::Result; +use crate::toolchain::toolchain::Toolchain; use crate::{ cli::{common::set_globals, job, self_update}, command::run_command_for_dir, @@ -51,7 +52,13 @@ async fn direct_proxy( ) -> Result { let cmd = match toolchain { None => cfg.create_command_for_dir(arg0).await?, - Some(tc) => cfg.create_command_for_toolchain(&tc, false, arg0).await?, + Some(tc) => { + let toolchain = Toolchain::from_local(&tc, false, cfg).await?; + + // NB this can only fail in race conditions since we handle existence above + // for dir. + cfg.create_command_for_toolchain_(toolchain, arg0)? + } }; run_command_for_dir(cmd, arg0, args) } diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index a500e5d4aa..177f0d4cb3 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -900,9 +900,11 @@ async fn run( install: bool, ) -> Result { let toolchain = toolchain.resolve(&cfg.get_default_host_triple()?)?; - let cmd = cfg - .create_command_for_toolchain(&toolchain, install, &command[0]) - .await?; + let toolchain = Toolchain::from_local(&toolchain, install, cfg).await?; + + // NB this can only fail in race conditions since we handle existence above + // for dir. + let cmd = cfg.create_command_for_toolchain_(toolchain, &command[0])?; command::run_command_for_dir(cmd, &command[0], &command[1..]) } diff --git a/src/config.rs b/src/config.rs index 5353666a23..230d45d073 100644 --- a/src/config.rs +++ b/src/config.rs @@ -939,20 +939,7 @@ impl Cfg { self.create_command_for_toolchain_(toolchain, binary) } - pub(crate) async fn create_command_for_toolchain( - &self, - toolchain_name: &LocalToolchainName, - install_if_missing: bool, - binary: &str, - ) -> Result { - let toolchain = Toolchain::from_local(toolchain_name, install_if_missing, self).await?; - - // NB this can only fail in race conditions since we handle existence above - // for dir. - self.create_command_for_toolchain_(toolchain, binary) - } - - fn create_command_for_toolchain_( + pub fn create_command_for_toolchain_( &self, toolchain: Toolchain<'_>, binary: &str, From 0a3619c2b1effc3d11c29733f86790855b6b149c Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 10:17:37 +0200 Subject: [PATCH 10/17] Inline trivial single-use function Cfg::create_command_for_dir() --- src/cli/proxy_mode.rs | 7 +++++-- src/cli/rustup_mode.rs | 2 +- src/config.rs | 7 +------ 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/cli/proxy_mode.rs b/src/cli/proxy_mode.rs index 88ed5e4bca..e9026a572c 100644 --- a/src/cli/proxy_mode.rs +++ b/src/cli/proxy_mode.rs @@ -51,13 +51,16 @@ async fn direct_proxy( args: &[OsString], ) -> Result { let cmd = match toolchain { - None => cfg.create_command_for_dir(arg0).await?, + None => { + let (toolchain, _) = cfg.find_or_install_active_toolchain().await?; + cfg.create_command_for_toolchain(toolchain, arg0)? + } Some(tc) => { let toolchain = Toolchain::from_local(&tc, false, cfg).await?; // NB this can only fail in race conditions since we handle existence above // for dir. - cfg.create_command_for_toolchain_(toolchain, arg0)? + cfg.create_command_for_toolchain(toolchain, arg0)? } }; run_command_for_dir(cmd, arg0, args) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 177f0d4cb3..433952f52f 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -904,7 +904,7 @@ async fn run( // NB this can only fail in race conditions since we handle existence above // for dir. - let cmd = cfg.create_command_for_toolchain_(toolchain, &command[0])?; + let cmd = cfg.create_command_for_toolchain(toolchain, &command[0])?; command::run_command_for_dir(cmd, &command[0], &command[1..]) } diff --git a/src/config.rs b/src/config.rs index 230d45d073..35bf847f1a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -934,12 +934,7 @@ impl Cfg { }) } - pub(crate) async fn create_command_for_dir(&self, binary: &str) -> Result { - let (toolchain, _) = self.find_or_install_active_toolchain().await?; - self.create_command_for_toolchain_(toolchain, binary) - } - - pub fn create_command_for_toolchain_( + pub fn create_command_for_toolchain( &self, toolchain: Toolchain<'_>, binary: &str, From 40844ea29eca61c338946da7e0437932b3e8eb7b Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 10:19:40 +0200 Subject: [PATCH 11/17] Extract common usage of Cfg::create_command_for_toolchain() --- src/cli/proxy_mode.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/cli/proxy_mode.rs b/src/cli/proxy_mode.rs index e9026a572c..2bb626a49e 100644 --- a/src/cli/proxy_mode.rs +++ b/src/cli/proxy_mode.rs @@ -50,18 +50,11 @@ async fn direct_proxy( toolchain: Option, args: &[OsString], ) -> Result { - let cmd = match toolchain { - None => { - let (toolchain, _) = cfg.find_or_install_active_toolchain().await?; - cfg.create_command_for_toolchain(toolchain, arg0)? - } - Some(tc) => { - let toolchain = Toolchain::from_local(&tc, false, cfg).await?; - - // NB this can only fail in race conditions since we handle existence above - // for dir. - cfg.create_command_for_toolchain(toolchain, arg0)? - } + let toolchain = match toolchain { + None => cfg.find_or_install_active_toolchain().await?.0, + Some(tc) => Toolchain::from_local(&tc, false, cfg).await?, }; + + let cmd = cfg.create_command_for_toolchain(toolchain, arg0)?; run_command_for_dir(cmd, arg0, args) } From 55d7cfdb5608ac8e7a557082c3d34f399922c2c8 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 10:25:07 +0200 Subject: [PATCH 12/17] Move Cfg::create_command_for_toolchain() to Toolchain::command() --- src/cli/proxy_mode.rs | 2 +- src/cli/rustup_mode.rs | 5 +---- src/config.rs | 21 +-------------------- src/toolchain/toolchain.rs | 20 ++++++++++++++++---- 4 files changed, 19 insertions(+), 29 deletions(-) diff --git a/src/cli/proxy_mode.rs b/src/cli/proxy_mode.rs index 2bb626a49e..eadf17e4ca 100644 --- a/src/cli/proxy_mode.rs +++ b/src/cli/proxy_mode.rs @@ -55,6 +55,6 @@ async fn direct_proxy( Some(tc) => Toolchain::from_local(&tc, false, cfg).await?, }; - let cmd = cfg.create_command_for_toolchain(toolchain, arg0)?; + let cmd = toolchain.command(arg0)?; run_command_for_dir(cmd, arg0, args) } diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 433952f52f..ee8619847a 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -901,10 +901,7 @@ async fn run( ) -> Result { let toolchain = toolchain.resolve(&cfg.get_default_host_triple()?)?; let toolchain = Toolchain::from_local(&toolchain, install, cfg).await?; - - // NB this can only fail in race conditions since we handle existence above - // for dir. - let cmd = cfg.create_command_for_toolchain(toolchain, &command[0])?; + let cmd = toolchain.command(&command[0])?; command::run_command_for_dir(cmd, &command[0], &command[1..]) } diff --git a/src/config.rs b/src/config.rs index 35bf847f1a..56d5f6ff80 100644 --- a/src/config.rs +++ b/src/config.rs @@ -934,28 +934,9 @@ impl Cfg { }) } - pub fn create_command_for_toolchain( - &self, - toolchain: Toolchain<'_>, - binary: &str, - ) -> Result { - // Should push the cargo fallback into a custom toolchain type? And then - // perhaps a trait that create command layers on? - if !matches!( - toolchain.name(), - LocalToolchainName::Named(ToolchainName::Official(_)) - ) { - if let Some(cmd) = self.maybe_do_cargo_fallback(&toolchain, binary)? { - return Ok(cmd); - } - } - - toolchain.create_command(binary) - } - // Custom toolchains don't have cargo, so here we detect that situation and // try to find a different cargo. - fn maybe_do_cargo_fallback( + pub(crate) fn maybe_do_cargo_fallback( &self, toolchain: &Toolchain<'_>, binary: &str, diff --git a/src/toolchain/toolchain.rs b/src/toolchain/toolchain.rs index a765f1ddee..ae1c05037f 100644 --- a/src/toolchain/toolchain.rs +++ b/src/toolchain/toolchain.rs @@ -286,11 +286,23 @@ impl<'a> Toolchain<'a> { } } + pub(crate) fn command(&self, binary: &str) -> anyhow::Result { + // Should push the cargo fallback into a custom toolchain type? And then + // perhaps a trait that create command layers on? + if !matches!( + self.name(), + LocalToolchainName::Named(ToolchainName::Official(_)) + ) { + if let Some(cmd) = self.cfg.maybe_do_cargo_fallback(self, binary)? { + return Ok(cmd); + } + } + + self.create_command(binary) + } + #[cfg_attr(feature="otel", tracing::instrument(err,fields(binary, recursion=process().var("RUST_RECURSION_COUNT").ok())))] - pub fn create_command + Debug>( - &self, - binary: T, - ) -> Result { + fn create_command + Debug>(&self, binary: T) -> Result { // Create the path to this binary within the current toolchain sysroot let binary = if let Some(binary_str) = binary.as_ref().to_str() { if binary_str.to_lowercase().ends_with(EXE_SUFFIX) { From d17056495857dc0181cdbe57aef72d65d2649865 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 10:28:26 +0200 Subject: [PATCH 13/17] Move Cfg::maybe_do_cargo_fallback() to Toolchain --- src/config.rs | 42 +------------------------------------- src/toolchain/toolchain.rs | 36 +++++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 42 deletions(-) diff --git a/src/config.rs b/src/config.rs index 56d5f6ff80..50d3be4ed4 100644 --- a/src/config.rs +++ b/src/config.rs @@ -2,7 +2,6 @@ use std::borrow::Cow; use std::fmt::{self, Debug, Display}; use std::io; use std::path::{Path, PathBuf}; -use std::process::Command; use std::str::FromStr; use std::sync::Arc; @@ -15,7 +14,7 @@ use crate::{ cli::self_update::SelfUpdateMode, currentprocess::process, dist::{ - dist::{self, PartialToolchainDesc, Profile, ToolchainDesc}, + dist::{self, Profile, ToolchainDesc}, download::DownloadCfg, temp, }, @@ -934,45 +933,6 @@ impl Cfg { }) } - // Custom toolchains don't have cargo, so here we detect that situation and - // try to find a different cargo. - pub(crate) fn maybe_do_cargo_fallback( - &self, - toolchain: &Toolchain<'_>, - binary: &str, - ) -> Result> { - if binary != "cargo" && binary != "cargo.exe" { - return Ok(None); - } - - let cargo_path = toolchain.binary_file("cargo"); - - // breadcrumb in case of regression: we used to get the cargo path and - // cargo.exe path separately, not using the binary_file helper. This may - // matter if calling a binary with some personality that allows .exe and - // not .exe to coexist (e.g. wine) - but that's not something we aim to - // support : the host should always be correct. - if cargo_path.exists() { - return Ok(None); - } - - let default_host_triple = self.get_default_host_triple()?; - // XXX: This could actually consider all installed distributable - // toolchains in principle. - for fallback in ["nightly", "beta", "stable"] { - let resolved = - PartialToolchainDesc::from_str(fallback)?.resolve(&default_host_triple)?; - if let Ok(fallback) = - crate::toolchain::distributable::DistributableToolchain::new(self, resolved) - { - let cmd = fallback.create_fallback_command("cargo", toolchain)?; - return Ok(Some(cmd)); - } - } - - Ok(None) - } - pub(crate) fn set_default_host_triple(&self, host_triple: String) -> Result<()> { // Ensure that the provided host_triple is capable of resolving // against the 'stable' toolchain. This provides early errors diff --git a/src/toolchain/toolchain.rs b/src/toolchain/toolchain.rs index ae1c05037f..a87568876f 100644 --- a/src/toolchain/toolchain.rs +++ b/src/toolchain/toolchain.rs @@ -6,6 +6,7 @@ use std::{ io::{self, BufRead, BufReader}, path::{Path, PathBuf}, process::{Command, Stdio}, + str::FromStr, time::Duration, }; @@ -293,7 +294,7 @@ impl<'a> Toolchain<'a> { self.name(), LocalToolchainName::Named(ToolchainName::Official(_)) ) { - if let Some(cmd) = self.cfg.maybe_do_cargo_fallback(self, binary)? { + if let Some(cmd) = self.maybe_do_cargo_fallback(binary)? { return Ok(cmd); } } @@ -301,6 +302,39 @@ impl<'a> Toolchain<'a> { self.create_command(binary) } + // Custom toolchains don't have cargo, so here we detect that situation and + // try to find a different cargo. + pub(crate) fn maybe_do_cargo_fallback(&self, binary: &str) -> anyhow::Result> { + if binary != "cargo" && binary != "cargo.exe" { + return Ok(None); + } + + let cargo_path = self.binary_file("cargo"); + + // breadcrumb in case of regression: we used to get the cargo path and + // cargo.exe path separately, not using the binary_file helper. This may + // matter if calling a binary with some personality that allows .exe and + // not .exe to coexist (e.g. wine) - but that's not something we aim to + // support : the host should always be correct. + if cargo_path.exists() { + return Ok(None); + } + + let default_host_triple = self.cfg.get_default_host_triple()?; + // XXX: This could actually consider all installed distributable + // toolchains in principle. + for fallback in ["nightly", "beta", "stable"] { + let resolved = + PartialToolchainDesc::from_str(fallback)?.resolve(&default_host_triple)?; + if let Ok(fallback) = DistributableToolchain::new(self.cfg, resolved) { + let cmd = fallback.create_fallback_command("cargo", self)?; + return Ok(Some(cmd)); + } + } + + Ok(None) + } + #[cfg_attr(feature="otel", tracing::instrument(err,fields(binary, recursion=process().var("RUST_RECURSION_COUNT").ok())))] fn create_command + Debug>(&self, binary: T) -> Result { // Create the path to this binary within the current toolchain sysroot From 842a7f7524a20043c7f5f7c060d6c3a9e17a3ea3 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 10:30:37 +0200 Subject: [PATCH 14/17] Rename new_toolchain_with_reason() to Toolchain::with_reason() --- src/cli/rustup_mode.rs | 6 ++--- src/config.rs | 45 +++----------------------------------- src/toolchain/toolchain.rs | 40 ++++++++++++++++++++++++++++++++- 3 files changed, 45 insertions(+), 46 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index ee8619847a..5101d4b549 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -22,7 +22,7 @@ use crate::{ topical_doc, }, command, - config::{new_toolchain_with_reason, ActiveReason, Cfg}, + config::{ActiveReason, Cfg}, currentprocess::{ process, terminalsource::{self, ColorableTerminal}, @@ -1021,7 +1021,7 @@ fn show(cfg: &Cfg, verbose: bool) -> Result { match active_toolchain_and_reason { Some((active_toolchain_name, active_reason)) => { - let active_toolchain = new_toolchain_with_reason( + let active_toolchain = Toolchain::with_reason( cfg, active_toolchain_name.clone().into(), &active_reason, @@ -1064,7 +1064,7 @@ fn show(cfg: &Cfg, verbose: bool) -> Result { fn show_active_toolchain(cfg: &Cfg, verbose: bool) -> Result { match cfg.find_active_toolchain()? { Some((toolchain_name, reason)) => { - let toolchain = new_toolchain_with_reason(cfg, toolchain_name.clone(), &reason)?; + let toolchain = Toolchain::with_reason(cfg, toolchain_name.clone(), &reason)?; writeln!( process().stdout().lock(), "{}\nactive because: {}", diff --git a/src/config.rs b/src/config.rs index 50d3be4ed4..ee44c8b7a0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -116,44 +116,6 @@ impl Display for ActiveReason { } } -/// Calls Toolchain::new(), but augments the error message with more context -/// from the ActiveReason if the toolchain isn't installed. -pub(crate) fn new_toolchain_with_reason<'a>( - cfg: &'a Cfg, - name: LocalToolchainName, - reason: &ActiveReason, -) -> Result> { - match Toolchain::new(cfg, name.clone()) { - Err(RustupError::ToolchainNotInstalled(_)) => (), - result => { - return Ok(result?); - } - } - - let reason_err = match reason { - ActiveReason::Environment => { - "the RUSTUP_TOOLCHAIN environment variable specifies an uninstalled toolchain" - .to_string() - } - ActiveReason::CommandLine => { - "the +toolchain on the command line specifies an uninstalled toolchain".to_string() - } - ActiveReason::OverrideDB(ref path) => format!( - "the directory override for '{}' specifies an uninstalled toolchain", - utils::canonicalize_path(path, cfg.notify_handler.as_ref()).display(), - ), - ActiveReason::ToolchainFile(ref path) => format!( - "the toolchain file at '{}' specifies an uninstalled toolchain", - utils::canonicalize_path(path, cfg.notify_handler.as_ref()).display(), - ), - ActiveReason::Default => { - "the default toolchain does not describe an installed toolchain".to_string() - } - }; - - Err(anyhow!(reason_err).context(format!("override toolchain '{name}' is not installed"))) -} - // Represents a toolchain override from a +toolchain command line option, // RUSTUP_TOOLCHAIN environment variable, or rust-toolchain.toml file etc. Can // include components and targets from a rust-toolchain.toml that should be @@ -746,12 +708,11 @@ impl Cfg { match self.find_override_config(path)? { Some((override_config, reason)) => match override_config { OverrideCfg::PathBased(path_based_name) => { - let toolchain = - new_toolchain_with_reason(self, path_based_name.into(), &reason)?; + let toolchain = Toolchain::with_reason(self, path_based_name.into(), &reason)?; Ok(Some((toolchain, reason))) } OverrideCfg::Custom(custom_name) => { - let toolchain = new_toolchain_with_reason(self, custom_name.into(), &reason)?; + let toolchain = Toolchain::with_reason(self, custom_name.into(), &reason)?; Ok(Some((toolchain, reason))) } OverrideCfg::Official { @@ -770,7 +731,7 @@ impl Cfg { None => Ok(None), Some(ToolchainName::Custom(custom_name)) => { let reason = ActiveReason::Default; - let toolchain = new_toolchain_with_reason(self, custom_name.into(), &reason)?; + let toolchain = Toolchain::with_reason(self, custom_name.into(), &reason)?; Ok(Some((toolchain, reason))) } Some(ToolchainName::Official(toolchain_desc)) => { diff --git a/src/toolchain/toolchain.rs b/src/toolchain/toolchain.rs index a87568876f..42bb6e6536 100644 --- a/src/toolchain/toolchain.rs +++ b/src/toolchain/toolchain.rs @@ -15,7 +15,7 @@ use fs_at::OpenOptions; use wait_timeout::ChildExt; use crate::{ - config::Cfg, + config::{ActiveReason, Cfg}, currentprocess::process, dist::dist::PartialToolchainDesc, env_var, install, @@ -87,6 +87,44 @@ impl<'a> Toolchain<'a> { } } + /// Calls Toolchain::new(), but augments the error message with more context + /// from the ActiveReason if the toolchain isn't installed. + pub(crate) fn with_reason( + cfg: &'a Cfg, + name: LocalToolchainName, + reason: &ActiveReason, + ) -> anyhow::Result { + match Self::new(cfg, name.clone()) { + Err(RustupError::ToolchainNotInstalled(_)) => (), + result => { + return Ok(result?); + } + } + + let reason_err = match reason { + ActiveReason::Environment => { + "the RUSTUP_TOOLCHAIN environment variable specifies an uninstalled toolchain" + .to_string() + } + ActiveReason::CommandLine => { + "the +toolchain on the command line specifies an uninstalled toolchain".to_string() + } + ActiveReason::OverrideDB(ref path) => format!( + "the directory override for '{}' specifies an uninstalled toolchain", + utils::canonicalize_path(path, cfg.notify_handler.as_ref()).display(), + ), + ActiveReason::ToolchainFile(ref path) => format!( + "the toolchain file at '{}' specifies an uninstalled toolchain", + utils::canonicalize_path(path, cfg.notify_handler.as_ref()).display(), + ), + ActiveReason::Default => { + "the default toolchain does not describe an installed toolchain".to_string() + } + }; + + Err(anyhow!(reason_err).context(format!("override toolchain '{name}' is not installed"))) + } + pub(crate) fn new(cfg: &'a Cfg, name: LocalToolchainName) -> Result { let path = cfg.toolchain_path(&name); if !Toolchain::exists(cfg, &name)? { From 21ade9569ce58bfcffff1c872fce2cdbce460498 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 10:37:45 +0200 Subject: [PATCH 15/17] Inline short single-use function direct_proxy() --- src/cli/proxy_mode.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/cli/proxy_mode.rs b/src/cli/proxy_mode.rs index eadf17e4ca..1a81999e5f 100644 --- a/src/cli/proxy_mode.rs +++ b/src/cli/proxy_mode.rs @@ -1,4 +1,4 @@ -use std::{ffi::OsString, path::PathBuf, process::ExitStatus}; +use std::{path::PathBuf, process::ExitStatus}; use anyhow::Result; @@ -6,9 +6,8 @@ use crate::toolchain::toolchain::Toolchain; use crate::{ cli::{common::set_globals, job, self_update}, command::run_command_for_dir, - config::Cfg, currentprocess::process, - toolchain::names::{LocalToolchainName, ResolvableLocalToolchainName}, + toolchain::names::ResolvableLocalToolchainName, }; #[cfg_attr(feature = "otel", tracing::instrument)] @@ -40,21 +39,12 @@ pub async fn main(arg0: &str, current_dir: PathBuf) -> Result { let toolchain = toolchain .map(|t| t.resolve(&cfg.get_default_host_triple()?)) .transpose()?; - direct_proxy(&cfg, arg0, toolchain, &cmd_args).await -} -#[cfg_attr(feature = "otel", tracing::instrument(skip(cfg)))] -async fn direct_proxy( - cfg: &Cfg, - arg0: &str, - toolchain: Option, - args: &[OsString], -) -> Result { let toolchain = match toolchain { None => cfg.find_or_install_active_toolchain().await?.0, - Some(tc) => Toolchain::from_local(&tc, false, cfg).await?, + Some(tc) => Toolchain::from_local(&tc, false, &cfg).await?, }; let cmd = toolchain.command(arg0)?; - run_command_for_dir(cmd, arg0, args) + run_command_for_dir(cmd, arg0, &cmd_args) } From 527678ec9d8182196116331bd9bc9939de585afe Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 10:41:34 +0200 Subject: [PATCH 16/17] Inline trivial single-use function Cfg::which_binary() --- src/cli/rustup_mode.rs | 3 ++- src/config.rs | 5 ----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 5101d4b549..347cd973eb 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -914,7 +914,8 @@ async fn which( let desc = toolchain.resolve(&cfg.get_default_host_triple()?)?; Toolchain::new(cfg, desc.into())?.binary_file(binary) } else { - cfg.which_binary(binary).await? + let (toolchain, _) = cfg.find_or_install_active_toolchain().await?; + toolchain.binary_file(binary) }; utils::assert_is_file(&binary_path)?; diff --git a/src/config.rs b/src/config.rs index ee44c8b7a0..3356ac28da 100644 --- a/src/config.rs +++ b/src/config.rs @@ -457,11 +457,6 @@ impl Cfg { Ok(self.update_hash_dir.join(toolchain.to_string())) } - pub(crate) async fn which_binary(&self, binary: &str) -> Result { - let (toolchain, _) = self.find_or_install_active_toolchain().await?; - Ok(toolchain.binary_file(binary)) - } - #[cfg_attr(feature = "otel", tracing::instrument(skip_all))] pub(crate) fn upgrade_data(&self) -> Result<()> { let current_version = self.settings_file.with(|s| Ok(s.version.clone()))?; From 0163f2ef4ae239072c8ba32c17a74a4160305914 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 5 Jun 2024 10:47:59 +0200 Subject: [PATCH 17/17] Inline trivial single-use function utils::to_absolute() --- src/cli/rustup_mode.rs | 2 +- src/utils/utils.rs | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 347cd973eb..ae8212b408 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -1267,7 +1267,7 @@ async fn toolchain_link( if true { InstallMethod::Link { - src: &utils::to_absolute(src, &cfg.current_dir), + src: &cfg.current_dir.join(src), dest, cfg, } diff --git a/src/utils/utils.rs b/src/utils/utils.rs index a0b15c45de..5dc3d53b60 100644 --- a/src/utils/utils.rs +++ b/src/utils/utils.rs @@ -495,12 +495,6 @@ pub fn current_exe() -> Result { env::current_exe().context(RustupError::LocatingWorkingDir) } -pub(crate) fn to_absolute>(path: P, cwd: &Path) -> PathBuf { - let mut new = PathBuf::from(cwd); - new.push(path); - new -} - pub(crate) fn home_dir() -> Option { home::home_dir_with_env(&home_process()) }