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

Upgrade jni to 0.21.1 #32216

Merged
merged 1 commit into from May 20, 2024
Merged

Upgrade jni to 0.21.1 #32216

merged 1 commit into from May 20, 2024

Conversation

delan
Copy link
Sponsor Member

@delan delan commented May 2, 2024

Split from #31278.

Co-authored-by: Martin Robinson [email protected]
Co-authored-by: Mukilan Thiyagarajan [email protected]


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because they do not change functionality.

@mrobinson
Copy link
Member

Here's my attempt as fixing these issues: 41bde52

I'm stuck fighting with the borrow checker. It's unclear how to create a JValue from a Rust string and pass it to JNIEnv::call_method without requiring a double mutable borrow of JNIEnv.

@mukilan mukilan self-requested a review May 3, 2024 04:16
@mukilan
Copy link
Member

mukilan commented May 4, 2024

With these changes on top of @mrobinson 's commit, I was able to get the android build to succeed and do a test run on my device.

The major change is the addition of the <'local> lifetime parameter to the JNI methods and methods that take both &mut JNIEnv and a some subtype of &JObject. I only needed to add them to the methods that actually used both arguments, but I've added them to all JNI methods, even though only the env parameter was used and the rest were ignored.

@mrobinson
Copy link
Member

@mukilan Would you mind pushing your changes to @delan's branch so we can land them?

JNI methods in `jniapi` now have explicit lifetime
annotations to adapt it to the new `jni` version.

Co-authored-by: Martin Robinson <[email protected]>
Co-authored-by: Mukilan Thiyagarajan <[email protected]>
Signed-off-by: Mukilan Thiyagarajan <[email protected]>
@mukilan
Copy link
Member

mukilan commented May 20, 2024

@mrobinson I've rebased and pushed all the changes in a single commit. Please review whenever you get a chance.

@mrobinson
Copy link
Member

Nice!

@mrobinson mrobinson enabled auto-merge May 20, 2024 13:46
@mrobinson mrobinson added this pull request to the merge queue May 20, 2024
Merged via the queue into servo:main with commit 8d2d955 May 20, 2024
11 checks passed
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 this pull request may close these issues.

None yet

3 participants