rust-mobile / ndk

Rust bindings to the Android NDK
Apache License 2.0
1.11k stars 110 forks source link

Bump `jni-sys` to `0.4.0` and `jni` to `0.22` #433

Open MarijnS95 opened 11 months ago

MarijnS95 commented 11 months ago

https://github.com/jni-rs/jni-sys/releases/tag/v0.4.0 https://github.com/jni-rs/jni-rs/releases/tag/v0.22.0

MarijnS95 commented 11 months ago

Expected a local compiler error, but upon closer inspection jni is only used in a doc-example. Let's double-check that the CI catches this, though we'll then block on https://github.com/jni-rs/jni-rs/pull/478.

MarijnS95 commented 11 months ago

It's not here yet, but I've prepared this PR for jni 0.22 (except possible Rust code changes) so that it should™ be as trivial as re-running the CI to get this merged, when that release happens.

rib commented 11 months ago

Cool, I'd like to make a jni release soon but if I'm honest I'm not currently expecting it to happen before the 0.5 release of android-activity or winit 0.29.

MarijnS95 commented 11 months ago

Sure, just keep in mind that that's going to be a hassle for the community as the ndk won't be able to make such a breaking release for a while until winit does... We seem to be on a ± yearly breaking release schedule.

rib commented 11 months ago

Surely the jni / jni-sys crates are only internal deps for the ndk crate so you should be ok to bump later?

I intentionally don't expose any jni types for android-activity so it wouldn't affect semver to bump.

MarijnS95 commented 11 months ago

Surely the jni / jni-sys crates are only internal deps for the ndk crate

This PR is marked as breaking for a reason :wink:

rib commented 11 months ago

Surely the jni / jni-sys crates are only internal deps for the ndk crate

This PR is marked as breaking for a reason 😉

where do the ndk / ndk-sys crates currently expose jni/jni-sys types? that doesn't sound good to me and maybe we can try to remove any exposure?

rib commented 11 months ago

We seem to be on a ± yearly breaking release schedule

It could be nice if we could do more frequent releases and maybe not worry too much whether every release necessarily gets picked up by Winit.

MarijnS95 commented 11 months ago

where do the ndk / ndk-sys crates currently expose jni/jni-sys types? that doesn't sound good to me and maybe we can try to remove any exposure?

Only jni-sys types. Quite a few JNIEnv/jobjects in constructors. jni should only be used for a doc-example.

It could be nice if we could do more frequent releases and maybe not worry too much whether every release necessarily gets picked up by Winit.

We could, but it's annoying to have a mismatch between all crates that are so intertwined with windowing systems.

rib commented 11 months ago

where do the ndk / ndk-sys crates currently expose jni/jni-sys types? that doesn't sound good to me and maybe we can try to remove any exposure?

Only jni-sys types. Quite a few JNIEnv/jobjects in constructors. jni should only be used for a doc-example.

I was pondering for a second whether we could do a 0.3.x release of the jni-sys crate with the semver trick. That's a tempting solution but really the JNIEnv type completely changed between jni-sys 0.3 and 0.4.

That's awkward in the sense that for 99% of things (and probably all the ndk crate usage) you only care about passing around an opaque pointer and probably don't ever dereference the JNIEnv pointer type.

MarijnS95 commented 11 months ago

@rib yeah we don't look at the innards of the type. Having a type-safe definition does help things not getting mixed up, though.