Auto merge of #16755 - Veykril:rustup-bins, r=Veykril

For toolchain binaries use the full path found in $PATH

Fixes https://github.com/rust-lang/rust-analyzer/issues/16754

Cargo.lock

@@ -1326,7 +1326,6 @@ name = "proc-macro-test"
 version = "0.0.0"
 dependencies = [
  "cargo_metadata",
- "toolchain",
 ]
 
 [[package]]

crates/ide/src/expand_macro.rs

@@ -189,7 +189,7 @@ fn _format(
     let &crate_id = db.relevant_crates(file_id).iter().next()?;
     let edition = db.crate_graph()[crate_id].edition;
 
-    let mut cmd = std::process::Command::new(toolchain::rustfmt());
+    let mut cmd = std::process::Command::new(toolchain::Tool::Rustfmt.path());
     cmd.arg("--edition");
     cmd.arg(edition.to_string());
 

crates/proc-macro-srv/proc-macro-test/Cargo.toml

@@ -11,6 +11,3 @@ doctest = false
 
 [build-dependencies]
 cargo_metadata = "0.18.1"
-
-# local deps
-toolchain.workspace = true

crates/proc-macro-srv/proc-macro-test/build.rs

@@ -18,12 +18,12 @@ use cargo_metadata::Message;
 fn main() {
     println!("cargo:rerun-if-changed=imp");
 
+    let cargo = env::var_os("CARGO").unwrap_or_else(|| "cargo".into());
+
     let has_features = env::var_os("RUSTC_BOOTSTRAP").is_some()
-        || String::from_utf8(
+        || String::from_utf8(Command::new(&cargo).arg("--version").output().unwrap().stdout)
-            Command::new(toolchain::cargo()).arg("--version").output().unwrap().stdout,
+            .unwrap()
-        )
+            .contains("nightly");
-        .unwrap()
-        .contains("nightly");
 
     let out_dir = env::var_os("OUT_DIR").unwrap();
     let out_dir = Path::new(&out_dir);
@@ -66,7 +66,7 @@ fn main() {
 
     let target_dir = out_dir.join("target");
 
-    let mut cmd = Command::new(toolchain::cargo());
+    let mut cmd = Command::new(&cargo);
     cmd.current_dir(&staging_dir)
         .args(["build", "-p", "proc-macro-test-impl", "--message-format", "json"])
         // Explicit override the target directory to avoid using the same one which the parent
@@ -96,7 +96,7 @@ fn main() {
     let repr = format!("{name} {version}");
     // New Package Id Spec since rust-lang/cargo#13311
     let pkgid = String::from_utf8(
-        Command::new(toolchain::cargo())
+        Command::new(cargo)
             .current_dir(&staging_dir)
             .args(["pkgid", name])
             .output()

crates/project-model/src/build_scripts.rs

@@ -71,8 +71,7 @@ impl WorkspaceBuildScripts {
                 cmd
             }
             _ => {
-                let mut cmd = Command::new(Tool::Cargo.path());
+                let mut cmd = Sysroot::tool(sysroot, Tool::Cargo);
-                Sysroot::set_rustup_toolchain_env(&mut cmd, sysroot);
 
                 cmd.args(["check", "--quiet", "--workspace", "--message-format=json"]);
                 cmd.args(&config.extra_args);
@@ -430,8 +429,7 @@ impl WorkspaceBuildScripts {
         }
         let res = (|| {
             let target_libdir = (|| {
-                let mut cargo_config = Command::new(Tool::Cargo.path());
+                let mut cargo_config = Sysroot::tool(sysroot, Tool::Cargo);
-                Sysroot::set_rustup_toolchain_env(&mut cargo_config, sysroot);
                 cargo_config.envs(extra_env);
                 cargo_config
                     .current_dir(current_dir)
@@ -440,7 +438,7 @@ impl WorkspaceBuildScripts {
                 if let Ok(it) = utf8_stdout(cargo_config) {
                     return Ok(it);
                 }
-                let mut cmd = Sysroot::rustc(sysroot);
+                let mut cmd = Sysroot::tool(sysroot, Tool::Rustc);
                 cmd.envs(extra_env);
                 cmd.args(["--print", "target-libdir"]);
                 utf8_stdout(cmd)

crates/project-model/src/cargo_workspace.rs

@@ -1,8 +1,8 @@
 //! See [`CargoWorkspace`].
 
+use std::ops;
 use std::path::PathBuf;
 use std::str::from_utf8;
-use std::{ops, process::Command};
 
 use anyhow::Context;
 use base_db::Edition;
@@ -243,8 +243,11 @@ impl CargoWorkspace {
     ) -> anyhow::Result<cargo_metadata::Metadata> {
         let targets = find_list_of_build_targets(config, cargo_toml, sysroot);
 
+        let cargo = Sysroot::tool(sysroot, Tool::Cargo);
         let mut meta = MetadataCommand::new();
-        meta.cargo_path(Tool::Cargo.path());
+        meta.cargo_path(cargo.get_program());
+        cargo.get_envs().for_each(|(var, val)| _ = meta.env(var, val.unwrap_or_default()));
+        config.extra_env.iter().for_each(|(var, val)| _ = meta.env(var, val));
         meta.manifest_path(cargo_toml.to_path_buf());
         match &config.features {
             CargoFeatures::All => {
@@ -291,10 +294,7 @@ impl CargoWorkspace {
         progress("metadata".to_owned());
 
         (|| -> Result<cargo_metadata::Metadata, cargo_metadata::Error> {
-            let mut command = meta.cargo_command();
+            let output = meta.cargo_command().output()?;
-            Sysroot::set_rustup_toolchain_env(&mut command, sysroot);
-            command.envs(&config.extra_env);
-            let output = command.output()?;
             if !output.status.success() {
                 return Err(cargo_metadata::Error::CargoMetadata {
                     stderr: String::from_utf8(output.stderr)?,
@@ -501,7 +501,7 @@ fn rustc_discover_host_triple(
     extra_env: &FxHashMap<String, String>,
     sysroot: Option<&Sysroot>,
 ) -> Option<String> {
-    let mut rustc = Sysroot::rustc(sysroot);
+    let mut rustc = Sysroot::tool(sysroot, Tool::Rustc);
     rustc.envs(extra_env);
     rustc.current_dir(cargo_toml.parent()).arg("-vV");
     tracing::debug!("Discovering host platform by {:?}", rustc);
@@ -529,8 +529,7 @@ fn cargo_config_build_target(
     extra_env: &FxHashMap<String, String>,
     sysroot: Option<&Sysroot>,
 ) -> Vec<String> {
-    let mut cargo_config = Command::new(Tool::Cargo.path());
+    let mut cargo_config = Sysroot::tool(sysroot, Tool::Cargo);
-    Sysroot::set_rustup_toolchain_env(&mut cargo_config, sysroot);
     cargo_config.envs(extra_env);
     cargo_config
         .current_dir(cargo_toml.parent())

crates/project-model/src/rustc_cfg.rs

@@ -1,9 +1,8 @@
 //! Runs `rustc --print cfg` to get built-in cfg flags.
 
-use std::process::Command;
-
 use anyhow::Context;
 use rustc_hash::FxHashMap;
+use toolchain::Tool;
 
 use crate::{cfg_flag::CfgFlag, utf8_stdout, ManifestPath, Sysroot};
 
@@ -69,8 +68,8 @@ fn get_rust_cfgs(
 ) -> anyhow::Result<String> {
     let sysroot = match config {
         RustcCfgConfig::Cargo(sysroot, cargo_toml) => {
-            let mut cmd = Command::new(toolchain::Tool::Cargo.path());
+            let mut cmd = Sysroot::tool(sysroot, Tool::Cargo);
-            Sysroot::set_rustup_toolchain_env(&mut cmd, sysroot);
+
             cmd.envs(extra_env);
             cmd.current_dir(cargo_toml.parent())
                 .args(["rustc", "-Z", "unstable-options", "--print", "cfg"])
@@ -90,7 +89,7 @@ fn get_rust_cfgs(
         RustcCfgConfig::Rustc(sysroot) => sysroot,
     };
 
-    let mut cmd = Sysroot::rustc(sysroot);
+    let mut cmd = Sysroot::tool(sysroot, Tool::Rustc);
     cmd.envs(extra_env);
     cmd.args(["--print", "cfg", "-O"]);
     if let Some(target) = target {

crates/project-model/src/sysroot.rs

@@ -12,7 +12,7 @@ use itertools::Itertools;
 use la_arena::{Arena, Idx};
 use paths::{AbsPath, AbsPathBuf};
 use rustc_hash::FxHashMap;
-use toolchain::probe_for_binary;
+use toolchain::{probe_for_binary, Tool};
 
 use crate::{utf8_stdout, CargoConfig, CargoWorkspace, ManifestPath};
 
@@ -193,23 +193,26 @@ impl Sysroot {
         Ok(Sysroot::load(sysroot_dir, Some(sysroot_src_dir), metadata))
     }
 
-    pub fn set_rustup_toolchain_env(cmd: &mut Command, sysroot: Option<&Self>) {
+    /// Returns a command to run a tool preferring the cargo proxies if the sysroot exists.
-        if let Some(sysroot) = sysroot {
+    pub fn tool(sysroot: Option<&Self>, tool: Tool) -> Command {
-            cmd.env("RUSTUP_TOOLCHAIN", AsRef::<std::path::Path>::as_ref(&sysroot.root));
+        match sysroot {
-        }
-    }
-
-    /// Returns a `Command` that is configured to run `rustc` from the sysroot if it exists,
-    /// otherwise returns what [toolchain::Tool::Rustc] returns.
-    pub fn rustc(sysroot: Option<&Self>) -> Command {
-        let mut cmd = Command::new(match sysroot {
             Some(sysroot) => {
-                toolchain::Tool::Rustc.path_in_or_discover(sysroot.root.join("bin").as_ref())
+                // special case rustc, we can look that up directly in the sysroot's bin folder
+                // as it should never invoke another cargo binary
+                if let Tool::Rustc = tool {
+                    if let Some(path) =
+                        probe_for_binary(sysroot.root.join("bin").join(Tool::Rustc.name()).into())
+                    {
+                        return Command::new(path);
+                    }
+                }
+
+                let mut cmd = Command::new(tool.prefer_proxy());
+                cmd.env("RUSTUP_TOOLCHAIN", AsRef::<std::path::Path>::as_ref(&sysroot.root));
+                cmd
             }
-            None => toolchain::Tool::Rustc.path(),
+            _ => Command::new(tool.path()),
-        });
+        }
-        Self::set_rustup_toolchain_env(&mut cmd, sysroot);
-        cmd
     }
 
     pub fn discover_proc_macro_srv(&self) -> anyhow::Result<AbsPathBuf> {
@@ -411,7 +414,7 @@ fn discover_sysroot_dir(
     current_dir: &AbsPath,
     extra_env: &FxHashMap<String, String>,
 ) -> Result<AbsPathBuf> {
-    let mut rustc = Command::new(toolchain::rustc());
+    let mut rustc = Command::new(Tool::Rustc.path());
     rustc.envs(extra_env);
     rustc.current_dir(current_dir).args(["--print", "sysroot"]);
     tracing::debug!("Discovering sysroot by {:?}", rustc);
@@ -443,7 +446,7 @@ fn discover_sysroot_src_dir_or_add_component(
 ) -> Result<AbsPathBuf> {
     discover_sysroot_src_dir(sysroot_path)
         .or_else(|| {
-            let mut rustup = Command::new(toolchain::rustup());
+            let mut rustup = Command::new(Tool::Rustup.prefer_proxy());
             rustup.envs(extra_env);
             rustup.current_dir(current_dir).args(["component", "add", "rust-src"]);
             tracing::info!("adding rust-src component by {:?}", rustup);

crates/project-model/src/target_data_layout.rs

@@ -1,7 +1,7 @@
 //! Runs `rustc --print target-spec-json` to get the target_data_layout.
-use std::process::Command;
 
 use rustc_hash::FxHashMap;
+use toolchain::Tool;
 
 use crate::{utf8_stdout, ManifestPath, Sysroot};
 
@@ -28,8 +28,7 @@ pub fn get(
     };
     let sysroot = match config {
         RustcDataLayoutConfig::Cargo(sysroot, cargo_toml) => {
-            let mut cmd = Command::new(toolchain::Tool::Cargo.path());
+            let mut cmd = Sysroot::tool(sysroot, Tool::Cargo);
-            Sysroot::set_rustup_toolchain_env(&mut cmd, sysroot);
             cmd.envs(extra_env);
             cmd.current_dir(cargo_toml.parent())
                 .args([
@@ -57,7 +56,7 @@ pub fn get(
         RustcDataLayoutConfig::Rustc(sysroot) => sysroot,
     };
 
-    let mut cmd = Sysroot::rustc(sysroot);
+    let mut cmd = Sysroot::tool(sysroot, Tool::Rustc);
     cmd.envs(extra_env)
         .args(["-Z", "unstable-options", "--print", "target-spec-json"])
         .env("RUSTC_BOOTSTRAP", "1");

crates/project-model/src/workspace.rs

@@ -2,7 +2,7 @@
 //! metadata` or `rust-project.json`) into representation stored in the salsa
 //! database -- `CrateGraph`.
 
-use std::{collections::VecDeque, fmt, fs, iter, process::Command, str::FromStr, sync};
+use std::{collections::VecDeque, fmt, fs, iter, str::FromStr, sync};
 
 use anyhow::{format_err, Context};
 use base_db::{
@@ -172,11 +172,13 @@ impl fmt::Debug for ProjectWorkspace {
 
 fn get_toolchain_version(
     current_dir: &AbsPath,
-    mut cmd: Command,
+    sysroot: Option<&Sysroot>,
+    tool: Tool,
     extra_env: &FxHashMap<String, String>,
     prefix: &str,
 ) -> Result<Option<Version>, anyhow::Error> {
     let cargo_version = utf8_stdout({
+        let mut cmd = Sysroot::tool(sysroot, tool);
         cmd.envs(extra_env);
         cmd.arg("--version").current_dir(current_dir);
         cmd
@@ -297,11 +299,8 @@ impl ProjectWorkspace {
 
                 let toolchain = get_toolchain_version(
                     cargo_toml.parent(),
-                    {
+                    sysroot_ref,
-                        let mut cmd = Command::new(toolchain::Tool::Cargo.path());
+                    Tool::Cargo,
-                        Sysroot::set_rustup_toolchain_env(&mut cmd, sysroot_ref);
-                        cmd
-                    },
                     &config.extra_env,
                     "cargo ",
                 )?;
@@ -386,7 +385,8 @@ impl ProjectWorkspace {
         let data_layout_config = RustcDataLayoutConfig::Rustc(sysroot_ref);
         let toolchain = match get_toolchain_version(
             project_json.path(),
-            Sysroot::rustc(sysroot_ref),
+            sysroot_ref,
+            Tool::Rustc,
             extra_env,
             "rustc ",
         ) {
@@ -433,18 +433,15 @@ impl ProjectWorkspace {
         };
 
         let sysroot_ref = sysroot.as_ref().ok();
-        let toolchain = match get_toolchain_version(
+        let toolchain =
-            dir,
+            match get_toolchain_version(dir, sysroot_ref, Tool::Rustc, &config.extra_env, "rustc ")
-            Sysroot::rustc(sysroot_ref),
+            {
-            &config.extra_env,
+                Ok(it) => it,
-            "rustc ",
+                Err(e) => {
-        ) {
+                    tracing::error!("{e}");
-            Ok(it) => it,
+                    None
-            Err(e) => {
+                }
-                tracing::error!("{e}");
+            };
-                None
-            }
-        };
 
         let rustc_cfg = rustc_cfg::get(None, &config.extra_env, RustcCfgConfig::Rustc(sysroot_ref));
         let data_layout = target_data_layout::get(
@@ -1573,8 +1570,7 @@ fn cargo_config_env(
     extra_env: &FxHashMap<String, String>,
     sysroot: Option<&Sysroot>,
 ) -> FxHashMap<String, String> {
-    let mut cargo_config = Command::new(Tool::Cargo.path());
+    let mut cargo_config = Sysroot::tool(sysroot, Tool::Cargo);
-    Sysroot::set_rustup_toolchain_env(&mut cargo_config, sysroot);
     cargo_config.envs(extra_env);
     cargo_config
         .current_dir(cargo_toml.parent())

crates/rust-analyzer/src/handlers/request.rs

@@ -2002,7 +2002,7 @@ fn run_rustfmt(
     let mut command = match snap.config.rustfmt() {
         RustfmtConfig::Rustfmt { extra_args, enable_range_formatting } => {
             // FIXME: Set RUSTUP_TOOLCHAIN
-            let mut cmd = process::Command::new(toolchain::rustfmt());
+            let mut cmd = process::Command::new(toolchain::Tool::Rustfmt.path());
             cmd.envs(snap.config.extra_env());
             cmd.args(extra_args);
 

crates/toolchain/src/lib.rs

@@ -16,18 +16,48 @@ pub enum Tool {
 }
 
 impl Tool {
+    pub fn proxy(self) -> Option<PathBuf> {
+        cargo_proxy(self.name())
+    }
+
+    /// Return a `PathBuf` to use for the given executable.
+    ///
+    /// The current implementation checks three places for an executable to use:
+    /// 1) `$CARGO_HOME/bin/<executable_name>`
+    ///      where $CARGO_HOME defaults to ~/.cargo (see https://doc.rust-lang.org/cargo/guide/cargo-home.html)
+    ///      example: for cargo, this tries $CARGO_HOME/bin/cargo, or ~/.cargo/bin/cargo if $CARGO_HOME is unset.
+    ///      It seems that this is a reasonable place to try for cargo, rustc, and rustup
+    /// 2) Appropriate environment variable (erroring if this is set but not a usable executable)
+    ///      example: for cargo, this checks $CARGO environment variable; for rustc, $RUSTC; etc
+    /// 3) $PATH/`<executable_name>`
+    ///      example: for cargo, this tries all paths in $PATH with appended `cargo`, returning the
+    ///      first that exists
+    /// 4) If all else fails, we just try to use the executable name directly
+    pub fn prefer_proxy(self) -> PathBuf {
+        invoke(&[cargo_proxy, lookup_as_env_var, lookup_in_path], self.name())
+    }
+
+    /// Return a `PathBuf` to use for the given executable.
+    ///
+    /// The current implementation checks three places for an executable to use:
+    /// 1) Appropriate environment variable (erroring if this is set but not a usable executable)
+    ///      example: for cargo, this checks $CARGO environment variable; for rustc, $RUSTC; etc
+    /// 2) $PATH/`<executable_name>`
+    ///      example: for cargo, this tries all paths in $PATH with appended `cargo`, returning the
+    ///      first that exists
+    /// 3) `$CARGO_HOME/bin/<executable_name>`
+    ///      where $CARGO_HOME defaults to ~/.cargo (see https://doc.rust-lang.org/cargo/guide/cargo-home.html)
+    ///      example: for cargo, this tries $CARGO_HOME/bin/cargo, or ~/.cargo/bin/cargo if $CARGO_HOME is unset.
+    ///      It seems that this is a reasonable place to try for cargo, rustc, and rustup
+    /// 4) If all else fails, we just try to use the executable name directly
     pub fn path(self) -> PathBuf {
-        get_path_for_executable(self.name())
+        invoke(&[lookup_as_env_var, lookup_in_path, cargo_proxy], self.name())
     }
 
     pub fn path_in(self, path: &Path) -> Option<PathBuf> {
         probe_for_binary(path.join(self.name()))
     }
 
-    pub fn path_in_or_discover(self, path: &Path) -> PathBuf {
-        probe_for_binary(path.join(self.name())).unwrap_or_else(|| self.path())
-    }
-
     pub fn name(self) -> &'static str {
         match self {
             Tool::Cargo => "cargo",
@@ -38,60 +68,21 @@ impl Tool {
     }
 }
 
-pub fn cargo() -> PathBuf {
+fn invoke(list: &[fn(&str) -> Option<PathBuf>], executable: &str) -> PathBuf {
-    get_path_for_executable("cargo")
+    list.iter().find_map(|it| it(executable)).unwrap_or_else(|| executable.into())
 }
 
-pub fn rustc() -> PathBuf {
+/// Looks up the binary as its SCREAMING upper case in the env variables.
-    get_path_for_executable("rustc")
+fn lookup_as_env_var(executable_name: &str) -> Option<PathBuf> {
+    env::var_os(executable_name.to_ascii_uppercase()).map(Into::into)
 }
 
-pub fn rustup() -> PathBuf {
+/// Looks up the binary in the cargo home directory if it exists.
-    get_path_for_executable("rustup")
+fn cargo_proxy(executable_name: &str) -> Option<PathBuf> {
-}
+    let mut path = get_cargo_home()?;
-
+    path.push("bin");
-pub fn rustfmt() -> PathBuf {
+    path.push(executable_name);
-    get_path_for_executable("rustfmt")
+    probe_for_binary(path)
-}
-
-/// Return a `PathBuf` to use for the given executable.
-///
-/// E.g., `get_path_for_executable("cargo")` may return just `cargo` if that
-/// gives a valid Cargo executable; or it may return a full path to a valid
-/// Cargo.
-fn get_path_for_executable(executable_name: &'static str) -> PathBuf {
-    // The current implementation checks three places for an executable to use:
-    // 1) Appropriate environment variable (erroring if this is set but not a usable executable)
-    //      example: for cargo, this checks $CARGO environment variable; for rustc, $RUSTC; etc
-    // 2) `$CARGO_HOME/bin/<executable_name>`
-    //      where $CARGO_HOME defaults to ~/.cargo (see https://doc.rust-lang.org/cargo/guide/cargo-home.html)
-    //      example: for cargo, this tries $CARGO_HOME/bin/cargo, or ~/.cargo/bin/cargo if $CARGO_HOME is unset.
-    //      It seems that this is a reasonable place to try for cargo, rustc, and rustup
-    // 3) `<executable_name>`
-    //      example: for cargo, this tries just `cargo`, which will succeed if `cargo` is on the $PATH
-    let env_var = executable_name.to_ascii_uppercase();
-    if let Some(path) = env::var_os(env_var) {
-        return path.into();
-    }
-
-    if let Some(mut path) = get_cargo_home() {
-        path.push("bin");
-        path.push(executable_name);
-        if let Some(path) = probe_for_binary(path) {
-            return path;
-        }
-    }
-
-    if lookup_in_path(executable_name) {
-        return executable_name.into();
-    }
-
-    executable_name.into()
-}
-
-fn lookup_in_path(exec: &str) -> bool {
-    let paths = env::var_os("PATH").unwrap_or_default();
-    env::split_paths(&paths).map(|path| path.join(exec)).find_map(probe_for_binary).is_some()
 }
 
 fn get_cargo_home() -> Option<PathBuf> {
@@ -107,6 +98,11 @@ fn get_cargo_home() -> Option<PathBuf> {
     None
 }
 
+fn lookup_in_path(exec: &str) -> Option<PathBuf> {
+    let paths = env::var_os("PATH").unwrap_or_default();
+    env::split_paths(&paths).map(|path| path.join(exec)).find_map(probe_for_binary)
+}
+
 pub fn probe_for_binary(path: PathBuf) -> Option<PathBuf> {
     let with_extension = match env::consts::EXE_EXTENSION {
         "" => None,