Skip to content

fix(android): upgrade jni to 0.22.4 for veilid-core compatibility#30

Open
evic2132 wants to merge 3 commits into
mainfrom
fix/jni-0.22-veilid-compat
Open

fix(android): upgrade jni to 0.22.4 for veilid-core compatibility#30
evic2132 wants to merge 3 commits into
mainfrom
fix/jni-0.22-veilid-compat

Conversation

@evic2132

@evic2132 evic2132 commented Jun 18, 2026

Copy link
Copy Markdown

Summary

Upgrades the Android jni dependency from 0.21.1 to 0.22.4 and migrates the JNI bridge code to match the API expected by veilid-core v0.5.3.

Without this change, Android builds fail when calling veilid_core_setup_android because veilid-core types its Android setup against jni 0.22 (EnvUnowned, JObject), while save-rust was still using jni 0.21 (JNIEnv).

Problem

veilid-core v0.5.3 depends on jni 0.22.x and exposes:

pub fn veilid_core_setup_android(mut env: EnvUnowned, ctx: JObject)

save-rust pinned jni = "0.21.1" for Android and passed incompatible types from android_bridge.rs, producing:

error[E0308]: arguments to this function are incorrect
expected `EnvUnowned<'_>`, found `JNIEnv<'_>`
expected `jni::objects::jobject::JObject<'_>`, found `JObject<'_>`
note: there are multiple different versions of crate `jni` in the dependency graph

Solution

Dependency

  • Bump [target.'cfg(target_os = "android")'.dependencies] jni to 0.22.4 in Cargo.toml

src/android_bridge.rs

  • Migrate native methods to jni 0.22's EnvUnowned + with_env(|env| ...) + resolve::<ThrowRuntimeExAndDefault>() pattern
  • Replace direct JNIEnv calls (get_string, new_string, get_native_interface, etc.) with Env APIs
  • Call veilid_core_setup_android(env, context) after JNI init/smoke-test and before spawning the server thread (Veilid must be ready before backend startup)
  • Use jni_str! / jni_sig! for static Java method invocation
  • Preserve existing comments (setup_android stub, stop-server flow comments, smoke-test annotations)

src/jni_globals.rs

  • GlobalRefGlobal<JClass<'static>>
  • init_jni / init_jni_inner take &mut Env
  • JavaVM::attach_current_thread updated to callback style required by jni 0.22
  • Preserve commented legacy with_env helper block

build-android.sh

  • Improve SDK/NDK auto-detection (macOS ~/Library/Android/sdk, glob-based NDK version selection avoids ANSI-colored ls output breaking path checks)
  • No change to jniLibs output path (../../Save-app-android/app/src/main/jniLibs)
  • Build ABIs: arm64-v8a (modern phones), armeabi-v7a (older 32-bit ARM), x86_64 (emulators/dev)
  • Skipped: 32-bit x86 (i686) — deprecated emulator ABI, not used by the Android app's Rust tooling
ABI Rust target Purpose
arm64-v8a aarch64-linux-android Primary production ABI
armeabi-v7a armv7-linux-androideabi Older 32-bit ARM devices
x86_64 x86_64-linux-android Emulators / dev machines

jni 0.22 migration notes

jni 0.21 jni 0.22
JNIEnv in native methods EnvUnowned entry + with_env&mut Env
env.get_string() jstring.try_to_string(env)
env.new_string() JString::from_str(env, ...)
GlobalRef Global<T>
attach_current_thread()AttachGuard attach_current_thread(|env| ...) callback

Testing

  • cargo build --release (host)
  • cargo ndk ... -t arm64-v8a -t armeabi-v7a build --release (Android)
  • RUST_MIN_STACK=8388608 cargo test -- --test-threads=113 passed, 0 failed, 2 ignored (~18 min)
  • On-device Android smoke test of SnowbirdBridge.startServer / stopServer (not run in this PR)

Follow-up

  • Runtime validation on an emulator/device is recommended before release
  • veilid-tools still transitively pulls jni 0.21.1; this is fine as long as save-rust and veilid-core share jni 0.22 at the integration boundary

evic2132 added 2 commits June 18, 2026 18:04
veilid-core v0.5.3 depends on jni 0.22.x and exposes
veilid_core_setup_android(EnvUnowned, JObject), which is incompatible
with the jni 0.21 API used by the Android bridge.

Migrate android_bridge.rs and jni_globals.rs to jni 0.22's EnvUnowned/
Env pattern, and harden build-android.sh NDK auto-detection on macOS
without changing the jniLibs output path.

Verified:
- cargo build --release (host)
- cargo ndk build (aarch64-linux-android)
- cargo test -- --test-threads=1 (13 passed, 2 ignored)
Add armeabi-v7a for older 32-bit ARM devices to match the Android
app's Rust tooling. Skip deprecated 32-bit x86 (i686) emulator ABI.

@tripledoublev tripledoublev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Builds clean locally (host cargo build --release + cargo ndk -t arm64-v8a build --release). One blocking issue: startServer falls through after a failed init instead of bailing.

Comment thread src/android_bridge.rs Outdated
Comment on lines +70 to +71
// Use another new JNIEnv for veilid_core_setup_android
veilid_core_setup_android(env, context);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: if the closure errors, resolve stages a Java exception and returns null/empty, yet we still call into native code and spawn the server with empty paths — return early instead.

Suggested change
// Use another new JNIEnv for veilid_core_setup_android
veilid_core_setup_android(env, context);
if output.is_null() {
return output;
}
veilid_core_setup_android(env, context);

When with_env().resolve::<ThrowRuntimeExAndDefault>() fails, jni 0.22
throws to Java and returns a null jstring. Bail out before calling
veilid_core_setup_android or spawning the server with empty paths.
@evic2132

Copy link
Copy Markdown
Author

Addresses the review feedback on startServer: when resolve::<ThrowRuntimeExAndDefault>() fails, we now return the null jstring immediately instead of falling through to veilid_core_setup_android and the server thread.

Pushed in 977aab4.

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.

2 participants