Take the version from the git tag instead of the manifest - #538
Merged
Conversation
versionCode and versionName were two numbers in AndroidManifest.xml that had to be walked forward by hand before tagging, and that were wrong the rest of the time: between releases they describe one that already went out, and a bump commit is a guess at the next. Forgetting one of them is only discoverable at the very end of a release run, when the store refuses a code it has already accepted. The tag is the one place a version cannot be stale, since it does not exist until the version is true. So the repository now holds no version at all. The release workflow hands gradle the tag it was triggered by as -Podr.version, and app/build.gradle derives both halves: v4.8.0 becomes name 4.8.0 and code 40800, two digits per part. A part above 99 is an error rather than a code that collides with the next release - 4.100.0 and 5.0.0 would otherwise both be 50000. Codes up to 204 were counted by hand, so the first derived one is a five digit jump, and that is one way: play only ever accepts a code above the last. A build handed no version is 0.0.0. Nothing reads it - no code in the app looks at its own version, and only what the release workflow builds is ever uploaded. Its code is spelled out as 1 rather than derived, because AGP rejects 0. A dispatched run has no tag to read, so it takes a version input, which is also how a release whose upload half failed gets finished off the branch it was cut from. Without either it can only be a dry run. The laptop path keeps working the same way: fastlane android deployPro version:v4.8.0, or ODR_VERSION in the environment. This drops the versionCode out of AndroidManifest.xml, which f-droid's UpdateCheckData greps for update detection. That metadata needs rewriting regardless - its build recipe seds the java files the kotlin migration and the package rename removed, and it has been stuck at 4.3 since December. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TgR3TTDbVm7HLybtCMDT15
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c00097b44
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Two things the review caught in the release workflow: A versionless dry run put 0.0.0 in -Podr.version rather than leaving the property off, and that name derives to version code 0, which AGP rejects. The fallback only applies when the property is absent, so the one path that is meant to run without a version was the one path that could not build. The step now leaves the output empty and the gradle line omits the property with it. Dispatched against a tag with a different version input, the input won, but the attach job still puts the apk on the release of the tag the run was started from - so the release would end up carrying an apk of a version it is not. On a tag the tag now decides, and an input that disagrees with it fails the run instead of being quietly overridden. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TgR3TTDbVm7HLybtCMDT15
The step had grown three error paths, a normalisation and a regex, and none of it could be run without pushing it - checking the behaviour meant copying the shell into a scratch file and testing that instead, which is a copy that can disagree with what actually runs. It is now .github/scripts/resolve-version.py, next to the service account key script and written for the same reason: fail before the six minutes of setup and building, with a message that names what is wrong. It takes the tag, the version input and what the run publishes, and writes the result to GITHUB_OUTPUT - empty when there is none, so the gradle line keeps leaving the property off entirely rather than passing 0.0.0. Run it by hand to see what a dispatch would build. The step moves behind the checkout it now needs, and takes the tag name and the version input through the environment rather than interpolating them into the run: line. The version code is deliberately not computed there. app/build.gradle derives it and is the one that counts; the script only checks the shape, so that a typo in a dispatched version does not cost the conan setup first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TgR3TTDbVm7HLybtCMDT15
The two remaining run: blocks that were more than a command: the keytool checks and the "is everything signed" pass. Both are about running a tool and reading its exit code, so both stay shell rather than becoming python. verify-keystore.sh takes the same ODR_* variables the build takes, so the check the release runs can be run against a keystore by hand, which is the whole point of it - it exists to say which password is wrong before six minutes of building. verify-signed.sh takes the outputs directory, so it can be pointed at a local build. Being able to run it turned up two things the inline version had: An unmatched glob left the loops iterating over nothing and passing, so a wrong path verified nothing and said so with a "signed:" for every file it did not look at. It now counts what it found first. apksigner exits non-zero both for an unsigned apk and for not being able to run, and only the first was reported - "not signed" is a poor way to learn that the runner has no java. What it said is now part of the message. And one the extraction introduced and the tests caught: grep -q stops at the first match, and the SIGPIPE unzip is left with fails the pipeline under pipefail, so a signed bundle came out unsigned whenever unzip had not finished writing. The listing is read once into a variable now. Left inline: everything that is a fixed sequence without branches - decoding the keystore, the gradle invocation, the fastlane lanes, the udev rules in build_test. A script would only hide what those runs do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TgR3TTDbVm7HLybtCMDT15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
versionCodeandversionNamelived inAndroidManifest.xmland had to be raised by hand before tagging. Between releases that number is wrong: it describes a release that already went out, or guesses the next one. Forgetting it only surfaces at the very end of a release run, asVersion code 203 has already been used— six minutes in, and unrepeatable, since the upload step failing takes theattachjob with it.The tag is the one place a version cannot be stale: it does not exist until the version is true. So the repository now carries no version number at all.
What changes
app/build.gradlederives both halves from-Podr.version(orODR_VERSION):v4.8.0→ name4.8.0, code40800, two digits per part.v4.8and4.8.0are accepted too.4.100.0and5.0.0would both be50000.AndroidManifest.xml. Nothing replaces them there: both spellings reach the merged manifest and the gradle one wins, so a second copy could only ever disagree with the tag.versioninput — which is also how a half-uploaded release gets finished off the branch it was cut from. Without either, it can only be a dry run, and says so before the build rather than after it.fastlane android deployPro version:v4.8.0for the laptop path, as a gradle property rather than an env var, since the daemon captures its environment at startup.Version codes jump to five digits
Codes up to 204 were counted by hand, so the first derived one is
40800. That is one way — Play only ever accepts a code above the last one it saw.Unversioned builds
A build handed no version is
0.0.0. Nothing reads it: no code in the app looks at its own version, and only what the release workflow builds is ever uploaded. Its code is spelled out as1rather than derived, because AGP rejects0(versionCode is set to 0, but it should be a positive integer).F-Droid
This drops the
versionCodethat F-Droid'sUpdateCheckDatagreps out of the manifest, so their auto-update stops finding releases. Their metadata needs rewriting regardless: the build recipe seds.javafiles underat/tomtasche/reader/, which the kotlin migration and the package rename removed, and the listing has been stuck at 4.3 since December. Worth a follow-up MR to fdroiddata either way.Verified locally
-Podr.version=v4.8.0→versionCode="40800",versionName="4.8.0-pro";v4.9.1viaODR_VERSION→40901; no version →1/0.0.0-prov4.100.0,lol,v4.8.0.1,4..8spotlessCheck,testProDebugUnitTest,lintProDebugall pass (noMissingVersionfinding)Does not touch the other half of the problem — the upload step is still not idempotent, so a duplicate code still fails the run and skips
attach. That is a separate change.