From f70205a5d44cc286de5dd59dfbd3f9675ebbd2b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 22:03:47 +0000 Subject: [PATCH] build(qr): guard libzxingcpp_android.so like libarti_android.so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #4146 committed libzxingcpp_android.so for all four ABIs but landed it outside both native-library guards main had just built for Arti, so two properties the QR decoder's README claims are not actually true of the APK: * verifyArtiAbis only ever looked for libarti_android.so. An ABI split missing libzxingcpp_android.so — or holding a truncated one, or arm64's copied into x86/ — builds and installs clean, and the scanner then fails to load on that architecture with nothing in the build to catch it. Renamed verifyNativeAbis and driven from a map of committed libraries, so each one is checked on every shipped ABI and the error names the build command to re-run. * keepDebugSymbols excluded only libarti_android.so, so AGP's llvm-strip pass rewrites the QR library on its way into the APK. That makes `unzip -p app.apk lib//libzxingcpp_android.so | sha256sum` a function of whoever built the APK rather than of the committed bytes, which is exactly the comparison tools/zxing-cpp-build exists to make possible. The same PR also added tools/zxing-cpp-build/ANDROID_NDK_VERSION as a second copy of the NDK pin. c2071ee82c had just made :amethyst read ndkVersion straight from tools/arti-build/ANDROID_NDK_VERSION rather than duplicate it ("the two can then never drift"), and ea8326f759 deleted verify-reproducible.sh's private copy of the ABI list for the same reason. build-zxingcpp.sh now reads that one file too and the copy is gone, so a bump moves both native builds and the strip toolchain together. Docs follow: BUILDING.md still described Arti as the only committed .so and named verifyArtiAbis, as did tools/arti-build/README.md. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0134jvyriixNTHST4WRbbqbX --- BUILDING.md | 52 ++++---- amethyst/build.gradle.kts | 137 ++++++++++++++-------- tools/arti-build/README.md | 5 +- tools/zxing-cpp-build/ANDROID_NDK_VERSION | 1 - tools/zxing-cpp-build/README.md | 8 +- tools/zxing-cpp-build/build-zxingcpp.sh | 20 +++- 6 files changed, 138 insertions(+), 85 deletions(-) delete mode 100644 tools/zxing-cpp-build/ANDROID_NDK_VERSION diff --git a/BUILDING.md b/BUILDING.md index 0c5be556fe..5e7aaf68af 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -89,8 +89,8 @@ cd amethyst ## Generated & vendored artifacts -Two build inputs are **generated by tools but committed to the repo**, so a -normal build or release does **not** run either — Gradle just consumes the +Three build inputs are **generated by tools but committed to the repo**, so a +normal build or release does **not** run any of them — Gradle just consumes the checked-in output. You only regenerate them under the specific conditions below, and each has its own guide: @@ -98,34 +98,44 @@ and each has its own guide: |---|---|---|---| | **Material Symbols subset font** | `commonsUI/src/commonMain/composeResources/font/material_symbols_outlined.ttf` | You add/remove a `MaterialSymbol("\uXXXX")` codepoint in `MaterialSymbols.kt`, or bump the upstream font | [`tools/material-symbols-subset/README.md`](tools/material-symbols-subset/README.md) — run `./tools/material-symbols-subset/subset.sh` | | **Arti (Tor) native libs** | `amethyst/src/main/jniLibs/*.so` | You update the pinned Arti version, change the JNI wrapper, or want to reproduce the binaries | [`tools/arti-build/README.md`](tools/arti-build/README.md) | +| **zxing-cpp (QR decoder) native libs** | `amethyst/src/main/jniLibs/*/libzxingcpp_android.so` | You bump `ZXING_CPP_VERSION`, change the JNI wrapper, or want to reproduce the binaries | [`tools/zxing-cpp-build/README.md`](tools/zxing-cpp-build/README.md) | > **Material Symbols is mandatory after icon changes.** The bundled font is a > ~210-glyph subset; a new codepoint that isn't in it renders as tofu (□) at > runtime. Regenerate and commit the `.ttf` alongside the `MaterialSymbols.kt` > change. Reusing an existing codepoint needs no regeneration. -Both tools have their own prerequisites (`fonttools`/`brotli` for the font; a -Rust toolchain + the exact Android NDK revision pinned in -`tools/arti-build/ANDROID_NDK_VERSION` for Arti) documented in their READMEs — -they are **not** required to build Amethyst from the committed sources. +Each tool has its own prerequisites (`fonttools`/`brotli` for the font; a Rust +toolchain for Arti; `cmake` + `ninja` for zxing-cpp; and, for both native +builds, the exact Android NDK revision pinned in +`tools/arti-build/ANDROID_NDK_VERSION`) documented in their READMEs — none of +them are required to build Amethyst from the committed sources. -The NDK half of that Arti pin does reach the ordinary Android build, though. -AGP strips every native library it packages with the NDK's `llvm-strip`, so -`:amethyst` sets `ndkVersion` from `ANDROID_NDK_VERSION` — one revision for the -libraries we build and the ones we merge from dependencies. `libarti_android.so` -is then excluded from that strip step (`packaging.jniLibs.keepDebugSymbols`): -the Cargo release profile already stripped it, and llvm-strip would only rewrite -its `.comment` stamps, so skipping the pass costs no size and lets the `.so` -inside an APK be compared byte-for-byte against the committed, independently -reproducible one. The Rust toolchain stays irrelevant either way; Studio/AGP -fetches the pinned NDK on demand, or pre-install it with +That NDK pin is a single file for the whole repo, not a copy per tool: +`build-arti.sh`, `build-zxingcpp.sh` and `:amethyst`'s `ndkVersion` all read it, +so bumping it moves every native build and the packaging toolchain together and +they cannot drift apart. (It lives under `tools/arti-build/` for history; it is +not Arti's alone.) + +The NDK half of that pin reaches the ordinary Android build. AGP strips every +native library it packages with the NDK's `llvm-strip`, so `:amethyst` sets +`ndkVersion` from `ANDROID_NDK_VERSION` — one revision for the libraries we +build and the ones we merge from dependencies. Both of our own libraries are +then excluded from that strip step (`packaging.jniLibs.keepDebugSymbols`): +they are already stripped by the pinned toolchain, so skipping the pass costs +no size and lets the `.so` inside an APK be compared byte-for-byte against the +committed, independently reproducible one. The Rust toolchain stays irrelevant +either way; Studio/AGP fetches the pinned NDK on demand, or pre-install it with `sdkmanager "ndk;$(cat tools/arti-build/ANDROID_NDK_VERSION)"`. -> **One Arti library per ABI split.** The APK is split four ways (`arm64-v8a`, -> `x86_64`, `armeabi-v7a`, `x86`) and every split needs its own -> `libarti_android.so`; a split without one installs and runs with Tor silently -> unavailable. The `verifyArtiAbis` Gradle task fails the build if the two lists -> drift, and names the `build-arti.sh --target=…` to run. +> **Every ABI split needs its own copy of each library.** The APK is split four +> ways (`arm64-v8a`, `x86_64`, `armeabi-v7a`, `x86`). A split missing +> `libarti_android.so` installs and runs with Tor silently unavailable; one +> missing `libzxingcpp_android.so` installs with the QR scanner broken. The +> `verifyNativeAbis` Gradle task fails the build if a library's ABI list drifts +> from the split list, and names the `build-arti.sh --target=…` / +> `build-zxingcpp.sh --abi …` to run — and rejects a file that is not an ELF +> built for that architecture, which a missing-file check would pass. --- diff --git a/amethyst/build.gradle.kts b/amethyst/build.gradle.kts index 45681f09b1..1f432394be 100644 --- a/amethyst/build.gradle.kts +++ b/amethyst/build.gradle.kts @@ -67,14 +67,27 @@ afterEvaluate { } } -// Every ABI we split the APK for, and therefore every ABI that needs its own -// libarti_android.so under src/main/jniLibs/ (see tools/arti-build/). The two -// lists drifted once: the splits shipped four ABIs while Arti was built for two, -// so the armeabi-v7a and x86 APKs installed and ran with the dependencies' native -// libraries all present (secp256k1's JNI ships every ABI) and Tor alone dead for -// the life of the install. `verifyArtiAbis` below keeps them in step. +// Every ABI we split the APK for, and therefore every ABI that needs its own copy of each +// library we build and commit ourselves under src/main/jniLibs/. The lists drifted once: the +// splits shipped four ABIs while Arti was built for two, so the armeabi-v7a and x86 APKs +// installed and ran with the dependencies' native libraries all present (secp256k1's JNI ships +// every ABI) and Tor alone dead for the life of the install. `verifyNativeAbis` below keeps +// them in step. val shippedAbis = listOf("x86", "x86_64", "arm64-v8a", "armeabi-v7a") +// The libraries that guard covers, and how to rebuild one when it is missing. Both are built +// from source by tools/ rather than pulled prebuilt, so both can go missing the same way — and +// a QR scanner that cannot load is as silently broken on that install as a dead Tor. +val committedNativeLibs = + mapOf( + "libarti_android.so" to { abi: String, triple: String -> + "./tools/arti-build/build-arti.sh --target=$triple" + }, + "libzxingcpp_android.so" to { abi: String, _: String -> + "./tools/zxing-cpp-build/build-zxingcpp.sh --abi $abi" + }, + ) + android { namespace = "com.vitorpamplona.amethyst" compileSdk = @@ -92,10 +105,14 @@ android { // libraries") — three different APKs from the same source, which is // exactly what F-Droid's rebuild verification cannot have. // - // Read straight from the Arti pin rather than copied into the version - // catalog: the two can then never drift, and bumping ANDROID_NDK_VERSION - // (which also means rebuilding the .so) moves the packaging toolchain with - // it. See tools/arti-build/README.md → "Reproducible builds". + // Read straight from the pin rather than copied into the version catalog: + // the two can then never drift, and bumping ANDROID_NDK_VERSION (which also + // means rebuilding the .so files) moves the packaging toolchain with it. + // That one file is the repo's only NDK pin -- tools/arti-build/build-arti.sh + // and tools/zxing-cpp-build/build-zxingcpp.sh read it too, so every + // committed .so is produced and stripped by the same revision. It lives + // under tools/arti-build for history; it is not Arti's alone. See + // tools/arti-build/README.md → "Reproducible builds". ndkVersion = providers .fileContents(layout.settingsDirectory.file("tools/arti-build/ANDROID_NDK_VERSION")) @@ -341,6 +358,17 @@ android { // symbols to drop) and makes that comparison exact. Dependency .so files // are still stripped, with the NDK pinned by ndkVersion above. keepDebugSymbols += "**/libarti_android.so" + + // Same guarantee for the QR decoder, for a different reason. Unlike Arti's, this + // library is *not* currently rewritten by AGP's pass -- verified by running the + // pinned NDK's `llvm-strip --strip-unneeded` over the committed file and getting + // identical bytes -- because tools/zxing-cpp-build strips it with that very same + // llvm-strip, which makes a second pass idempotent. That idempotence is a property + // of one NDK revision, though, and reading it back from the APK should not depend + // on a strip pass staying a no-op across bumps. Excluding it makes + // `unzip -p app.apk lib//libzxingcpp_android.so | sha256sum` match + // src/main/jniLibs by construction, at no size cost. + keepDebugSymbols += "**/libzxingcpp_android.so" } } @@ -381,20 +409,20 @@ android { // only surfaces at System.loadLibrary time on a user's device — where // TorManager's flow swallows the UnsatisfiedLinkError and leaves the status Off // forever. Checked at build time instead, against the same list the splits use. -val verifyArtiAbis = - tasks.register("verifyArtiAbis") { +val verifyNativeAbis = + tasks.register("verifyNativeAbis") { group = "verification" - description = "Checks that every ABI in the APK splits has a libarti_android.so for that architecture." + description = "Checks that every ABI in the APK splits has each committed native library, built for that architecture." val jniLibs = file("src/main/jniLibs") val abis = shippedAbis - // Per ABI: the Rust target triple (so a failure names the exact build - // command) and the ELF identity the library must have — 32/64-bit class - // (header byte 4) and e_machine (bytes 18-19, little-endian on every - // Android ABI we ship). Existence alone is not enough: a truncated file, - // an empty placeholder, or arm64's .so copied into x86/ all load as - // nothing on device, which is the same silent dead Tor this task exists - // to prevent — and unlike a missing file, those look fine in git. + val libs = committedNativeLibs + // Per ABI: the Rust target triple (so an Arti failure names the exact build command) and + // the ELF identity every library must have — 32/64-bit class (header byte 4) and + // e_machine (bytes 18-19, little-endian on every Android ABI we ship). Existence alone is + // not enough: a truncated file, an empty placeholder, or arm64's .so copied into x86/ all + // load as nothing on device, which is the same silent failure this task exists to prevent + // — and unlike a missing file, those look fine in git. val expected = mapOf( "arm64-v8a" to Triple("aarch64-linux-android", 2, 0xB7), @@ -405,48 +433,51 @@ val verifyArtiAbis = doLast { val bitness = mapOf(1 to "32-bit", 2 to "64-bit") - val problems = mutableListOf>() + val problems = mutableListOf>() - abis.forEach { abi -> - val lib = File(jniLibs, "$abi/libarti_android.so") - val want = expected[abi] - val header = ByteArray(20) - val read = if (lib.isFile) lib.inputStream().use { it.read(header) } else -1 + libs.keys.forEach { libName -> + abis.forEach { abi -> + val lib = File(jniLibs, "$abi/$libName") + val want = expected[abi] + val header = ByteArray(20) + val read = if (lib.isFile) lib.inputStream().use { it.read(header) } else -1 - val problem = - when { - !lib.isFile -> "no libarti_android.so" - want == null -> "no expected ELF identity recorded for this ABI" - read < header.size || - header[0] != 0x7F.toByte() || - header[1] != 'E'.code.toByte() || - header[2] != 'L'.code.toByte() || - header[3] != 'F'.code.toByte() -> "not an ELF file (truncated or corrupt)" - header[4].toInt() != want.second -> - "${bitness[header[4].toInt()] ?: "unknown-class"} ELF, expected ${bitness[want.second]}" - else -> { - val machine = (header[18].toInt() and 0xFF) or ((header[19].toInt() and 0xFF) shl 8) - if (machine != want.third) { - "built for ELF machine 0x%02x, expected 0x%02x".format(machine, want.third) - } else { - null + val problem = + when { + !lib.isFile -> "no $libName" + want == null -> "no expected ELF identity recorded for this ABI" + read < header.size || + header[0] != 0x7F.toByte() || + header[1] != 'E'.code.toByte() || + header[2] != 'L'.code.toByte() || + header[3] != 'F'.code.toByte() -> "not an ELF file (truncated or corrupt)" + header[4].toInt() != want.second -> + "${bitness[header[4].toInt()] ?: "unknown-class"} ELF, expected ${bitness[want.second]}" + else -> { + val machine = (header[18].toInt() and 0xFF) or ((header[19].toInt() and 0xFF) shl 8) + if (machine != want.third) { + "built for ELF machine 0x%02x, expected 0x%02x".format(machine, want.third) + } else { + null + } } } - } - if (problem != null) problems += abi to problem + if (problem != null) problems += Triple(libName, abi, problem) + } } if (problems.isNotEmpty()) { throw GradleException( buildString { - appendLine("libarti_android.so is missing or wrong for ${problems.size} ABI split(s):") - problems.forEach { (abi, problem) -> appendLine(" $abi: $problem") } - appendLine("Those APK splits would install with Tor permanently unavailable.") - appendLine("Rebuild them (tools/arti-build/README.md):") - problems.forEach { (abi, _) -> + appendLine("Committed native libraries are missing or wrong for ${problems.size} (library, ABI split) pair(s):") + problems.forEach { (libName, abi, problem) -> appendLine(" $libName / $abi: $problem") } + appendLine("Those APK splits would install with that library permanently unavailable.") + appendLine("Rebuild them:") + problems.forEach { (libName, abi, _) -> val triple = expected[abi]?.first ?: "" - appendLine(" ./tools/arti-build/build-arti.sh --target=$triple") + val rebuild = libs[libName]?.invoke(abi, triple) ?: "" + appendLine(" $rebuild") } append("…or drop the ABI from `shippedAbis` in amethyst/build.gradle.kts.") }, @@ -455,7 +486,9 @@ val verifyArtiAbis = } } -tasks.named("preBuild") { dependsOn(verifyArtiAbis) } +tasks.named("preBuild") { + dependsOn(verifyNativeAbis) +} // androidx.appfunctions-compiler runs in a per-module mode by default, // emitting only the dispatcher Kotlin code. The aggregator that builds diff --git a/tools/arti-build/README.md b/tools/arti-build/README.md index 9d9987a064..bf29ccabb1 100644 --- a/tools/arti-build/README.md +++ b/tools/arti-build/README.md @@ -196,8 +196,9 @@ The ABI list therefore lives in three places that must agree: `splits.abi` in `amethyst/build.gradle.kts`, `targets` in `rust-toolchain.toml`, and `TARGETS` in `build-arti.sh`. (`verify-reproducible.sh` has no copy of its own — it asks `build-arti.sh --print-abis`, so it can never hash a different set than the one -it just rebuilt.) The `verifyArtiAbis` Gradle task, wired into `preBuild`, fails -the build when an ABI split has no `libarti_android.so` **or** has one that is +it just rebuilt.) The `verifyNativeAbis` Gradle task, wired into `preBuild`, fails +the build when an ABI split is missing any committed native library +(`libarti_android.so`, `libzxingcpp_android.so`) **or** has one that is not an ELF of that architecture — a truncated file or arm64's library copied into `x86/` loads as nothing on device, exactly like a missing one, and unlike a missing one it looks fine in `git status`. diff --git a/tools/zxing-cpp-build/ANDROID_NDK_VERSION b/tools/zxing-cpp-build/ANDROID_NDK_VERSION deleted file mode 100644 index e7aa49e341..0000000000 --- a/tools/zxing-cpp-build/ANDROID_NDK_VERSION +++ /dev/null @@ -1 +0,0 @@ -30.0.16248370 diff --git a/tools/zxing-cpp-build/README.md b/tools/zxing-cpp-build/README.md index 68fea2ad38..48d1426725 100644 --- a/tools/zxing-cpp-build/README.md +++ b/tools/zxing-cpp-build/README.md @@ -30,8 +30,10 @@ verify the binaries, bump the zxing-cpp version, or change the build flags. ``` Prerequisites: `git`, `cmake`, `ninja`, and the exact NDK revision in -[`ANDROID_NDK_VERSION`](ANDROID_NDK_VERSION) — which is deliberately the same revision -`tools/arti-build` pins, so one NDK install serves both native builds. +[`tools/arti-build/ANDROID_NDK_VERSION`](../arti-build/ANDROID_NDK_VERSION). That is the +repo's single NDK pin, not a copy: `build-zxingcpp.sh`, `build-arti.sh` and `:amethyst`'s +`ndkVersion` all read that one file, so one NDK install serves every native build and the +three can never drift apart. ## Reproducible builds @@ -39,7 +41,7 @@ Five things have to be fixed, and each is: | Source of non-determinism | Pinned by | |---|---| -| compiler + linker version | [`ANDROID_NDK_VERSION`](ANDROID_NDK_VERSION); `build-zxingcpp.sh` refuses any other revision | +| compiler + linker version | [`tools/arti-build/ANDROID_NDK_VERSION`](../arti-build/ANDROID_NDK_VERSION); `build-zxingcpp.sh` refuses any other revision | | upstream source | [`ZXING_CPP_VERSION`](ZXING_CPP_VERSION), cloned at that tag and nothing else | | absolute paths baked into `__FILE__`, assertions, debug records | `-ffile-prefix-map` / `-fdebug-prefix-map` in [`repro-env.sh`](repro-env.sh) | | timestamps | `SOURCE_DATE_EPOCH`, derived from the pinned tag's commit rather than from build time; `__DATE__`/`__TIME__` redacted | diff --git a/tools/zxing-cpp-build/build-zxingcpp.sh b/tools/zxing-cpp-build/build-zxingcpp.sh index a1b253ab14..50b45f9df6 100755 --- a/tools/zxing-cpp-build/build-zxingcpp.sh +++ b/tools/zxing-cpp-build/build-zxingcpp.sh @@ -13,15 +13,22 @@ # ./build-zxingcpp.sh --out DIR # write .so somewhere else (verify uses this) # # Prerequisites: git, cmake, ninja, and the exact NDK revision in -# ANDROID_NDK_VERSION. Any other revision is refused — it would change the -# output bytes, which is the whole point. +# tools/arti-build/ANDROID_NDK_VERSION. Any other revision is refused — it +# would change the output bytes, which is the whole point. set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" ZXING_VERSION="$(tr -d '[:space:]' < "$SCRIPT_DIR/ZXING_CPP_VERSION")" -NDK_VERSION="$(tr -d '[:space:]' < "$SCRIPT_DIR/ANDROID_NDK_VERSION")" +# One pin for the whole repo, deliberately not a copy of our own. :amethyst +# already reads this same file for `ndkVersion` (the NDK that strips whatever +# lands in src/main/jniLibs), so a second copy here could only ever be a way +# to disagree with the toolchain that packages the .so we produce — the exact +# byte-level drift the pin exists to prevent. Bumping it rebuilds both +# libarti_android.so and libzxingcpp_android.so, which is correct: they must +# be built and stripped by one NDK. +NDK_VERSION="$(tr -d '[:space:]' < "$PROJECT_ROOT/tools/arti-build/ANDROID_NDK_VERSION")" # Canonical build path. Codegen and link ordering can key on the real build # directory even with path remapping in place, so everyone builds here or @@ -33,9 +40,10 @@ OUTPUT_DIR="$PROJECT_ROOT/amethyst/src/main/jniLibs" LIB_NAME="libzxingcpp_android.so" MIN_SDK_VERSION=26 -# Every ABI the app splits on. Unlike Tor — an optional feature that ships on -# two ABIs — a QR scanner that does not load is a broken core feature, and the -# decoder this replaced was pure Java and worked everywhere. +# Every ABI the app splits on. A QR scanner that does not load is a broken core +# feature — the decoder this replaced was pure Java and worked everywhere — so +# every split gets one, and :amethyst's verifyNativeAbis fails the build if one +# is missing or built for the wrong architecture. ABIS=(arm64-v8a armeabi-v7a x86 x86_64) while [ $# -gt 0 ]; do