Repository: rust-lang/cc-rs (rust)

Issue:
1.0.88 breaks the Rust compile-ui tests for arm-android
When trying to update cc-rs to 1.0.88 in the Rust repo, we hit an error with the compile-ui test for arm-android:
<https://github.com/rust-lang/rust/pull/121854#issuecomment-1973518078>
```
2024-03-01T16:42:03.5816834Z ---- [ui] tests/ui/thread-local/tls.rs stdout ----
2024-03-01T16:42:03.5844499Z --- stderr -------------------------------
2024-03-01T16:42:03.5845537Z error: linking with `/android/ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/armv7a-linux-androideabi19-clang` failed: exit status: 1
2024-03-01T16:42:03.5846544Z    |
2024-03-01T16:42:03.5854489Z    = note: LC_ALL="C" PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/android/sdk/emulator:/android/sdk/tools:/android/sdk/platform-tools" VSLANG="1033" "/android/ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/armv7a-linux-androideabi19-clang" "/tmp/rustc0dOpvq/symbols.o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/thread-local/tls/a.tls.ca29c82538305dca-cgu.0.rcgu.o" "-Wl,--as-needed" "-L" "/checkout/obj/build/arm-linux-androideabi/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/thread-local/tls/auxiliary" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/arm-linux-androideabi/lib" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/arm-linux-androideabi/lib" "-Wl,-Bdynamic" "-lstd-8e24d49e9c2a7cc4" "-Wl,-Bstatic" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/arm-linux-androideabi/lib/libcompiler_builtins-23518b53a80b1f29.rlib" "-Wl,-Bdynamic" "-ldl" "-llog" "-lunwind" "-ldl" "-lm" "-lc" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/arm-linux-androideabi/lib" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/thread-local/tls/a" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-Wl,-O1" "-Wl,--strip-debug" "-nodefaultlibs" "-Wl,-rpath,$ORIGIN/../../../../stage2/lib/rustlib/arm-linux-androideabi/lib" "-Wl,--enable-new-dtags" "-Wl,-z,origin"
2024-03-01T16:42:03.5862557Z    = note: ld: error: undefined symbol: __atomic_load_4
2024-03-01T16:42:03.5863027Z            >>> referenced by emutls.c
2024-03-01T16:42:03.5864373Z            >>>               45c91108d938afe8-emutls.o:(__emutls_get_address) in archive /checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/arm-linux-androideabi/lib/libcompiler_builtins-23518b53a80b1f29.rlib
2024-03-01T16:42:03.5865772Z            
2024-03-01T16:42:03.5866209Z            ld: error: undefined symbol: __atomic_store_4
2024-03-01T16:42:03.5866677Z            >>> referenced by emutls.c
2024-03-01T16:42:03.5867943Z            >>>               45c91108d938afe8-emutls.o:(__emutls_get_address) in archive /checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/arm-linux-androideabi/lib/libcompiler_builtins-23518b53a80b1f29.rlib
2024-03-01T16:42:03.5869403Z            clang-14: error: linker command failed with exit code 1 (use -v to see invocation)
```

I've narrowed down the failure to <https://github.com/rust-lang/cc-rs/commit/53564e00498156c9be00361c4b039952cbf7ff59>

Using 53564e00498156c9be00361c4b039952cbf7ff59 reproduced the issue: <https://github.com/rust-lang/rust/pull/121874#issuecomment-1974171096>
The previous commit did not have the issue: https://github.com/rust-lang/rust/actions/runs/8145164818/job/22260797425?pr=121874
And reverting 53564e00498156c9be00361c4b039952cbf7ff59 from 1.0.88 also did not have the issue: https://github.com/rust-lang/rust/actions/runs/8145164818/job/22260797425?pr=121874

Pull request:
Fix regression: remove `--target` when targeting Android on non-Windows platforms
The `--target` argument should not be passed to Clang when targeting Android, as the Android NDK contains wrappers for Clang that automatically set the target.

However, <https://github.com/rust-lang/cc-rs/commit/53564e00498156c9be00361c4b039952cbf7ff59> introduced a regression where `--target` was being passed on non-Windows platforms, which broke the `compile-ui` tests for the Rust compiler when building for `arm-android`: <https://github.com/rust-lang/rust/pull/121854#issuecomment-1973518078>

This fix restores the original logic AND respects the new `has_internal_target_arg` flag (regardless if we're targeting Android or not).

I've verified that the `arm-android` build succeeds with this change: <https://github.com/rust-lang/rust/actions/runs/8146073082/job/22263762750?pr=121874>

Fixes #990

Patch:
diff --git a/src/lib.rs b/src/lib.rs
index 6999d14..0238961 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1894,7 +1894,10 @@ impl Build {
         // Target flags
         match cmd.family {
             ToolFamily::Clang => {
-                if !(target.contains("android") && cmd.has_internal_target_arg) {
+                if !cmd.has_internal_target_arg
+                    && !(target.contains("android")
+                        && android_clang_compiler_uses_target_arg_internally(&cmd.path))
+                {
                     if target.contains("darwin") {
                         if let Some(arch) =
                             map_darwin_target_from_rust_to_compiler_architecture(target)


Test patch:
diff --git a/tests/test.rs b/tests/test.rs
index f8c950d..f48e05d 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -617,3 +617,33 @@ fn compile_intermediates() {
     assert!(intermediates[1].display().to_string().contains("x86_64"));
     assert!(intermediates[2].display().to_string().contains("x86_64"));
 }
+
+#[test]
+fn clang_android() {
+    let target = "arm-linux-androideabi";
+
+    // On Windows, we don't use the Android NDK shims for Clang, so verify that
+    // we use "clang" and set the target correctly.
+    #[cfg(windows)]
+    {
+        let test = Test::new();
+        test.shim("clang").shim("llvm-ar");
+        test.gcc()
+            .target(target)
+            .host("x86_64-pc-windows-msvc")
+            .file("foo.c")
+            .compile("foo");
+        test.cmd(0).must_have("--target=arm-linux-androideabi");
+    }
+
+    // On non-Windows, we do use the shims, so make sure that we use the shim
+    // and don't set the target.
+    #[cfg(not(windows))]
+    {
+        let test = Test::new();
+        test.shim("arm-linux-androideabi-clang")
+            .shim("arm-linux-androideabi-ar");
+        test.gcc().target(target).file("foo.c").compile("foo");
+        test.cmd(0).must_not_have("--target=arm-linux-androideabi");
+    }
+}