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.

Sign up for GitHub

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

give rustc_driver users a chance to overwrite the default sysroot #122993

Closed
wants to merge 1 commit into from
Closed

give rustc_driver users a chance to overwrite the default sysroot #122993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 0 additions & 4 deletions compiler/rustc_interface/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,10 +642,6 @@ fn test_top_level_options_tracked_no_crate() {

// Make sure that changing a [TRACKED_NO_CRATE_HASH] option leaves the crate hash unchanged but changes the incremental hash.
// tidy-alphabetical-start
tracked!(
real_rust_source_base_dir,
Some("/home/bors/rust/.rustup/toolchains/nightly/lib/rustlib/src/rust".into())
);
tracked!(remap_path_prefix, vec![("/home/bors/rust".into(), "src".into())]);
// tidy-alphabetical-end
}
Expand Down
29 changes: 21 additions & 8 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use rustc_span::{BytePos, Pos, SpanData, SpanDecoder, SyntaxContext, DUMMY_SP};

use proc_macro::bridge::client::ProcMacro;
use std::iter::TrustedLen;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::{io, iter, mem};

pub(super) use cstore_impl::provide;
Expand Down Expand Up @@ -1589,10 +1589,14 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
/// Proc macro crates don't currently export spans, so this function does not have
/// to work for them.
fn imported_source_file(self, source_file_index: u32, sess: &Session) -> ImportedSourceFile {
fn filter<'a>(sess: &Session, path: Option<&'a Path>) -> Option<&'a Path> {
fn filter<'a>(
sess: &Session,
real_rust_source_base_dir: &Option<PathBuf>,
path: Option<&'a Path>,
) -> Option<&'a Path> {
path.filter(|_| {
// Only spend time on further checks if we have what to translate *to*.
sess.opts.real_rust_source_base_dir.is_some()
real_rust_source_base_dir.is_some()
// Some tests need the translation to be always skipped.
&& sess.opts.unstable_opts.translate_remapped_path_to_local_path
})
Expand All @@ -1604,25 +1608,34 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
})
}

let real_rust_source_base_dir = sess.real_rust_source_base_dir();
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only place where real_rust_source_base_dir gets called. Should I just move that logic into this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, should I be worried about perf here? We now re-compute real_rust_source_base_dir each time imported_source_file gets called. Not sure what would be the right place for a cache though.

Copy link
Member Author

@RalfJung RalfJung Mar 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Before #84233 this got computed after the config callback, in build_session. That would be a lot better. But it seems that was changed for good reasons.

Copy link
Contributor

Choose a reason for hiding this comment

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

This fn called a lot, you should wrap real_rust_source_base_dir into some once_cell.

let try_to_translate_virtual_to_real = |name: &mut rustc_span::FileName| {
// Translate the virtual `/rustc/$hash` prefix back to a real directory
// that should hold actual sources, where possible.
//
// NOTE: if you update this, you might need to also update bootstrap's code for generating
// the `rust-src` component in `Src::run` in `src/bootstrap/dist.rs`.
let virtual_rust_source_base_dir = [
filter(sess, option_env!("CFG_VIRTUAL_RUST_SOURCE_BASE_DIR").map(Path::new)),
filter(sess, sess.opts.unstable_opts.simulate_remapped_rust_src_base.as_deref()),
filter(
sess,
&real_rust_source_base_dir,
option_env!("CFG_VIRTUAL_RUST_SOURCE_BASE_DIR").map(Path::new),
),
filter(
sess,
&real_rust_source_base_dir,
sess.opts.unstable_opts.simulate_remapped_rust_src_base.as_deref(),
),
];

debug!(
"try_to_translate_virtual_to_real(name={:?}): \
virtual_rust_source_base_dir={:?}, real_rust_source_base_dir={:?}",
name, virtual_rust_source_base_dir, sess.opts.real_rust_source_base_dir,
name, virtual_rust_source_base_dir, real_rust_source_base_dir,
);

for virtual_dir in virtual_rust_source_base_dir.iter().flatten() {
if let Some(real_dir) = &sess.opts.real_rust_source_base_dir {
if let Some(real_dir) = &real_rust_source_base_dir {
if let rustc_span::FileName::Real(old_name) = name {
if let rustc_span::RealFileName::Remapped { local_path: _, virtual_name } =
old_name
Expand Down Expand Up @@ -1713,7 +1726,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
// `try_to_translate_virtual_to_real` don't have to worry about how the
// compiler is bootstrapped.
if let Some(virtual_dir) = &sess.opts.unstable_opts.simulate_remapped_rust_src_base
&& let Some(real_dir) = &sess.opts.real_rust_source_base_dir
&& let Some(real_dir) = &real_rust_source_base_dir
&& let rustc_span::FileName::Real(ref mut old_name) = name
{
let relative_path = match old_name {
Expand Down
28 changes: 3 additions & 25 deletions compiler/rustc_session/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ pub use crate::options::*;
use crate::errors::FileWriteFail;
use crate::search_paths::SearchPath;
use crate::utils::{CanonicalizedPath, NativeLib, NativeLibKind};
use crate::{filesearch, lint, HashStableContext};
use crate::{lint, HashStableContext};
use crate::{EarlyDiagCtxt, Session};
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
use rustc_data_structures::stable_hasher::{StableOrd, ToStableHashKey};
Expand Down Expand Up @@ -1066,7 +1066,6 @@ impl Default for Options {
cli_forced_codegen_units: None,
cli_forced_local_thinlto_off: false,
remap_path_prefix: Vec::new(),
real_rust_source_base_dir: None,
edition: DEFAULT_EDITION,
json_artifact_notifications: false,
json_unused_externs: JsonUnusedExterns::No,
Expand Down Expand Up @@ -2780,7 +2779,7 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M

let cg = cg;

let sysroot_opt = matches.opt_str("sysroot").map(|m| PathBuf::from(&m));
let maybe_sysroot = matches.opt_str("sysroot").map(|m| PathBuf::from(&m));
let target_triple = parse_target_triple(early_dcx, matches);
let opt_level = parse_opt_level(early_dcx, matches, &cg);
// The `-g` and `-C debuginfo` flags specify the same setting, so we want to be able
Expand Down Expand Up @@ -2823,26 +2822,6 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M

let logical_env = parse_logical_env(early_dcx, matches);

let sysroot = filesearch::materialize_sysroot(sysroot_opt);

let real_rust_source_base_dir = {
// This is the location used by the `rust-src` `rustup` component.
let mut candidate = sysroot.join("lib/rustlib/src/rust");
if let Ok(metadata) = candidate.symlink_metadata() {
// Replace the symlink rustbuild creates, with its destination.
// We could try to use `fs::canonicalize` instead, but that might
// produce unnecessarily verbose path.
if metadata.file_type().is_symlink() {
if let Ok(symlink_dest) = std::fs::read_link(&candidate) {
candidate = symlink_dest;
}
}
}

// Only use this directory if it has a file we can expect to always find.
candidate.join("library/std/src/lib.rs").is_file().then_some(candidate)
};

let working_dir = std::env::current_dir().unwrap_or_else(|e| {
early_dcx.early_fatal(format!("Current directory is invalid: {e}"));
});
Expand All @@ -2868,7 +2847,7 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
describe_lints,
output_types,
search_paths,
maybe_sysroot: Some(sysroot),
maybe_sysroot,
target_triple,
test,
incremental,
Expand All @@ -2889,7 +2868,6 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
cli_forced_codegen_units: codegen_units,
cli_forced_local_thinlto_off: disable_local_thinlto,
remap_path_prefix,
real_rust_source_base_dir,
edition,
json_artifact_notifications,
json_unused_externs,
Expand Down
8 changes: 0 additions & 8 deletions compiler/rustc_session/src/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,6 @@ top_level_options!(

/// Remap source path prefixes in all output (messages, object files, debug, etc.).
remap_path_prefix: Vec<(PathBuf, PathBuf)> [TRACKED_NO_CRATE_HASH],
/// Base directory containing the `src/` for the Rust standard library, and
/// potentially `rustc` as well, if we can find it. Right now it's always
/// `$sysroot/lib/rustlib/src/rust` (i.e. the `rustup` `rust-src` component).
///
/// This directory is what the virtual `/rustc/$hash` is translated back to,
/// if Rust was built with path remapping to `/rustc/$hash` enabled
/// (the `rust.remap-debuginfo` option in `config.toml`).
real_rust_source_base_dir: Option<PathBuf> [TRACKED_NO_CRATE_HASH],

edition: Edition [TRACKED],

Expand Down
26 changes: 26 additions & 0 deletions compiler/rustc_session/src/session.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,32 @@ impl Session {
.default_hidden_visibility
.unwrap_or(self.target.options.default_hidden_visibility)
}

/// Base directory containing the `src/` for the Rust standard library, and
/// potentially `rustc` as well, if we can find it. Right now it's always
/// `$sysroot/lib/rustlib/src/rust` (i.e. the `rustup` `rust-src` component).
///
/// This directory is what the virtual `/rustc/$hash` is translated back to,
/// if Rust was built with path remapping to `/rustc/$hash` enabled
/// (the `rust.remap-debuginfo` option in `config.toml`).
pub fn real_rust_source_base_dir(&self) -> Option<PathBuf> {
// This is the location used by the `rust-src` `rustup` component.
let sysroot = filesearch::materialize_sysroot(self.opts.maybe_sysroot.clone());
let mut candidate = sysroot.join("lib/rustlib/src/rust");
if let Ok(metadata) = candidate.symlink_metadata() {
// Replace the symlink rustbuild creates, with its destination.
// We could try to use `fs::canonicalize` instead, but that might
// produce an unnecessarily verbose path.
if metadata.file_type().is_symlink() {
if let Ok(symlink_dest) = std::fs::read_link(&candidate) {
candidate = symlink_dest;
}
}
}

// Only use this directory if it has a file we can expect to always find.
candidate.join("library/std/src/lib.rs").is_file().then_some(candidate)
}
}

// JUSTIFICATION: defn of the suggested wrapper fns
Expand Down
53 changes: 25 additions & 28 deletions src/tools/miri/src/bin/miri.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,34 @@ use rustc_session::{CtfeBacktrace, EarlyDiagCtxt};

use miri::{BacktraceStyle, BorrowTrackerMethod, ProvenanceMode, RetagFields};

fn show_error(msg: &impl std::fmt::Display) -> ! {
eprintln!("fatal error: {msg}");
std::process::exit(1)
}

macro_rules! show_error {
($($tt:tt)*) => { show_error(&format_args!($($tt)*)) };
}

struct MiriCompilerCalls {
miri_config: miri::MiriConfig,
}

fn set_default_sysroot_for_target_crate(config: &mut Config) {
if config.opts.maybe_sysroot.is_none() {
// Overwrite the default.
let miri_sysroot = env::var("MIRI_SYSROOT").unwrap_or_else(|_| {
show_error!(
"Miri was invoked in 'target' mode without `MIRI_SYSROOT` or `--sysroot` being set"
)
});
config.opts.maybe_sysroot = Some(PathBuf::from(miri_sysroot));
}
}

impl rustc_driver::Callbacks for MiriCompilerCalls {
fn config(&mut self, config: &mut Config) {
set_default_sysroot_for_target_crate(config);
config.override_queries = Some(|_, providers| {
providers.extern_queries.used_crate_source = |tcx, cnum| {
let mut providers = Providers::default();
Expand Down Expand Up @@ -130,6 +152,9 @@ struct MiriBeRustCompilerCalls {
impl rustc_driver::Callbacks for MiriBeRustCompilerCalls {
#[allow(rustc::potential_query_instability)] // rustc_codegen_ssa (where this code is copied from) also allows this lint
fn config(&mut self, config: &mut Config) {
if self.target_crate {
set_default_sysroot_for_target_crate(config);
}
if config.opts.prints.is_empty() && self.target_crate {
// Queries overridden here affect the data stored in `rmeta` files of dependencies,
// which will be used later in non-`MIRI_BE_RUSTC` mode.
Expand Down Expand Up @@ -201,15 +226,6 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls {
}
}

fn show_error(msg: &impl std::fmt::Display) -> ! {
eprintln!("fatal error: {msg}");
std::process::exit(1)
}

macro_rules! show_error {
($($tt:tt)*) => { show_error(&format_args!($($tt)*)) };
}

fn rustc_logger_config() -> rustc_log::LoggerConfig {
// Start with the usual env vars.
let mut cfg = rustc_log::LoggerConfig::from_env("RUSTC_LOG");
Expand Down Expand Up @@ -271,25 +287,6 @@ fn run_compiler(
callbacks: &mut (dyn rustc_driver::Callbacks + Send),
using_internal_features: std::sync::Arc<std::sync::atomic::AtomicBool>,
) -> ! {
if target_crate {
// Miri needs a custom sysroot for target crates.
// If no `--sysroot` is given, the `MIRI_SYSROOT` env var is consulted to find where
// that sysroot lives, and that is passed to rustc.
let sysroot_flag = "--sysroot";
if !args.iter().any(|e| e == sysroot_flag) {
// Using the built-in default here would be plain wrong, so we *require*
// the env var to make sure things make sense.
let miri_sysroot = env::var("MIRI_SYSROOT").unwrap_or_else(|_| {
show_error!(
"Miri was invoked in 'target' mode without `MIRI_SYSROOT` or `--sysroot` being set"
)
});

args.push(sysroot_flag.to_owned());
args.push(miri_sysroot);
}
}

// Don't insert `MIRI_DEFAULT_ARGS`, in particular, `--cfg=miri`, if we are building
// a "host" crate. That may cause procedural macros (and probably build scripts) to
// depend on Miri-only symbols, such as `miri_resolve_frame`:
Expand Down
Loading