search

rust-mobile / ndk

Rust bindings to the Android NDK
Apache License 2.0
1.14k stars 112 forks source link

what's the plan on binder? #490

Open suguruwataru opened 6 days ago

suguruwataru commented 6 days ago

NDK has native Binder API https://developer.android.com/ndk/reference/group/ndk-binder But a quick search in the crate docs doesn't find any binder related items. I'm guessing it's just not supported at this moment?

suguruwataru commented 6 days ago

found this commit which removed all binder APIs from ndk-sys https://github.com/rust-mobile/ndk/commit/63cbffa77a97206c66bd5b070632a4f462326017

So they are not there because they include 'c++ like directives'. Not sure what this means. The Binder API look pretty C to me. Perhaps if I try to build it myself I can find why.

MarijnS95 commented 4 days ago

found this commit which removed all binder APIs from ndk-sys https://github.com/rust-mobile/ndk/commit/63cbffa77a97206c66bd5b070632a4f462326017

This commit doesn't remove any binder API from ndk-sys because they weren't there in the first place. The relevant headers are merely added while being commented out to have a complete list of all the Android-specific headers in the entire NDK sysroot.

So they are not there because they include 'c++ like directives'. Not sure what this means. The Binder API look pretty C to me.

Do you have any source to back this up? Some but not all of these files are C++-only and cannot be wrapped using our C-only bindgen execution.

The files that can however be wrapped are enabled on some of these branches, with a few prototypes to extend the ndk crate with binder APIs: https://github.com/MarijnS95/ndk/branches/all?query=binder

None of that gained any traction though, as Google already has quite "mature" binder bindings together with a code generator for internal AOSP usage: https://cs.android.com/android/platform/superproject/main/+/main:frameworks/native/libs/binder/rust/. This requires quite some modification to fit in a regular (i.e. non-Android.bp) crate and was already done over at https://github.com/Kernel-SU/binder_rs.

According to a recent crate search @maurer and @emarteca just published the exact same thing 5 days ago. Without a repository link it's still quite easy to browse the source and see that it's also a copy of the AOSP code. EDIT: Looking further, these two GitHub accounts turn out to be associated with Googlers that are making the upstream crate publishable directly to crates.io: https://cs.android.com/android//android/platform/frameworks/native/+/75d711270053226fea61534cac871d8967adb744. While this looks like official Google/Android-provided bindings and saves everyone from having outdated community ~forks~ copy-pastes like https://github.com/Kernel-SU/binder_rs, it does yet again mean no interop with the ndk crate._

I'm happy to work together with anyone to land proper binder APIs in the ndk crate, especially if it helps against publishing yet another crate with the same source code to crates.io. but it's hard to keep this "in sync" with an AOSP codebase. EDIT: Perhaps I can find a middle-ground where binder types are exempt from ndk-sys and instead provided from android-binder-ndk-sys. Likewise, the ndk crate can provide type-specific bindings (i.e. parcel functions on specific types) against the android-binder crate.

maurer commented 3 days ago

We're not ready to make the binder crate's API properly stable, which would be somewhat required for an official NDK-style release. The primary purpose of those crates is to make it easier for those libraries to move from the platform ecosystem to another, which is why you didn't see much fanfare around them, just a quiet cargo publish. That said, if it helps anyone, great, they're just not currently intended to be stable, come with no expectation of working outside our environment, etc. It will probably be updated about once a year or if a bug is found, pending an official release.

Overall, we would like ndk to work with this, but currently have no official plans for 3p app Rust support.

With regards to parcel functions on specific types, those should mostly be useful through integration with AIDL, yes? AIDL expects those to be exported safely.

I can't promise anything here, but would it be helpful if these libraries had a similar unsupported export? The downside with either binder or nativewindow is that if the aidl tool you're using to generate sources (I believe people usually get this out of the SDK) gets too far ahead, it might try to use features that don't exist.

Is there something that your users want binder for aside from AIDL usage?

MarijnS95 commented 3 days ago

@maurer that's completely understandable and I'm already more than happy to receive a response at all from a Googler. We've had similar discussions around the linked nativewindow AOSP Rust crate in the past and while true that Google owns and maintains the NDK API for that, we would've liked to share code to benefit both AOSP and the wider Rust "native" app/library community. There was however a preference to keep the bindings in-house because of both ownership and complications that sharing would bring. At least you've "figured out" here to make an AOSP Android.bp-driven crate available on crates.io already :+1:

If there's one suggestion I can make right now, it's to add repository = to Cargo.toml with links to the source code in the AOSP tree; that should help everyone find where the code originates from.

Thanks for the links regarding safely parceling specific types. So if the aidl tool expects these functions to be available there'd be very complicated interplay in Rust when your android-binder crate has to be used in conjunction with the AOSP-only nativewindow crate rather than the public and app-focused ndk crate.

I can't promise anything here, but would it be helpful if these libraries had a similar unsupported export?

What exactly do you mean with "similar unsupported export"; publishing nativewindow to crates.io? That would conflict with the types and functionality that the ndk crate already provides.

Hence why I want to be early in discussing code shareability for the NDK Rust wrappers as it is used both internally in AOSP as well as available to Rust-built apps. Rather than both building the exact same thing.

I am not sure what users want. @suguruwataru who opened this issue did not specify. Personally, as a maintainer of this ndk crate, I want to provide Rust bindings to every API that the NDK provides, including i.e. the safe parcel functions on types that are represented by this crate. And separate to that, talk to APIs on the system or vendor that don't have a corresponding NDK API yet, i.e. https://github.com/Traverse-Research/android-powerstats-rs.

DanAlbert commented 3 days ago

Some but not all of these files are C++-only

FWIW the C++ parts of the binder "API" are not actually API. It's all inline helper stuff for aidl afaik. Those headers shouldn't have actually been shipped in the first place, but it took a while to clean up the dependencies on them that of course sprang up immediately. r28 doesn't include them any more. It wouldn't make much sense to bindgen those. If rust needs something similar to what's in those (and I'm not sure you will) it'd make the most sense to just write that rather than try to machine translate it.

MarijnS95 commented 3 days ago

@DanAlbert yes, there is a reason why https://github.com/MarijnS95/ndk/commit/935851df20caca5146f70a882083fe299e622ecd didn't enable those headers. Haven't found a need for inline functions that we could just as well implement ourselves. Glad to hear they'll be removed.

suguruwataru commented 2 days ago

This commit doesn't remove any binder API from ndk-sys because they weren't there in the first place. The relevant headers are merely added while being commented out to have a complete list of all the Android-specific headers in the entire NDK sysroot.

Ack. My misunderstanding here...

Do you have any source to back this up? Some but not all of these files are C++-only and cannot be wrapped using our C-only bindgen execution.

No... I just took a quick look at https://developer.android.com/ndk/reference/group/ndk-binder and didn't find anything that looked very C++ only to me. Possibly that's relevant to what @DanAlbert said. If those C++ parts are not supposed to be exposed it make sense they don't make a big appearance in docs.

I am not sure what users want. @suguruwataru who opened this issue did not specify.

I am writing multiple apps that communicate with each other, and was very happy to see Binder which looks like a great IPC mechanism. Those apps are mostly written in Rust so it would be great if I can just use the ndk crate to use Binder.

Also Binder is how IPC "should" be implemented on Android. Android forbids other mechanisms like unix domain sockets, and any background app that isn't bound to a foreground app using the "bound service" mechanism can get killed. Using Binder immediately both prevents the background side of the IPC from getting killed and creates a channel for communication.