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

Fixes iOS compiling. #33

Merged
merged 19 commits into from
Oct 4, 2020
Merged

Conversation

simlay
Copy link
Member

@simlay simlay commented Dec 31, 2019

It's unclear if this finishes #29 but this deals with a pretty annoying edge case when building foraarch64-apple-ios as described here: rust-lang/rust-bindgen#1211.

Also, you might notice that rustup target list | grep ios yields something like:

aarch64-apple-ios (installed)
armv7-apple-ios (installed)
armv7s-apple-ios (installed)
i386-apple-ios (installed)
x86_64-apple-ios (installed)

The i386 and x86_64 targets don't have architecture support in the iPhoneOS13.2.sdk it seems. I can share the stack trace if you'd like. For now compiling for aarch64, armv7 and armv7s I think is more than enough.

.travis.yml Outdated Show resolved Hide resolved
build.rs Outdated Show resolved Hide resolved
@simlay
Copy link
Member Author

simlay commented Jan 4, 2020

I've updated this PR to use the objective-c feature found in rust-lang/rust-bindgen#1702. We should wait for that to land and be pushed to crates.io to actually merge.

build.rs Show resolved Hide resolved
@catt-io
Copy link

catt-io commented Jan 17, 2020

Hi @simlay, cheers for looking into iOS support. When I try to build this for aarch64-apple-ios I get the following error:

error: failed to run custom build command for `coreaudio-sys v0.2.3 (https://github.com/simlay/coreaudio-sys.git?branch=add-ios-support#be5a43d2)`

Caused by:
  process didn't exit successfully: `.../target/release/build/coreaudio-sys-abd2e647f3203db2/build-script-build` (signal: 6, SIGABRT: process abort signal)
--- stderr
dyld: Library not loaded: @rpath/libclang.dylib
  Referenced from: .../target/release/build/coreaudio-sys-abd2e647f3203db2/build-script-build
  Reason: image not found

Xcode version is 11.3 and xcrun output is:

xcrun --sdk iphoneos --show-sdk-path
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS13.2.sdk

Same error with x86_64-apple-darwin, which the master branch is building fine for.

@simlay
Copy link
Member Author

simlay commented Jan 17, 2020

Hi @simlay, cheers for looking into iOS support. When I try to build this for aarch64-apple-ios I get the following error:
Hmm... That looks like an issue with llvm/clang. Maybe brew reinstall llvm?

@mitchmindtree
Copy link
Member

Thanks so much for your work on iOS support @simlay!

Unfortunately I don't have the means to test macOS and iOS at the moment. If we can get travis building successfully and a tick of approval from another macOS/iOS user I'd be happy to merge!

Also, would you be interested in being added as a collaborator at both the coreaudio-rs and coreaudio-sys crates? I realise it can be tricky finding iOS testers/reviewers and prefer you weren't endlessly blocked by this!

@simlay
Copy link
Member Author

simlay commented Jan 23, 2020

I’ll fix the build. This PR is dependent on a new release of rust-bindgen to be published on crates.io as more objective-c support has been added.

@slmjkdbtl
Copy link

Is there any update on this? @simlay 's fixes works and I wonder when can this get merged?

@simlay
Copy link
Member Author

simlay commented Aug 6, 2020

Is there any update on this? @simlay 's fixes works and I wonder when can this get merged?

So, I've added a ton to rust-bindgen in the last 8 months that makes the sys bindings much better but would also be breaking to whatever uses it to generate. If you're familiar with UIKit, I've got a WIP bindings crate for uikit-sys.

I'd like to wait until we have rust-lang/rust-bindgen#1847 and rust-lang/rust-bindgen#1784 as those kinda complete the objective-c binding generation from what I can tell.

@slmjkdbtl
Copy link

@simlay I see. Thanks a lot for your work on those iOS related subjects!

@simlay
Copy link
Member Author

simlay commented Sep 19, 2020

I've tested this feature branch out again with Xcode 12 (using the latest iOS SDK) on macOS 10.15.6 and for some unclear reason, I get the following stacktrace when compiling for aarch64-apple-ios:

~/projects/coreaudio-sys (add-ios-support)$ cargo build --target aarch64-apple-ios                                                                                                                                                                          
   Compiling coreaudio-sys v0.2.3 (/Users/simlay/projects/coreaudio-sys)
error[E0204]: the trait `Copy` may not be implemented for this type
     --> /Users/simlay/projects/coreaudio-sys/target/aarch64-apple-ios/debug/build/coreaudio-sys-b95864b357b6f856/out/coreaudio.rs:31119:17
      |
31119 | #[derive(Debug, Copy, Clone)]
      |                 ^^^^
31120 | pub struct AudioUnitRenderContext {
31121 |     pub workgroup: os_workgroup_t,
      |     ----------------------------- this field does not implement `Copy`
      |
      = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

For more information about this error, try `rustc --explain E0204`.
error: could not compile `coreaudio-sys`.

To learn more, run the command again with --verbose.

I don't have this issue when targeting x86_64-apple-ios which is pretty annoying.

@MichaelHills
Copy link
Contributor

@simlay here's the patch of this PR I used when I got audio working on bevy -> rodio -> cpal. I rebased on master, which was 8d27848335dea18129dffe60643572497cc42d6f, and then disabled the objective C stuff. So given that this works, maybe we can push to land this as a first pass?

+        //builder = builder.clang_args(&["-x", "objective-c", "-fblocks"]);
+        builder = builder.objc_extern_crate(false);
+        builder = builder.generate_block(false);
+        builder = builder.block_extern_crate(false);

Full diff

diff --git a/Cargo.toml b/Cargo.toml
index cac8dc3..55779a5 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -10,8 +10,12 @@ homepage = "https://github.com/RustAudio/coreaudio-sys"
 repository = "https://github.com/RustAudio/coreaudio-sys.git"
 build = "build.rs"
 
+[target.'cfg(target_os = "ios")'.dependencies]
+objc = "0.2.7"
+block = "0.1.6"
+
 [build-dependencies.bindgen]
-version = "0.53"
+version = "0.55"
 default-features = false
 features = ["runtime"]
 
diff --git a/build.rs b/build.rs
index 3a7a80e..e1494c8 100644
--- a/build.rs
+++ b/build.rs
@@ -11,7 +11,12 @@ fn sdk_path(target: &str) -> Result<String, std::io::Error> {
 
     let sdk = if target.contains("apple-darwin") {
         "macosx"
-    } else if target.contains("apple-ios") {
+    } else if target == "x86_64-apple-ios" || target == "i386-apple-ios" {
+        "iphonesimulator"
+    } else if target == "aarch64-apple-ios"
+        || target == "armv7-apple-ios"
+        || target == "armv7s-apple-ios"
+    {
         "iphoneos"
     } else {
         unreachable!();
@@ -37,7 +42,7 @@ fn build(sdk_path: Option<&str>, target: &str) {
     use std::env;
     use std::path::PathBuf;
 
-    let mut headers: Vec<&'static str> = vec![];
+    let mut headers: Vec<&str> = vec![];
 
     #[cfg(feature = "audio_toolbox")]
     {
@@ -47,14 +52,19 @@ fn build(sdk_path: Option<&str>, target: &str) {
 
     #[cfg(feature = "audio_unit")]
     {
-        println!("cargo:rustc-link-lib=framework=AudioUnit");
+        println!("cargo:rustc-link-lib=framework=AudioToolbox");
         headers.push("AudioUnit/AudioUnit.h");
     }
 
     #[cfg(feature = "core_audio")]
     {
         println!("cargo:rustc-link-lib=framework=CoreAudio");
-        headers.push("CoreAudio/CoreAudio.h");
+
+        if target.contains("apple-ios") {
+            headers.push("CoreAudio/CoreAudioTypes.h");
+        } else {
+            headers.push("CoreAudio/CoreAudio.h");
+        }
     }
 
     #[cfg(feature = "open_al")]
@@ -81,11 +91,31 @@ fn build(sdk_path: Option<&str>, target: &str) {
 
     builder = builder.size_t_is_usize(true);
 
+    // See https://github.com/rust-lang/rust-bindgen/issues/1211
+    // Technically according to the llvm mailing list, the argument to clang here should be
+    // -arch arm64 but it looks cleaner to just change the target.
+    let target = if target == "aarch64-apple-ios" {
+        "arm64-apple-ios"
+    } else {
+        target
+    };
+
     builder = builder.clang_args(&[&format!("--target={}", target)]);
 
     if let Some(sdk_path) = sdk_path {
         builder = builder.clang_args(&["-isysroot", sdk_path]);
     }
+    if target.contains("apple-ios") {
+        //builder = builder.clang_args(&["-x", "objective-c", "-fblocks"]);
+        builder = builder.objc_extern_crate(false);
+        builder = builder.generate_block(false);
+        builder = builder.block_extern_crate(false);
+        builder = builder.rustfmt_bindings(true);
+        // time.h as has a variable called timezone that conflicts with some of the objective-c
+        // calls from NSCalendar.h in the Foundation framework. This removes that one variable.
+        builder = builder.blacklist_item("timezone");
+        builder = builder.blacklist_item("objc_object");
+    }
 
     let meta_header: Vec<_> = headers
         .iter()
@@ -95,9 +125,7 @@ fn build(sdk_path: Option<&str>, target: &str) {
     builder = builder.header_contents("coreaudio.h", &meta_header.concat());
 
     // Generate the bindings.
-    builder = builder
-        .trust_clang_mangling(false)
-        .derive_default(true);
+    builder = builder.trust_clang_mangling(false).derive_default(true);
 
     let bindings = builder.generate().expect("unable to generate bindings");
 

@simlay simlay requested a review from est31 September 27, 2020 02:24
@mitchmindtree
Copy link
Member

Thanks so much for all your work on this folks!

@simlay & @MichaelHills, just let me know if you'd like to become a co-maintainer of the repo and crate - I don't want my lack of time or the fact I don't have a Mac anymore to hold any progress back here.

@MichaelHills
Copy link
Contributor

@mitchmindtree probably a good idea for one or both of us to be a co-maintainer especially since you don't have a Mac. @simlay is very active in the Rust iOS community and has been for a long time it seems. :) I have been focused on just getting the bevy game engine fully working on iOS and audio is the last piece of the puzzle.

Given the amount of effort I've spent getting audio working on iOS, it makes sense for me to help continue to refine it. Would love to have someone else help me test though!

In case you didn't see it, I have this up on coreaudio-rs RustAudio/coreaudio-rs#72 Which RustAudio projects do you currently maintain?

@mitchmindtree
Copy link
Member

Thanks @MichaelHills, I've invited you and @simlay to a rustaudio:coreaudio-maintainers group which should give maintainer access to both coreaudio repos along with the ability to publish to both as well.

Also, manually publishing coreaudio-sys should rarely be necessary as we have github actions that should auto-publish when a commit lands in master that updates the version - it might be worth moving coreaudio-rs from travis onto a similar github workflow?

@MichaelHills
Copy link
Contributor

Thanks @mitchmindtree, auto publish for coreaudio-rs sounds good though not necessary. I'm a bit time poor after the big push I did on bevy iOS so will focus my efforts on coreaudio-rs and cpal actual code changes for the time being. Might chase up some testers to help me test. :)

@MichaelHills
Copy link
Contributor

@simlay we should publish a new version so that RustAudio/coreaudio-rs#72 can be landed and subsequently RustAudio/cpal#485.

I'm not familiar with the release process of rust crates and versioning. What should the new version number be?

@MichaelHills
Copy link
Contributor

Might be nice to also clean up all the warnings. One thing at a time I guess.

@simlay
Copy link
Member Author

simlay commented Oct 17, 2020

Might be nice to also clean up all the warnings. One thing at a time I guess.

You mean the 128 warnings emitted that are pretty much all warning: externblock uses typeu128, which is not FFI-safe? I'm pretty sure the reason why those are emitted is because of rust-lang/rust#54341

I'm not familiar with the release process of rust crates and versioning. What should the new version number be?

Well, for everyone else in the non-iOS rust ecosystem this is a pretty small change and it's non breaking to anything. I'd say going from 0.2.5 to 0.2.6 is reasonable.

@MichaelHills
Copy link
Contributor

Might be nice to also clean up all the warnings. One thing at a time I guess.

You mean the 128 warnings emitted that are pretty much all warning: externblock uses typeu128, which is not FFI-safe? I'm pretty sure the reason why those are emitted is because of rust-lang/rust#54341

Oh maybe I meant in coreaudio-rs, there's a bunch of try! and dyn trait warnings. Looks like there's nothing we can do about the u128 ones at the moment?

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

6 participants