8727fccba1ddff7be1ca2b5ce0a7e25c6d967fee
11
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  JMR-devandClaude Opus 5
|
8727fccba1 |
Declare read-only permissions on the status-check workflow
CodeQL flagged the new static-analysis job for relying on the repository's default GITHUB_TOKEN scope. Fair, and the repo already holds the opposite opinion elsewhere: build.yml's release job spells out contents: write with a comment saying the token's reach should be visible at the point of use. Set at workflow level rather than on the one job that was flagged, because none of these four write anything -- they read the code, build it and attach reports. It also means a repository default that widens later cannot quietly widen these jobs with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
fd6e5325cf |
Raise Kotlin to 2.4.10 so the bytecode can join the toolchain on Java 25
The previous commit settled for Java 24 everywhere because Kotlin 2.2.10 refuses jvmTarget 25. That was the wrong constraint to accept, for two reasons. The first is that 24 turned out to be unbuyable. Adoptium's repository carries 8, 11, 17, 21, 25 and 26 -- no 24, because it is a non-LTS that went end of life in July 2025. The builds passed only because Gradle quietly auto-provisioned 24.0.2+12 through foojay, and .idea/misc.xml had been pointed at a temurin-24 that cannot be installed. A toolchain nobody can install is not pinned, it is lucky. The second is that the cap was never on the toolchain at all. Kotlin's ceiling applies to jvmTarget -- the bytecode -- and the JDK running the build is a separate axis. Conflating them is what steered this at 24 in the first place. So the fix is the one the sibling repo already uses: put KGP on the root buildscript classpath, where AGP's built-in Kotlin picks it up instead of the 2.2.10 it bundles. Kotlin 2.4.10 supports jvmTarget through 26, which lifts the ceiling above the toolchain rather than under it. The Compose compiler plugin is versioned in lockstep and reads the same catalog entry, so the two cannot drift, and the module now applies both by id() because they come from the classpath rather than from plugin resolution. Checked rather than assumed, since a silent downgrade would look identical to success: compiled classes report major version 69, which is Java 25. D8 dexes them, R8 minifies them, and ktlint, detekt, lint, the unit tests and the androidTest compile are all green on top. 25 is the right landing place independent of all this: it is LTS, it is in the Adoptium repository, and temurin-25-jdk is already installed here -- so the daemon runs on a real system JDK rather than a provisioned copy of an unpatched one. Two catalog plugin aliases went with it. android-application and kotlin-compose now resolve from the buildscript classpath, so leaving aliases behind would have left two entries that read like the source of truth and control nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
3841f58c74 |
Put the whole toolchain on Java 24, and take Gradle to 9.7.1
Java was scattered across four numbers that nobody had chosen together: the
daemon ran on 25 (pinned in gradle-daemon-jvm.properties), CI installed 17, the
IDE was set to 25, and the app compiled to 17 bytecode. Now all four say 24.
24 rather than 25 because 25 is not reachable end to end. Kotlin 2.2.10 refuses
jvmTarget 25 outright -- "available targets are 1.8 ... 23, 24" -- so the app's
bytecode could never have joined a 25 toolchain, and "everything on the same
version" would have stayed false in the one place it is hardest to notice. 24 is
the highest number all four can actually hold. Checked, not assumed: D8 dexes
Java 24 class files, and R8 full mode minifies them, so the shipped artifact
builds on this too.
Floating where floating is native:
- java-version: '24' -- setup-java resolves the newest 24.x at run time.
- toolchainVersion=24 -- Gradle reports it as "Compatible with Java 24, any
vendor", and provisions whatever 24.x it finds or downloads.
The Gradle wrapper deliberately does NOT float, because it cannot: distributionUrl
names one archive and distributionSha256Sum is the checksum of that exact file.
That pairing is the wrapper's integrity check, and it is the same reasoning the
workflows already apply to action SHAs. Set via `./gradlew wrapper`, not by hand,
so the checksum matches the URL.
Dependencies were audited against Google Maven and Maven Central rather than
guessed at, and almost everything was already current: AGP, the Compose BOM,
core-ktx, activity, lifecycle, navigation, work, datastore, media3, room,
documentfile, annotation, espresso, androidx-junit, junit and ktlint are all at
their newest stable. Only two had moved -- detekt to 2.0.0-alpha.6 and JaCoCo to
0.8.15 -- and both are here.
Kotlin stays at 2.2.10 and that is now recorded as a verified fact rather than a
warning: the AGP 9.3.1 POM declares kotlin-gradle-plugin 2.2.10 at runtime scope,
which is what AGP's built-in Kotlin actually compiles with. Android lint suggests
2.4.10 and taking that suggestion breaks the build unless KGP is also forced onto
the root buildscript classpath. agp, kotlin and ksp move together or not at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
JMR-devandClaude Opus 5
|
65a94b4ec1 |
Clear the 35 findings the new tools reported
detekt found 29 and Android lint 6, on a codebase neither had ever seen. Each
one was either fixed or relaxed with the reason written next to it; nothing was
suppressed to make the build quiet.
Fixed, because the tool was right:
- ConversionDependencies constructs Media3Engine, which is @UnstableApi, and
was not marked. Every other type here that touches Media3 propagates the
marker rather than swallowing it with @OptIn, so this one does too. Lint
was the only thing that had ever noticed.
- MediaProbe converted microseconds to milliseconds with a bare 1000, twice,
in a file that also handles a seconds-based duration from a different API.
US_PER_MS and MS_PER_SECOND now say which is which -- that confusion is a
real bug source in media code, not a style question.
- Foreground service types compared SDK_INT against 34 and 35 as raw ints
while the doc comment above spelled the version names out. VERSION_CODES
says it in the code.
- take(3) is a product decision about how many alternatives an error offers.
It means nothing until it is named; MAX_SUGGESTIONS does.
- setProgress(100, ...) is a percentage max, now PERCENT_MAX.
Relaxed, because the rule did not fit:
- The model package is excluded from ReturnCount and CyclomaticComplexMethod
ONLY. It is the decision layer: ConversionRouter.route scores 17 because
the app can give 17 distinct answers to "which engine, and why", each with
its own user-visible reason, and route's own comment records that their
ORDER decides which message is shown. Counting those as complexity measures
how many answers exist, not how hard the code is to follow. Everything else
-- LongMethod, NestedBlockDepth, ComplexCondition -- still applies there.
- A flat `when` used as a lookup table scores a point per entry, so
MediaProbe's demuxer-name to Container map read as complexity 21 with no
nesting and no state. ignoreSingleWhenExpression is the rule's own answer.
- TooGenericExceptionCaught off. MediaProbe, ConversionWorker and ConcatWorker
sit in front of native code that reports a malformed file as anything from
IllegalArgumentException to a bare RuntimeException, undocumented.
Enumerating that list means guessing, and a wrong guess crashes the app on a
file it could have reported as unreadable. SwallowedException stays on, so
these still have to log and handle.
- SI thresholds in the byte formatter, via ignoreNumbers. Each literal sits on
the line with the unit string it belongs to; BYTES_PER_MB would need a Long
and a Double and say nothing the line does not.
- allowedFunctionsPerObject, which the first pass simply missed.
Lint's three version-freshness nags are off. They do not describe this code --
they go red the day someone else publishes a release, which turns a PR red for
something its author cannot see in their diff, and they want the network at
lint time. Upgrades here are deliberate; Kotlin in particular is pinned to AGP's
bundled KGP and is not free to follow the newest release.
UsableSpace is informational rather than disabled, because it is a real finding
that this commit is choosing not to act on. hasSpaceFor reads File.usableSpace,
which ignores reclaimable cache, so the app can refuse a conversion it had room
for. StorageManager.getAllocatableBytes is the better answer, but it changes
when a job is rejected and can throw -- a behaviour change to a safety check,
which deserves its own commit and its own test rather than a drive-by here.
informational keeps it in every lint report instead of hiding it.
Also adds the CI gate and a CLAUDE.md. Coverage is reported and not gated: the
measured baseline is 31% of lines, which is exactly why LibreMail's 0.84 floor
was evidence about LibreMail and not a number to copy.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
JMR-devandClaude Opus 5
|
4e6fe6b75a |
Drop API 37 from the E2E matrix and write down why
The android-37.0 emulator image crash-loops surfaceflinger inside its own gralloc mapper: RegionSamplingThread calls GraphicBuffer::lock, which reaches GoldfishMapper::readFromHost, which asserts that the host has not negotiated ReadColorBufferDma. It has, so surfaceflinger aborts, restarts, and aborts again. Nothing this app does can survive that, and it reproduces on a GitHub runner under swiftshader_indirect and on a workstation under -gpu host alike. There is no ATD image at android-37.0 to fall back to, and -feature -GLDMA is accepted by the emulator but does not prevent the assertion. Correcting the previous commit, which is already pushed so its message stands: ram-size was not the cause of that failure. Setting it did move the job from failing at install to failing during the test run, which is how the real crash became visible, but at 2560M the guest had 1.5 GB free when it died. The setting is kept because the emulator's own floor varies by API level -- 2048M at 33, 2560M at 34 to 36 -- and pinning it makes the matrix uniform. Also corrected: a comment claiming this could not be reproduced locally. It can, and the local crash was the same one all along. Dropped the dmesg probe. adb shell is not root, so klogctl is denied and it only ever printed a permission error -- which a later reader would reasonably misread as "no OOM kills". docs/api-37-emulator-crash.md carries the evidence, the ruled-out fixes, the reproduction, and how to file it upstream, so re-adding the row later starts from what is already known rather than from scratch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
2fe1aa9f9c |
Stop the memory probe from being able to fail the run
The probe line runs before the tests and its exit status is grep's, so a run where adb returned nothing would have exited 1 on the first line and reded the job before Gradle started -- on all five levels, four of them currently green. The action passes no ignoreReturnCode, so exec throws straight into setFailed. A diagnostic must never be the thing that turns a run red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
a79ff62b61 |
Give the API 37 emulator the RAM every other level already gets
API 37 was the only red job in the matrix, and the last two fixes each corrected a real problem only to reveal the next one. This is the cause of the third failure. The emulator raises an undersized guest to 2560M on its own, but only for API levels it recognises, and it does not recognise "37.0". Comparing the two CI logs from the same emulator binary (37.1.11.0) shows the asymmetry directly: the API 36 job logs "Increasing RAM size to 2560MB" and the API 37 job has no such line. So four levels were quietly running at 2560M while API 37 ran at the pixel_6 default of 1536M, lost system_server partway through installing the 82 MB APK, and surfaced it as "Can't find service: package". 2560M is not a guess at a sufficient value -- it is the value the other four levels already pass at, so this makes the matrix uniform rather than introducing a fifth configuration. Verified that the setting actually lands: the action appends hw.ramSize to a config.ini that already has one from the profile, so the fix only works if the later key wins. Appending a distinctive 3072M to an API 36 AVD produced MemTotal 3047924 kB and suppressed the automatic bump, confirming it does. This failure cannot be reproduced locally -- API 37 will not boot on a workstation under either GPU mode, aborting surfaceflinger in the goldfish mapper under -gpu host and segfaulting the emulator under swiftshader_indirect -- so the job now reports guest memory on every run and dumps OOM kills and native crashes on failure. That makes the next run conclusive either way instead of producing another bare "Can't find service: package". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
5b47764c70 |
Build only the emulator's own ABI for instrumented tests
API 37 got as far as running the suite this time and then failed to install: 'package install-create ... -S 117817978' java.io.IOException: Requested internal only, but not enough space The 117 MB debug APK did not fit on the emulator's data partition. The DELETE_FAILED_INTERNAL_ERROR that followed was the same exhaustion, not a second problem. It surfaced on API 37 because that system image is the largest and leaves the least free userdata. The margin was thin at every level, so this was never really an API 37 bug -- the others were simply further from the edge and would have caught up as the APK grew. Roughly half that APK is arm64-v8a FFmpeg libraries that an x86_64 emulator can never load. abiFilters is now overridable, so a test run builds only what it will execute: 114 MB becomes 80 MB. Release builds ignore the property and still ship both ABIs, so nothing about what gets distributed changes. disk-size is raised to 8G for every level rather than only the one that failed, since fixing just API 37 would leave the rest waiting their turn. Verified locally on an API 36 emulator with an x86_64-only APK: 40 instrumented tests, 0 failures, and the installed APK contains lib/x86_64 only. 66 unit tests still pass, and a release build still carries both ABIs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
2d2687aa3c |
Pin CI actions to commit hashes and fix the API 37 emulator run
The API 37 job failed after 23 seconds, before an emulator ever started. The CI log names the cause exactly: sdkmanager --install 'build-tools;37.0.0' platform-tools 'platforms;android-37' Warning: Failed to find package 'platforms;android-37' There is no platforms;android-37. The release is published as android-37.0, alongside 37.1 and the 37.2 betas. The earlier attempt to fix this with system-image-api-level was aimed at the wrong package: that input only names the system image, while the platform is installed from api-level directly. Setting api-level to 37.0 resolves all three packages, and build-tools is a hardcoded constant in the action rather than derived from api-level, so it is unaffected. A separate label field keeps the job name reading "API 37". Every action is now pinned to a commit hash with its release in a trailing comment. A tag is mutable: the owner can repoint v4 at new code whenever they like, so a tag reference amounts to running whatever that repository contains tomorrow. Each hash was verified to resolve to the tag its comment claims, because a wrong hash is worse than a tag -- it looks deliberate. Versions moved a long way in the process: checkout v4 -> v7.0.1, setup-java v4 -> v5.7.0, upload-artifact v4 -> v7.0.1. gradle/actions is gone rather than upgraded. Its v6 release moved the caching component closed-source and states that upgrading accepts Gradle's Terms of Use for it. That has no bearing on the project's own licence -- a CI tool is never combined with or distributed alongside the app, unlike the FFmpeg libraries that make the APK GPL -- but it is a component in the build path that cannot be audited or forked. Gradle now runs through the committed wrapper, which verifies its own distribution against distributionSha256Sum, and caching is a handful of lines of actions/cache. The rest of the matrix passed on this run: API 33, 34, 35 and 36 all green, along with the unit tests and the FFmpeg archive check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
128763e99c |
Commit the FFmpeg binary so test runs stop depending on a rebuild
CI rebuilt FFmpeg on every cold cache, which made results ambiguous: a red run could mean the code was broken or that a forty-minute cross-compile of FFmpeg, x264, x265 and SVT-AV1 had hiccuped. Those are not the same signal, and only one of them is worth a developer's attention. The archive is now checked in under bin/, so a failing run points at code. It also removes roughly forty minutes from a cold run and lets a fresh clone build without a container toolchain. bin/README.md records provenance -- upstream tag, FFmpeg version, NDK, ABIs, SHA-256 and the full configure line read back out of the shipped libavutil -- so the binary is auditable rather than opaque. The recipe in tools/ffmpeg remains the authority: this archive is its output, and is also what satisfies the GPL corresponding-source obligation. The status check is now seven independent runners: one validating the archive, one for the JVM tests, and one per API level from 33 to 37. The FFmpeg job verifies rather than builds. It asserts native libraries are present for both ABIs and that every one is 16 KB aligned, which is a Play requirement that is easy to lose in a rebuild and expensive to discover at submission. Checking for file existence alone would not do: a Git LFS pointer checked out without LFS passes that and then surfaces as an obscure linker error much later. It is a separate job rather than a step in each emulator run so a bad archive reports once, clearly, instead of five confusing emulator failures. build.yml no longer builds FFmpeg either, and keeps only its post-merge and release duties. Two costs, deliberately accepted. The repository goes from about 1 MB to 35 MB, and every future rebuild adds another 35 MB blob to history permanently, so bin/README.md says to regenerate only when the FFmpeg version or the configure flags actually change. And F-Droid's scanner flags checked-in native libraries, so submitting there needs a scandelete entry for bin/ -- noted in bin/README.md, and nothing prevents a from-source build. Verified against the relocated archive: 66 unit tests, and 40 instrumented tests on an API 36 emulator, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
54d6e9c57b |
Add a pull-request status check across API 33-37
Runs the JVM tests and the instrumented suite on every pull request to main, with one emulator job per supported API level. The matrix is the whole range rather than a single level because the foreground-service type differs across it -- none below 34, dataSync at 34, mediaProcessing from 35 -- so testing one level would leave two thirds of that branch unexercised. Running the range locally is what caught a test that had baked in an assumption about the host's encoders. API 37 needs its image level stated separately. It is published as android-37.0, not android-37, so a plain integer resolves to nothing and the image download silently finds no package. FFmpeg is built once and shared. The AAR is not committed -- 35 MB of native code, and F-Droid strips checked-in binaries -- but every job needs it, since the app compiles against it and the instrumented tests exercise it for real. Building it is a full cross-compile of FFmpeg, x264, x265 and SVT-AV1, so it is cached on the contents of tools/ffmpeg, which is what actually determines the output. The job also asserts the AAR carries native libraries for both ABIs: a truncated or stub archive would otherwise pass a file-exists check and send the matrix off to fail confusingly five times over. fail-fast is off. Knowing whether a failure is universal or specific to one API level is most of the diagnosis. build.yml no longer runs on pull requests. It triggered on every PR with no branch filter, so both workflows would have run, and its unit job falls back to a stub AAR -- a weaker check that could mask a compile break the real one would catch. It keeps its post-merge and release duties. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |