Auto merge of #15560 - davidbarsky:davidbarsky/use-sysroot-rustc-to-d…

…etermine-cfgs, r=Veykril

project-model: when using `rust-project.json`, prefer the sysroot-defined rustc over discovery in `$PATH`

At the moment, rust-analyzer discovers `rustc` via the `$PATH` even if the `sysroot` field is defined in a `rust-project.json`. However, this does not work for users who do not have rustup installed, resulting in any `cfg`-based inference in rust-analzyer not working correctly. In my (decently naive!) opinion, it makes more sense to rely on the `sysroot` field in the `rust-project.json`.

One might ask "why not add `rustc` to the `$PATH`?" That is a reasonable question, but that doesn't work for my use case:
- The path to the sysroot in my employer's monorepo changes depending on which platform a user is on. For example, if they're on Linux, they'd want to use the sysroot defined at path `a`, whereas if they're on macOS, they'd want to use the sysroot at path `b` (I wrote the sysroot resolution functionality [here](https://github.com/facebook/buck2/blob/765da4ca1e0b97a0de1e82b2fb3af52766fd06f4/integrations/rust-project/src/sysroot.rs#L39), if you're curious).
- The location of the sysroot can (and does!) change, especially as people figure out how to make Rust run successfully on non-Linux platforms (e.g., iOS, Android, etc.) in a monorepo. Updating people's `$PATH` company-wide is hard while updating a config inside a CLI is pretty easy.

## Testing

I've created a `rust-project.json` using [rust-project](https://github.com/facebook/buck2/tree/main/integrations/rust-project) and was able to successfully load a project with and without the `sysroot`/`sysroot_src` fields—without those fields, rust-analyzer fell back to the `$PATH` based approach, as evidenced by `[DEBUG project_model::rustc_cfg] using rustc from env rustc="rustc"` showing up in the logs.

crates/project-model/src/rustc_cfg.rs

@@ -2,14 +2,29 @@
 
 use std::process::Command;
 
+use anyhow::Context;
 use rustc_hash::FxHashMap;
 
-use crate::{cfg_flag::CfgFlag, utf8_stdout, ManifestPath};
+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,
+}
 
 pub(crate) fn get(
-    cargo_toml: Option<&ManifestPath>,
     target: Option<&str>,
     extra_env: &FxHashMap<String, String>,
+    config: RustcCfgConfig<'_>,
 ) -> Vec<CfgFlag> {
     let _p = profile::span("rustc_cfg::get");
     let mut res = Vec::with_capacity(6 * 2 + 1);
@@ -25,49 +40,67 @@ pub(crate) fn get(
     // Add miri cfg, which is useful for mir eval in stdlib
     res.push(CfgFlag::Atom("miri".into()));
 
-    match get_rust_cfgs(cargo_toml, target, extra_env) {
+    let rustc_cfgs = get_rust_cfgs(target, extra_env, config);
+
+    let rustc_cfgs = match rustc_cfgs {
+        Ok(cfgs) => cfgs,
+        Err(e) => {
+            tracing::error!(?e, "failed to get rustc cfgs");
+            return res;
+        }
+    };
+
+    let rustc_cfgs =
+        rustc_cfgs.lines().map(|it| it.parse::<CfgFlag>()).collect::<Result<Vec<_>, _>>();
+
+    match rustc_cfgs {
         Ok(rustc_cfgs) => {
-            tracing::debug!(
-                "rustc cfgs found: {:?}",
-                rustc_cfgs
-                    .lines()
-                    .map(|it| it.parse::<CfgFlag>().map(|it| it.to_string()))
-                    .collect::<Vec<_>>()
-            );
-            res.extend(rustc_cfgs.lines().filter_map(|it| it.parse().ok()));
+            tracing::debug!(?rustc_cfgs, "rustc cfgs found");
+            res.extend(rustc_cfgs);
+        }
+        Err(e) => {
+            tracing::error!(?e, "failed to get rustc cfgs")
         }
-        Err(e) => tracing::error!("failed to get rustc cfgs: {e:?}"),
     }
 
     res
 }
 
 fn get_rust_cfgs(
-    cargo_toml: Option<&ManifestPath>,
     target: Option<&str>,
     extra_env: &FxHashMap<String, String>,
+    config: RustcCfgConfig<'_>,
 ) -> anyhow::Result<String> {
-    if let Some(cargo_toml) = cargo_toml {
-        let mut cargo_config = Command::new(toolchain::cargo());
-        cargo_config.envs(extra_env);
-        cargo_config
-            .current_dir(cargo_toml.parent())
-            .args(["rustc", "-Z", "unstable-options", "--print", "cfg"])
-            .env("RUSTC_BOOTSTRAP", "1");
-        if let Some(target) = target {
-            cargo_config.args(["--target", target]);
+    let mut cmd = match config {
+        RustcCfgConfig::Cargo(cargo_toml) => {
+            let mut cmd = Command::new(toolchain::cargo());
+            cmd.envs(extra_env);
+            cmd.current_dir(cargo_toml.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`");
         }
-        match utf8_stdout(cargo_config) {
-            Ok(it) => return Ok(it),
-            Err(e) => tracing::debug!("{e:?}: falling back to querying rustc for cfgs"),
+        RustcCfgConfig::Explicit(sysroot) => {
+            let rustc: std::path::PathBuf = sysroot.discover_rustc()?.into();
+            tracing::debug!(?rustc, "using explicit rustc from sysroot");
+            Command::new(rustc)
         }
-    }
-    // using unstable cargo features failed, fall back to using plain rustc
-    let mut cmd = Command::new(toolchain::rustc());
+        RustcCfgConfig::Discover => {
+            let rustc = toolchain::rustc();
+            tracing::debug!(?rustc, "using rustc from env");
+            Command::new(rustc)
+        }
+    };
+
     cmd.envs(extra_env);
     cmd.args(["--print", "cfg", "-O"]);
     if let Some(target) = target {
         cmd.args(["--target", target]);
     }
-    utf8_stdout(cmd)
+
+    utf8_stdout(cmd).context("Unable to run `rustc`")
 }

crates/project-model/src/sysroot.rs

@@ -115,10 +115,19 @@ impl Sysroot {
         Ok(Sysroot::load(sysroot_dir, src))
     }
 
-    pub fn discover_rustc(&self) -> Option<ManifestPath> {
+    pub fn discover_rustc_src(&self) -> Option<ManifestPath> {
         get_rustc_src(&self.root)
     }
 
+    pub fn discover_rustc(&self) -> Result<AbsPathBuf, std::io::Error> {
+        let rustc = self.root.join("bin/rustc");
+        tracing::debug!(?rustc, "checking for rustc binary at location");
+        match fs::metadata(&rustc) {
+            Ok(_) => Ok(rustc),
+            Err(e) => Err(e),
+        }
+    }
+
     pub fn with_sysroot_dir(sysroot_dir: AbsPathBuf) -> Result<Sysroot> {
         let sysroot_src_dir = discover_sysroot_src_dir(&sysroot_dir).ok_or_else(|| {
             format_err!("can't load standard library from sysroot path {sysroot_dir}")

crates/project-model/src/workspace.rs

@@ -21,7 +21,7 @@ use crate::{
     cargo_workspace::{DepKind, PackageData, RustLibSource},
     cfg_flag::CfgFlag,
     project_json::Crate,
-    rustc_cfg,
+    rustc_cfg::{self, RustcCfgConfig},
     sysroot::SysrootCrate,
     target_data_layout, utf8_stdout, CargoConfig, CargoWorkspace, InvocationStrategy, ManifestPath,
     Package, ProjectJson, ProjectManifest, Sysroot, TargetData, TargetKind, WorkspaceBuildScripts,
@@ -240,9 +240,9 @@ impl ProjectWorkspace {
                     Some(RustLibSource::Path(path)) => ManifestPath::try_from(path.clone())
                         .map_err(|p| Some(format!("rustc source path is not absolute: {p}"))),
                     Some(RustLibSource::Discover) => {
-                        sysroot.as_ref().ok().and_then(Sysroot::discover_rustc).ok_or_else(|| {
-                            Some(format!("Failed to discover rustc source for sysroot."))
-                        })
+                        sysroot.as_ref().ok().and_then(Sysroot::discover_rustc_src).ok_or_else(
+                            || Some(format!("Failed to discover rustc source for sysroot.")),
+                        )
                     }
                     None => Err(None),
                 };
@@ -279,8 +279,11 @@ impl ProjectWorkspace {
                     }
                 });
 
-                let rustc_cfg =
-                    rustc_cfg::get(Some(&cargo_toml), config.target.as_deref(), &config.extra_env);
+                let rustc_cfg = rustc_cfg::get(
+                    config.target.as_deref(),
+                    &config.extra_env,
+                    RustcCfgConfig::Cargo(cargo_toml),
+                );
 
                 let cfg_overrides = config.cfg_overrides.clone();
                 let data_layout = target_data_layout::get(
@@ -331,11 +334,18 @@ impl ProjectWorkspace {
             }
             (None, None) => Err(None),
         };
-        if let Ok(sysroot) = &sysroot {
-            tracing::info!(src_root = %sysroot.src_root(), root = %sysroot.root(), "Using sysroot");
-        }
+        let config = match &sysroot {
+            Ok(sysroot) => {
+                tracing::debug!(src_root = %sysroot.src_root(), root = %sysroot.root(), "Using sysroot");
+                RustcCfgConfig::Explicit(sysroot)
+            }
+            Err(_) => {
+                tracing::debug!("discovering sysroot");
+                RustcCfgConfig::Discover
+            }
+        };
 
-        let rustc_cfg = rustc_cfg::get(None, target, extra_env);
+        let rustc_cfg = rustc_cfg::get(target, extra_env, config);
         ProjectWorkspace::Json { project: project_json, sysroot, rustc_cfg, toolchain }
     }
 
@@ -357,10 +367,18 @@ impl ProjectWorkspace {
             }
             None => Err(None),
         };
-        if let Ok(sysroot) = &sysroot {
-            tracing::info!(src_root = %sysroot.src_root(), root = %sysroot.root(), "Using sysroot");
-        }
-        let rustc_cfg = rustc_cfg::get(None, None, &Default::default());
+        let rustc_config = match &sysroot {
+            Ok(sysroot) => {
+                tracing::info!(src_root = %sysroot.src_root(), root = %sysroot.root(), "Using sysroot");
+                RustcCfgConfig::Explicit(sysroot)
+            }
+            Err(_) => {
+                tracing::info!("discovering sysroot");
+                RustcCfgConfig::Discover
+            }
+        };
+
+        let rustc_cfg = rustc_cfg::get(None, &FxHashMap::default(), rustc_config);
         Ok(ProjectWorkspace::DetachedFiles { files: detached_files, sysroot, rustc_cfg })
     }
 
@@ -755,9 +773,14 @@ fn project_json_to_crate_graph(
                 let env = env.clone().into_iter().collect();
 
                 let target_cfgs = match target.as_deref() {
-                    Some(target) => cfg_cache
-                        .entry(target)
-                        .or_insert_with(|| rustc_cfg::get(None, Some(target), extra_env)),
+                    Some(target) => cfg_cache.entry(target).or_insert_with(|| {
+                        let rustc_cfg = match sysroot {
+                            Some(sysroot) => RustcCfgConfig::Explicit(sysroot),
+                            None => RustcCfgConfig::Discover,
+                        };
+
+                        rustc_cfg::get(Some(target), extra_env, rustc_cfg)
+                    }),
                     None => &rustc_cfg,
                 };