ReviewOS

also looking at this

craft-native/craft

fix(android): make every Zig decline speak, and prove Zig answers

#205
Merged glennmichael123 wants to merge fix/android-zig-serves into main
8 files +616 -255
9 days ago

Follow-up to #199. That PR got the Zig runtime loading on Android for the first time, and its own evidence showed the next problem:

I CraftNative: craft: registered 103 natives on com/craft/runtime/CraftNative
W System.err: java.lang.NoSuchFieldError: no "J" field "longVersionCode" in class "Landroid/content/pm/PackageInfo;"
W CraftNative: craft: getDeviceInfo fell through to the shim (JavaException)

Bound, every case passing — and getDeviceInfo not served by Zig at all.

The bug

PackageInfo.longVersionCode is not a field. The Kotlin shim writes .longVersionCode, which Kotlin compiles to getLongVersionCode(); the Zig port read it with GetFieldID. It calls the getter now.

I swept every other field read in the Android Zig code for the same property-vs-field mistake. The rest are real public fields (DisplayMetrics, PackageInfo.versionName/versionCode, ContentProviderResult.uri, R.drawable.ic_dialog_info) or static constants.

Why it nearly went unnoticed

A native that declines hands the action to Kotlin, the page still gets an answer, and the suite passes. That decline happened to log. About 170 others did not — catch return null, catch return jni.JNI_FALSE, catch {} — so a JNI failure in any of them looked identical to a native doing its job.

Every one goes through a helper now, and each says something different because each means something different:

helpermeaninglevel
fellThrougha native returned null/false; Kotlin served insteadwarn
failedWithoutFallbacka callback into Zig gave up; nothing picks it uperror
undelivereda settle/reject/event could not reach the pageerror

The last two are the serious ones. All 29 void natives are callbacks into operations Zig already owns — location results, review outcomes, task runs — so their failures used to mean a promise that never settled, with no trace.

ownedUtf8 is removed: it turned a failure into null for seven callers that then returned silently.

Keeping it that way

  • test/android_declines_test.zig — no catch in the dispatcher may discard its error (the compiler already refuses an unused capture). The scanner is shown silent and handled shapes before it's trusted with the file, and it's mutation-tested: putting catch return null back into getDeviceInfo fails it, naming the line.
  • The Android E2E runtime leg fails on any decline line, reading the full logcat buffer so the System.err stack trace is there to quote. Registration proves the library bound; no declines, with every decline made to speak, proves Zig answered.
  • protocol.test.ts reads android_dispatch.zig to check the phrases it matches are the ones Zig writes, so rewording a helper can't blind the leg.

Verification

Full Zig suite green; test:android green on 0.17.0-dev.1441 and on CI's 1963; fmt --check on 1963; 34 protocol tests, typecheck, and Pickier clean. Whether Zig now actually serves getDeviceInfo on a device is this PR's CI to answer — the runtime leg will say declines:0 or name the decline.

8 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.