From ed975019dc601c42fe6013dc7c8661cc3d9458e1 Mon Sep 17 00:00:00 2001 From: Logan Date: Sun, 16 Aug 2026 16:25:33 -0400 Subject: [PATCH] fix(dev): the smoke target died silently on a genuinely dead app Two harness bugs and the finding that exposed them. **`pidof` exits 1 when it finds nothing**, and under `set -e` a failing command substitution killed the script before it could print anything -- rc=1, no output. That was invisible for as long as the app crash-*looped*, because there is always some pid in that state. It appeared the moment the app died for good and ActivityManager stopped respawning it, which is precisely the run you most want output from. **And an install failure said nothing useful.** Both ways it fails are about identity rather than the build: INSTALL_FAILED_VERSION_DOWNGRADE when a bare `make android` (versionCode 1) meets something a versioned build left behind, and a signature mismatch when a debug-signed local build meets a release-signed one. Both were hit in one session, and both are fixed by uninstalling. The target says so now instead of leaving someone to read the constant name. The finding: with the startup bug fixed the app reaches the database and takes SIGSYS on the x86_64 emulator, because modernc.org/libc's Xlstat64 issues a raw lstat syscall on linux/amd64 and Android's seccomp filter forbids it -- bionic never issues it. arm64 has no lstat syscall at all, so ccgo_linux_arm64.go routes Xlstat through fstatat and is structurally unaffected; Go's own syscall package already used fstatat on both. So the default emulator cannot verify this app, and the skill says so rather than letting the next session read a tombstone as a regression. --- .../references/android-tier.md | 75 ++++++++++++------- .planning/NOTES.md | 56 ++++++++++++++ scripts/android-emulator.sh | 51 +++++++++++-- 3 files changed, 152 insertions(+), 30 deletions(-) diff --git a/.pi/skills/yellowjacket-dev/references/android-tier.md b/.pi/skills/yellowjacket-dev/references/android-tier.md index 6853517..1c7a8a0 100644 --- a/.pi/skills/yellowjacket-dev/references/android-tier.md +++ b/.pi/skills/yellowjacket-dev/references/android-tier.md @@ -96,34 +96,59 @@ command. The emulator is addressed by its saved pid in ## The current state of the build -`make android-smoke` **fails today, and the cause is known.** -`backend/system`'s `buildUserDirPath` switches on `runtime.GOOS` with -cases for darwin, linux and windows, and a `default:` that returns -`errUnsupportedOS`. `runtime.GOOS` is `"android"`, so it takes the -default, `NewYellowJacketApp` fails, and `main()` calls `os.Exit(1)` — -about 6 ms after the bridge initialises, which is exactly the signature -described above. +**The app starts. The x86_64 emulator cannot run it, and that is not a +bug in the app.** -**The fix is a documented Wails API and needs no build tags.** -`application.Mobile.StoragePath()` returns the app's private files -directory and returns `""` on desktop (`mobile_stub.go`), and -`resolveUserDirPath` already lets `YJ_HOME` override the path on every -OS — so setting that override from `StoragePath()` early in `main()`, -when it is non-empty, is the whole change. Do *not* import -`pkg/application` into `backend/system`: that package is deliberately -Wails-free, which is what the `indexbuild` tag split is protecting. +`modernc.org/libc` — which `modernc.org/sqlite`, and therefore the whole +database layer, sits on — issues a **raw `lstat` syscall on +linux/amd64** (`libc_linux_amd64.go`'s `Xlstat64` calls +`unix.Syscall(unix.SYS_LSTAT, …)`). Android's seccomp policy forbids +syscall 6 on x86_64, because bionic never issues it, so the process +takes `SIGSYS` the first time anything touches the database: -It is the *first* thing that stops it, not the only one. MPRIS is -compiled in (`android` implies the `linux` build tag, so -`mpris_linux.go` is in the build and will look for a session bus that -does not exist), and the desktop shell is still a desktop shell. Fixing -one and re-running the smoke is how you find the next. +``` +F/libc: Fatal signal 31 (SIGSYS), code 1 (SYS_SECCOMP), syscall 6 +F/DEBUG: Cause: seccomp prevented call to disallowed x86_64 system call 6 +``` -**And one that no amount of porting will fix:** open-*directory* -dialogs return an error on Android — the Storage Access Framework gives -tree URIs, not filesystem paths — as do save-file dialogs. This app's -first run is "choose your music folder" and its library model is -filesystem paths, so that is a design question, not a port. +**arm64 is unaffected, and structurally so.** There is no `lstat` +syscall on arm64 at all, so `ccgo_linux_arm64.go`'s `Xlstat` is +`Xfstatat(…, AT_SYMLINK_NOFOLLOW)` → `SYS_newfstatat` (79), which +Android permits. `grep -c SYS_LSTAT ccgo_linux_arm64.go` is 0. Go's own +`syscall` package already uses `fstatat` on both architectures, which +is why this is *only* the modernc path. + +So: **verify on arm64, not on the default emulator.** Use an +`arm64-v8a` system image (slow on an x86_64 host — it is full software +emulation) or a real device. `make android-smoke` on an x86_64 AVD will +report a `SIGSYS` tombstone that says nothing about your change. + +Two consequences worth holding onto. The x86_64 half of the fat APK is +*only* useful for emulators, and cannot work on any Android until +modernc fixes this — including x86 Chromebooks. And the tombstone is at +least honest: unlike the `os.Exit` that came before it, this one leaves +a real crash record with a backtrace. + +### What was fixed to get here + +`backend/system`'s `buildUserDirPath` switched on `runtime.GOOS` with a +`default:` returning `errUnsupportedOS`, so Android failed at startup +and `main()` called `os.Exit(1)` six milliseconds after the bridge came +up. `main()` now calls `system.UseHomeOverride(application.Mobile. +StoragePath())` before anything asks for a path — a documented, +build-tag-free API that returns `""` on desktop, where the setter is a +no-op. `backend/system` gained no import of the Wails application +package, which matters for the same reason `backend/events` is split by +the `indexbuild` tag. + +### What is still not done + +MPRIS is compiled in (`android` implies the `linux` build tag), the +shell is still a desktop shell, and — the largest one — open-*directory* +dialogs return an error on Android, because the Storage Access Framework +yields tree URIs rather than filesystem paths. This app's first run is +"choose your music folder" and its library model is filesystem paths, so +that is a design question rather than a port. ## The scaffold's own tasks diff --git a/.planning/NOTES.md b/.planning/NOTES.md index 9d73af7..65f6ffd 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -2587,3 +2587,59 @@ under the `Wails` tag and are inspectable from `chrome://inspect`; production builds compile that out — so a debug APK is the more informative one when something is wrong. And the docs recommend `build-tools;35.0.0`; 34.0.0 is what is installed here and builds fine. + +## The app starts on Android; x86_64 Android cannot run it (2026-08-16) + +Two findings, and the second is the one with consequences. + +**The startup bug is fixed.** `backend/system`'s `buildUserDirPath` +switched on `runtime.GOOS` and Android took the `default:` branch, so +`main()` called `os.Exit(1)` six milliseconds after the JNI bridge came +up. `main()` now calls +`system.UseHomeOverride(application.Mobile.StoragePath())` before +anything asks for a path. `StoragePath()` is `getFilesDir()` on +Android, Application Support on iOS and `""` on desktop — where +`UseHomeOverride` is a no-op — so the change needs no build tag and +alters nothing off mobile. `backend/system` gained no import of the +Wails application package, deliberately: that is the same constraint +the `indexbuild` split protects in `backend/events`. + +**And then it takes SIGSYS on the x86_64 emulator.** + +``` +F/libc: Fatal signal 31 (SIGSYS), code 1 (SYS_SECCOMP), syscall 6 +F/DEBUG: Cause: seccomp prevented call to disallowed x86_64 system call 6 +``` + +Syscall 6 on x86_64 is `lstat`, and the caller is **not our code and +not Go's**. Go's `syscall` package already routes both `Stat` and +`Lstat` through `fstatat` on amd64 *and* arm64. The caller is +`modernc.org/libc`, which `modernc.org/sqlite` sits on and therefore +the entire database layer: `libc_linux_amd64.go`'s `Xlstat64` issues +`unix.Syscall(unix.SYS_LSTAT, …)` directly. Android's seccomp filter +forbids it because bionic never issues it. + +**arm64 is unaffected, structurally rather than by luck.** arm64 has no +`lstat` syscall at all, so `ccgo_linux_arm64.go`'s `Xlstat` is +`Xfstatat(…, AT_SYMLINK_NOFOLLOW)` → `SYS_newfstatat` (79), which is +permitted. `grep -c SYS_LSTAT ccgo_linux_arm64.go` returns 0 against 1 +for amd64. + +Three consequences: + +- **The default emulator cannot verify this app.** `make android-smoke` + on an x86_64 AVD reports a tombstone that says nothing about your + change. Verification needs an `arm64-v8a` image (full software + emulation on an x86_64 host, so slow) or a real device. +- **The x86_64 half of the fat APK is dead weight on every Android**, + not just emulators — an x86 Chromebook would hit exactly this. It is + 31 MB of a 27 MB compressed artifact. Dropping it is a real option; + keeping it costs size and buys an emulator target that does not work. + Not decided here. +- The failure is at least *legible*. Unlike the `os.Exit` it replaced, + SIGSYS leaves a tombstone with a backtrace into `libwails.so`, which + is how it was identified in one pass. + +Worth knowing for anything else that reaches for a pure-Go C library: +this class of bug is invisible to every build and every desktop test, +and appears only under a platform's syscall filter. diff --git a/scripts/android-emulator.sh b/scripts/android-emulator.sh index e7d7673..a76c384 100755 --- a/scripts/android-emulator.sh +++ b/scripts/android-emulator.sh @@ -162,7 +162,42 @@ cmd_install() { need_sdk [ -f bin/yellowjacket.apk ] || die "no bin/yellowjacket.apk — run 'make android' first" "$ADB" get-state >/dev/null 2>&1 || die "no device — run 'make android-emulator' first" - "$ADB" install -r bin/yellowjacket.apk + + # The two ways this fails are both about identity rather than the + # build, and neither error says what to do about it. + # + # A *downgrade* is the versionCode rule working as designed: a bare + # `make android` produces versionCode 1, so it will not install over + # anything a versioned build left behind. A *signature* mismatch is + # the rule this whole pipeline exists for — a debug-signed local + # build cannot replace a release-signed one. + # + # Both are fixed by uninstalling, and on a throwaway emulator that + # costs nothing, so say so rather than making someone read the + # constant name. + out=$("$ADB" install -r bin/yellowjacket.apk 2>&1) || { + printf '%s\n' "$out" + case "$out" in + *INSTALL_FAILED_VERSION_DOWNGRADE*) + echo + echo "The installed copy has a higher versionCode than this build." + echo "A bare 'make android' builds versionCode 1; a versioned one" + echo "builds e.g. 10301. Either uninstall:" + echo " $ADB uninstall $PKG" + echo "or build with a version:" + echo " YJ_VERSION=1.3.1 YJ_VERSION_CODE=10301 make android" + ;; + *INSTALL_FAILED_UPDATE_INCOMPATIBLE* | *signatures do not match*) + echo + echo "The installed copy was signed with a different key. Android" + echo "never allows that as an update — which is exactly why CI" + echo "refuses to publish a debug-signed APK. Uninstall:" + echo " $ADB uninstall $PKG" + ;; + esac + return 1 + } + printf '%s\n' "$out" } cmd_launch() { @@ -190,11 +225,17 @@ cmd_smoke() { cmd_launch sleep 3 - local first - first=$("$ADB" shell pidof "$PKG" 2>/dev/null | tr -d '\r' | awk '{print $1}') + # **`|| true` is load-bearing.** `pidof` exits 1 when it finds + # nothing, and under `set -e` a failing command substitution kills + # the script -- silently, before it can print why. That is invisible + # for as long as the app crash-*loops*, because there is always some + # pid; it appears the moment the app dies for good and ActivityManager + # stops respawning it, which is exactly the run you most want output + # from. + local first second + first=$("$ADB" shell pidof "$PKG" 2>/dev/null | tr -d '\r' | awk '{print $1}' || true) sleep "$wait_s" - local second - second=$("$ADB" shell pidof "$PKG" 2>/dev/null | tr -d '\r' | awk '{print $1}') + second=$("$ADB" shell pidof "$PKG" 2>/dev/null | tr -d '\r' | awk '{print $1}' || true) if [ -n "$first" ] && [ "$first" = "$second" ]; then echo "PASS: $PKG alive as pid $first after ${wait_s}s"