Heads up for planned jni-sys updates

[rib]

Wasn't sure whether to open a discussion for this, but essentially just wanted to make sure to give a heads up here that I've been chipping away at some maintenance chores for jni-sys and based on issues I've seen while working on the jni crate I was planning a few semver breaking changes.

It'd be good if you can take a look at the 'Release 0.4' milestone here: https://github.com/jni-rs/jni-sys/milestone/1

The two notable changes are:

1. making jboolean an alias for bool: ref Considering redefining jboolean as either an bool or our own #[repr(u8)] enum jni-rs/jni-sys#19, Add Array wrapper types with lifetimes jni-rs/jni-rs#400 (comment) and Make jboolean an alias for bool instead of u8 jni-rs/jni-sys#23
2. Turning JNINativeInterface + JNIInvokeInterface_ into unions that namespace function pointers by the JNI version they were added (and remove the misleading Option wrappers), ref: Turn JNINativeInterface + JNIInvokeInterface_ into unions jni-rs/jni-sys#28 and Removes the Options around the function pointers jni-rs/jni-sys#25

[Dushistov]

Thanks for information. Obviously I'm not happy with changes that break compatibility with bindgen output,
because flapigen supports the both variant of usage: jni-sys and generated by bindgen code for jni.h.
But I understand why these changes were made.

[rib]

I guess if you aim to have compatible support for layering on jni-sys vs direct bindgen for jni.h the main thing that would be exposed is the jboolean change and you could also alias jboolean as bool for bindgen for consistency?

To clarify, I also haven't landed / released any changes yet and was first just reaching out to others that are building on jni-sys to avoid any surprises and see if there would be any big problems for anyone if we make these changes.

In general I guess it's reasonable that jni-sys may not be exactly compatible with bindgen (we could just drop jni-sys in that case) but also want to be considerate of current users of the API.

[Dushistov]

be exposed is the jboolean change and you could also alias jboolean as bool for bindgen for consistency?

No, I can not see any impact of "jboolean change" on flapigen.

The problematic part, at the first glance, were JNINativeInterface, JNIInvokeInterface and replacing Option.

But at the second glance, flapigen uses JNIEnv to call Java functions, so any changes in JNIInvokeInterface should
not affect flapigen.

In general I guess it's reasonable that jni-sys may not be exactly compatible with bindgen (we could just drop jni-sys in that case

I completely disagree here. The main feature of jni-sys for me is that there is no depency on bindgen,
and so there is no dependency onclang library. Plus no build script that generate Rust from C during build,
that hurt compile time.
Plus no need of jni.h exists during build.

So if you just generate with bindgen jni-sys crate, and that publish it on crates.io it would be the same usability for me as existing jni-sys.

[rib]

be exposed is the jboolean change and you could also alias jboolean as bool for bindgen for consistency?

No, I can not see any impact of "jboolean change" on flapigen.

👍

The problematic part, at the first glance, were JNINativeInterface, JNIInvokeInterface and replacing Option.

But at the second glance, flapigen uses JNIEnv to call Java functions, so any changes in JNIInvokeInterface should not affect flapigen.

okey, cool, that's good to know.

In general I guess it's reasonable that jni-sys may not be exactly compatible with bindgen (we could just drop jni-sys in that case

I completely disagree here. The main feature of jni-sys for me is that there is no depency on bindgen, and so there is no dependency onclang library. Plus no build script that generate Rust from C during build, that hurt compile time. Plus no need of jni.h exists during build.

Ah, okey, so you effectively want to treat it like a pre-generated bindgen output.

For reference, for a similar situation in android-activity I pre-generate ffi bindings with bindgen offline so that there's no need for downstream crates to deal with bindgen at build time: https://github.com/rust-mobile/android-activity/blob/main/android-activity/generate-bindings.sh

That's a slightly more constrained case because at least there we only care about a small set of targets but maybe it'd be possible to do something similar here if you strictly want bindgen compatibility.

Since it sounds like the details are encapsulated by what flapigen generates though it sounds like you'd still be able to use jni-sys as a way to avoid depending on bindgen which sounds like the simpler option.

Considering that jni-sys is a hand-written binding instead of auto-generated, I still tend to think it's reasonable that the crate shouldn't be constrained to being exactly API compatible with cached bindgen output. That would also be ambiguous to maintain considering that bindgen is configurable and has various options for tailoring the bindings it generates.

Btw, is there anything in particular you think is missing with the current jni-sys bindings that you get from bindgen?

One thing I've missed before is Debug implementations.