android: Migrate from ndk-glue to ndk-context

[MarijnS95]

ndk-glue suffers one fatal flaw: it's "only" supposed to be used by the crate providing fn main() and only supposed to end up in the dependency graph once as it has static globals which get duplicated across versions.

In the current case with winit 0.26 still on ndk-glue 0.5 but cpal on ndk-glue 0.6 it'll always panic in fn native_activity() as the static globals on this version is not initialized.

Introducing ndk-context: a crate that holds these statics, with the intention/premise to not see a breaking release /ever/ and make this a problem of the past. The crate is currently initialized with the VM and Android Context on ndk-glue 0.5.1 and 0.6.1 (0.4.1 pending) making it compatible with whatever is current, and the possibility for backporting to older ndk-glue versions too.

See also:
rust-mobile/ndk#211
rust-mobile/ndk#223

[est31]

Thanks!

[est31]

Hmm wait it failed in CI:

error: custom attribute panicked
 --> examples/android.rs:8:35
  |
8 | #[cfg_attr(target_os = "android", ndk_glue::main(backtrace = "full"))]
  |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: message: `"ndk-glue"` is not a valid identifier

warning: unused import: `HostTrait`
 --> examples/android.rs:6:33
  |
6 | use cpal::traits::{DeviceTrait, HostTrait, StreamTrait};
  |                                 ^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: `cpal` (example "android") generated 1 warning
error: could not compile `cpal` due to previous error; 1 warning emitted

[MarijnS95]

It seems that target.'cfg(target_os = "android")' doesn't work for dev-dependencies? Removing it makes the build succeed again (but breaks other targets for obvious reasons). https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#development-dependencies contains such an example with cfg() for dev-dependencies, any idea what I'm missing?

[MarijnS95]

This looks to be an odd issues inside our macros, investigating...

[est31]

This might be because cpal is still on the 2015 edition? The macro might be emitting a wrong span that points to cpal's sources so rustc treats it as edition 2015 code.

[MarijnS95]

The issue is that proc-macro-crate doesn't lookup target.xy.dev-dependencies, only target.xy.dependencies:

https://github.com/bkchr/proc-macro-crate/blob/9ddaaac13fc04182a525c85e89edc1c29f3d4456/src/lib.rs#L178-L189

And our fallback in ndk-macro is wrong in that it doesn't replace the dash in the crate_path("ndk-glue", ...) call with an underscore, hence we get `"ndk-glue"` is not a valid identifier:

https://github.com/rust-windowing/android-ndk-rs/blob/9060d5dbea8bdd9aee9e42ef8e8375c35adbc271/ndk-macro/src/helper.rs#L64

I'm fixing both and will get back to you when it's done, sorry for the noise!

[MarijnS95]

bkchr/proc-macro-crate#15

I'll think about removing the erroneous fallback in ndk-macro too.

[MarijnS95]

@est31 the fix to proc-macro-crate has been merged and released, re-running the CI should now succeed!

(The workaround has also been removed from ndk-macro, but since it's not critical I haven't bothered to create a release yet)

[MarijnS95]

Well, I'm not sure how this happened: I made sure to test the fix locally, yet it seems we're now hit by bkchr/proc-macro-crate#16... Not sure how I got it working without that at some point.

[est31]

@MarijnS95 it's alright, take your time. I'm impressed by how quickly you figured out the cause of the problem and came up with a solution!

[MarijnS95]

@est31 I just don't like making PRs (to proc-macro-crate) that are "half finished" and didn't actually fix the issue, shows that my testing methodology was/is messed up somewhere 😳

Anyway, the second fix is merged and released now too. I double checked it locally and the CI should not fail on it now 🤞

[MarijnS95]

Looks like we're all set now, the current issues also appear on master. Perhaps these CI steps should be made infallible so that it looks a bit nicer in the git log, instead of seeing everything failed?

[est31]

Thanks again, please write if you want a release. As for the CI failures, yeah maybe the three failling jobs should be commented out or be allowed to fail. Ideally at least the windows job would be fixed tho. PRs welcome :).

[MarijnS95]

No need for a release from my side yet, just propagating our android-ndk-rs changes through the most popular crates in the ecosystem. Perhaps @zarik5?

[zmerp]

I don't have any hurry, I'm keeping my cpal and oboe forks for my project. But once both this PR and the oboe one hit a new release I will be able to remove some duplicate code, thanks to ndk_context::initialize_android_context() (my project can't use ndk-glue yet)