Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 57 additions & 33 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -321,7 +345,7 @@ jobs:
name: Tests pass
needs:
- test
- check-tvos
- check-build-std
- check-wasm
- test-wasm32-wasip1-thread
- cuda
Expand Down
11 changes: 9 additions & 2 deletions dev-tools/cc-test/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
7 changes: 7 additions & 0 deletions dev-tools/cc-test/src/armv7.S
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
.globl asm
.balign 4
asm:
mov r0, #7
bx lr

.globl _asm
.balign 4
_asm:
mov r0, #7
bx lr
37 changes: 24 additions & 13 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 } => {
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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
Expand Down
18 changes: 13 additions & 5 deletions tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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");
}
Expand Down Expand Up @@ -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");
}

Expand All @@ -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(
Expand Down Expand Up @@ -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");
}

Expand All @@ -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");
}
Expand Down