From 387dae9092414888e9291efd9317068458250a49 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 5 Jun 2025 14:47:00 +0000 Subject: [PATCH 1/3] Move canonicalization into current_dll_path And consistently use try_canonicalize rather than canonicalize. --- compiler/rustc_session/src/filesearch.rs | 30 ++++++++++-------------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_session/src/filesearch.rs b/compiler/rustc_session/src/filesearch.rs index 0e711890e076f..1fd74c914b290 100644 --- a/compiler/rustc_session/src/filesearch.rs +++ b/compiler/rustc_session/src/filesearch.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use std::{env, fs}; -use rustc_fs_util::{fix_windows_verbatim_for_gcc, try_canonicalize}; +use rustc_fs_util::try_canonicalize; use rustc_target::spec::Target; use smallvec::{SmallVec, smallvec}; @@ -87,7 +87,7 @@ fn current_dll_path() -> Result { }; let bytes = CStr::from_ptr(fname_ptr).to_bytes(); let os = OsStr::from_bytes(bytes); - Ok(PathBuf::from(os)) + try_canonicalize(Path::new(os)).map_err(|e| e.to_string()) } #[cfg(target_os = "aix")] @@ -122,7 +122,7 @@ fn current_dll_path() -> Result { if (data_base..data_end).contains(&addr) { let bytes = CStr::from_ptr(&(*current).ldinfo_filename[0]).to_bytes(); let os = OsStr::from_bytes(bytes); - return Ok(PathBuf::from(os)); + return try_canonicalize(Path::new(os)).map_err(|e| e.to_string()); } if (*current).ldinfo_next == 0 { break; @@ -169,7 +169,12 @@ fn current_dll_path() -> Result { filename.truncate(n); - Ok(OsString::from_wide(&filename).into()) + let path = try_canonicalize(OsString::from_wide(&filename)).map_err(|e| e.to_string())?; + + // See comments on this target function, but the gist is that + // gcc chokes on verbatim paths which fs::canonicalize generates + // so we try to avoid those kinds of paths. + Ok(rustc_fs_util::fix_windows_verbatim_for_gcc(&path)) } #[cfg(target_os = "wasi")] @@ -178,10 +183,8 @@ fn current_dll_path() -> Result { } pub fn sysroot_candidates() -> SmallVec<[PathBuf; 2]> { - let target = crate::config::host_tuple(); let mut sysroot_candidates: SmallVec<[PathBuf; 2]> = smallvec![get_or_default_sysroot()]; - let path = current_dll_path().and_then(|s| try_canonicalize(s).map_err(|e| e.to_string())); - if let Ok(dll) = path { + if let Ok(dll) = current_dll_path() { // use `parent` twice to chop off the file name and then also the // directory containing the dll which should be either `lib` or `bin`. if let Some(path) = dll.parent().and_then(|p| p.parent()) { @@ -196,7 +199,7 @@ pub fn sysroot_candidates() -> SmallVec<[PathBuf; 2]> { // assume that we may be in the main libdir. sysroot_candidates.push(path.to_owned()); - if path.ends_with(target) { + if path.ends_with(crate::config::host_tuple()) { sysroot_candidates.extend( path.parent() // chop off `$target` .and_then(|p| p.parent()) // chop off `rustlib` @@ -219,17 +222,8 @@ pub fn materialize_sysroot(maybe_sysroot: Option) -> PathBuf { /// This function checks if sysroot is found using env::args().next(), and if it /// is not found, finds sysroot from current rustc_driver dll. pub fn get_or_default_sysroot() -> PathBuf { - // Follow symlinks. If the resolved path is relative, make it absolute. - fn canonicalize(path: PathBuf) -> PathBuf { - let path = try_canonicalize(&path).unwrap_or(path); - // See comments on this target function, but the gist is that - // gcc chokes on verbatim paths which fs::canonicalize generates - // so we try to avoid those kinds of paths. - fix_windows_verbatim_for_gcc(&path) - } - fn default_from_rustc_driver_dll() -> Result { - let dll = current_dll_path().map(|s| canonicalize(s))?; + let dll = current_dll_path()?; // `dll` will be in one of the following two: // - compiler's libdir: $sysroot/lib/*.dll From dff8ee5b01b7251937860da41afd55958c13456a Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 5 Jun 2025 16:53:12 +0000 Subject: [PATCH 2/3] Replace all uses of sysroot_candidates with get_or_default_sysroot Before this change we had two different ways to attempt to locate the sysroot which are inconsistently used: * get_or_default_sysroot which tries to locate based on the 0th cli argument and if that doesn't work falls back to locating it using the librustc_driver.so location and returns a single path., * sysroot_candidates which takes the former and additionally does another attempt at locating using librustc_driver.so except without linux multiarch handling and then returns both paths., The latter was originally introduced to be able to locate the codegen backend back when cg_llvm was dynamically linked even for a custom driver when the --sysroot passed in does not contain a copy of cg_llvm. Back then get_or_default_sysroot did not attempt to locate the sysroot based on the location of librustc_driver.so yet. Because that is now done, the only case where removing sysroot_candidates can break things is if you have a custom driver inside what looks like a sysroot including the lib/rustlib directory, but which is missing some parts of the full sysroot like eg rust-lld. --- compiler/rustc_error_messages/src/lib.rs | 4 +-- compiler/rustc_interface/src/interface.rs | 4 +-- compiler/rustc_interface/src/util.rs | 11 ++++--- compiler/rustc_session/src/filesearch.rs | 36 +++++------------------ compiler/rustc_session/src/session.rs | 6 +--- 5 files changed, 17 insertions(+), 44 deletions(-) diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index 3c6df147b1ba5..a1f787b00d98f 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -107,7 +107,7 @@ impl From> for TranslationBundleError { #[instrument(level = "trace")] pub fn fluent_bundle( sysroot: PathBuf, - sysroot_candidates: Vec, + default_sysroot: PathBuf, requested_locale: Option, additional_ftl_path: Option<&Path>, with_directionality_markers: bool, @@ -141,7 +141,7 @@ pub fn fluent_bundle( // If the user requests the default locale then don't try to load anything. if let Some(requested_locale) = requested_locale { let mut found_resources = false; - for mut sysroot in Some(sysroot).into_iter().chain(sysroot_candidates.into_iter()) { + for mut sysroot in [sysroot, default_sysroot] { sysroot.push("share"); sysroot.push("locale"); sysroot.push(requested_locale.to_string()); diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index cf494f8d686e8..068b8c6d04459 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -18,7 +18,7 @@ use rustc_parse::parser::attr::AllowLeadingUnsafe; use rustc_query_impl::QueryCtxt; use rustc_query_system::query::print_query_stack; use rustc_session::config::{self, Cfg, CheckCfg, ExpectedValues, Input, OutFileName}; -use rustc_session::filesearch::sysroot_candidates; +use rustc_session::filesearch::get_or_default_sysroot; use rustc_session::parse::ParseSess; use rustc_session::{CompilerIO, EarlyDiagCtxt, Session, lint}; use rustc_span::source_map::{FileLoader, RealFileLoader, SourceMapInputs}; @@ -443,7 +443,7 @@ pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Se let bundle = match rustc_errors::fluent_bundle( config.opts.sysroot.clone(), - sysroot_candidates().to_vec(), + get_or_default_sysroot(), config.opts.unstable_opts.translate_lang.clone(), config.opts.unstable_opts.translate_additional_ftl.as_deref(), config.opts.unstable_opts.translate_directionality_markers, diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 087b11fdf9d9c..f769c6156aebc 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -2,7 +2,7 @@ use std::env::consts::{DLL_PREFIX, DLL_SUFFIX}; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, OnceLock}; -use std::{env, iter, thread}; +use std::{env, thread}; use rustc_ast as ast; use rustc_codegen_ssa::traits::CodegenBackend; @@ -12,7 +12,6 @@ use rustc_metadata::{DylibError, load_symbol_from_dylib}; use rustc_middle::ty::CurrentGcx; use rustc_parse::validate_attr; use rustc_session::config::{Cfg, OutFileName, OutputFilenames, OutputTypes, host_tuple}; -use rustc_session::filesearch::sysroot_candidates; use rustc_session::lint::{self, BuiltinLintDiag, LintBuffer}; use rustc_session::output::{CRATE_TYPES, categorize_crate_type}; use rustc_session::{EarlyDiagCtxt, Session, filesearch}; @@ -346,7 +345,7 @@ pub fn rustc_path<'a>() -> Option<&'a Path> { } fn get_rustc_path_inner(bin_path: &str) -> Option { - sysroot_candidates().iter().find_map(|sysroot| { + Some(filesearch::get_or_default_sysroot()).iter().find_map(|sysroot| { let candidate = sysroot.join(bin_path).join(if cfg!(target_os = "windows") { "rustc.exe" } else { @@ -374,10 +373,10 @@ fn get_codegen_sysroot( ); let target = host_tuple(); - let sysroot_candidates = sysroot_candidates(); + let sysroot_candidates = filesearch::sysroot_with_fallback(&sysroot); - let sysroot = iter::once(sysroot) - .chain(sysroot_candidates.iter().map(<_>::as_ref)) + let sysroot = sysroot_candidates + .iter() .map(|sysroot| { filesearch::make_target_lib_path(sysroot, target).with_file_name("codegen-backends") }) diff --git a/compiler/rustc_session/src/filesearch.rs b/compiler/rustc_session/src/filesearch.rs index 1fd74c914b290..def2cc97f061f 100644 --- a/compiler/rustc_session/src/filesearch.rs +++ b/compiler/rustc_session/src/filesearch.rs @@ -182,35 +182,13 @@ fn current_dll_path() -> Result { Err("current_dll_path is not supported on WASI".to_string()) } -pub fn sysroot_candidates() -> SmallVec<[PathBuf; 2]> { - let mut sysroot_candidates: SmallVec<[PathBuf; 2]> = smallvec![get_or_default_sysroot()]; - if let Ok(dll) = current_dll_path() { - // use `parent` twice to chop off the file name and then also the - // directory containing the dll which should be either `lib` or `bin`. - if let Some(path) = dll.parent().and_then(|p| p.parent()) { - // The original `path` pointed at the `rustc_driver` crate's dll. - // Now that dll should only be in one of two locations. The first is - // in the compiler's libdir, for example `$sysroot/lib/*.dll`. The - // other is the target's libdir, for example - // `$sysroot/lib/rustlib/$target/lib/*.dll`. - // - // We don't know which, so let's assume that if our `path` above - // ends in `$target` we *could* be in the target libdir, and always - // assume that we may be in the main libdir. - sysroot_candidates.push(path.to_owned()); - - if path.ends_with(crate::config::host_tuple()) { - sysroot_candidates.extend( - path.parent() // chop off `$target` - .and_then(|p| p.parent()) // chop off `rustlib` - .and_then(|p| p.parent()) // chop off `lib` - .map(|s| s.to_owned()), - ); - } - } +pub fn sysroot_with_fallback(sysroot: &Path) -> SmallVec<[PathBuf; 2]> { + let mut candidates = smallvec![sysroot.to_owned()]; + let default_sysroot = get_or_default_sysroot(); + if default_sysroot != sysroot { + candidates.push(default_sysroot); } - - sysroot_candidates + candidates } /// Returns the provided sysroot or calls [`get_or_default_sysroot`] if it's none. @@ -236,7 +214,7 @@ pub fn get_or_default_sysroot() -> PathBuf { dll.display() ))?; - // if `dir` points target's dir, move up to the sysroot + // if `dir` points to target's dir, move up to the sysroot let mut sysroot_dir = if dir.ends_with(crate::config::host_tuple()) { dir.parent() // chop off `$target` .and_then(|p| p.parent()) // chop off `rustlib` diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 010ae42c2802d..6b85e0abc8683 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -458,13 +458,9 @@ impl Session { /// directories are also returned, for example if `--sysroot` is used but tools are missing /// (#125246): we also add the bin directories to the sysroot where rustc is located. pub fn get_tools_search_paths(&self, self_contained: bool) -> Vec { - let bin_path = filesearch::make_target_bin_path(&self.sysroot, config::host_tuple()); - let fallback_sysroot_paths = filesearch::sysroot_candidates() + let search_paths = filesearch::sysroot_with_fallback(&self.sysroot) .into_iter() - // Ignore sysroot candidate if it was the same as the sysroot path we just used. - .filter(|sysroot| *sysroot != self.sysroot) .map(|sysroot| filesearch::make_target_bin_path(&sysroot, config::host_tuple())); - let search_paths = std::iter::once(bin_path).chain(fallback_sysroot_paths); if self_contained { // The self-contained tools are expected to be e.g. in `bin/self-contained` in the From e4c4c4c677f00a35aef0cfbe5e5ab3b30eafda4c Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 6 Jun 2025 14:20:48 +0000 Subject: [PATCH 3/3] Fix review comments --- Cargo.lock | 1 + compiler/rustc_error_messages/Cargo.toml | 1 + compiler/rustc_error_messages/src/lib.rs | 6 +++--- compiler/rustc_interface/src/interface.rs | 5 ++--- compiler/rustc_interface/src/util.rs | 12 ++++-------- 5 files changed, 11 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 43f5f40925b66..b9258fb185575 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3640,6 +3640,7 @@ dependencies = [ "rustc_macros", "rustc_serialize", "rustc_span", + "smallvec", "tracing", "unic-langid", ] diff --git a/compiler/rustc_error_messages/Cargo.toml b/compiler/rustc_error_messages/Cargo.toml index 0951859fa531f..5dc582b9c3a56 100644 --- a/compiler/rustc_error_messages/Cargo.toml +++ b/compiler/rustc_error_messages/Cargo.toml @@ -16,6 +16,7 @@ rustc_data_structures = { path = "../rustc_data_structures" } rustc_macros = { path = "../rustc_macros" } rustc_serialize = { path = "../rustc_serialize" } rustc_span = { path = "../rustc_span" } +smallvec = { version = "1.8.1", features = ["union", "may_dangle"] } tracing = "0.1" unic-langid = { version = "0.9.0", features = ["macros"] } # tidy-alphabetical-end diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index a1f787b00d98f..1d3b5b20751a7 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -21,6 +21,7 @@ use intl_memoizer::concurrent::IntlLangMemoizer; use rustc_data_structures::sync::IntoDynSyncSend; use rustc_macros::{Decodable, Encodable}; use rustc_span::Span; +use smallvec::SmallVec; use tracing::{instrument, trace}; pub use unic_langid::{LanguageIdentifier, langid}; @@ -106,8 +107,7 @@ impl From> for TranslationBundleError { /// (overriding any conflicting messages). #[instrument(level = "trace")] pub fn fluent_bundle( - sysroot: PathBuf, - default_sysroot: PathBuf, + sysroot_candidates: SmallVec<[PathBuf; 2]>, requested_locale: Option, additional_ftl_path: Option<&Path>, with_directionality_markers: bool, @@ -141,7 +141,7 @@ pub fn fluent_bundle( // If the user requests the default locale then don't try to load anything. if let Some(requested_locale) = requested_locale { let mut found_resources = false; - for mut sysroot in [sysroot, default_sysroot] { + for mut sysroot in sysroot_candidates { sysroot.push("share"); sysroot.push("locale"); sysroot.push(requested_locale.to_string()); diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 068b8c6d04459..e824e9d4aa91c 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -18,7 +18,7 @@ use rustc_parse::parser::attr::AllowLeadingUnsafe; use rustc_query_impl::QueryCtxt; use rustc_query_system::query::print_query_stack; use rustc_session::config::{self, Cfg, CheckCfg, ExpectedValues, Input, OutFileName}; -use rustc_session::filesearch::get_or_default_sysroot; +use rustc_session::filesearch::sysroot_with_fallback; use rustc_session::parse::ParseSess; use rustc_session::{CompilerIO, EarlyDiagCtxt, Session, lint}; use rustc_span::source_map::{FileLoader, RealFileLoader, SourceMapInputs}; @@ -442,8 +442,7 @@ pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Se let temps_dir = config.opts.unstable_opts.temps_dir.as_deref().map(PathBuf::from); let bundle = match rustc_errors::fluent_bundle( - config.opts.sysroot.clone(), - get_or_default_sysroot(), + sysroot_with_fallback(&config.opts.sysroot), config.opts.unstable_opts.translate_lang.clone(), config.opts.unstable_opts.translate_additional_ftl.as_deref(), config.opts.unstable_opts.translate_directionality_markers, diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index f769c6156aebc..8bdc24d47d98a 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -345,14 +345,10 @@ pub fn rustc_path<'a>() -> Option<&'a Path> { } fn get_rustc_path_inner(bin_path: &str) -> Option { - Some(filesearch::get_or_default_sysroot()).iter().find_map(|sysroot| { - let candidate = sysroot.join(bin_path).join(if cfg!(target_os = "windows") { - "rustc.exe" - } else { - "rustc" - }); - candidate.exists().then_some(candidate) - }) + let candidate = filesearch::get_or_default_sysroot() + .join(bin_path) + .join(if cfg!(target_os = "windows") { "rustc.exe" } else { "rustc" }); + candidate.exists().then_some(candidate) } #[allow(rustc::untranslatable_diagnostic)] // FIXME: make this translatable