diff --git a/.claude/skills/android-expert/references/proguard-rules.md b/.claude/skills/android-expert/references/proguard-rules.md index 2adabf4593..9ecf2bf0b1 100644 --- a/.claude/skills/android-expert/references/proguard-rules.md +++ b/.claude/skills/android-expert/references/proguard-rules.md @@ -76,10 +76,8 @@ What does **not** need a keep, and where the temptation usually comes from: ### Attributes ```proguard -# Retraceable stack traces from the uploaded mapping.txt, without leaking the -# class name back through the file name. +# What makes a crash report retraceable. -keepattributes SourceFile,LineNumberTable --renamesourcefileattribute SourceFile # jackson-module-kotlin reads @kotlin.Metadata; R8 requires InnerClasses and # EnclosingMethod alongside Signature. @@ -91,6 +89,36 @@ What does **not** need a keep, and where the temptation usually comes from: jackson-module-kotlin takes parameter names from `@kotlin.Metadata`, not from `MethodParameters`. They were removed; don't add them back. +`-renamesourcefileattribute` is deliberately **not** set, and the reason is not +the usual one. + +Once R8 is minifying it rewrites every class's `SourceFile` to the marker +`r8-map-id-` on its own — there is no rule that restores the original +per-class `.kt` name. Verified by building it both ways: with +`-renamesourcefileattribute SourceFile`, all 24,440 classes report the literal +`"SourceFile"`; without it, they report the marker. So the rule cannot buy +readability, it can only *destroy* the marker — and that marker is the +`pg_map_id` header of the mapping that produced the build, which is what lets a +pasted stack trace name the exact mapping file it needs. + +Two related facts worth knowing before someone tries to "fix" stack traces with +keep rules: + +- **Raw line numbers are no longer source line numbers.** R8 renumbers them so + that one obfuscated line can encode a whole inlined frame stack. This is a + cost of *optimization*, not of renaming — it is new only because the old + blanket `-keepnames` had optimization switched off across the program, which + is the thing Play was flagging. +- **Retrace therefore returns more than the old raw traces did**: it expands + the frames R8 inlined instead of collapsing them into one misleading line. A + one-frame crash can retrace to three. + +The workflow is `scripts/retrace.sh [trace]`, documented in +[`RELEASE_OPS.md` § 7](../../../../RELEASE_OPS.md). Every GitHub Release carries +`amethyst-{googleplay,fdroid}-mapping-.txt.gz`; Play Console needs +nothing because AGP embeds the mapping in the `.aab` under +`BUNDLE-METADATA/com.android.tools.build.obfuscation/proguard.map`. + ### Verifying a change to these rules R8 cannot see reflection, so a wrong keep rule fails **only in a release build, diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index 78bb1aa0f6..5661030b8f 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -1041,6 +1041,34 @@ jobs: "dist/amethyst-googleplay-${TAG}.aab" cp "amethyst/build/outputs/bundle/fdroidRelease/amethyst-fdroid-release.aab" \ "dist/amethyst-fdroid-${TAG}.aab" + + # R8 mapping files — the ONLY way a crash report from this build is + # ever readable again. The release build is minified, so every class + # in every artifact above reports `r8-map-id-` as its source + # file and a renamed class/method; `scripts/retrace.sh ` + # turns that back into real names, files and lines (and expands the + # frames R8 inlined). + # + # Play Console deobfuscates by itself because AGP embeds the mapping + # in the .aab it was given. Nothing else does: a trace from an + # F-Droid, Zapstore, Accrescent or GitHub-APK user is unreadable + # without the matching file, and the mapping only exists on this + # runner. If it is not published here it is gone when the job ends. + # + # Gzipped because the raw text mapping is ~500 MB (~29 MB + # compressed). retrace.sh reads the .gz directly. + for flavor in play fdroid; do + case "$flavor" in + play) name=googleplay ;; + fdroid) name=fdroid ;; + esac + src="amethyst/build/outputs/mapping/${flavor}Release/mapping.txt" + if [ ! -f "$src" ]; then + echo "::error::$src is missing — the release would ship with no way to read its crash reports." + exit 1 + fi + gzip -c "$src" > "dist/amethyst-${name}-mapping-${TAG}.txt.gz" + done ls -la dist # Accrescent does not accept AABs or monolithic APKs — it requires a signed diff --git a/BUILDING.md b/BUILDING.md index 0c5be556fe..1a67b65b82 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -352,7 +352,7 @@ Quartz library in one pipeline. 3. **Wait** for the `Create Release Assets` workflow to finish (~25–30 min). -4. **Verify** — the GH Release should hold **47 assets**: +4. **Verify** — the GH Release should hold **49 assets**: - **14 desktop**, one per matrix leg × format: - macOS arm64: `dmg` (1) - Windows x64: `msi` + portable `zip` (2) @@ -367,8 +367,12 @@ Quartz library in one pipeline. ships no WiX (`windows-latest` has WiX 3.14 preinstalled, which is why the x64 leg gets an MSI). Revisit if that image gains WiX, or if jpackage learns the WiX 4+ `wix build` CLI. - - **13 Android** — 5 Google Play APKs + 5 F-Droid APKs + 2 AABs + the - F-Droid `.apks` set built for Accrescent. + - **15 Android** — 5 Google Play APKs + 5 F-Droid APKs + 2 AABs + the + F-Droid `.apks` set built for Accrescent + **2 R8 mapping files** + (`amethyst-{googleplay,fdroid}-mapping-.txt.gz`). The mappings + are not optional extras: the release build is minified, so without them + no crash report from an APK/`.apks` user can be read. See + [`RELEASE_OPS.md` § Crash reports](RELEASE_OPS.md#7-crash-reports--retrace). - **10 amy** — `tar.gz` (macOS arm64, Linux x64, Linux arm64), `deb` + `rpm` per Linux arch, portable `zip` per Windows arch, and the one arch-independent no-JRE `amy--jvm.tar.gz` for Homebrew-core. diff --git a/RELEASE_OPS.md b/RELEASE_OPS.md index 202e9a2418..6cb754698f 100644 --- a/RELEASE_OPS.md +++ b/RELEASE_OPS.md @@ -130,6 +130,11 @@ Nothing to do beyond pushing the tag. Verify the asset count (BUILDING.md § Verify). macOS is **arm64-only** — there is no Intel DMG, so a single `amethyst-desktop--macos-arm64.dmg` is the expected, correct result. +Two of those assets are the R8 mapping files +(`amethyst-{googleplay,fdroid}-mapping-.txt.gz`). Do not prune them +from old releases — they are the only way to read a crash report from a build +that old (§ 7). + ### Google Play — manual upload 1. Download `amethyst-googleplay-.aab` from the GH Release. 2. Play Console → app `com.vitorpamplona.amethyst` → **Production** (or the @@ -304,7 +309,7 @@ Owner assignments and rotation reminders live with the team (issue tracker). ## 6. Post-release verification -- [ ] GH Release: 47 assets, sizes sane, and the asset-name set matches the +- [ ] GH Release: 49 assets, sizes sane, and the asset-name set matches the previous release (see the `diff` one-liner in BUILDING.md § Release runbook). macOS is arm64-only — do **not** look for an Intel DMG. - [ ] Maven Central: `quartz:` resolves (allow tens of minutes of @@ -325,3 +330,67 @@ Owner assignments and rotation reminders live with the team (issue tracker). see § 4); UnifiedPush still works on an `fdroid` build. If anything ships broken, see [`BUILDING.md` § Incident response](BUILDING.md#incident-response). + +--- + +## 7. Crash reports & retrace + +Release builds are minified **and obfuscated** (they have to be: Play Console +drops apps whose DEX is under 25% optimized or obfuscated out of store surfaces +— see `amethyst/proguard-rules.pro` for the whole story). So a raw stack trace +from a release build looks like this: + +``` +java.lang.IllegalStateException: something blew up + at onh.B(r8-map-id-12c710927a584543dbe1e2e867db95460bc44482efe53283f86798648c1cfc00:7) +``` + +That is not lost information, it is encoded information. Run it back through the +mapping: + +```bash +scripts/retrace.sh amethyst-googleplay-mapping-v1.13.1.txt.gz crash.txt +# or: pbpaste | scripts/retrace.sh amethyst-googleplay-mapping-v1.13.1.txt.gz +``` + +``` +java.lang.IllegalStateException: something blew up + at androidx.compose.foundation.text.input.TextFieldCharSequence.getText(TextFieldCharSequence.kt:58) + at androidx.compose.foundation.text.input.TextFieldState.getText(TextFieldState.kt:146) + at com.vitorpamplona.amethyst.ui.screen.loggedIn.home.ShortNotePostViewModel.onMessageChanged(ShortNotePostViewModel.kt:1727) +``` + +Note that retrace gave back **three** frames where the crash reported one: R8 +had inlined two of them. That is worth internalising — it is the reason a raw +trace's line number cannot be trusted even in the pre-obfuscation builds, where +optimization was already inlining. Retracing is not a tax obfuscation imposed; +it is how you read an optimized build at all. + +**Which mapping?** Never guess. The `r8-map-id-` in the trace *is* the +`pg_map_id` header of the mapping that produced it, so: + +```bash +gh release download -p 'amethyst-*-mapping-*.txt.gz' +zcat amethyst-googleplay-mapping-.txt.gz | grep -m1 pg_map_id +``` + +If the hashes match, that is the right file, full stop. (`googleplay` vs +`fdroid` matters — the two flavors are separate R8 runs with different +mappings.) + +**Per channel:** + +| Where the report came from | What to do | +|---|---| +| Play Console / Android vitals | Nothing. AGP embeds the mapping in the `.aab` (`BUNDLE-METADATA/com.android.tools.build.obfuscation/proguard.map`), so Play deobfuscates automatically. | +| A GitHub issue, Nostr DM, F-Droid, Zapstore, Accrescent | `scripts/retrace.sh` against that release's mapping asset. | +| A build you made locally | `scripts/retrace.sh amethyst/build/outputs/mapping//mapping.txt` | + +`scripts/retrace.sh` downloads the R8 version named in the mapping's own header +from Google's Maven and caches it, so it needs no pinned tooling and keeps +working across AGP bumps. + +**Do not delete mapping assets from old releases.** They are the only copy — +CI's are gone when the job ends, and a mapping cannot be regenerated after the +fact (it would need a bit-identical rebuild, and R8's renaming is not stable +across runs). diff --git a/amethyst/proguard-rules.pro b/amethyst/proguard-rules.pro index 1f225dbea8..38fa407b3c 100644 --- a/amethyst/proguard-rules.pro +++ b/amethyst/proguard-rules.pro @@ -27,12 +27,41 @@ # ----------------------------------------------------------------------------- # Attributes # ----------------------------------------------------------------------------- -# SourceFile + LineNumberTable are what let Play (and `retrace`) turn an -# obfuscated stack trace back into real line numbers via mapping.txt. -# -renamesourcefileattribute replaces the real file name with a constant so the -# class name cannot simply be read back off it. +# These two are what make a crash report readable again. Keep them. +# +# There is no setting that gives readable stack traces *in the raw trace* once +# R8 is minifying, and that is worth being precise about, because it is the +# thing -dontobfuscate used to buy us: +# +# * R8 overwrites every class's SourceFile with the marker +# `r8-map-id-` whatever we do here. Verified by building it both +# ways: with `-renamesourcefileattribute SourceFile` all 24,440 classes +# report the literal "SourceFile"; without it they report the marker. +# There is no rule that restores the original per-class .kt name. +# * R8 renumbers lines even with LineNumberTable kept, because one +# obfuscated line now has to encode a whole INLINED frame stack. In this +# build, line 7 of one method carries three source frames: +# TextFieldCharSequence.getText():58 inlined into TextFieldState.getText() +# :146 inlined into ShortNotePostViewModel.onMessageChanged():1727. A raw +# line number is no longer a source line. +# +# That second point is a cost of OPTIMIZATION, not of renaming, and it is new +# here only because the old `-keepnames class ** { *; }` had optimization off +# program-wide — which is exactly what Play was complaining about. +# +# So the answer is retrace, not a keep rule. And retrace hands back more than +# the old raw traces did: it expands those inlined frames instead of collapsing +# them into one misleading line. See `scripts/retrace.sh` and RELEASE_OPS.md +# § 7. Two things make that painless, and both depend on this file: +# +# * `-renamesourcefileattribute` is deliberately NOT set, so the map-id +# marker survives. A pasted trace then names the exact mapping file it +# needs (the marker is the `pg_map_id` header of that mapping), so there is +# never any doubt about which release a report came from. +# * mapping.txt.gz ships as a GitHub Release asset for every build, so traces +# from F-Droid / Zapstore / Accrescent users are retraceable too — Play +# Console only auto-deobfuscates the AAB it was given. -keepattributes SourceFile,LineNumberTable --renamesourcefileattribute SourceFile # Annotations (jackson-module-kotlin reads @kotlin.Metadata; Jackson mixins and # kotlinx.serialization read their own), generic signatures, and the diff --git a/scripts/retrace.sh b/scripts/retrace.sh new file mode 100755 index 0000000000..752db089ed --- /dev/null +++ b/scripts/retrace.sh @@ -0,0 +1,88 @@ +#!/usr/bin/env bash +# +# Retrace an obfuscated Amethyst stack trace back to real class, method, file +# and line names. +# +# scripts/retrace.sh [stacktrace-file] # trace on stdin if omitted +# +# is the mapping file for the EXACT build the crash came from: +# * mapping--.txt.gz — attached to every GitHub Release +# * mapping.prt — R8's partition map (faster, same content) +# * amethyst/build/outputs/mapping//mapping.txt — a local build +# .txt, .txt.gz and .prt are all accepted. +# +# Picking the right one is not guesswork: since the release build is minified, +# R8 replaces every class's SourceFile attribute with `r8-map-id-`, and +# that hash is the `pg_map_id` on line 6 of the matching mapping file. A trace +# therefore names its own mapping — grep the releases for that id. +# +# The R8 jar that does the work is fetched from Google's Maven at the version +# recorded in the mapping's own header, so this keeps working across AGP bumps +# with nothing to pin. It is cached under ~/.cache/amethyst-retrace/. +set -euo pipefail + +MAP="${1:-}" +TRACE="${2:-}" + +if [ -z "$MAP" ] || [ ! -f "$MAP" ]; then + echo "usage: $0 [stacktrace-file]" >&2 + exit 2 +fi + +CACHE="${XDG_CACHE_HOME:-$HOME/.cache}/amethyst-retrace" +mkdir -p "$CACHE" + +# ---- resolve the mapping to a plain .txt (Retrace cannot read .gz) ---------- +PARTITION=0 +case "$MAP" in + *.prt) + PARTITION=1 + ;; + *.gz) + PLAIN="$CACHE/$(basename "${MAP%.gz}")" + if [ ! -s "$PLAIN" ] || [ "$MAP" -nt "$PLAIN" ]; then + echo "decompressing $(basename "$MAP") ..." >&2 + gunzip -c "$MAP" > "$PLAIN" + fi + MAP="$PLAIN" + ;; +esac + +# ---- fetch the matching R8 ------------------------------------------------ +if [ "$PARTITION" -eq 1 ]; then + # A partition map is a zip; its header is not greppable. Fall back to the + # newest R8 we already have, else ask for an explicit version. + R8_VERSION="${R8_VERSION:-}" + if [ -z "$R8_VERSION" ]; then + R8_VERSION=$(ls "$CACHE"/r8-*.jar 2>/dev/null | sed 's/.*r8-\(.*\)\.jar/\1/' | sort -V | tail -1 || true) + fi + if [ -z "$R8_VERSION" ]; then + echo "error: cannot read the R8 version out of a .prt partition map." >&2 + echo " Set R8_VERSION= (see 'compiler_version' in the matching" >&2 + echo " mapping.txt), or retrace against the .txt.gz instead." >&2 + exit 2 + fi +else + R8_VERSION=$(head -c 4096 "$MAP" | sed -n 's/^# compiler_version: //p' | head -1) + if [ -z "$R8_VERSION" ]; then + echo "error: no '# compiler_version:' header in $MAP — is it really an R8 mapping?" >&2 + exit 2 + fi +fi + +R8_JAR="$CACHE/r8-${R8_VERSION}.jar" +if [ ! -s "$R8_JAR" ]; then + echo "fetching R8 ${R8_VERSION} ..." >&2 + curl -fsSL -o "$R8_JAR.tmp" \ + "https://maven.google.com/com/android/tools/r8/${R8_VERSION}/r8-${R8_VERSION}.jar" + mv "$R8_JAR.tmp" "$R8_JAR" +fi + +# ---- retrace --------------------------------------------------------------- +if [ "$PARTITION" -eq 1 ]; then + exec java -cp "$R8_JAR" com.android.tools.r8.retrace.Retrace \ + --partition-map "$MAP" ${TRACE:+"$TRACE"} +else + exec java -cp "$R8_JAR" com.android.tools.r8.retrace.Retrace \ + "$MAP" ${TRACE:+"$TRACE"} +fi