rust-mobile / android-activity

Glue for building Rust applications on Android with NativeActivity or GameActivity
238 stars 46 forks source link

Disable `ndk` default features to remove `raw-window-handle 0.6` #142

Closed Vrixyz closed 10 months ago

Vrixyz commented 10 months ago

Fixes #141

The ndk crate enables raw-window-handle 0.6 by default (because of https://github.com/rust-mobile/ndk/pull/434#issuecomment-1752089087) which might not be used by consumers of the android-activity crate at all, or might (still) be a mismatching version. Even if the rwh_0x features are additive, figuring that out leads to cryptic errors and it is best to turn off these defaults completely and leave it to the user to turn it back on in their own [dependencies] section if desired.


I couldn't run the test because of that error, I'm on mac, not sure how my android dev setup is right now 😅

image
MarijnS95 commented 10 months ago

This error is triggered based on compile target, how should disabling features have an effect?

Vrixyz commented 10 months ago

To be honest I don't know exactly how to test this crate (android-activity), I don't think my change is the root cause of my error, but rather something on my end, I just quickly got the PR running to start the discussion because I'm curious on how to address the issue.

I'm not sure how to answer your question, I guess I expected cargo test to select the correct target if only one were available, so I was surprised and unsure how to proceed.

MarijnS95 commented 10 months ago

@Vrixyz right, the way the description is set up (no context what you want to achieve, only something about an error) made me think this PR tried to address the error in the screenshot. Rather, you meant that that error shows up making you unable to test.

Please solve that by providing a commit message and PR description detailing what you want to achieve, because I'd approve the code changes in a pinch.

MarijnS95 commented 10 months ago

I just quickly got the PR running to start the discussion because I'm curious on how to address the issue.

In that case I've done you a solid and wrote a sensible description for you. Note that it was my decision to have rwh_06 in the default feature set for the ndk crate, so I believe I'm the right person to document why this makes sense.


There's likely no need to re-expose ndk-rwh_0x features, even though android-activity reexports the ndk crate. It'll be trivial enough for crate users to add this dependency to their own list, but if undesired I should remove this from the default feature set after all.

Vrixyz commented 10 months ago

Thanks! I edited the commit to reflect that, but I guess you can also squash and edit it from github UI, let me know if I can be of assistance further.

rib commented 10 months ago

Hey thanks for poking at this @Vrixyz

That error was essentially Cargo saying that the ndk-sys crate (same for the android-activity crate itself) needs to be compiled for Android whereas cargo test will by default try and build and test a crate for your host environment (macOS in this case).

Cargo lets you cross compile crates for other platforms by passing a --target like --target aarch64-linux-android or E.g.:

cargo build --target aarch64-linux-android --features=native-activity

but more generally for Rust Android development you also need to have downloaded the latest Android NDK from https://developer.android.com/ndk/downloads?authuser=3 and have pointed the ANDROID_NDK_ROOT environment variable at that.

I tend to use the cargo ndk crate to help deal with some of those Android cross-compilation details.

E.g. something like:

cargo install cargo-ndk
export ANDROID_NDK_ROOT=/path/to/ndk-r26
cargo ndk build --features=native-activity

Currently though we don't have any tests for this crate besides testing various apps or examples like these: https://github.com/rust-mobile/rust-android-examples

cargo test doesn't generally work for Android development since that's geared for trying to build and run tests on your host machine and android-activity is only designed to be usable on an Android device or emulator.

There are also some examples in this repo that each have a README that shows how they can be built and installed.

MarijnS95 commented 10 months ago

@Vrixyz thanks! I've gone ahead and squash-merged this while doing some patching (in particular, putting a title back into the commit message).