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.

Sign up for GitHub

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

Jump to bottom

Apple: Remove redundant flags #1256

Merged
merged 5 commits into from
Nov 2, 2024
Merged

Apple: Remove redundant flags #1256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4fc7f95
Refactor Apple flags
madsmtm Nov 2, 2024
544c494
Apple: Remove redundant architecture flags
madsmtm Nov 2, 2024
f86048a
Don't output version flag on Clang
madsmtm Nov 2, 2024
9185bab
Merge branch 'main' into refactor-apple-flags
madsmtm Nov 2, 2024
4f8fb27
Appease clippy
madsmtm Nov 2, 2024
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
162 changes: 25 additions & 137 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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"),
NobodyXu marked this conversation as resolved.
Show resolved Hide resolved
"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.
Expand Down Expand Up @@ -3719,7 +3642,9 @@ impl Build {
Ok(Arc::from(OsStr::new(sdk_path.trim())))
}

fn apple_sdk_root(&self, sdk: &str) -> Result<Arc<OsStr>, Error> {
fn apple_sdk_root(&self, target: &TargetInfo) -> Result<Arc<OsStr>, Error> {
let sdk = target.apple_sdk_name();

if let Some(ret) = self
.build_cache
.apple_sdk_root_cache
Expand Down Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions src/target/apple.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
}
24 changes: 13 additions & 11 deletions tests/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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.
Expand All @@ -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");
}
}
Expand Down Expand Up @@ -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::<u32>().unwrap());
Expand Down Expand Up @@ -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")]
Expand All @@ -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")]
Expand Down Expand Up @@ -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"
));
}
Expand All @@ -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")]
Expand Down