Apply PR feedback

Author: dennisameling

Cargo.toml

@@ -33,6 +33,10 @@ parallel = ["dep:libc", "dep:jobserver"]
 # This is a placeholder feature for people who incorrectly used `cc` with `features = ["jobserver"]`
 # so that they aren't broken. This has never enabled `parallel`, so we won't do that.
 jobserver = []
+# Visual Studio doesn't have clang-cl (Clang/LLVM) installed by default as it's an optional module.
+# While GitHub Actions runners have it installed, it might cause some failing tests if you don't
+# have it. Run the tests with "--features disable-clang-cl-tests" if you want to disable related tests.
+disable-clang-cl-tests = []
 
 [dev-dependencies]
 tempfile = "3"

src/lib.rs

@@ -3802,7 +3802,12 @@ impl Build {
             self.cargo_output
                 .print_metadata(&format_args!("cargo:rerun-if-env-changed={v}"));
         }
-        let r = env::var_os(v).map(Arc::from);
+        let r = self
+            .env
+            .iter()
+            .find(|(k, _)| k.as_ref() == v)
+            .map(|(_, value)| value.clone())
+            .or_else(|| env::var_os(v).map(Arc::from));
         self.cargo_output.print_metadata(&format_args!(
             "{} = {}",
             v,

src/windows/find_tools.rs

@@ -554,9 +554,10 @@ mod impl_ {
                 // E.g. C:\...\VC\Tools\LLVM\x64\bin\clang.exe
                 base_path.push("bin");
                 base_path.push(tool);
+                let clang_cl = tool.contains("clang-cl");
                 base_path
                     .is_file()
-                    .then(|| Tool::with_family(base_path, ToolFamily::Msvc { clang_cl: true }))
+                    .then(|| Tool::with_family(base_path, ToolFamily::Msvc { clang_cl }))
             })
             .next()
     }
@@ -1139,8 +1140,6 @@ mod impl_ {
         use std::path::Path;
         // Import the find function from the module level
         use crate::windows::find_tools::find;
-        // Import StdEnvGetter from the parent module
-        use crate::windows::find_tools::StdEnvGetter;
 
         fn host_arch_to_string(host_arch_value: u16) -> &'static str {
             match host_arch_value {
@@ -1203,7 +1202,11 @@ mod impl_ {
         }
 
         #[test]
+        #[cfg(not(feature = "disable-clang-cl-tests"))]
         fn test_find_llvm_tools() {
+            // Import StdEnvGetter from the parent module
+            use crate::windows::find_tools::StdEnvGetter;
+
             // Test the actual find_llvm_tool function with various LLVM tools
             // This test assumes CI environment has Visual Studio + Clang installed
             // We test against x64 target since clang can cross-compile to any target

tests/test.rs

@@ -852,96 +852,99 @@ fn clang_android() {
     }
 }
 
-#[test]
 #[cfg(windows)]
-fn msvc_prefer_clang_cl_over_msvc_disabled_by_default() {
-    reset_env();
+#[cfg(not(feature = "disable-clang-cl-tests"))]
+mod msvc_clang_cl_tests {
+    use super::{reset_env, Test};
 
-    let test = Test::msvc_autodetect();
+    #[test]
+    fn msvc_prefer_clang_cl_over_msvc_disabled_by_default() {
+        reset_env();
 
-    // When prefer_clang_cl_over_msvc is not called (default false), should use MSVC
-    let compiler = test
-        .gcc()
-        .try_get_compiler()
-        .expect("Failed to get compiler");
-
-    // By default, should be using MSVC (cl.exe) and NOT clang-cl
-    assert!(compiler.is_like_msvc(), "Should use MSVC by default");
-    assert!(
-        !compiler.is_like_clang_cl(),
-        "Should not use clang-cl by default"
-    );
-}
+        let test = Test::msvc_autodetect();
 
-#[test]
-#[cfg(windows)]
-fn msvc_prefer_clang_cl_over_msvc_enabled() {
-    reset_env();
+        // When prefer_clang_cl_over_msvc is not called (default false), should use MSVC
+        let compiler = test
+            .gcc()
+            .try_get_compiler()
+            .expect("Failed to get compiler");
+
+        // By default, should be using MSVC (cl.exe) and NOT clang-cl
+        assert!(compiler.is_like_msvc(), "Should use MSVC by default");
+        assert!(
+            !compiler.is_like_clang_cl(),
+            "Should not use clang-cl by default"
+        );
+    }
 
-    let test = Test::msvc_autodetect();
+    #[test]
+    fn msvc_prefer_clang_cl_over_msvc_enabled() {
+        reset_env();
 
-    let compiler = test
-        .gcc()
-        // When prefer_clang_cl_over_msvc is true, should use clang-cl.exe
-        .prefer_clang_cl_over_msvc(true)
-        .try_get_compiler()
-        .expect("Failed to get compiler");
-
-    assert!(
-        compiler.is_like_clang_cl(),
-        "clang-cl.exe should be identified as clang-cl-like, got {:?}",
-        compiler
-    );
-    assert!(
-        compiler.is_like_msvc(),
-        "clang-cl should still be MSVC-like"
-    );
-}
+        let test = Test::msvc_autodetect();
 
-#[test]
-#[cfg(windows)]
-fn msvc_prefer_clang_cl_over_msvc_respects_explicit_cc_env() {
-    reset_env();
+        let compiler = test
+            .gcc()
+            // When prefer_clang_cl_over_msvc is true, should use clang-cl.exe
+            .prefer_clang_cl_over_msvc(true)
+            .try_get_compiler()
+            .expect("Failed to get compiler");
+
+        assert!(
+            compiler.is_like_clang_cl(),
+            "clang-cl.exe should be identified as clang-cl-like, got {:?}",
+            compiler
+        );
+        assert!(
+            compiler.is_like_msvc(),
+            "clang-cl should still be MSVC-like"
+        );
+    }
 
-    let test = Test::msvc_autodetect();
-    let compiler = test
-        .gcc()
-        // We can't set the CC=cl.exe environment variable directly in the test as it's removed
-        // in mod.rs, so we simulate it by setting the compiler directly
-        .compiler("cl.exe")
-        .prefer_clang_cl_over_msvc(true)
-        .try_get_compiler()
-        .expect("Failed to get compiler");
-
-    // The preference should not override explicit compiler setting
-    assert!(compiler.is_like_msvc(), "Should still be MSVC-like");
-    assert!(
-        !compiler.is_like_clang_cl(),
-        "Should NOT use clang-cl when CC is explicitly set to cl.exe, got {:?}",
-        compiler
-    );
-}
+    #[test]
+    fn msvc_prefer_clang_cl_over_msvc_respects_explicit_cc_env() {
+        reset_env();
 
-#[test]
-#[cfg(windows)]
-fn msvc_prefer_clang_cl_over_msvc_cpp_mode() {
-    reset_env();
+        let test = Test::msvc_autodetect();
 
-    let test = Test::msvc_autodetect();
-    let compiler = test
-        .gcc()
-        .cpp(true)
-        .prefer_clang_cl_over_msvc(true)
-        .try_get_compiler()
-        .expect("Failed to get compiler");
-
-    // Verify clang-cl.exe works correctly in C++ mode
-    assert!(
-        compiler.is_like_clang_cl(),
-        "clang-cl.exe should be identified as clang-cl-like in C++ mode"
-    );
-    assert!(
-        compiler.is_like_msvc(),
-        "clang-cl should still be MSVC-like in C++ mode"
-    );
+        //std::env::set_var("CC", "cl.exe");
+        let compiler = test
+            .gcc()
+            .__set_env("CC", "cl.exe")
+            .prefer_clang_cl_over_msvc(true)
+            .try_get_compiler()
+            .expect("Failed to get compiler");
+
+        // The preference should not override explicit compiler setting
+        assert!(compiler.is_like_msvc(), "Should still be MSVC-like");
+        assert!(
+            !compiler.is_like_clang_cl(),
+            "Should NOT use clang-cl when CC is explicitly set to cl.exe, got {:?}",
+            compiler
+        );
+        std::env::remove_var("CC"); // Clean up after test
+    }
+
+    #[test]
+    fn msvc_prefer_clang_cl_over_msvc_cpp_mode() {
+        reset_env();
+
+        let test = Test::msvc_autodetect();
+        let compiler = test
+            .gcc()
+            .cpp(true)
+            .prefer_clang_cl_over_msvc(true)
+            .try_get_compiler()
+            .expect("Failed to get compiler");
+
+        // Verify clang-cl.exe works correctly in C++ mode
+        assert!(
+            compiler.is_like_clang_cl(),
+            "clang-cl.exe should be identified as clang-cl-like in C++ mode"
+        );
+        assert!(
+            compiler.is_like_msvc(),
+            "clang-cl should still be MSVC-like in C++ mode"
+        );
+    }
 }