Don't specify both -target and -mtargetos= on Apple targets (#1384)

Author: madsmtm

.github/workflows/main.yml

@@ -22,9 +22,13 @@ jobs:
             beta,
             nightly,
             linux32,
-            macos,
             aarch64-macos,
+            x86_64-macos,
             aarch64-ios,
+            aarch64-ios-sim,
+            x86_64-ios-sim,
+            aarch64-ios-macabi,
+            x86_64-ios-macabi,
             win32,
             win64,
             mingw32,
@@ -49,19 +53,39 @@ jobs:
             os: ubuntu-latest
             rust: stable
             target: i686-unknown-linux-gnu
-          - build: macos
-            os: macos-latest
-            rust: stable
-            target: x86_64-apple-darwin
           - build: aarch64-macos
             os: macos-14
             rust: stable
             target: aarch64-apple-darwin
+          - build: x86_64-macos
+            os: macos-13 # x86
+            rust: stable
+            target: x86_64-apple-darwin
           - build: aarch64-ios
             os: macos-latest
             rust: stable
             target: aarch64-apple-ios
             no_run: --no-run
+          - build: aarch64-ios-sim
+            os: macos-latest
+            rust: stable
+            target: aarch64-apple-ios-sim
+            no_run: --no-run
+          - build: x86_64-ios-sim
+            os: macos-13 # x86
+            rust: stable
+            target: x86_64-apple-ios # Simulator
+            no_run: --no-run
+          - build: aarch64-ios-macabi
+            os: macos-latest
+            rust: stable
+            target: aarch64-apple-ios-macabi
+            no_run: --no-run # FIXME(madsmtm): Fix running tests
+          - build: x86_64-ios-macabi
+            os: macos-13 # x86
+            rust: stable
+            target: x86_64-apple-ios-macabi
+            no_run: --no-run # FIXME(madsmtm): Fix running tests
           - build: windows-aarch64
             os: windows-latest
             rust: stable
@@ -139,42 +163,42 @@ jobs:
       - run: cargo test ${{ matrix.no_run }} --workspace --target ${{ matrix.target }} ${{ matrix.cargo_flags }}
 
   # This is separate from the matrix above because there is no prebuilt rust-std component for these targets.
-  check-tvos:
+  check-build-std:
     name: Test build-std
-    runs-on: ${{ matrix.os }}
+    runs-on: macos-latest
     strategy:
       matrix:
-        build: [aarch64-tvos, aarch64-tvos-sim, x86_64-tvos]
-        include:
-          - build: aarch64-tvos
-            os: macos-latest
-            rust: nightly
-            target: aarch64-apple-tvos
-            no_run: --no-run
-          - build: aarch64-tvos-sim
-            os: macos-latest
-            rust: nightly
-            target: aarch64-apple-tvos-sim
-            no_run: --no-run
-          - build: x86_64-tvos
-            os: macos-latest
-            rust: nightly
-            target: x86_64-apple-tvos
-            no_run: --no-run
+        target:
+          - x86_64h-apple-darwin
+          # FIXME(madsmtm): needs deployment target
+          # - armv7s-apple-ios
+          # FIXME(madsmtm): needs deployment target
+          # - i386-apple-ios # Simulator
+          - aarch64-apple-tvos
+          - aarch64-apple-tvos-sim
+          - x86_64-apple-tvos # Simulator
+          - aarch64-apple-watchos
+          - aarch64-apple-watchos-sim
+          - x86_64-apple-watchos-sim
+          # FIXME(madsmtm): needs deployment target
+          # - arm64_32-apple-watchos
+          - armv7k-apple-watchos
+          - aarch64-apple-visionos
+          - aarch64-apple-visionos-sim
     steps:
       - uses: actions/checkout@v4
       - name: Install Rust (rustup)
         run: |
           set -euxo pipefail
-          rustup toolchain install ${{ matrix.rust }} --no-self-update --profile minimal
-          rustup component add rust-src --toolchain ${{ matrix.rust }}
-          rustup default ${{ matrix.rust }}
+          rustup toolchain install nightly --no-self-update --profile minimal
+          rustup component add rust-src --toolchain nightly
+          rustup default nightly
         shell: bash
       - run: cargo update
       - uses: Swatinem/rust-cache@v2
-      - run: cargo test -Z build-std=std ${{ matrix.no_run }} --workspace --target ${{ matrix.target }}
-      - run: cargo test -Z build-std=std ${{ matrix.no_run }} --workspace --target ${{ matrix.target }} --release
-      - run: cargo test -Z build-std=std ${{ matrix.no_run }} --workspace --target ${{ matrix.target }} --features parallel
+      - run: cargo test -Z build-std=std --no-run --workspace --target ${{ matrix.target }}
+      - run: cargo test -Z build-std=std --no-run --workspace --target ${{ matrix.target }} --release
+      - run: cargo test -Z build-std=std --no-run --workspace --target ${{ matrix.target }} --features parallel
 
   check-wasm:
     name: Test wasm
@@ -188,7 +212,7 @@ jobs:
         run: |
           rustup target add ${{ matrix.target }}
         shell: bash
-      - run: cargo update 
+      - run: cargo update
       - uses: Swatinem/rust-cache@v2
       - run: cargo test --no-run --target ${{ matrix.target }}
       - run: cargo test --no-run --target ${{ matrix.target }} --release
@@ -251,7 +275,7 @@ jobs:
           sudo dpkg -i cuda-keyring_1.0-1_all.deb
           sudo apt-get update
           sudo apt-get -y install cuda-minimal-build-11-8
-      - run: cargo update 
+      - run: cargo update
       - uses: Swatinem/rust-cache@v2
       - name: Test 'cudart' feature
         shell: bash
@@ -321,7 +345,7 @@ jobs:
     name: Tests pass
     needs:
       - test
-      - check-tvos
+      - check-build-std
       - check-wasm
       - test-wasm32-wasip1-thread
       - cuda

dev-tools/cc-test/build.rs

@@ -50,10 +50,17 @@ fn main() {
         .compile("bar");
 
     let target = std::env::var("TARGET").unwrap();
-    let file = target.split('-').next().unwrap();
+    let arch = match target.split('-').next().unwrap() {
+        "arm64_32" => "aarch64",
+        "armv7k" => "armv7",
+        "armv7s" => "armv7",
+        "i386" => "i686",
+        "x86_64h" => "x86_64",
+        arch => arch,
+    };
     let file = format!(
         "src/{}.{}",
-        file,
+        arch,
         if target.contains("msvc") { "asm" } else { "S" }
     );
     cc::Build::new().file(file).compile("asm");

dev-tools/cc-test/src/armv7.S

@@ -1,4 +1,11 @@
 .globl asm
+.balign 4
 asm:
     mov r0, #7
     bx lr
+
+.globl _asm
+.balign 4
+_asm:
+    mov r0, #7
+    bx lr

src/lib.rs

@@ -2188,14 +2188,23 @@ impl Build {
                         }
                     }
 
-                    // Pass `--target` with the LLVM target to properly configure Clang even when
-                    // cross-compiling.
+                    // Pass `--target` with the LLVM target to configure Clang for cross-compiling.
                     //
-                    // We intentionally don't put the deployment version in here on Apple targets,
-                    // and instead pass that via `-mmacosx-version-min=` and similar flags, for
-                    // better compatibility with older versions of Clang that has poor support for
-                    // versioned target names (especially when it comes to configuration files).
-                    cmd.push_cc_arg(format!("--target={}", target.llvm_target).into());
+                    // NOTE: In the past, we passed this, along with the deployment version in here
+                    // on Apple targets, but versioned targets were found to have poor compatibility
+                    // with older versions of Clang, especially around comes to configuration files.
+                    //
+                    // Instead, we specify `-arch` along with `-mmacosx-version-min=`, `-mtargetos=`
+                    // and similar flags in `.apple_flags()`.
+                    //
+                    // Note that Clang errors when both `-mtargetos=` and `-target` are specified,
+                    // so we omit this entirely on Apple targets (it's redundant when specifying
+                    // both the `-arch` and the deployment target / OS flag) (in theory we _could_
+                    // specify this on some of the Apple targets that use the older
+                    // `-m*-version-min=`, but for consistency we omit it entirely).
+                    if target.vendor != "apple" {
+                        cmd.push_cc_arg(format!("--target={}", target.llvm_target).into());
+                    }
                 }
             }
             ToolFamily::Msvc { clang_cl } => {
@@ -2233,12 +2242,6 @@ impl Build {
                 }
             }
             ToolFamily::Gnu => {
-                if target.vendor == "apple" {
-                    let arch = map_darwin_target_from_rust_to_compiler_architecture(target);
-                    cmd.args.push("-arch".into());
-                    cmd.args.push(arch.into());
-                }
-
                 if target.vendor == "kmc" {
                     cmd.args.push("-finput-charset=utf-8".into());
                 }
@@ -2641,6 +2644,14 @@ impl Build {
     fn apple_flags(&self, cmd: &mut Tool) -> Result<(), Error> {
         let target = self.get_target()?;
 
+        // Add `-arch` on all compilers. This is a Darwin/Apple-specific flag
+        // that works both on GCC and Clang.
+        // https://gcc.gnu.org/onlinedocs/gcc/Darwin-Options.html#:~:text=arch
+        // https://clang.llvm.org/docs/CommandGuide/clang.html#cmdoption-arch
+        let arch = map_darwin_target_from_rust_to_compiler_architecture(&target);
+        cmd.args.push("-arch".into());
+        cmd.args.push(arch.into());
+
         // Pass the deployment target via `-mmacosx-version-min=`, `-mtargetos=` and similar.
         //
         // It is also necessary on GCC, as it forces a compilation error if the compiler is not

tests/test.rs

@@ -525,7 +525,8 @@ fn asm_flags() {
 
 #[test]
 fn gnu_apple_darwin() {
-    for (arch, version) in &[("x86_64", "10.7"), ("aarch64", "11.0")] {
+    for (arch, ld64_arch, version) in &[("x86_64", "x86_64", "10.7"), ("aarch64", "arm64", "11.0")]
+    {
         let target = format!("{}-apple-darwin", arch);
         let test = Test::gnu();
         test.shim("fake-gcc")
@@ -539,6 +540,7 @@ fn gnu_apple_darwin() {
             .compile("foo");
 
         let cmd = test.cmd(0);
+        cmd.must_have_in_order("-arch", ld64_arch);
         cmd.must_have(format!("-mmacosx-version-min={version}"));
         cmd.must_not_have("-isysroot");
     }
@@ -614,6 +616,7 @@ fn clang_apple_tvos() {
         .file("foo.c")
         .compile("foo");
 
+    test.cmd(0).must_have_in_order("-arch", "arm64");
     test.cmd(0).must_have("-mappletvos-version-min=9.0");
 }
 
@@ -637,7 +640,9 @@ fn clang_apple_mac_catalyst() {
         .compile("foo");
     let execution = test.cmd(0);
 
-    execution.must_have("--target=arm64-apple-ios-macabi");
+    execution.must_have_in_order("-arch", "arm64");
+    // --target and -mtargetos= don't mix
+    execution.must_not_have("--target=arm64-apple-ios-macabi");
     execution.must_have("-mtargetos=ios15.0-macabi");
     execution.must_have_in_order("-isysroot", sdkroot);
     execution.must_have_in_order(
@@ -667,8 +672,7 @@ fn clang_apple_tvsimulator() {
         .file("foo.c")
         .compile("foo");
 
-    test.cmd(0)
-        .must_have("--target=x86_64-apple-tvos-simulator");
+    test.cmd(0).must_have_in_order("-arch", "x86_64");
     test.cmd(0).must_have("-mappletvsimulator-version-min=9.0");
 }
 
@@ -693,8 +697,12 @@ fn clang_apple_visionos() {
 
     dbg!(test.cmd(0).args);
 
-    test.cmd(0).must_have("--target=arm64-apple-xros");
+    test.cmd(0).must_have_in_order("-arch", "arm64");
+    // --target and -mtargetos= don't mix.
+    test.cmd(0).must_not_have("--target=arm64-apple-xros");
     test.cmd(0).must_have("-mtargetos=xros1.0");
+
+    // Flags that don't exist.
     test.cmd(0).must_not_have("-mxros-version-min=1.0");
     test.cmd(0).must_not_have("-mxrsimulator-version-min=1.0");
 }