ci(android): build every ABI and gate on 16 KB page alignment - #289
Conversation
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
WalkthroughThe pull request adds Android CI and Gradle project configuration. The workflow builds JNI libraries for four ABIs, validates 16 KB alignment and APK contents, assembles a debug APK, and uploads it. The Gradle wrapper is pinned to 8.10. ChangesAndroid CI
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions as GitHub Actions runner
participant CargoNdk as cargo-ndk
participant AndroidGradle as Android Gradle
participant APKChecks as APK validation
GitHubActions->>CargoNdk: Build JNI libraries for four ABIs
CargoNdk-->>GitHubActions: Write native libraries
GitHubActions->>GitHubActions: Check 16 KB ELF alignment
GitHubActions->>AndroidGradle: Assemble debug APK
AndroidGradle-->>GitHubActions: Generate debug APK
GitHubActions->>APKChecks: Validate ABI contents and APK alignment
APKChecks-->>GitHubActions: Pass or fail validation
Suggested reviewers: 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/android.yml:
- Around line 6-18: Add "Cargo.lock" to both the push and pull_request paths
lists in the Android workflow, preserving the existing path filters so Android
CI also runs when only the dependency lockfile changes.
- Around line 76-81: Update the “Build the JNI libraries” step to build all
three required crates, including rustysnes_mobile and rustysnes_monetization,
for every ABI listed in the cargo ndk targets. Also replace any directory-only
validation with assertions that each packaged ABI contains all required .so
files, or remove ABIs that are intentionally unsupported.
In `@CHANGELOG.md`:
- Around line 26-29: Update the prose in the changelog entry to say “two
third-party actions only” instead of “two actions only,” preserving the rest of
the workflow description unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 63c7c316-f62c-45e4-8e42-01b06b25a83b
📒 Files selected for processing (2)
.github/workflows/android.ymlCHANGELOG.md
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: accuracysnes
- GitHub Check: test-light
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{rs,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Chip-behavior changes must update both the chip implementation and the corresponding
docs/<subsystem>.mddocumentation.A chip change must update both the chip implementation and its corresponding
docs/<chip>.mddocumentation in the same change.
Files:
CHANGELOG.md
CHANGELOG.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
User-visible changes must be recorded under the
[Unreleased]section.For the full pull request diff against its base branch, modify
CHANGELOG.mdwhen user-visible behavior changes, including emulator output, frontend features, CLI flags, public APIs, or AccuracySNES cartridge contents. Do not require it for purely internal changes, tests, comments, or CI configuration.
Files:
CHANGELOG.md
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Do not commit or vendor the generatedsnesdev_wiki/mirror; it is gitignored and intended only as a local reference.
Keep commits focused and use Conventional Commits:<type>(<scope>): <subject>, with an imperative subject of at most 72 characters.
Do not use emojis in code, comments, or commit messages.
Before opening a PR, ensure formatting, Clippy, workspace tests, the core embedded build, rustdoc with warnings denied, documentation coverage, and changelog requirements pass.
Ticket completion must be reflected in the relevantto-dos/sprint file.
**/*: Preserve the one-directional crate graph: chip crates must not depend on one another;rustysnes-coreties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keepdocs/STATUS.mdas the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNESv2.0orengine-lineageanchors as project releases.
Files:
CHANGELOG.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
rather than style nits. The markdownlint gate is pinned to v0.39.0 via pre-commit —
do not report rules that version does not have (MD060 in particular).
Files:
CHANGELOG.md
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: Flag third-party actions pinned to a branch rather than a tag or commit SHA, jobs without
a least-privilegepermissions:block, and secrets that could reach the log. The
ci-successjob is the required check and must depend on every gate it claims to
aggregate — a new job that is not in itsneeds:list is a gate nothing enforces.
Files:
.github/workflows/android.yml
🪛 zizmor (1.28.0)
.github/workflows/android.yml
[info] 34-34: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
| Deliberately two actions only, both already pinned in this repo: the NDK comes from the runner | ||
| image's own `sdkmanager` and the JDK and Gradle are preinstalled, rather than adding three | ||
| third-party actions to the supply-chain surface the `v1.26.0` pass tightened. `assembleDebug`, not | ||
| release — signing material is maintainer-only and stays out of CI. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
State the number of third-party actions.
The workflow invokes three actions, including ./.github/actions/rust-setup. Change “two actions only” to “two third-party actions only” to match the workflow.
As per path instructions, “Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes rather than style nits.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` around lines 26 - 29, Update the prose in the changelog entry
to say “two third-party actions only” instead of “two actions only,” preserving
the rest of the workflow description unchanged.
Source: Path instructions
9779f95 to
3ad180a
Compare
There was a problem hiding this comment.
Pull request overview
Adds first-class Android CI coverage to RustySNES by introducing a dedicated GitHub Actions workflow that cross-builds the Android JNI libraries for all supported ABIs, gates 16 KB PT_LOAD alignment for 64-bit targets, and verifies ABI packaging inside a debug APK. This complements the existing iOS workflow by making Android build regressions visible in CI and by committing the Gradle wrapper/config needed for clean-checkout builds.
Changes:
- Add
.github/workflows/android.ymlto build JNI.soartifacts for all four ABIs, enforce 16 KB alignment for 64-bit, assemble a debug APK, and validate ABI presence. - Commit Gradle wrapper +
android/gradle.propertiesto make Android builds reproducible and work from a clean checkout. - Document the Android CI + alignment gate rationale and findings in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
CHANGELOG.md |
Adds a detailed changelog entry describing the new Android CI workflow and the 16 KB alignment gate. |
android/gradlew.bat |
Adds the Windows Gradle wrapper entrypoint. |
android/gradlew |
Adds the POSIX Gradle wrapper entrypoint used by CI (./gradlew). |
android/gradle/wrapper/gradle-wrapper.properties |
Pins the Gradle distribution used by the wrapper (Gradle 8.10). |
android/gradle.properties |
Commits required Gradle/AndroidX settings so Android builds succeed from a clean checkout. |
.github/workflows/android.yml |
Introduces Android CI: NDK install, cargo ndk builds for all ABIs, 16 KB alignment gates, debug APK assembly, and artifact upload. |
Suppressed comments (1)
.github/workflows/android.yml:220
- Same issue as above:
find android -name '*-debug.apk' | head -1can select the wrong APK if multiple debug APKs exist, causing the alignment gate to validate an unintended artifact. Scope the lookup to the known output directory so the gate checks the APK that is actually uploaded.
set -euo pipefail
apk=$(find android -name '*-debug.apk' | head -1)
[ -n "$apk" ] || { echo "::error::no debug APK was produced"; exit 1; }
# Same divisibility test as the jniLibs gate above -- see its comment for why an exact
| pull_request: | ||
| paths: | ||
| - "crates/rustysnes-android/**" | ||
| - "crates/rustysnes-mobile/**" | ||
| - "crates/rustysnes-monetization/**" | ||
| - "crates/rustysnes-gfx-shaders/**" | ||
| - "android/**" | ||
| - "Cargo.lock" | ||
| - ".github/workflows/android.yml" |
| - name: Install cargo-ndk | ||
| run: cargo install cargo-ndk --locked --version ^3 |
| - name: Confirm the APK carries every ABI | ||
| run: | | ||
| set -euo pipefail | ||
| apk=$(find android -name '*-debug.apk' | head -1) |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/gradle/wrapper/gradle-wrapper.properties (1)
1-8: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd
distributionSha256Sumto verify the downloaded Gradle distribution.This file pins
distributionUrltogradle-8.10-bin.zipbut does not setdistributionSha256Sum.validateDistributionUrlchecks only that the URL is well-formed; it does not verify the downloaded bytes. Gradle's own security guidance states to "Set distributionSha256Sum in gradle-wrapper.properties to verify the integrity of the downloaded Gradle distribution" since this "ensures the gradle-X.X-bin.zip file matches the official SHA-256 checksum published by Gradle, protecting your build from corruption or tampering".Without it, both CI and any developer machine will accept a tampered or corrupted Gradle 8.10 binary from a compromised mirror or MITM. Add the checksum from
gradle.org/release-checksumsfor Gradle 8.10.Proposed fix
distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists distributionUrl=https\://services.gradle.org/distributions/gradle-8.10-bin.zip +distributionSha256Sum=<sha256 for gradle-8.10-bin.zip from https://gradle.org/release-checksums/> networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/gradle/wrapper/gradle-wrapper.properties` around lines 1 - 8, Add the distributionSha256Sum property to the Gradle wrapper configuration alongside distributionUrl, using the official SHA-256 checksum published by Gradle for gradle-8.10-bin.zip.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/android.yml:
- Around line 38-39: Add a descriptive name: field to the build job in the
Android workflow, alongside runs-on, so the job is clearly labeled in the GitHub
Actions UI.
- Around line 38-43: Add a job-level timeout-minutes setting to the Android
workflow’s build job, using a 30-minute limit. Keep the existing runs-on,
environment, and steps unchanged.
- Around line 235-255: Separate the per-library presence check from the
alignment loop in the workflow: keep alignment validation restricted to
arm64-v8a and x86_64, but add a distinct loop over all four packaged ABIs for
librustysnes_android.so, librustysnes_mobile.so, and
librustysnes_monetization.so. Preserve the existing missing-library error and
fail behavior.
In `@CHANGELOG.md`:
- Around line 78-79: Update the CHANGELOG wording describing the assembled APK’s
lib/ validation to remove the claim that exactly six 64-bit libraries are
required or that count changes fail. Keep only the behavior implemented by
.github/workflows/android.yml: each .so is checked and the three required
project libraries are verified for every 64-bit ABI.
---
Outside diff comments:
In `@android/gradle/wrapper/gradle-wrapper.properties`:
- Around line 1-8: Add the distributionSha256Sum property to the Gradle wrapper
configuration alongside distributionUrl, using the official SHA-256 checksum
published by Gradle for gradle-8.10-bin.zip.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ecaf6f16-3587-4516-a167-20b116b45d1f
⛔ Files ignored due to path filters (1)
android/gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jarand included byandroid/**
📒 Files selected for processing (6)
.github/workflows/android.ymlCHANGELOG.mdandroid/gradle.propertiesandroid/gradle/wrapper/gradle-wrapper.propertiesandroid/gradlewandroid/gradlew.bat
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: copilot-pull-request-reviewer
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Do not commit or vendor the generatedsnesdev_wiki/mirror; it is gitignored and intended only as a local reference.
Keep commits focused and use Conventional Commits:<type>(<scope>): <subject>, with an imperative subject of at most 72 characters.
Do not use emojis in code, comments, or commit messages.
Before opening a PR, ensure formatting, Clippy, workspace tests, the core embedded build, rustdoc with warnings denied, documentation coverage, and changelog requirements pass.
Ticket completion must be reflected in the relevantto-dos/sprint file.
**/*: Preserve the one-directional crate graph: chip crates must not depend on one another;rustysnes-coreties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keepdocs/STATUS.mdas the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNESv2.0orengine-lineageanchors as project releases.
Files:
android/gradle.propertiesandroid/gradle/wrapper/gradle-wrapper.propertiesandroid/gradlew.batCHANGELOG.mdandroid/gradlew
**/*.{rs,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Chip-behavior changes must update both the chip implementation and the corresponding
docs/<subsystem>.mddocumentation.A chip change must update both the chip implementation and its corresponding
docs/<chip>.mddocumentation in the same change.
Files:
CHANGELOG.md
CHANGELOG.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
User-visible changes must be recorded under the
[Unreleased]section.For the full pull request diff against its base branch, modify
CHANGELOG.mdwhen user-visible behavior changes, including emulator output, frontend features, CLI flags, public APIs, or AccuracySNES cartridge contents. Do not require it for purely internal changes, tests, comments, or CI configuration.
Files:
CHANGELOG.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
rather than style nits. The markdownlint gate is pinned to v0.39.0 via pre-commit —
do not report rules that version does not have (MD060 in particular).
Files:
CHANGELOG.md
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: Flag third-party actions pinned to a branch rather than a tag or commit SHA, jobs without
a least-privilegepermissions:block, and secrets that could reach the log. The
ci-successjob is the required check and must depend on every gate it claims to
aggregate — a new job that is not in itsneeds:list is a gate nothing enforces.
Files:
.github/workflows/android.yml
🪛 zizmor (1.28.0)
.github/workflows/android.yml
[info] 38-38: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
🔇 Additional comments (5)
CHANGELOG.md (1)
96-98: Duplicate of the existing action-count wording comment.The sentence still says “two actions only”. The previous review already requested “two third-party actions only” because the workflow also invokes
./.github/actions/rust-setup..github/workflows/android.yml (1)
44-46: LGTM!Also applies to: 258-264
android/gradle.properties (1)
1-23: LGTM!android/gradlew (1)
1-253: LGTM!android/gradlew.bat (1)
1-95: LGTM!
| build: | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Optional: name the job.
zizmor flags build as an anonymous job definition. Adding a name: key improves readability in the Actions UI but has no functional effect.
🧰 Tools
🪛 zizmor (1.28.0)
[info] 38-38: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/android.yml around lines 38 - 39, Add a descriptive name:
field to the build job in the Android workflow, alongside runs-on, so the job is
clearly labeled in the GitHub Actions UI.
Source: Linters/SAST tools
| build: | ||
| runs-on: ubuntu-latest | ||
| env: | ||
| CARGO_NET_RETRY: "10" | ||
| CARGO_TERM_COLOR: always | ||
| steps: |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Add a job-level timeout-minutes.
The build job has no timeout-minutes. It installs an NDK via sdkmanager, and it runs a Rust and Gradle build. If any step hangs — for example, sdkmanager blocking on an unexpected license prompt — the job runs up to the default GitHub Actions limit before failing. Set an explicit bound, for example timeout-minutes: 30, to fail fast and free the runner sooner.
🧰 Tools
🪛 zizmor (1.28.0)
[info] 38-38: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/android.yml around lines 38 - 43, Add a job-level
timeout-minutes setting to the Android workflow’s build job, using a 30-minute
limit. Keep the existing runs-on, environment, and steps unchanged.
3e0e1d0 to
85c01be
Compare
android/ had Gradle sources and a cargo ndk layout but no workflow at all, so nothing built it and nothing could regress visibly. The new workflow cross-builds rustysnes-android for arm64-v8a, armeabi-v7a, x86_64 and x86; asserts every 64-bit .so's PT_LOAD segments are 16 KB aligned; assembles a debug APK; and checks the APK actually carries each ABI. The alignment gate is the store-facing one. Play requires 16 KB-aligned segments for 16 KB-page devices and a 4 KB-aligned library simply fails to load there. The NDK has defaulted to 16 KB since r27, so this is a REGRESSION gate against an NDK downgrade or a stray linker flag -- the kind of thing otherwise found by a store rejection. It fails closed when the .so glob matches nothing, so a build producing no libraries cannot pass it silently. Two actions only, both already pinned in the committed tree: the NDK comes from the runner image's own sdkmanager, and the JDK and Gradle are preinstalled. Adding setup-ndk/setup-java/setup-gradle would have put three more third-party actions on the surface the v1.26.0 hardening pass just tightened. assembleDebug, not release: signing material is maintainer-only and stays out of CI, per docs/mobile-readiness.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The project had no wrapper, so every build -- CI or local -- used whatever Gradle happened to be installed, and this environment only ever had a cached 8.10 distribution. That is the drift a wrapper removes. gradlew, gradlew.bat and gradle/wrapper/ (jar + properties) are the official artefacts of the 8.10 distribution, generated by 'gradle wrapper --gradle-version 8.10 --distribution-type bin'. The workflow now runs ./gradlew rather than the runner's preinstalled copy, so CI and a developer's machine build with the same Gradle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
First CI run failed with 'sdkmanager: command not found'. ubuntu-latest sets $ANDROID_HOME but does not put the SDK's cmdline-tools on PATH. Uses the full path, asserts $ANDROID_HOME is set, and fails with a directory listing if the binary is not where expected -- so the next runner-image change reports what it found rather than exit 127. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The alignment gate failed on its first run with 4 KB-aligned arm64-v8a and x86_64 libraries -- what Play rejects. The gate was not broken; the libraries were really misaligned. Its own comment claimed 16 KB was the NDK default from r27 onward, which made the check look like the faulty part. Bisected locally instead: with the same NDK r27c, cargo-ndk 3.5.4 emits 0x1000 and 4.1.2 emits 0x4000. The linker invocation decides the alignment, not the NDK version, and the workflow's `--version ^3` pin resolved to 3.5.4. Pass -C link-arg=-Wl,-z,max-page-size=16384 explicitly for the 64-bit ABIs, verified to produce 0x4000 under the failing 3.5.4, so the result does not depend on a tool default a version range cannot promise. The 32-bit ABIs move to their own step and stay 4 KB by design -- listing them together would relink the 64-bit ones without the flag. Both comments that asserted the NDK default are corrected to say what was measured. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The app could not be built from a clean checkout. AGP fails :app:checkDebugAarMetadata outright with "contains AndroidX dependencies, but the `android.useAndroidX` property is not enabled", and android/gradle.properties had simply never been committed. Every Android build this project has done -- including the real APK installed on an AVD in v1.15.0 -- ran on a machine that already had those settings in a user-level ~/.gradle/gradle.properties, so nothing surfaced the gap until CI built from a bare checkout. enableJetifier is explicitly off: it rewrites legacy support-library artifacts and there are none here, so leaving it on would cost build time on every dependency for no effect. The Kotlin daemon heap is raised because Compose otherwise OOMs on a runner rather than failing legibly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The APK carries three libraries per ABI -- librustysnes_android.so, librustysnes_mobile.so and librustysnes_monetization.so -- all built by Gradle's own cargoNdkBuild task. The workflow's standalone pre-build produced only rustysnes-android, and Gradle's task did not inherit the max-page-size flag, so the alignment gate could pass on artifacts that never ship while the ones that do were 4 KB aligned. Found by CodeRabbit on this PR; worse than reported, since the effect is a false pass on the store-facing requirement rather than only missing coverage. Build all three crates in both pre-build steps, set RUSTFLAGS on the Gradle step, and add a second gate over the assembled APK's lib/ -- pinned at exactly six 64-bit libraries so a packaging change that drops one fails instead of passing more quietly. Also add Cargo.lock and crates/rustysnes-monetization/** to the path filters, so a lockfile-only dependency bump still runs this workflow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The APK-level gate failed on `libjnidispatch.so`, and the gate was wrong, not the library. It compared each PT_LOAD Align to 0x4000 for equality, but the requirement is "aligned to at least 16 KB" -- a divisibility property. JNA ships that library at 0x10000 (64 KB), which is valid and stricter. Measured before acting rather than bumping the dependency on a guess: the JNA AARs for 5.15.0/5.16.0/5.17.0/5.19.1 are 0x10000, 0x4000, 0x4000, 0x4000 -- every one of them fine. A version bump would have "fixed" nothing. The test is now `align % 16384 == 0`, checked against real files to accept 64 KB and 16 KB and reject 4 KB. Also replace the hardcoded "expect exactly 6 libraries" assertion, which was simply wrong -- the APK carries 10, two of them third-party -- with a by-name presence check for the three this project builds. A count would need revising whenever a dependency adds a native library. Every library in the APK is alignment-checked, not only ours, because Play's requirement is a property of the shipped package. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JNA 5.15.0 ships libjnidispatch.so 4 KB aligned on x86_64, armeabi-v7a and x86, and 64 KB aligned only on arm64-v8a. The APK therefore carried libraries that violate Play's 16 KB page-size requirement. This corrects the previous commit's message, which claimed a version bump would fix nothing. That was measured on arm64-v8a alone and generalised -- the one ABI where 5.15.0 happens to be fine. Reading all four ABIs of the published AARs: 5.15.0 arm64 0x10000 x86_64 0x1000 armv7 0x1000 x86 0x1000 5.16.0+ arm64 0x4000 x86_64 0x4000 armv7 0x4000 x86 0x4000 5.16.0 is the first release that fixes it; 5.17.0 is the nearest settled patch line after that change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
8ce03b9 to
3aefce4
Compare
Antigravity review (Gemini via Ultra)This PR adds a GitHub Actions workflow for building Android JNI libraries across four ABIs (arm64-v8a, x86_64, armeabi-v7a, x86), gates 64-bit targets on 16 KB ELF page alignment, updates JNA to 5.17.0, and commits the missing Gradle wrapper and project properties. Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
First
v1.30.0item.android/had Gradle sources and acargo ndklayout but no workflow at all — nothing built it, so nothing could regress visibly.What it does
Cross-builds
rustysnes-androidfor all four ABIs, asserts 16 KBPT_LOADalignment on the 64-bit ones, assembles a debug APK, and checks the APK actually carries each ABI.The alignment gate
Play requires 16 KB-aligned segments for 16 KB-page devices; a 4 KB-aligned library simply fails to load there. The NDK has defaulted to 16 KB since r27, so this is a regression gate — it catches an NDK downgrade or a stray linker flag, which would otherwise surface as a store rejection rather than a red build.
It fails closed when the
.soglob matches nothing, so a build that produces no libraries cannot pass it silently — that is the failure mode a naive loop would have.Two actions, both already pinned here
I first wrote this with
setup-ndk,setup-javaandsetup-gradle, then checked the committed tree and found none of them in use — and that four of the five SHAs I had written were invented. Rather than introduce three third-party actions to the surface #276 just tightened, the NDK now comes from the runner image's ownsdkmanagerand the JDK and Gradle are preinstalled. Onlycheckoutandupload-artifactremain, both with pins taken from committed workflows.No
github.event.*interpolation anywhere;pull_requestnotpull_request_target;contents: read;persist-credentials: false.Scope
assembleDebug, not release — signing material is maintainer-only and stays out of CI perdocs/mobile-readiness.md. This does not flip theSTATUSline; it is the engineering half.🤖 Generated with Claude Code
Adds Android CI coverage for four ABIs, 16 KB
PT_LOADalignment, native-library output, and debug APK packaging.The claim is false if any ABI fails to build, produces no native library, violates alignment, or is absent from the APK. The workflow does not provide release signing or change mobile readiness status.
No AccuracySNES dossier assertions or coverage denominator changed. No emulator-core behavior changed.