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

Set --host flag for autotools to support cross compilation #19

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

Conversation

skywhale
Copy link
Member

@skywhale skywhale commented Jun 5, 2020

https://docs.rs/autotools/0.2.1/autotools/struct.Config.html#method.host
It's set automatically, but based on the compilation host. We want the target architecture to be set for --host flag.

@skywhale skywhale requested a review from bschwind June 5, 2020 13:31
@bschwind
Copy link
Member

bschwind commented Jun 5, 2020

LGTM!

@goodhoko
Copy link
Member

goodhoko commented Dec 5, 2023

@skywhale @bschwind Sorry to dig this up after 3 years. Let's finish this. .) Seems like we could just merge? If not, let's close this.

@bschwind
Copy link
Member

bschwind commented Dec 5, 2023

We should probably test before merging though, by successfully cross-compiling this library with this branch.

@goodhoko
Copy link
Member

goodhoko commented Dec 5, 2023

I'll give it a try. I just need to make the regular compilation work first. (something something c++ errors out)

@goodhoko
Copy link
Member

goodhoko commented Dec 6, 2023

@bschwind Tested this by successfully compiling for x86 macOS on an arm macOS. You need to have have the target's rust toolchaing installed and you also need to set PKG_CONFIG_SYSROOT_DIR to this weird value (found out here)

I pushed an additional commit that documents this in the README.

PKG_CONFIG_SYSROOT_DIR=/ cargo build --target x86_64-apple-darwin 

// Remove the vendor component.
target_tuple.remove(1);

assert_eq!(3, target_tuple.len());
Copy link
Member

@bschwind bschwind Dec 6, 2023

Choose a reason for hiding this comment

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

I think this is an incorrect assumption, as some targets can have 3 components, some can have 4, etc.

When testing it's important to pass the --features "bundled" flag so the C++ code is built from source.

brian webrtc-audio-processing-sys $ PKG_CONFIG_SYSROOT_DIR=/ cargo build --features "bundled" --target aarch64-apple-ios
   Compiling webrtc-audio-processing-sys v0.3.0 (/Users/brian/projects/tonari/webrtc-audio-processing/webrtc-audio-processing-sys)
error: failed to run custom build command for `webrtc-audio-processing-sys v0.3.0 (/Users/brian/projects/tonari/webrtc-audio-processing/webrtc-audio-processing-sys)`

Caused by:
  process didn't exit successfully: `/Users/brian/projects/tonari/webrtc-audio-processing/webrtc-audio-processing-sys/target/debug/build/webrtc-audio-processing-sys-e044a990654280ee/build-script-build` (exit status: 101)
  --- stderr
  thread 'main' panicked at build.rs:72:9:
  assertion `left == right` failed
    left: 3
   right: 2
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Copy link
Member

@goodhoko goodhoko Dec 7, 2023

Choose a reason for hiding this comment

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

Good point!

When I use the bundled feature and a 4-component target I get a configure errors like:


  --- stderr
  configure: error: in `/Users/jentak/workspace/tonari/webrtc-audio-processing/target/aarch64-unknown-linux-musl/debug/build/webrtc-audio-processing-sys-7f94f1d16145798d/out/build':
  configure: error: C compiler cannot create executables

When I comment out https://github.com/tonarino/webrtc-audio-processing/pull/19/files#diff-4038f237c46940dc12666050b832d77271ddb0171c9bc64700009de2a982a403R106 (i.e. essentially ignore changes from this PR) and cross-compile for eg. x86_64-apple-darwin the build finishes successfully.

PKG_CONFIG_SYSROOT_DIR=/ cargo -v build --target x86_64-apple-darwin --features "bundled"

So I'm not sure if this PR is even needed? See also my comment below.

@goodhoko
Copy link
Member

goodhoko commented Dec 7, 2023

@skywhale

Do you remember why you wrote this?

We want the target architecture to be set for --host flag.

Yes, rust's "target" is "host" in autotools's nomenclature, but the autotools crate docs say:

The host and target methods on this package’s autotools::Config structure (as well as the $HOST and $TARGET variables set by cargo) are understood with their Rust meaning.

Anyway, I think I've lost too much time on this trying to debug opaque autoconf//gcc/cc errors. If anyone else won't pick this up let's close it as stale.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants