From 1a9f345ff859db85b5c048942486b1459058bc72 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 2 Nov 2024 15:38:59 +0100 Subject: [PATCH] Apple: Remove redundant flags (#1256) * Refactor Apple flags Non-functional change. * Apple: Remove redundant architecture flags The architecture is specified for Clang with the --target option, and both -arch and -m32/-m64 are passed to GCC elsewhere (should probably be changed to -march, but that's a different issue). * Don't output version flag on Clang It is redundant, the version is already specified in `-target`. * Appease clippy --- src/lib.rs | 162 +++++++------------------------------------- src/target/apple.rs | 18 +++++ tests/test.rs | 24 ++++--- 3 files changed, 56 insertions(+), 148 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c2110822..b64bff58 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -325,8 +325,6 @@ pub struct Build { enum ErrorKind { /// Error occurred while performing I/O. IOError, - /// Invalid architecture supplied. - ArchitectureInvalid, /// Environment variable not found, with the var in question as extra info. EnvVarNotFound, /// Error occurred while using external tools (ie: invocation of compiler). @@ -2134,7 +2132,7 @@ impl Build { } } ToolFamily::Gnu => { - if target.os == "macos" { + 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()); @@ -2532,113 +2530,38 @@ impl Build { fn apple_flags(&self, cmd: &mut Tool) -> Result<(), Error> { let target = self.get_target()?; - let arch_str = target.full_arch; - let arch = if target.os == "macos" { - match arch_str { - "i686" => AppleArchSpec::Device("-m32"), - "x86_64" | "x86_64h" | "aarch64" | "arm64e" => AppleArchSpec::Device("-m64"), - _ => { - return Err(Error::new( - ErrorKind::ArchitectureInvalid, - "Unknown architecture for macOS target.", - )); - } - } - } else if target.abi == "macabi" { - match arch_str { - "arm64e" => AppleArchSpec::Catalyst("arm64e"), - "arm64" | "aarch64" => AppleArchSpec::Catalyst("arm64"), - "x86_64" | "x86_64h" => AppleArchSpec::Catalyst("-m64"), - _ => { - return Err(Error::new( - ErrorKind::ArchitectureInvalid, - "Unknown architecture for iOS target.", - )); - } - } - } else if target.abi == "sim" { - match arch_str { - "arm64" | "aarch64" => AppleArchSpec::Simulator("arm64"), - "i386" | "i686" => AppleArchSpec::Simulator("-m32"), - "x86_64" | "x86_64h" => AppleArchSpec::Simulator("-m64"), - _ => { - return Err(Error::new( - ErrorKind::ArchitectureInvalid, - "Unknown architecture for simulator target.", - )); - } - } - } else { - match arch_str { - "arm" | "armv7" | "thumbv7" => AppleArchSpec::Device("armv7"), - "armv7k" => AppleArchSpec::Device("armv7k"), - "armv7s" | "thumbv7s" => AppleArchSpec::Device("armv7s"), - "arm64e" => AppleArchSpec::Device("arm64e"), - "arm64" | "aarch64" => AppleArchSpec::Device("arm64"), - "arm64_32" => AppleArchSpec::Device("arm64_32"), - _ => { - return Err(Error::new( - ErrorKind::ArchitectureInvalid, - format!("Unknown architecture for {:?} target.", target.os), - )); - } - } - }; - - let sdk_details = apple_os_sdk_parts(target.os, &arch); - let min_version = self.apple_deployment_target(&target); - - match arch { - AppleArchSpec::Device(_) if target.os == "macos" => { - cmd.args - .push(format!("-mmacosx-version-min={}", min_version).into()); - } - AppleArchSpec::Device(arch) => { - cmd.args.push("-arch".into()); - cmd.args.push(arch.into()); - // `-mxros-version-min` does not exist - // https://github.com/llvm/llvm-project/issues/88271 - if target.os != "visionos" { - cmd.args.push( - format!("-m{}os-version-min={}", sdk_details.sdk_prefix, min_version) - .into(), - ); - } - } - AppleArchSpec::Simulator(arch) => { - if arch.starts_with('-') { - // -m32 or -m64 - cmd.args.push(arch.into()); - } else { - cmd.args.push("-arch".into()); - cmd.args.push(arch.into()); - } - if target.os != "visionos" { - cmd.args.push( - format!( - "-m{}simulator-version-min={}", - sdk_details.sim_prefix, min_version - ) - .into(), - ); - } - } - AppleArchSpec::Catalyst(_) => {} - }; + // If the compiler is Clang, then we've already specifed the target + // information (including the deployment target) with the `--target` + // option, so we don't need to do anything further here. + // + // If the compiler is GCC, then we need to specify + // `-mmacosx-version-min` to set the deployment target, as well + // as to say that the target OS is macOS. + // + // NOTE: GCC does not support `-miphoneos-version-min=` etc. (because + // it does not support iOS in general), but we specify them anyhow in + // case we actually have a Clang-like compiler disguised as a GNU-like + // compiler, or in case GCC adds support for these in the future. + if !cmd.is_like_clang() { + let min_version = self.apple_deployment_target(&target); + cmd.args + .push(target.apple_version_flag(&min_version).into()); + } // AppleClang sometimes requires sysroot even on macOS if cmd.is_xctoolchain_clang() || target.os != "macos" { self.cargo_output.print_metadata(&format_args!( "Detecting {:?} SDK path for {}", - target.os, sdk_details.sdk + target.os, + target.apple_sdk_name(), )); - let sdk_path = self.apple_sdk_root(&sdk_details.sdk)?; + let sdk_path = self.apple_sdk_root(&target)?; cmd.args.push("-isysroot".into()); cmd.args.push(OsStr::new(&sdk_path).to_owned()); - if let AppleArchSpec::Catalyst(_) = arch { + if target.abi == "macabi" { // Mac Catalyst uses the macOS SDK, but to compile against and // link to iOS-specific frameworks, we should have the support // library stubs in the include and library search path. @@ -3719,7 +3642,9 @@ impl Build { Ok(Arc::from(OsStr::new(sdk_path.trim()))) } - fn apple_sdk_root(&self, sdk: &str) -> Result, Error> { + fn apple_sdk_root(&self, target: &TargetInfo) -> Result, Error> { + let sdk = target.apple_sdk_name(); + if let Some(ret) = self .build_cache .apple_sdk_root_cache @@ -3974,43 +3899,6 @@ fn fail(s: &str) -> ! { std::process::exit(1); } -struct AppleSdkTargetParts { - sdk_prefix: &'static str, - sim_prefix: &'static str, - sdk: Cow<'static, str>, -} - -fn apple_os_sdk_parts(os: &str, arch: &AppleArchSpec) -> AppleSdkTargetParts { - let (sdk_prefix, sim_prefix) = match os { - "macos" => ("macosx", ""), - "ios" => ("iphone", "ios-"), - "watchos" => ("watch", "watch"), - "tvos" => ("appletv", "appletv"), - "visionos" => ("xr", "xr"), - os => unreachable!("unknown Apple OS {}", os), - }; - let sdk = match arch { - AppleArchSpec::Device(_) if os == "macos" => Cow::Borrowed("macosx"), - AppleArchSpec::Device(_) => format!("{}os", sdk_prefix).into(), - AppleArchSpec::Simulator(_) => format!("{}simulator", sdk_prefix).into(), - AppleArchSpec::Catalyst(_) => Cow::Borrowed("macosx"), - }; - - AppleSdkTargetParts { - sdk_prefix, - sim_prefix, - sdk, - } -} - -#[allow(dead_code)] -enum AppleArchSpec { - Device(&'static str), - Simulator(&'static str), - #[allow(dead_code)] - Catalyst(&'static str), -} - // Use by default minimum available API level // See note about naming here // https://android.googlesource.com/platform/ndk/+/refs/heads/ndk-release-r21/docs/BuildSystemMaintainers.md#Clang diff --git a/src/target/apple.rs b/src/target/apple.rs index 7869aaaf..6eedcd89 100644 --- a/src/target/apple.rs +++ b/src/target/apple.rs @@ -16,4 +16,22 @@ impl TargetInfo<'_> { (os, _) => panic!("invalid Apple target OS {}", os), } } + + pub(crate) fn apple_version_flag(&self, min_version: &str) -> String { + match (self.os, self.abi) { + ("macos", "") => format!("-mmacosx-version-min={min_version}"), + ("ios", "") => format!("-miphoneos-version-min={min_version}"), + ("ios", "sim") => format!("-mios-simulator-version-min={min_version}"), + ("ios", "macabi") => format!("-mtargetos=ios{min_version}-macabi"), + ("tvos", "") => format!("-mappletvos-version-min={min_version}"), + ("tvos", "sim") => format!("-mappletvsimulator-version-min={min_version}"), + ("watchos", "") => format!("-mwatchos-version-min={min_version}"), + ("watchos", "sim") => format!("-mwatchsimulator-version-min={min_version}"), + // `-mxros-version-min` does not exist + // https://github.com/llvm/llvm-project/issues/88271 + ("visionos", "") => format!("-mtargetos=xros{min_version}"), + ("visionos", "sim") => format!("-mtargetos=xros{min_version}-simulator"), + (os, _) => panic!("invalid Apple target OS {}", os), + } + } } diff --git a/tests/test.rs b/tests/test.rs index 939e56e3..14e72ec4 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -274,8 +274,8 @@ fn gnu_aarch64_none_no_pic() { for target in &["aarch64-unknown-none-softfloat", "aarch64-unknown-none"] { let test = Test::gnu(); test.gcc() - .target(&target) - .host(&target) + .target(target) + .host(target) .file("foo.c") .compile("foo"); @@ -511,7 +511,9 @@ fn gnu_apple_darwin() { for (arch, version) in &[("x86_64", "10.7"), ("aarch64", "11.0")] { let target = format!("{}-apple-darwin", arch); let test = Test::gnu(); - test.gcc() + test.shim("fake-gcc") + .gcc() + .compiler("fake-gcc") .target(&target) .host(&target) // Avoid test maintenance when minimum supported OSes change. @@ -520,8 +522,7 @@ fn gnu_apple_darwin() { .compile("foo"); let cmd = test.cmd(0); - test.cmd(0) - .must_have(format!("-mmacosx-version-min={}", version)); + cmd.must_have(format!("-mmacosx-version-min={version}")); cmd.must_not_have("-isysroot"); } } @@ -553,7 +554,7 @@ fn macos_cpp_minimums() { let deployment_arg = exec .args .iter() - .find_map(|arg| arg.strip_prefix("-mmacosx-version-min=")) + .find_map(|arg| arg.strip_prefix("--target=x86_64-apple-macosx")) .expect("no deployment target argument was set"); let mut deployment_parts = deployment_arg.split('.').map(|v| v.parse::().unwrap()); @@ -581,7 +582,7 @@ fn macos_cpp_minimums() { .compile("foo"); // No C++ leaves it untouched - test.cmd(0).must_have("-mmacosx-version-min=10.7"); + test.cmd(0).must_have("--target=x86_64-apple-macosx10.7"); } #[cfg(target_os = "macos")] @@ -596,7 +597,7 @@ fn clang_apple_tvos() { .file("foo.c") .compile("foo"); - test.cmd(0).must_have("-mappletvos-version-min=9.0"); + test.cmd(0).must_have("--target=arm64-apple-tvos9.0"); } #[cfg(target_os = "macos")] @@ -629,8 +630,8 @@ fn clang_apple_mac_catalyst() { "-iframework", &format!("{sdkroot}/System/iOSSupport/System/Library/Frameworks"), ); - execution.must_have(&format!("-L{sdkroot}/System/iOSSupport/usr/lib")); - execution.must_have(&format!( + execution.must_have(format!("-L{sdkroot}/System/iOSSupport/usr/lib")); + execution.must_have(format!( "-F{sdkroot}/System/iOSSupport/System/Library/Frameworks" )); } @@ -648,7 +649,8 @@ fn clang_apple_tvsimulator() { .file("foo.c") .compile("foo"); - test.cmd(0).must_have("-mappletvsimulator-version-min=9.0"); + test.cmd(0) + .must_have("--target=x86_64-apple-tvos9.0-simulator"); } #[cfg(target_os = "macos")]