Upgrade jni to 0.22

[djc]

To give feedback upstream.

[complexspaces]

It looks like we'll need to finally bump our MSRV with this update, as jni now has an MSRV of 1.77. This is 8 months newer, but hopefully by this point most users have moved on? Debian stable is on 1.85 so I think we're OK personally.

[complexspaces]

I've pushed up an initial attempt to adopt the new API of jni 0.22. It looks like there's some problems and lints that still need to be fixed but I'm out of steam for further fiddling today.

[atouchet]

jni 0.22 is out now.

[djc]

Filed:

• Avoid the paste dependency jni-rs/jni-rs#746
• Cost/benefit for the proc-macro-crate dependency jni-rs/jni-rs#747

[djc]

error:
       … while checking the derivation 'packages.x86_64-linux.rustls-platform-verifier'

       … while calling the 'derivationStrict' builtin
         at «nix-internal»/derivation-internal.nix:38:12:
           37|
           38|   strict = drvFunc drvAttrs;
             |            ^
           39|

       … while evaluating derivation 'rustls-platform-verifier'
         whose name attribute is located at «github:NixOS/nixpkgs/8c50662»/pkgs/stdenv/generic/make-derivation.nix:333:7

       (stack trace truncated; use '--show-trace' to show the full, detailed trace)

       error: No hash was found while vendoring the git dependency jni-0.22.0. You can add
       a hash through the `outputHashes` argument of `importCargoLock`:

       outputHashes = {
         "jni-0.22.0" = "<hash>";
       };

       If you use `buildRustPackage`, you can add this attribute to the `cargoLock`
       attribute set.

Not sure what this means, nor am I really motivated to learn...

[cpu]

Not sure what this means, nor am I really motivated to learn...

The error message suggests how to fix the problem (add an explicit outputHashes for the patched crate), but it will also go away when you stop using a cargo patch to take a git dependency on jni. I would recommend ignoring it for now.

[djc]

Not sure what this means, nor am I really motivated to learn...

The error message suggests how to fix the problem (add an explicit outputHashes for the patched crate), but it will also go away when you stop using a cargo patch to take a git dependency on jni. I would recommend ignoring it for now.

Dropped the patch section, which is no longer necessary anyway.

[djc]

Okay, so the test suite doesn't compile yet, and I don't think I have enough context on how this should work to fix that.

Also, the Nix stuff fails, probably because the compiler is too old and the flake.lock rust-overlay needs an update?

[djc]

Upgraded the PR to 0.22.2. Still needs someone with more JVM and/or Nix context to move forward.

[neunenak]

I'm working on a project involving using reqwests version 0.13 on Android, and this version bump would be very helpful for integrating it into the Rust-Kotlin FFI pipeline I already have which is on jni 0.22. I'm hoping to see this get merged and released quickly.

[neunenak]

Okay, so the test suite doesn't compile yet, and I don't think I have enough context on how this should work to fix that.

Also, the Nix stuff fails, probably because the compiler is too old and the flake.lock rust-overlay needs an update?

Looking at the error messages in CI, it seems like there are some complaints about edition2024 not being supported by the current rust compiler version, which the flake is getting from the Cargo.toml file, which was set to 1.77 in this commit. 1.85 is the earliest version that supports edition 2024, so the version bump might need to go there to make this work.

[cpu]

Also, the Nix stuff fails, probably because the compiler is too old and the flake.lock rust-overlay needs an update?

Indeed, here's a fix commit that fixes the Nix CI jobs on this branch. Do you want to cherry-pick it in here, or should I open a separate PR?

(Meta note: I added that flake+CI integration and would be OK with it being removed if i'm the only thing keeping it from annoying other contributors. Fighting Android+native platform APIs doesn't spark joy and unless there's a sponsorship motivation I'd prefer to prioritize volunteering time in other Rustls repos)

[neunenak]

I just tried making #224 to see if that version bump might fix the CI problems, but it seems the CI pipeline won't run for me.

[complexspaces]

This looks good so far and I'll be finishing up my review of the global/env changes specifically Saturday morning 👍

[rnikander]

I just used this branch in my Android app, along with latest jni crate, and it appears to work. I needed to change one character to get things to compile. Add a ? here because get_java_vm() is a Result<JavaVM, _>, not a JavaVM.

rustls-platform-verifier/rustls-platform-verifier/src/android.rs

Line 102 in cae50a9

java_vm: env.get_java_vm(),

[djc]

Alright, looks like this can finally pass CI. There was a problem with the earlier version of android::init_with_env() that seemingly prevented it from working in CI. Does someone want to give this a final round of review?

[jplatte]

As somebody who just likes their Cargo.lock tidy and has no domain knowledge of JNI, this looks good. Left a few minor comments.

[djc]

Thanks! Would especially appreciate a review from someone with substantial Android/jni expertise.

[rnikander]

appreciate a review from someone with substantial Android/jni expertise.

I can't say I have substantial JNI expertise, but I've been using the API a fair amount in the last few weeks. One suggestion I'd have is to take advantage of the macros like jni_sig!, so this code:

rustls-platform-verifier/rustls-platform-verifier/src/verification/android.rs

Line 175 in d8ab444

jni_sig!("(Landroid/content/Context;Ljava/lang/String;Ljava/lang/String;[Ljava/lang/String;[BJ[[B)Lorg/rustls/platformverifier/VerificationResult;");

Could be more readable:

 jni_sig!((android.content.Context, JString, JString, JString[], byte[], long, byte[][]) -> org.rustls.platformverifier.VerificationResult)

I think that's correct. I didn't try to compile it.

I've also found the bind_java_type! macro to very helpful at cleaning up JNI code. The method calls become more direct instead of env.call_method(...). However, in this case, we're only calling a few methods, so I think it's fine to use env.call_static_method or env.call_method.