From 4e1c4e68c7fe7ee7a29ecf37b2d84e3761cccaf7 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Thu, 30 Jan 2025 16:09:25 -0500 Subject: [PATCH 1/5] =?UTF-8?q?refactor:=20use=20`find=5Fmap`-`strip=5Fsuf?= =?UTF-8?q?fix`=20instead=20of=20`find`-`map`-`split`=20for=20`--driver-mo?= =?UTF-8?q?de=3D=E2=80=A6`=20search?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c0b468e3..97f622dd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2755,10 +2755,7 @@ impl Build { .map(|(tool, wrapper, args)| { // find the driver mode, if any const DRIVER_MODE: &str = "--driver-mode="; - let driver_mode = args - .iter() - .find(|a| a.starts_with(DRIVER_MODE)) - .map(|a| &a[DRIVER_MODE.len()..]); + let driver_mode = args.iter().find_map(|a| a.strip_suffix(DRIVER_MODE)); // Chop off leading/trailing whitespace to work around // semi-buggy build scripts which are shared in // makefiles/configure scripts (where spaces are far more From dfb54916caa24b62ccf1a20cd88621db67ffd049 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Thu, 30 Jan 2025 13:59:44 -0500 Subject: [PATCH 2/5] refactor: move both sides of `--driver-mode=cl` check to `Tool::get_base_compiler` Do this by plumbing through `args` associated with the compiler wrapper, if any, instead of handling it in calling code. --- src/lib.rs | 9 +++------ src/tool.rs | 28 +++++++++++++++++----------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 97f622dd..7a6da682 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2753,16 +2753,13 @@ impl Build { let tool_opt: Option = self .env_tool(env) .map(|(tool, wrapper, args)| { - // find the driver mode, if any - const DRIVER_MODE: &str = "--driver-mode="; - let driver_mode = args.iter().find_map(|a| a.strip_suffix(DRIVER_MODE)); // Chop off leading/trailing whitespace to work around // semi-buggy build scripts which are shared in // makefiles/configure scripts (where spaces are far more // lenient) - let mut t = Tool::with_clang_driver( + let mut t = Tool::with_args( tool, - driver_mode, + args.clone(), &self.build_cache.cached_compiler_family, &self.cargo_output, out_dir, @@ -2882,7 +2879,7 @@ impl Build { }; let mut nvcc_tool = Tool::with_features( nvcc, - None, + vec![], self.cuda, &self.build_cache.cached_compiler_family, &self.cargo_output, diff --git a/src/tool.rs b/src/tool.rs index 4233d591..990440aa 100644 --- a/src/tool.rs +++ b/src/tool.rs @@ -46,7 +46,7 @@ impl Tool { ) -> Self { Self::with_features( path, - None, + vec![], false, cached_compiler_family, cargo_output, @@ -54,16 +54,16 @@ impl Tool { ) } - pub(crate) fn with_clang_driver( + pub(crate) fn with_args( path: PathBuf, - clang_driver: Option<&str>, + args: Vec, cached_compiler_family: &RwLock, ToolFamily>>, cargo_output: &CargoOutput, out_dir: Option<&Path>, ) -> Self { Self::with_features( path, - clang_driver, + args, false, cached_compiler_family, cargo_output, @@ -88,7 +88,7 @@ impl Tool { pub(crate) fn with_features( path: PathBuf, - clang_driver: Option<&str>, + args: Vec, cuda: bool, cached_compiler_family: &RwLock, ToolFamily>>, cargo_output: &CargoOutput, @@ -235,12 +235,18 @@ impl Tool { Some(fname) if fname.ends_with("cl") || fname == "cl.exe" => { ToolFamily::Msvc { clang_cl: false } } - Some(fname) if fname.contains("clang") => match clang_driver { - Some("cl") => ToolFamily::Msvc { clang_cl: true }, - _ => ToolFamily::Clang { - zig_cc: is_zig_cc(&path, cargo_output), - }, - }, + Some(fname) if fname.contains("clang") => { + let is_clang_cl = args + .iter() + .find_map(|a| a.strip_prefix("--driver-mode=") == Some("cl")) + if is_clang_cl { + ToolFamily::Msvc { clang_cl: true } + } else { + ToolFamily::Clang { + zig_cc: is_zig_cc(&path, cargo_output), + } + } + } Some(fname) if fname.contains("zig") => ToolFamily::Clang { zig_cc: true }, _ => ToolFamily::Gnu, } From 50a3f70d0e6ab6e0dd079bfb40a873d0bf3683e2 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Thu, 30 Jan 2025 22:56:49 -0500 Subject: [PATCH 3/5] chore: `type CompilerFamilyLookupCache` to preempt `clippy::type_complexity` --- src/lib.rs | 4 ++-- src/tool.rs | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 7a6da682..46c23fcd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -254,7 +254,7 @@ use command_helpers::*; mod tool; pub use tool::Tool; -use tool::ToolFamily; +use tool::{CompilerFamilyLookupCache, ToolFamily}; mod tempfile; @@ -277,7 +277,7 @@ struct BuildCache { env_cache: RwLock, Env>>, apple_sdk_root_cache: RwLock, Arc>>, apple_versions_cache: RwLock, Arc>>, - cached_compiler_family: RwLock, ToolFamily>>, + cached_compiler_family: RwLock, known_flag_support_status_cache: RwLock>, target_info_parser: target::TargetInfoParser, } diff --git a/src/tool.rs b/src/tool.rs index 990440aa..953e082c 100644 --- a/src/tool.rs +++ b/src/tool.rs @@ -16,6 +16,8 @@ use crate::{ Error, ErrorKind, OutputKind, }; +pub(crate) type CompilerFamilyLookupCache = HashMap, ToolFamily>; + /// Configuration used to represent an invocation of a C compiler. /// /// This can be used to figure out what compiler is in use, what the arguments @@ -40,7 +42,7 @@ pub struct Tool { impl Tool { pub(crate) fn new( path: PathBuf, - cached_compiler_family: &RwLock, ToolFamily>>, + cached_compiler_family: &RwLock, cargo_output: &CargoOutput, out_dir: Option<&Path>, ) -> Self { @@ -57,7 +59,7 @@ impl Tool { pub(crate) fn with_args( path: PathBuf, args: Vec, - cached_compiler_family: &RwLock, ToolFamily>>, + cached_compiler_family: &RwLock, cargo_output: &CargoOutput, out_dir: Option<&Path>, ) -> Self { @@ -90,7 +92,7 @@ impl Tool { path: PathBuf, args: Vec, cuda: bool, - cached_compiler_family: &RwLock, ToolFamily>>, + cached_compiler_family: &RwLock, cargo_output: &CargoOutput, out_dir: Option<&Path>, ) -> Self { @@ -238,7 +240,7 @@ impl Tool { Some(fname) if fname.contains("clang") => { let is_clang_cl = args .iter() - .find_map(|a| a.strip_prefix("--driver-mode=") == Some("cl")) + .any(|a| a.strip_prefix("--driver-mode=") == Some("cl")); if is_clang_cl { ToolFamily::Msvc { clang_cl: true } } else { From 002c6d87e7186fcc50462697b44b3bc2d6dfaea8 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Thu, 30 Jan 2025 13:59:44 -0500 Subject: [PATCH 4/5] fix: include wrapper args. in `stdout` family heuristics This can be particularly significant for compilers that can dynamically change what options they accept based on arguments, like `clang --driver-mode=cl`. --- src/tool.rs | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/tool.rs b/src/tool.rs index 953e082c..04d366df 100644 --- a/src/tool.rs +++ b/src/tool.rs @@ -16,7 +16,7 @@ use crate::{ Error, ErrorKind, OutputKind, }; -pub(crate) type CompilerFamilyLookupCache = HashMap, ToolFamily>; +pub(crate) type CompilerFamilyLookupCache = HashMap]>, ToolFamily>; /// Configuration used to represent an invocation of a C compiler. /// @@ -116,21 +116,25 @@ impl Tool { fn guess_family_from_stdout( stdout: &str, path: &Path, + args: &[String], cargo_output: &CargoOutput, ) -> Result { cargo_output.print_debug(&stdout); // https://gitlab.kitware.com/cmake/cmake/-/blob/69a2eeb9dff5b60f2f1e5b425002a0fd45b7cadb/Modules/CMakeDetermineCompilerId.cmake#L267-271 // stdin is set to null to ensure that the help output is never paginated. - let accepts_cl_style_flags = - run(Command::new(path).arg("-?").stdin(Stdio::null()), path, &{ + let accepts_cl_style_flags = run( + Command::new(path).args(args).arg("-?").stdin(Stdio::null()), + path, + &{ // the errors are not errors! let mut cargo_output = cargo_output.clone(); cargo_output.warnings = cargo_output.debug; cargo_output.output = OutputKind::Discard; cargo_output - }) - .is_ok(); + }, + ) + .is_ok(); let clang = stdout.contains(r#""clang""#); let gcc = stdout.contains(r#""gcc""#); @@ -155,6 +159,7 @@ impl Tool { fn detect_family_inner( path: &Path, + args: &[String], cargo_output: &CargoOutput, out_dir: Option<&Path>, ) -> Result { @@ -209,25 +214,31 @@ impl Tool { &compiler_detect_output, )?; let stdout = String::from_utf8_lossy(&stdout); - guess_family_from_stdout(&stdout, path, cargo_output) + guess_family_from_stdout(&stdout, path, args, cargo_output) } else { - guess_family_from_stdout(&stdout, path, cargo_output) + guess_family_from_stdout(&stdout, path, args, cargo_output) } } - let detect_family = |path: &Path| -> Result { - if let Some(family) = cached_compiler_family.read().unwrap().get(path) { + let detect_family = |path: &Path, args: &[String]| -> Result { + let cache_key = [path.as_os_str()] + .iter() + .cloned() + .chain(args.iter().map(OsStr::new)) + .map(Into::into) + .collect(); + if let Some(family) = cached_compiler_family.read().unwrap().get(&cache_key) { return Ok(*family); } - let family = detect_family_inner(path, cargo_output, out_dir)?; + let family = detect_family_inner(path, args, cargo_output, out_dir)?; cached_compiler_family .write() .unwrap() - .insert(path.into(), family); + .insert(cache_key, family); Ok(family) }; - let family = detect_family(&path).unwrap_or_else(|e| { + let family = detect_family(&path, &args).unwrap_or_else(|e| { cargo_output.print_warning(&format_args!( "Compiler family detection failed due to error: {}", e From 5d2908124311708917ceee5a59a4dfe01f35d5ff Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Fri, 31 Jan 2025 12:48:11 -0500 Subject: [PATCH 5/5] test: `cfg` `Test` import behind `not(windows)` --- tests/rustflags.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/rustflags.rs b/tests/rustflags.rs index c9c6fc14..cee86f05 100644 --- a/tests/rustflags.rs +++ b/tests/rustflags.rs @@ -1,3 +1,4 @@ +#[cfg(not(windows))] use crate::support::Test; mod support;