Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 62 additions & 29 deletions crates/project-model/src/rustc_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name here is a little generic, but I'm struggling to think of anything better. In any case, I hope that the doc comment helps a little.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire module is slightly mislabeled by now I feel like, not sure what a better name for this would be though. Maybe builtin_cfg and the like

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

builtin_cfg is a good option. While we're bikeshedding, what do you think something like target_cfg since the output consists of things like:

``` rust-analyzer on davidbarsky/use-sysroot-rustc-to-determine-cfgs [$] via 🦀 v1.71.0 ❯ rustc --print cfg -O panic="unwind" target_arch="aarch64" target_endian="little" target_env="" target_family="unix" target_feature="aes" target_feature="crc" target_feature="dit" target_feature="dotprod" target_feature="dpb" target_feature="dpb2" target_feature="fcma" target_feature="fhm" target_feature="flagm" target_feature="fp16" target_feature="frintts" target_feature="jsconv" target_feature="lor" target_feature="lse" target_feature="neon" target_feature="paca" target_feature="pacg" target_feature="pan" target_feature="pmuv3" target_feature="ras" target_feature="rcpc" target_feature="rcpc2" target_feature="rdm" target_feature="sb" target_feature="sha2" target_feature="sha3" target_feature="ssbs" target_feature="vh" target_has_atomic="128" target_has_atomic="16" target_has_atomic="32" target_has_atomic="64" target_has_atomic="8" target_has_atomic="ptr" target_os="macos" target_pointer_width="64" target_vendor="apple" unix ```

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<'_>,
Veykril marked this conversation as resolved.
Show resolved Hide resolved
) -> Vec<CfgFlag> {
let _p = profile::span("rustc_cfg::get");
let mut res = Vec::with_capacity(6 * 2 + 1);
Expand All @@ -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`");
Veykril marked this conversation as resolved.
Show resolved Hide resolved
}
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`")
}
11 changes: 10 additions & 1 deletion crates/project-model/src/sysroot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Veykril marked this conversation as resolved.
Show resolved Hide resolved
get_rustc_src(&self.root)
}

pub fn discover_rustc(&self) -> Result<AbsPathBuf, std::io::Error> {
Veykril marked this conversation as resolved.
Show resolved Hide resolved
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}")
Expand Down
57 changes: 40 additions & 17 deletions crates/project-model/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
};
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 }
}

Expand All @@ -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 })
}

Expand Down Expand Up @@ -754,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, 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,
};

Expand Down