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

Cross-compilation fixes for Apple M1/x86 #30

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Cross-compilation fixes for Apple M1/x86 #30

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
802077b
Fix unwrap on compiler names without -
kornelski Nov 22, 2021
d6facaa
Check for actual target OS
kornelski Nov 22, 2021
57c190e
Use the os from the target triple, rather than Rust's nickname
kornelski Nov 22, 2021
e9f53e4
Explicitly enable cross-compilation in ./configure
kornelski Nov 22, 2021
a2b4ca7
Set --target flag for Apple's clang when cross-compiling
kornelski Nov 22, 2021
9f75f57
Fix cross-compilation for Windows on Linux
chrox Jan 4, 2022
2c0ff86
Clippy lint
kornelski Jan 19, 2022
c6dc629
rustfmt
kornelski Feb 6, 2022
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
63 changes: 48 additions & 15 deletions build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ fn switch(configure: &mut Command, feature: &str, name: &str) {
configure.arg(arg.to_string() + name);
}

fn build() -> io::Result<()> {
fn build(target_os: &str) -> io::Result<()> {
let source_dir = source();

// Command's path is not relative to command's current_dir
Expand All @@ -200,25 +200,41 @@ fn build() -> io::Result<()> {

configure.arg(format!("--prefix={}", search().to_string_lossy()));

if env::var("TARGET").unwrap() != env::var("HOST").unwrap() {
let target = env::var("TARGET").unwrap();
let host = env::var("HOST").unwrap();
if target != host {
configure.arg("--enable-cross-compile");

// Rust targets are subtly different than naming scheme for compiler prefixes.
// The cc crate has the messy logic of guessing a working prefix,
// and this is a messy way of reusing that logic.
let cc = cc::Build::new();

// Apple-clang needs this, -arch is not enough.
let target_flag = format!("--target={}", target);
if cc.is_flag_supported(&target_flag).unwrap_or(false) {
configure.arg(format!("--extra-cflags={}", target_flag));
configure.arg(format!("--extra-ldflags={}", target_flag));
}

let compiler = cc.get_compiler();
let compiler = compiler.path().file_stem().unwrap().to_str().unwrap();
let suffix_pos = compiler.rfind('-').unwrap(); // cut off "-gcc"
let prefix = compiler[0..suffix_pos].trim_end_matches("-wr"); // "wr-c++" compiler
if let Some(suffix_pos) = compiler.rfind('-') {
// cut off "-gcc"
let prefix = compiler[0..suffix_pos].trim_end_matches("-wr"); // "wr-c++" compiler
configure.arg(format!("--cross-prefix={}-", prefix));
}

configure.arg(format!("--cross-prefix={}-", prefix));
configure.arg(format!(
"--arch={}",
env::var("CARGO_CFG_TARGET_ARCH").unwrap()
));
configure.arg(format!(
"--target_os={}",
env::var("CARGO_CFG_TARGET_OS").unwrap()
));
if target_os == "windows" {
// fix `configure: Unknown OS 'windows'`
configure.arg(format!("--target_os={}", "mingw32"));
} else {
configure.arg(format!("--target_os={}", target_os));
}
}

// control debug build
Expand Down Expand Up @@ -343,6 +359,8 @@ fn build() -> io::Result<()> {
// configure misc build options
enable!(configure, "BUILD_PIC", "pic");

println!("configure cmd: {:?}", configure);

// run ./configure
let output = configure
.output()
Expand Down Expand Up @@ -383,6 +401,19 @@ fn build() -> io::Result<()> {
Ok(())
}

fn os_from_triple(triple: &str) -> &str {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there really no way to use rust's target_os! or something more standard? I'm afraid this will bite us if other architectures start to pop up

Copy link
Contributor Author

@kornelski kornelski Feb 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Rust's target definitions are Rust's, and have no guarantee of compatibility with anything, e.g. Rust uses macos, but the rest of the world uses darwin.

So yes, it is already broken, and will keep breaking on every new OS.

Copy link
Collaborator

@Polochon-street Polochon-street Feb 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

would it be possible to have something that just checks if target_os! is macos, and replace it by darwin if it is? And maybe that does that for other OSes, if it happens for others? Maybe just a fn target_os() that wraps target_os! and does the changes or something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suspect it's needed for more OSes, but I don't have them to test. Whatever you do, it's going to be an incomplete list that may break on OSes that haven't been tested.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Quick question, but what's the value of CARGO_CFG_TARGET_OS on macos? Can't you just use a wrapper that gets CARGO_CFG_TARGET_OS and if it's macos, replace it by darwin?

Copy link
Contributor Author

@kornelski kornelski Feb 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

CARGO_CFG_TARGET_* variables are derived from rustc's cfg values, same as cfg!(target_*), so they use the same naming scheme.

I can hardcode the result to be correct only on darwin/macos, and leave it incorrect everywhere else, but why should I leave it incorrect everywhere else?

Copy link
Collaborator

Choose a reason for hiding this comment

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

my question is just, why do this triple splitting magic (and explode if it doesn't work) instead of just using CARGO_CFG_TARGET and use a hashmap or something like that to substitute the wrong stuff with whatever you know is right? Or maybe I'm missing the point here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Split doesn't work, because Rust's triples are inconsistently named, and sometimes have the "vendor" and sometimes they don't, so you don't know where the OS part starts.

HashMap would work for a closed set. I'm trying to make it work for an open set.

Copy link
Collaborator

Choose a reason for hiding this comment

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

my point was that maybe we could do a hashmap like {'macos': 'darwin'} and then just have a custom target_os function that would just return either the string itself, if it's not in the hashmap? Like

fn target_os(target: &str) -> &str {
    let mut os_replacements = HashMap::new();

    os_replacements.insert(
        "macos".to_string(),
         "darwin".to_string(),
     );
     os_replacements.get(&target).unwrap_or("target")
}

But I feel like we're talking about different things though, so I could be completely wrong 😄

https://github.com/rustsec/rustsec/blob/main/platforms/src/target/os.rs#L112-L134 seems to have a pretty consistent list of OS if we wanna list more stuff here?

let platform = triple.split_once('-').map(|x| x.1).expect("bad triple");
platform
.trim_start_matches("unknown-")
.trim_start_matches("pc-")
.trim_start_matches("wrs-")
.trim_start_matches("apple-")
.trim_start_matches("uwp-")
.split('-')
.next()
.unwrap()
}

#[cfg(not(target_env = "msvc"))]
fn try_vcpkg(_statik: bool) -> Option<Vec<PathBuf>> {
None
Expand Down Expand Up @@ -623,33 +654,35 @@ fn maybe_search_include(include_paths: &[PathBuf], header: &str) -> Option<Strin
}
}

fn link_to_libraries(statik: bool) {
fn link_to_libraries(statik: bool, target_os: &str) {
let ffmpeg_ty = if statik { "static" } else { "dylib" };
for lib in LIBRARIES {
let feat_is_enabled = lib.feature_name().and_then(|f| env::var(&f).ok()).is_some();
if !lib.is_feature || feat_is_enabled {
println!("cargo:rustc-link-lib={}={}", ffmpeg_ty, lib.name);
}
}
if env::var("CARGO_FEATURE_BUILD_ZLIB").is_ok() && cfg!(target_os = "linux") {
if env::var("CARGO_FEATURE_BUILD_ZLIB").is_ok() && target_os == "linux" {
println!("cargo:rustc-link-lib=z");
}
}

fn main() {
let statik = env::var("CARGO_FEATURE_STATIC").is_ok();
let ffmpeg_major_version: u32 = env!("CARGO_PKG_VERSION_MAJOR").parse().unwrap();
let target = env::var("TARGET").unwrap();
let target_os = os_from_triple(&target); // it's different than Rust's target_os! but ./configure likes these better

let include_paths: Vec<PathBuf> = if env::var("CARGO_FEATURE_BUILD").is_ok() {
println!(
"cargo:rustc-link-search=native={}",
search().join("lib").to_string_lossy()
);
link_to_libraries(statik);
link_to_libraries(statik, target_os);
if fs::metadata(&search().join("lib").join("libavutil.a")).is_err() {
fs::create_dir_all(&output()).expect("failed to create build directory");
fetch().unwrap();
build().unwrap();
build(target_os).unwrap();
}

// Check additional required libraries.
Expand Down Expand Up @@ -682,7 +715,7 @@ fn main() {
"cargo:rustc-link-search=native={}",
ffmpeg_dir.join("lib").to_string_lossy()
);
link_to_libraries(statik);
link_to_libraries(statik, target_os);
vec![ffmpeg_dir.join("include")]
} else if let Some(paths) = try_vcpkg(statik) {
// vcpkg doesn't detect the "system" dependencies
Expand Down Expand Up @@ -737,7 +770,7 @@ fn main() {
.include_paths
};

if statik && cfg!(target_os = "macos") {
if statik && target_os == "darwin" {
let frameworks = vec![
"AppKit",
"AudioToolbox",
Expand Down