Skip to content

Fix cross compilation for non-macOS apple targets #244

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
99 changes: 66 additions & 33 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,7 @@ impl Config {
"release" | "bench" => RustProfile::Release,
unknown => {
eprintln!(
"Warning: unknown Rust profile={}; defaulting to a release build.",
unknown
"Warning: unknown Rust profile={unknown}; defaulting to a release build."
);
RustProfile::Release
}
Expand All @@ -153,8 +152,7 @@ impl Config {
RustProfile::Release => OptLevel::Release,
};
eprintln!(
"Warning: unknown opt-level={}; defaulting to a {:?} build.",
unknown, default_opt_level
"Warning: unknown opt-level={unknown}; defaulting to a {default_opt_level:?} build."
);
default_opt_level
}
Expand All @@ -164,7 +162,7 @@ impl Config {
"false" => false,
"true" => true,
unknown => {
eprintln!("Warning: unknown debug={}; defaulting to `true`.", unknown);
eprintln!("Warning: unknown debug={unknown}; defaulting to `true`.");
true
}
};
Expand Down Expand Up @@ -562,7 +560,7 @@ impl Config {
let mut cmake_prefix_path = Vec::new();
for dep in &self.deps {
let dep = dep.to_uppercase().replace('-', "_");
if let Some(root) = env::var_os(format!("DEP_{}_ROOT", dep)) {
if let Some(root) = env::var_os(format!("DEP_{dep}_ROOT")) {
cmake_prefix_path.push(PathBuf::from(root));
}
}
Expand Down Expand Up @@ -667,16 +665,54 @@ impl Config {
}
cmd.arg("-AWin32");
} else {
panic!("unsupported msvc target: {}", target);
panic!("unsupported msvc target: {target}");
}
}
} else if target.contains("darwin") && !self.defined("CMAKE_OSX_ARCHITECTURES") {
if target.contains("x86_64") {
cmd.arg("-DCMAKE_OSX_ARCHITECTURES=x86_64");
} else if target.contains("aarch64") {
cmd.arg("-DCMAKE_OSX_ARCHITECTURES=arm64");
} else {
panic!("unsupported darwin target: {}", target);
} else if target.contains("apple") {
if !self.defined("CMAKE_OSX_ARCHITECTURES") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Link to the docs. Same for CMAKE_OSX_SYSROOT.

Suggested change
if !self.defined("CMAKE_OSX_ARCHITECTURES") {
// <https://cmake.org/cmake/help/latest/variable/CMAKE_OSX_ARCHITECTURES.html>
if !self.defined("CMAKE_OSX_ARCHITECTURES") {

if target.contains("x86_64") {
cmd.arg("-DCMAKE_OSX_ARCHITECTURES=x86_64");
} else if target.contains("aarch64") {
cmd.arg("-DCMAKE_OSX_ARCHITECTURES=arm64");
} else if target.contains("i686") {
cmd.arg("-DCMAKE_OSX_ARCHITECTURES=i686");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ld64 prefers i386, so we should probably pass that here as well.

Suggested change
cmd.arg("-DCMAKE_OSX_ARCHITECTURES=i686");
cmd.arg("-DCMAKE_OSX_ARCHITECTURES=i386");

} else if target.contains("arm64e") {
cmd.arg("-DCMAKE_OSX_ARCHITECTURES=arm64e");
} else if target.contains("i386") {
cmd.arg("-DCMAKE_OSX_ARCHITECTURES=i386");
} else if target.contains("armv7s") {
cmd.arg("-DCMAKE_OSX_ARCHITECTURES=armv7s");
} else if target.contains("armv7k") {
cmd.arg("-DCMAKE_OSX_ARCHITECTURES=armv7k");
} else if target.contains("arm64_32") {
cmd.arg("-DCMAKE_OSX_ARCHITECTURES=arm64_32");
} else {
panic!("unsupported apple target: {target}");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Maybe we can make this simpler, and remove the panic? See bootstrap's implementation:
https://github.com/rust-lang/rust/blob/718ddf660e6a1802c39b4962cf7eaa4db57025ef/src/bootstrap/src/core/build_steps/llvm.rs#L698-L706

}
}

if !self.defined("CMAKE_OSX_SYSROOT") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the docs for this, it seems to me that we can (and should) use SDKROOT instead (to allow the user to override with that env var set). And cc-rs already contains the logic to invoke xcrun at the appropriate times.

So maybe the solution here is to instead make cc-rs set the SDKROOT env var here instead of passing -isysroot? That would automatically set SDKROOT for CMake invocations as well.

if target.contains("macabi") {
cmd.arg("-DCMAKE_OSX_SYSROOT=macosx");
} else if target.contains("sim") {
if target.contains("ios") {
cmd.arg("-DCMAKE_OSX_SYSROOT=iphonesimulator");
} else if target.contains("tvos") {
cmd.arg("-DCMAKE_OSX_SYSROOT=appletvsimulator");
} else if target.contains("visionos") {
cmd.arg("-DCMAKE_OSX_SYSROOT=xrsimulator");
} else if target.contains("watchos") {
cmd.arg("-DCMAKE_OSX_SYSROOT=watchsimulator");
}
} else if target.contains("ios") {
cmd.arg("-DCMAKE_OSX_SYSROOT=iphoneos");
} else if target.contains("tvos") {
cmd.arg("-DCMAKE_OSX_SYSROOT=appletvos");
} else if target.contains("visionos") {
cmd.arg("-DCMAKE_OSX_SYSROOT=xros");
} else if target.contains("watchos") {
cmd.arg("-DCMAKE_OSX_SYSROOT=watchos");
}
}
}
if let Some(ref generator) = generator {
Expand Down Expand Up @@ -718,8 +754,8 @@ impl Config {
None => false,
};
let mut set_compiler = |kind: &str, compiler: &cc::Tool, extra: &OsString| {
let flag_var = format!("CMAKE_{}_FLAGS", kind);
let tool_var = format!("CMAKE_{}_COMPILER", kind);
let flag_var = format!("CMAKE_{kind}_FLAGS");
let tool_var = format!("CMAKE_{kind}_COMPILER");
if !self.defined(&flag_var) {
let mut flagsflag = OsString::from("-D");
flagsflag.push(&flag_var);
Expand All @@ -743,7 +779,7 @@ impl Config {
// Note that for other generators, though, this *overrides*
// things like the optimization flags, which is bad.
if generator.is_none() && msvc {
let flag_var_alt = format!("CMAKE_{}_FLAGS_{}", kind, build_type_upcase);
let flag_var_alt = format!("CMAKE_{kind}_FLAGS_{build_type_upcase}");
if !self.defined(&flag_var_alt) {
let mut flagsflag = OsString::from("-D");
flagsflag.push(&flag_var_alt);
Expand Down Expand Up @@ -805,7 +841,7 @@ impl Config {
}

if !self.defined("CMAKE_BUILD_TYPE") {
cmd.arg(format!("-DCMAKE_BUILD_TYPE={}", profile));
cmd.arg(format!("-DCMAKE_BUILD_TYPE={profile}"));
}

if self.verbose_make {
Expand Down Expand Up @@ -930,7 +966,7 @@ impl Config {
return val.clone();
}
let r = env::var_os(v);
println!("{} = {:?}", v, r);
println!("{v} = {r:?}");
self.env_cache.insert(v.to_string(), r.clone());
r
}
Expand All @@ -945,9 +981,9 @@ impl Config {

let kind = if host == target { "HOST" } else { "TARGET" };
let target_u = target.replace('-', "_");
self.getenv_os(&format!("{}_{}", var_base, target))
.or_else(|| self.getenv_os(&format!("{}_{}", var_base, target_u)))
.or_else(|| self.getenv_os(&format!("{}_{}", kind, var_base)))
self.getenv_os(&format!("{var_base}_{target}"))
.or_else(|| self.getenv_os(&format!("{var_base}_{target_u}")))
.or_else(|| self.getenv_os(&format!("{kind}_{var_base}")))
.or_else(|| self.getenv_os(var_base))
}

Expand Down Expand Up @@ -975,7 +1011,7 @@ impl Config {
{
base.to_string()
} else {
panic!("unsupported msvc target: {}", target);
panic!("unsupported msvc target: {target}");
}
}

Expand Down Expand Up @@ -1076,27 +1112,24 @@ impl Default for Version {
}

fn run(cmd: &mut Command, program: &str) {
println!("running: {:?}", cmd);
println!("running: {cmd:?}");
let status = match cmd.status() {
Ok(status) => status,
Err(ref e) if e.kind() == ErrorKind::NotFound => {
fail(&format!(
"failed to execute command: {}\nis `{}` not installed?",
e, program
"failed to execute command: {e}\nis `{program}` not installed?"
));
}
Err(e) => fail(&format!("failed to execute command: {}", e)),
Err(e) => fail(&format!("failed to execute command: {e}")),
};
if !status.success() {
if status.code() == Some(127) {
fail(&format!(
"command did not execute successfully, got: {}, is `{}` not installed?",
status, program
"command did not execute successfully, got: {status}, is `{program}` not installed?"
));
}
fail(&format!(
"command did not execute successfully, got: {}",
status
"command did not execute successfully, got: {status}"
));
}
}
Expand All @@ -1111,12 +1144,12 @@ fn find_exe(path: &Path) -> PathBuf {
fn getenv_unwrap(v: &str) -> String {
match env::var(v) {
Ok(s) => s,
Err(..) => fail(&format!("environment variable `{}` not defined", v)),
Err(..) => fail(&format!("environment variable `{v}` not defined")),
}
}

fn fail(s: &str) -> ! {
panic!("\n{}\n\nbuild script failed, must exit now", s)
panic!("\n{s}\n\nbuild script failed, must exit now")
}

/// Returns whether the given MAKEFLAGS indicate that there is an available
Expand Down
Loading