From 052a15c618c23a537b883d8e23f6d830b42439e9 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 3 Feb 2025 21:15:40 +0100 Subject: [PATCH 1/3] Don't specify both -target and -mtargetos= --- src/lib.rs | 37 ++++++++++++++++++++++++------------- tests/test.rs | 18 +++++++++++++----- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 328ac465..ab896c1c 100644 --- a/src/lib.rs +++ b/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 diff --git a/tests/test.rs b/tests/test.rs index 063a923f..231be41e 100644 --- a/tests/test.rs +++ b/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"); } From 661f12ec083804604faabb5697f28ed08958d6d8 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 3 Feb 2025 22:14:33 +0100 Subject: [PATCH 2/3] Fix tests for older Apple targets --- dev-tools/cc-test/build.rs | 11 +++++++++-- dev-tools/cc-test/src/armv7.S | 7 +++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/dev-tools/cc-test/build.rs b/dev-tools/cc-test/build.rs index 4adb870c..23c0a6f9 100644 --- a/dev-tools/cc-test/build.rs +++ b/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"); diff --git a/dev-tools/cc-test/src/armv7.S b/dev-tools/cc-test/src/armv7.S index 5c161c22..bc42479d 100644 --- a/dev-tools/cc-test/src/armv7.S +++ b/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 From ab175eafd7ea961787d5838092082c70c3194b5b Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 3 Feb 2025 22:33:52 +0100 Subject: [PATCH 3/3] Add more Apple targets to CI --- .github/workflows/main.yml | 90 ++++++++++++++++++++++++-------------- 1 file changed, 57 insertions(+), 33 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 4259216f..110551e4 100644 --- a/.github/workflows/main.yml +++ b/.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