From 5b5bce8aafe275502b773d5950165a3993251147 Mon Sep 17 00:00:00 2001 From: David Barsky Date: Tue, 5 Sep 2023 15:21:14 -0400 Subject: [PATCH 1/5] project-model: when using `rust-project.json`, prefer the sysroot-defined rustc over an env-based one --- crates/project-model/src/rustc_cfg.rs | 22 +++++++++++++++++++--- crates/project-model/src/sysroot.rs | 11 ++++++++++- crates/project-model/src/workspace.rs | 20 ++++++++++++-------- 3 files changed, 41 insertions(+), 12 deletions(-) diff --git a/crates/project-model/src/rustc_cfg.rs b/crates/project-model/src/rustc_cfg.rs index 8392718b227..f8f9a5d5447 100644 --- a/crates/project-model/src/rustc_cfg.rs +++ b/crates/project-model/src/rustc_cfg.rs @@ -4,10 +4,11 @@ use std::process::Command; use rustc_hash::FxHashMap; -use crate::{cfg_flag::CfgFlag, utf8_stdout, ManifestPath}; +use crate::{cfg_flag::CfgFlag, utf8_stdout, ManifestPath, Sysroot}; pub(crate) fn get( cargo_toml: Option<&ManifestPath>, + sysroot: Option<&Sysroot>, target: Option<&str>, extra_env: &FxHashMap, ) -> Vec { @@ -25,7 +26,7 @@ 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) { + match get_rust_cfgs(cargo_toml, sysroot, target, extra_env) { Ok(rustc_cfgs) => { tracing::debug!( "rustc cfgs found: {:?}", @@ -44,6 +45,7 @@ pub(crate) fn get( fn get_rust_cfgs( cargo_toml: Option<&ManifestPath>, + sysroot: Option<&Sysroot>, target: Option<&str>, extra_env: &FxHashMap, ) -> anyhow::Result { @@ -62,8 +64,22 @@ fn get_rust_cfgs( Err(e) => tracing::debug!("{e:?}: falling back to querying rustc for cfgs"), } } + + let rustc = match sysroot { + Some(sysroot) => { + let rustc = sysroot.discover_rustc()?.into(); + tracing::debug!(?rustc, "using rustc from sysroot"); + rustc + } + None => { + let rustc = toolchain::rustc(); + tracing::debug!(?rustc, "using rustc from env"); + rustc + } + }; + // using unstable cargo features failed, fall back to using plain rustc - let mut cmd = Command::new(toolchain::rustc()); + let mut cmd = Command::new(rustc); cmd.envs(extra_env); cmd.args(["--print", "cfg", "-O"]); if let Some(target) = target { diff --git a/crates/project-model/src/sysroot.rs b/crates/project-model/src/sysroot.rs index da862c9e87f..fe046dd1463 100644 --- a/crates/project-model/src/sysroot.rs +++ b/crates/project-model/src/sysroot.rs @@ -115,10 +115,19 @@ impl Sysroot { Ok(Sysroot::load(sysroot_dir, src)) } - pub fn discover_rustc(&self) -> Option { + pub fn discover_rustc_src(&self) -> Option { get_rustc_src(&self.root) } + pub fn discover_rustc(&self) -> Result { + 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 { 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}") diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index 13463e9f72e..0eb88332908 100644 --- a/crates/project-model/src/workspace.rs +++ b/crates/project-model/src/workspace.rs @@ -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,12 @@ impl ProjectWorkspace { } }); - let rustc_cfg = - rustc_cfg::get(Some(&cargo_toml), config.target.as_deref(), &config.extra_env); + let rustc_cfg = rustc_cfg::get( + Some(&cargo_toml), + None, + config.target.as_deref(), + &config.extra_env, + ); let cfg_overrides = config.cfg_overrides.clone(); let data_layout = target_data_layout::get( @@ -335,7 +339,7 @@ impl ProjectWorkspace { tracing::info!(src_root = %sysroot.src_root(), root = %sysroot.root(), "Using sysroot"); } - let rustc_cfg = rustc_cfg::get(None, target, extra_env); + let rustc_cfg = rustc_cfg::get(None, sysroot.as_ref().ok(), target, extra_env); ProjectWorkspace::Json { project: project_json, sysroot, rustc_cfg, toolchain } } @@ -360,7 +364,7 @@ impl ProjectWorkspace { 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_cfg = rustc_cfg::get(None, sysroot.as_ref().ok(), None, &Default::default()); Ok(ProjectWorkspace::DetachedFiles { files: detached_files, sysroot, rustc_cfg }) } @@ -756,7 +760,7 @@ fn project_json_to_crate_graph( let target_cfgs = match target.as_deref() { Some(target) => cfg_cache .entry(target) - .or_insert_with(|| rustc_cfg::get(None, Some(target), extra_env)), + .or_insert_with(|| rustc_cfg::get(None, sysroot, Some(target), extra_env)), None => &rustc_cfg, }; From 553152e2d568dd06406bd1cb93c2a6fe86a579e6 Mon Sep 17 00:00:00 2001 From: David Barsky Date: Thu, 7 Sep 2023 15:06:51 -0400 Subject: [PATCH 2/5] refactor `rustc_cfg` to be more inline with `docs/dev/style.md` --- crates/project-model/src/rustc_cfg.rs | 90 +++++++++++++++------------ crates/project-model/src/workspace.rs | 45 ++++++++++---- 2 files changed, 82 insertions(+), 53 deletions(-) diff --git a/crates/project-model/src/rustc_cfg.rs b/crates/project-model/src/rustc_cfg.rs index f8f9a5d5447..d6e041b3adf 100644 --- a/crates/project-model/src/rustc_cfg.rs +++ b/crates/project-model/src/rustc_cfg.rs @@ -2,15 +2,21 @@ use std::process::Command; +use anyhow::Context; use rustc_hash::FxHashMap; use crate::{cfg_flag::CfgFlag, utf8_stdout, ManifestPath, Sysroot}; +pub(crate) enum Config<'a> { + Cargo(&'a ManifestPath), + Explicit(&'a Sysroot), + Discover, +} + pub(crate) fn get( - cargo_toml: Option<&ManifestPath>, - sysroot: Option<&Sysroot>, target: Option<&str>, extra_env: &FxHashMap, + config: Config<'_>, ) -> Vec { let _p = profile::span("rustc_cfg::get"); let mut res = Vec::with_capacity(6 * 2 + 1); @@ -26,64 +32,68 @@ 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, sysroot, target, extra_env) { - Ok(rustc_cfgs) => { - tracing::debug!( - "rustc cfgs found: {:?}", - rustc_cfgs - .lines() - .map(|it| it.parse::().map(|it| it.to_string())) - .collect::>() - ); - res.extend(rustc_cfgs.lines().filter_map(|it| it.parse().ok())); + 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::()).collect::, _>>(); + + match rustc_cfgs { + Ok(rustc_cfgs) => { + 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>, - sysroot: Option<&Sysroot>, target: Option<&str>, extra_env: &FxHashMap, + config: Config<'_>, ) -> anyhow::Result { - 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]); - } - match utf8_stdout(cargo_config) { - Ok(it) => return Ok(it), - Err(e) => tracing::debug!("{e:?}: falling back to querying rustc for cfgs"), - } - } + let mut cmd = match config { + Config::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]); + } - let rustc = match sysroot { - Some(sysroot) => { - let rustc = sysroot.discover_rustc()?.into(); - tracing::debug!(?rustc, "using rustc from sysroot"); - rustc + return utf8_stdout(cmd).context("Unable to run `cargo rustc`"); } - None => { + Config::Explicit(sysroot) => { + let rustc: std::path::PathBuf = sysroot.discover_rustc()?.into(); + tracing::debug!(?rustc, "using explicit rustc from sysroot"); + Command::new(rustc) + } + Config::Discover => { let rustc = toolchain::rustc(); tracing::debug!(?rustc, "using rustc from env"); - rustc + Command::new(rustc) } }; - // using unstable cargo features failed, fall back to using plain 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]); } - utf8_stdout(cmd) + + let out = utf8_stdout(cmd).context("Unable to run `rustc`")?; + Ok(out) } diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index 0eb88332908..6add6bf3b63 100644 --- a/crates/project-model/src/workspace.rs +++ b/crates/project-model/src/workspace.rs @@ -280,10 +280,9 @@ impl ProjectWorkspace { }); let rustc_cfg = rustc_cfg::get( - Some(&cargo_toml), - None, config.target.as_deref(), &config.extra_env, + rustc_cfg::Config::Cargo(cargo_toml), ); let cfg_overrides = config.cfg_overrides.clone(); @@ -335,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"); + rustc_cfg::Config::Explicit(sysroot) + } + Err(_) => { + tracing::debug!("discovering sysroot"); + rustc_cfg::Config::Discover + } + }; - let rustc_cfg = rustc_cfg::get(None, sysroot.as_ref().ok(), target, extra_env); + let rustc_cfg = rustc_cfg::get(target, extra_env, config); ProjectWorkspace::Json { project: project_json, sysroot, rustc_cfg, toolchain } } @@ -361,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, sysroot.as_ref().ok(), None, &Default::default()); + let rustc_config = match &sysroot { + Ok(sysroot) => { + tracing::info!(src_root = %sysroot.src_root(), root = %sysroot.root(), "Using sysroot"); + rustc_cfg::Config::Explicit(sysroot) + } + Err(_) => { + tracing::info!("discovering sysroot"); + rustc_cfg::Config::Discover + } + }; + + let rustc_cfg = rustc_cfg::get(None, &FxHashMap::default(), rustc_config); Ok(ProjectWorkspace::DetachedFiles { files: detached_files, sysroot, rustc_cfg }) } @@ -758,9 +772,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, sysroot, Some(target), extra_env)), + Some(target) => cfg_cache.entry(target).or_insert_with(|| { + let rustc_cfg = match sysroot { + Some(sysroot) => rustc_cfg::Config::Explicit(sysroot), + None => rustc_cfg::Config::Discover, + }; + + rustc_cfg::get(Some(target), extra_env, rustc_cfg) + }), None => &rustc_cfg, }; From fad3823a20c453907ee311953fedc69b7ca11d8d Mon Sep 17 00:00:00 2001 From: David Barsky Date: Thu, 7 Sep 2023 15:19:04 -0400 Subject: [PATCH 3/5] rename `rustc_cfg::Config` to `rustc_cfg::RustcCfgConfig` and import --- crates/project-model/src/rustc_cfg.rs | 12 ++++++------ crates/project-model/src/workspace.rs | 16 ++++++++-------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/crates/project-model/src/rustc_cfg.rs b/crates/project-model/src/rustc_cfg.rs index d6e041b3adf..f4af7b0d2f0 100644 --- a/crates/project-model/src/rustc_cfg.rs +++ b/crates/project-model/src/rustc_cfg.rs @@ -7,7 +7,7 @@ use rustc_hash::FxHashMap; use crate::{cfg_flag::CfgFlag, utf8_stdout, ManifestPath, Sysroot}; -pub(crate) enum Config<'a> { +pub(crate) enum RustcCfgConfig<'a> { Cargo(&'a ManifestPath), Explicit(&'a Sysroot), Discover, @@ -16,7 +16,7 @@ pub(crate) enum Config<'a> { pub(crate) fn get( target: Option<&str>, extra_env: &FxHashMap, - config: Config<'_>, + config: RustcCfgConfig<'_>, ) -> Vec { let _p = profile::span("rustc_cfg::get"); let mut res = Vec::with_capacity(6 * 2 + 1); @@ -61,10 +61,10 @@ pub(crate) fn get( fn get_rust_cfgs( target: Option<&str>, extra_env: &FxHashMap, - config: Config<'_>, + config: RustcCfgConfig<'_>, ) -> anyhow::Result { let mut cmd = match config { - Config::Cargo(cargo_toml) => { + RustcCfgConfig::Cargo(cargo_toml) => { let mut cmd = Command::new(toolchain::cargo()); cmd.envs(extra_env); cmd.current_dir(cargo_toml.parent()) @@ -76,12 +76,12 @@ fn get_rust_cfgs( return utf8_stdout(cmd).context("Unable to run `cargo rustc`"); } - Config::Explicit(sysroot) => { + RustcCfgConfig::Explicit(sysroot) => { let rustc: std::path::PathBuf = sysroot.discover_rustc()?.into(); tracing::debug!(?rustc, "using explicit rustc from sysroot"); Command::new(rustc) } - Config::Discover => { + RustcCfgConfig::Discover => { let rustc = toolchain::rustc(); tracing::debug!(?rustc, "using rustc from env"); Command::new(rustc) diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index 6add6bf3b63..5d6c21d5972 100644 --- a/crates/project-model/src/workspace.rs +++ b/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, @@ -282,7 +282,7 @@ impl ProjectWorkspace { let rustc_cfg = rustc_cfg::get( config.target.as_deref(), &config.extra_env, - rustc_cfg::Config::Cargo(cargo_toml), + RustcCfgConfig::Cargo(cargo_toml), ); let cfg_overrides = config.cfg_overrides.clone(); @@ -337,11 +337,11 @@ impl ProjectWorkspace { let config = match &sysroot { Ok(sysroot) => { tracing::debug!(src_root = %sysroot.src_root(), root = %sysroot.root(), "Using sysroot"); - rustc_cfg::Config::Explicit(sysroot) + RustcCfgConfig::Explicit(sysroot) } Err(_) => { tracing::debug!("discovering sysroot"); - rustc_cfg::Config::Discover + RustcCfgConfig::Discover } }; @@ -370,11 +370,11 @@ impl ProjectWorkspace { let rustc_config = match &sysroot { Ok(sysroot) => { tracing::info!(src_root = %sysroot.src_root(), root = %sysroot.root(), "Using sysroot"); - rustc_cfg::Config::Explicit(sysroot) + RustcCfgConfig::Explicit(sysroot) } Err(_) => { tracing::info!("discovering sysroot"); - rustc_cfg::Config::Discover + RustcCfgConfig::Discover } }; @@ -774,8 +774,8 @@ fn project_json_to_crate_graph( let target_cfgs = match target.as_deref() { Some(target) => cfg_cache.entry(target).or_insert_with(|| { let rustc_cfg = match sysroot { - Some(sysroot) => rustc_cfg::Config::Explicit(sysroot), - None => rustc_cfg::Config::Discover, + Some(sysroot) => RustcCfgConfig::Explicit(sysroot), + None => RustcCfgConfig::Discover, }; rustc_cfg::get(Some(target), extra_env, rustc_cfg) From 912b22fa0795b56aab514ed2801fb1a7b40f7704 Mon Sep 17 00:00:00 2001 From: David Barsky Date: Thu, 7 Sep 2023 15:30:11 -0400 Subject: [PATCH 4/5] add doc comment to `rustc_cfg::RustcCfgConfig` --- crates/project-model/src/rustc_cfg.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/project-model/src/rustc_cfg.rs b/crates/project-model/src/rustc_cfg.rs index f4af7b0d2f0..905adff4e5c 100644 --- a/crates/project-model/src/rustc_cfg.rs +++ b/crates/project-model/src/rustc_cfg.rs @@ -7,6 +7,14 @@ 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), From 0bf6ffaf822d6057d114425dfb59de99619247e4 Mon Sep 17 00:00:00 2001 From: David Barsky Date: Fri, 8 Sep 2023 14:07:59 -0400 Subject: [PATCH 5/5] Update crates/project-model/src/rustc_cfg.rs Co-authored-by: Lukas Wirth --- crates/project-model/src/rustc_cfg.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/project-model/src/rustc_cfg.rs b/crates/project-model/src/rustc_cfg.rs index 905adff4e5c..c5d55f7d217 100644 --- a/crates/project-model/src/rustc_cfg.rs +++ b/crates/project-model/src/rustc_cfg.rs @@ -102,6 +102,5 @@ fn get_rust_cfgs( cmd.args(["--target", target]); } - let out = utf8_stdout(cmd).context("Unable to run `rustc`")?; - Ok(out) + utf8_stdout(cmd).context("Unable to run `rustc`") }