Auto merge of rust-lang#16537 - Veykril:sysroot-tools, r=Veykril

internal: tool discovery prefers sysroot tools

Fixes rust-lang/rust-analyzer#15927, Fixes rust-lang/rust-analyzer#16523

After this PR we will look for `cargo` and `rustc` in the sysroot if it was succesfully loaded instead of using the current lookup scheme. This should be more correct than the current approach as that relies on the working directory of the server binary or loade workspace, meaning it can behave a bit odd wrt overrides.

Additionally, rust-project.json projects now get the target data layout set so there should be better const eval support now.

Author: bors

crates/flycheck/src/lib.rs

@@ -89,9 +89,10 @@ impl FlycheckHandle {
         id: usize,
         sender: Box<dyn Fn(Message) + Send>,
         config: FlycheckConfig,
+        cargo: PathBuf,
         workspace_root: AbsPathBuf,
     ) -> FlycheckHandle {
-        let actor = FlycheckActor::new(id, sender, config, workspace_root);
+        let actor = FlycheckActor::new(id, sender, config, cargo, workspace_root);
         let (sender, receiver) = unbounded::<StateChange>();
         let thread = stdx::thread::Builder::new(stdx::thread::ThreadIntent::Worker)
             .name("Flycheck".to_owned())
@@ -171,6 +172,7 @@ struct FlycheckActor {
     /// Either the workspace root of the workspace we are flychecking,
     /// or the project root of the project.
     root: AbsPathBuf,
+    cargo: PathBuf,
     /// CargoHandle exists to wrap around the communication needed to be able to
     /// run `cargo check` without blocking. Currently the Rust standard library
     /// doesn't provide a way to read sub-process output without blocking, so we
@@ -189,10 +191,11 @@ impl FlycheckActor {
         id: usize,
         sender: Box<dyn Fn(Message) + Send>,
         config: FlycheckConfig,
+        cargo: PathBuf,
         workspace_root: AbsPathBuf,
     ) -> FlycheckActor {
         tracing::info!(%id, ?workspace_root, "Spawning flycheck");
-        FlycheckActor { id, sender, config, root: workspace_root, command_handle: None }
+        FlycheckActor { id, sender, config, cargo, root: workspace_root, command_handle: None }
     }
 
     fn report_progress(&self, progress: Progress) {
@@ -316,7 +319,7 @@ impl FlycheckActor {
                 ansi_color_output,
                 target_dir,
             } => {
-                let mut cmd = Command::new(toolchain::cargo());
+                let mut cmd = Command::new(&self.cargo);
                 cmd.arg(command);
                 cmd.current_dir(&self.root);
 

crates/hir-ty/src/layout/tests.rs

@@ -1,6 +1,7 @@
 use chalk_ir::{AdtId, TyKind};
 use either::Either;
 use hir_def::db::DefDatabase;
+use project_model::target_data_layout::RustcDataLayoutConfig;
 use rustc_hash::FxHashMap;
 use test_fixture::WithFixture;
 use triomphe::Arc;
@@ -15,7 +16,12 @@ use crate::{
 mod closure;
 
 fn current_machine_data_layout() -> String {
-    project_model::target_data_layout::get(None, None, &FxHashMap::default()).unwrap()
+    project_model::target_data_layout::get(
+        RustcDataLayoutConfig::Rustc(None),
+        None,
+        &FxHashMap::default(),
+    )
+    .unwrap()
 }
 
 fn eval_goal(ra_fixture: &str, minicore: &str) -> Result<Arc<Layout>, LayoutError> {

crates/project-model/src/build_scripts.rs

@@ -20,10 +20,11 @@ use paths::{AbsPath, AbsPathBuf};
 use rustc_hash::{FxHashMap, FxHashSet};
 use semver::Version;
 use serde::Deserialize;
+use toolchain::Tool;
 
 use crate::{
     cfg_flag::CfgFlag, utf8_stdout, CargoConfig, CargoFeatures, CargoWorkspace, InvocationLocation,
-    InvocationStrategy, Package,
+    InvocationStrategy, Package, Sysroot,
 };
 
 #[derive(Debug, Default, Clone, PartialEq, Eq)]
@@ -61,6 +62,7 @@ impl WorkspaceBuildScripts {
         config: &CargoConfig,
         allowed_features: &FxHashSet<String>,
         workspace_root: &AbsPathBuf,
+        sysroot: Option<&Sysroot>,
     ) -> io::Result<Command> {
         let mut cmd = match config.run_build_script_command.as_deref() {
             Some([program, args @ ..]) => {
@@ -69,7 +71,10 @@ impl WorkspaceBuildScripts {
                 cmd
             }
             _ => {
-                let mut cmd = Command::new(toolchain::cargo());
+                let mut cmd = Command::new(
+                    Sysroot::discover_tool(sysroot, Tool::Cargo)
+                        .map_err(|e| io::Error::new(io::ErrorKind::NotFound, e))?,
+                );
 
                 cmd.args(["check", "--quiet", "--workspace", "--message-format=json"]);
                 cmd.args(&config.extra_args);
@@ -133,6 +138,7 @@ impl WorkspaceBuildScripts {
         workspace: &CargoWorkspace,
         progress: &dyn Fn(String),
         toolchain: &Option<Version>,
+        sysroot: Option<&Sysroot>,
     ) -> io::Result<WorkspaceBuildScripts> {
         const RUST_1_62: Version = Version::new(1, 62, 0);
 
@@ -151,6 +157,7 @@ impl WorkspaceBuildScripts {
                 config,
                 &allowed_features,
                 &workspace.workspace_root().to_path_buf(),
+                sysroot,
             )?,
             workspace,
             current_dir,
@@ -165,6 +172,7 @@ impl WorkspaceBuildScripts {
                     config,
                     &allowed_features,
                     &workspace.workspace_root().to_path_buf(),
+                    sysroot,
                 )?;
                 cmd.args(["-Z", "unstable-options", "--keep-going"]).env("RUSTC_BOOTSTRAP", "1");
                 let mut res = Self::run_per_ws(cmd, workspace, current_dir, progress)?;
@@ -194,7 +202,7 @@ impl WorkspaceBuildScripts {
                 ))
             }
         };
-        let cmd = Self::build_command(config, &Default::default(), workspace_root)?;
+        let cmd = Self::build_command(config, &Default::default(), workspace_root, None)?;
         // NB: Cargo.toml could have been modified between `cargo metadata` and
         // `cargo check`. We shouldn't assume that package ids we see here are
         // exactly those from `config`.
@@ -415,14 +423,15 @@ impl WorkspaceBuildScripts {
         rustc: &CargoWorkspace,
         current_dir: &AbsPath,
         extra_env: &FxHashMap<String, String>,
+        sysroot: Option<&Sysroot>,
     ) -> Self {
         let mut bs = WorkspaceBuildScripts::default();
         for p in rustc.packages() {
             bs.outputs.insert(p, BuildScriptOutput::default());
         }
         let res = (|| {
             let target_libdir = (|| {
-                let mut cargo_config = Command::new(toolchain::cargo());
+                let mut cargo_config = Command::new(Sysroot::discover_tool(sysroot, Tool::Cargo)?);
                 cargo_config.envs(extra_env);
                 cargo_config
                     .current_dir(current_dir)
@@ -431,7 +440,7 @@ impl WorkspaceBuildScripts {
                 if let Ok(it) = utf8_stdout(cargo_config) {
                     return Ok(it);
                 }
-                let mut cmd = Command::new(toolchain::rustc());
+                let mut cmd = Command::new(Sysroot::discover_tool(sysroot, Tool::Rustc)?);
                 cmd.envs(extra_env);
                 cmd.args(["--print", "target-libdir"]);
                 utf8_stdout(cmd)

crates/project-model/src/cargo_workspace.rs

@@ -12,8 +12,9 @@ use paths::{AbsPath, AbsPathBuf};
 use rustc_hash::{FxHashMap, FxHashSet};
 use serde::Deserialize;
 use serde_json::from_value;
+use toolchain::Tool;
 
-use crate::{utf8_stdout, InvocationLocation, ManifestPath};
+use crate::{utf8_stdout, InvocationLocation, ManifestPath, Sysroot};
 use crate::{CfgOverrides, InvocationStrategy};
 
 /// [`CargoWorkspace`] represents the logical structure of, well, a Cargo
@@ -236,12 +237,13 @@ impl CargoWorkspace {
         cargo_toml: &ManifestPath,
         current_dir: &AbsPath,
         config: &CargoConfig,
+        sysroot: Option<&Sysroot>,
         progress: &dyn Fn(String),
     ) -> anyhow::Result<cargo_metadata::Metadata> {
-        let targets = find_list_of_build_targets(config, cargo_toml);
+        let targets = find_list_of_build_targets(config, cargo_toml, sysroot);
 
         let mut meta = MetadataCommand::new();
-        meta.cargo_path(toolchain::cargo());
+        meta.cargo_path(Sysroot::discover_tool(sysroot, Tool::Cargo)?);
         meta.manifest_path(cargo_toml.to_path_buf());
         match &config.features {
             CargoFeatures::All => {
@@ -476,24 +478,29 @@ impl CargoWorkspace {
     }
 }
 
-fn find_list_of_build_targets(config: &CargoConfig, cargo_toml: &ManifestPath) -> Vec<String> {
+fn find_list_of_build_targets(
+    config: &CargoConfig,
+    cargo_toml: &ManifestPath,
+    sysroot: Option<&Sysroot>,
+) -> Vec<String> {
     if let Some(target) = &config.target {
         return [target.into()].to_vec();
     }
 
-    let build_targets = cargo_config_build_target(cargo_toml, &config.extra_env);
+    let build_targets = cargo_config_build_target(cargo_toml, &config.extra_env, sysroot);
     if !build_targets.is_empty() {
         return build_targets;
     }
 
-    rustc_discover_host_triple(cargo_toml, &config.extra_env).into_iter().collect()
+    rustc_discover_host_triple(cargo_toml, &config.extra_env, sysroot).into_iter().collect()
 }
 
 fn rustc_discover_host_triple(
     cargo_toml: &ManifestPath,
     extra_env: &FxHashMap<String, String>,
+    sysroot: Option<&Sysroot>,
 ) -> Option<String> {
-    let mut rustc = Command::new(toolchain::rustc());
+    let mut rustc = Command::new(Sysroot::discover_tool(sysroot, Tool::Rustc).ok()?);
     rustc.envs(extra_env);
     rustc.current_dir(cargo_toml.parent()).arg("-vV");
     tracing::debug!("Discovering host platform by {:?}", rustc);
@@ -519,8 +526,10 @@ fn rustc_discover_host_triple(
 fn cargo_config_build_target(
     cargo_toml: &ManifestPath,
     extra_env: &FxHashMap<String, String>,
+    sysroot: Option<&Sysroot>,
 ) -> Vec<String> {
-    let mut cargo_config = Command::new(toolchain::cargo());
+    let Ok(program) = Sysroot::discover_tool(sysroot, Tool::Cargo) else { return vec![] };
+    let mut cargo_config = Command::new(program);
     cargo_config.envs(extra_env);
     cargo_config
         .current_dir(cargo_toml.parent())

crates/project-model/src/rustc_cfg.rs

@@ -8,17 +8,13 @@ use rustc_hash::FxHashMap;
 use crate::{cfg_flag::CfgFlag, utf8_stdout, ManifestPath, Sysroot};
 
 /// Determines how `rustc --print cfg` is discovered and invoked.
-///
-/// There options are supported:
-/// - [`RustcCfgConfig::Cargo`], which relies on `cargo rustc --print cfg`
-///   and `RUSTC_BOOTSTRAP`.
-/// - [`RustcCfgConfig::Explicit`], which uses an explicit path to the `rustc`
-///   binary in the sysroot.
-/// - [`RustcCfgConfig::Discover`], which uses [`toolchain::rustc`].
 pub(crate) enum RustcCfgConfig<'a> {
-    Cargo(&'a ManifestPath),
-    Explicit(&'a Sysroot),
-    Discover,
+    /// Use `rustc --print cfg`, either from with the binary from the sysroot or by discovering via
+    /// [`toolchain::rustc`].
+    Rustc(Option<&'a Sysroot>),
+    /// Use `cargo --print cfg`, either from with the binary from the sysroot or by discovering via
+    /// [`toolchain::cargo`].
+    Cargo(Option<&'a Sysroot>, &'a ManifestPath),
 }
 
 pub(crate) fn get(
@@ -71,36 +67,31 @@ fn get_rust_cfgs(
     extra_env: &FxHashMap<String, String>,
     config: RustcCfgConfig<'_>,
 ) -> anyhow::Result<String> {
-    let mut cmd = match config {
-        RustcCfgConfig::Cargo(cargo_toml) => {
-            let mut cmd = Command::new(toolchain::cargo());
+    match config {
+        RustcCfgConfig::Cargo(sysroot, cargo_oml) => {
+            let cargo = Sysroot::discover_tool(sysroot, toolchain::Tool::Cargo)?;
+            let mut cmd = Command::new(cargo);
             cmd.envs(extra_env);
-            cmd.current_dir(cargo_toml.parent())
+            cmd.current_dir(cargo_oml.parent())
                 .args(["rustc", "-Z", "unstable-options", "--print", "cfg"])
                 .env("RUSTC_BOOTSTRAP", "1");
             if let Some(target) = target {
                 cmd.args(["--target", target]);
             }
 
-            return utf8_stdout(cmd).context("Unable to run `cargo rustc`");
+            utf8_stdout(cmd).context("Unable to run `cargo rustc`")
         }
-        RustcCfgConfig::Explicit(sysroot) => {
-            let rustc: std::path::PathBuf = sysroot.discover_rustc()?.into();
+        RustcCfgConfig::Rustc(sysroot) => {
+            let rustc = Sysroot::discover_tool(sysroot, toolchain::Tool::Rustc)?;
             tracing::debug!(?rustc, "using explicit rustc from sysroot");
-            Command::new(rustc)
-        }
-        RustcCfgConfig::Discover => {
-            let rustc = toolchain::rustc();
-            tracing::debug!(?rustc, "using rustc from env");
-            Command::new(rustc)
-        }
-    };
+            let mut cmd = Command::new(rustc);
+            cmd.envs(extra_env);
+            cmd.args(["--print", "cfg", "-O"]);
+            if let Some(target) = target {
+                cmd.args(["--target", target]);
+            }
 
-    cmd.envs(extra_env);
-    cmd.args(["--print", "cfg", "-O"]);
-    if let Some(target) = target {
-        cmd.args(["--target", target]);
+            utf8_stdout(cmd).context("Unable to run `rustc`")
+        }
     }
-
-    utf8_stdout(cmd).context("Unable to run `rustc`")
 }