next-version-code.sh: derive the candidate from the commit, not the build clock
fdroid-repo-ci#6's "Superseded" sections change the rule: the candidate is now the committer date of the commit being built, clamped on both sides, rather than whatever minute the runner happened to pick the job up in. Backwards: rebase, amend and cherry-pick can produce a commit older than one already published, so the clamp keys off the committer date, not the author date -- author date survives a rebase unchanged and would replay a stale, already-superseded number. Forwards: a commit's timestamp is attacker- and typo-controlled and the number only ever rises, so a bad one can never be walked back. A value placed just below the int32 ceiling would still publish and then no later build could ever exceed it -- the app becomes permanently un-updatable. TOLERANCE absorbs honest clock skew and the clamp warns loudly rather than rejecting, since rejecting would block a legitimate build over a few minutes of skew while buying nothing the clamp doesn't already cover. next_version_code.py now takes the committer timestamp as a third argument (Unix seconds, what `git log -1 --format=%ct` prints) instead of reading the system clock, keeping it pure and testable without a git checkout. next-version-code.sh, README.md and action.yml updated to match. Granularity stays minutes: concurrency is handled separately (a workflow-level concurrency group, plus publish.sh's refuse-on-existing), so it is now a pure headroom choice. Tests (test_next_version_code.py, test_next_version_code.sh): a normal commit yields its own minute, a commit older than the published maximum still yields published+1, a future-dated commit within tolerance passes through untouched while one beyond it is clamped and warns, a commit dated near the int32 ceiling does not produce a near-int32 result, and a dedicated property test checks the result always strictly exceeds every versionCode in the fixture across all of the above. Each was verified by injecting the corresponding fault (removing the backwards clamp, removing the forwards clamp, and ignoring the commit's date entirely) and confirming the relevant test failed, then reverting. 98 assertions offline (0 failed, 4 skipped -- pre-existing, live-bucket only), up from 91. Co-authored-by: bit <bit@das-labor.org>
This commit is contained in:
@@ -21,7 +21,7 @@ here.
|
||||
|
||||
| script | what it does |
|
||||
|---|---|
|
||||
| `next-version-code.sh` | print the next monotonic `versionCode` for an application id, read from a repo index -- called before the build, CI and a manual build alike (see "One versionCode rule" below) |
|
||||
| `next-version-code.sh` | print the next monotonic `versionCode` for an application id and a commit's committer date, read from a repo index -- called before the build, CI and a manual build alike (see "One versionCode rule" below) |
|
||||
| `sign-apk.sh` | check alignment, then sign a prebuilt unsigned release APK with a keystore the caller supplies |
|
||||
| `fdroid-update.sh` | run `fdroid update` over a `repo/` directory of already-signed APKs and produce the signed index |
|
||||
| `publish.sh` | upload the resulting repo tree to an S3-compatible bucket -- files first, index last, and refuses to overwrite an already-published APK (see "Publish ordering and safety" below) |
|
||||
@@ -47,7 +47,7 @@ below for what that costs and why it is still the right call here.
|
||||
|
||||
- run: |
|
||||
version_code=$(bash "${{ steps.ci.outputs.path }}/next-version-code.sh" \
|
||||
"$FDROID_REPO_INDEX_URL" "$APPLICATION_ID")
|
||||
"$FDROID_REPO_INDEX_URL" "$APPLICATION_ID" "$(git log -1 --format=%ct)")
|
||||
echo "version_code=$version_code" >> "$GITHUB_ENV"
|
||||
# ... feed $version_code into whatever builds app-release-unsigned.apk
|
||||
|
||||
@@ -94,21 +94,49 @@ variable name.
|
||||
|
||||
Two implementations of "the next version code" is the exact failure mode
|
||||
`next-version-code.sh` exists to avoid, so both CI and a manual build call
|
||||
this one script rather than each computing their own:
|
||||
this one script rather than each computing their own. The candidate is
|
||||
derived from the **committer date of the commit being built**, not the
|
||||
build clock -- deterministic per commit, rather than depending on when a
|
||||
runner happened to pick the job up -- and clamped on **both** sides:
|
||||
|
||||
```
|
||||
candidate = whole minutes since 2026-01-01T00:00:00Z
|
||||
candidate = whole minutes since 2026-01-01T00:00:00Z, of the commit's committer date
|
||||
candidate = min(candidate, minutes(now) + TOLERANCE) # forwards
|
||||
published = highest versionCode already in the repo index for this applicationId
|
||||
result = max(candidate, published + 1)
|
||||
result = max(candidate, published + 1) # backwards
|
||||
```
|
||||
|
||||
Minutes, not seconds or raw Unix time: `versionCode` is a signed 32-bit int,
|
||||
and this scheme does not exhaust it for 4,082 years, against 67 years for
|
||||
seconds and 11 for Unix time. The clamp against `published` exists because
|
||||
minute granularity means two builds started in the same minute would
|
||||
otherwise compute the same number -- and it also covers a clock stepping
|
||||
backwards or an old commit being rebuilt, both of which would otherwise
|
||||
produce a number lower than one a device has already seen.
|
||||
seconds and 11 for Unix time. Granularity is no longer what keeps
|
||||
same-commit rebuilds from colliding, though -- that is a concurrency-group
|
||||
plus refuse-on-existing-APK problem now (see below) -- so minutes stand
|
||||
purely as a wide, cheap headroom choice, not a load-bearing part of the
|
||||
concurrency story.
|
||||
|
||||
**Backwards.** Rebase, amend and cherry-pick can all produce a commit older
|
||||
than one already published. The rule uses the commit's **committer** date,
|
||||
not its author date, specifically because of this: author date survives a
|
||||
rebase unchanged, so a rebased branch would keep re-emitting its original
|
||||
(now stale) timestamp and the backwards clamp would never see it move.
|
||||
Committer date changes on every rewrite, which is what lets `published + 1`
|
||||
actually catch it.
|
||||
|
||||
**Forwards.** A commit's timestamp is attacker- and typo-controlled, and the
|
||||
number this rule produces only ever rises -- so a bad one can never be
|
||||
walked back down. A commit dated a century ahead burns decades of headroom
|
||||
permanently; worse, a value placed just *below* the int32 ceiling is still
|
||||
accepted by Android, still publishes, and then **no later build can ever
|
||||
exceed it** -- the app becomes permanently un-updatable, recoverable only by
|
||||
shipping a new `applicationId` and asking every user to reinstall by hand.
|
||||
`TOLERANCE` only has to absorb honest clock skew between the committer's
|
||||
machine and the runner -- a few hours is generous, a day would still be
|
||||
harmless, since its only job is keeping the number near the present. A
|
||||
commit that lands beyond it is **clamped and a warning is printed, never
|
||||
rejected outright**: rejecting would block a legitimate build over a few
|
||||
minutes of innocent skew, and the clamp already removes the actual harm (a
|
||||
permanently reserved version-code range), so a hard failure would buy
|
||||
nothing further.
|
||||
|
||||
**The clamp is monotonic against *observed* state, not monotonic by
|
||||
construction.** It does not close the race where two jobs start in the same
|
||||
@@ -119,16 +147,24 @@ before it exists. Nothing in `next-version-code.sh` can prevent that.
|
||||
`publish.sh`'s APK-existence check (below) is the other half of this: it
|
||||
cannot stop the wasted build, but it turns the collision from a silent,
|
||||
permanent wrong state into a loud failure at the last moment, when the
|
||||
information is finally reliable.
|
||||
information is finally reliable. Concurrent same-commit builds are now also
|
||||
addressed at the workflow level with a concurrency group
|
||||
(cancel-in-progress) -- measured working on this Gitea instance -- which is
|
||||
an optimization on top of the refusal, not a replacement for it.
|
||||
|
||||
Takes an index-v2.json URL or local file path, and an application id;
|
||||
prints the integer and nothing else, so a caller does
|
||||
`versionCode=$(next-version-code.sh "$URL" "$APPLICATION_ID")`. The URL form
|
||||
needs no credentials, since both this repo and the F-Droid repo it reads are
|
||||
public -- the point of weblib-viewer#16's "buildable when CI is down". Fails
|
||||
loudly (non-zero exit, nothing on stdout) if the index cannot be read; an
|
||||
application id with no entry at all in the index is not an error -- that is
|
||||
simply the first-ever publish for a channel, and yields the bare candidate.
|
||||
Takes an index-v2.json URL or local file path, an application id, and the
|
||||
committer timestamp of the commit being built as Unix seconds (exactly what
|
||||
`git log -1 --format=%ct` prints); prints the integer and nothing else, so a
|
||||
caller does
|
||||
`versionCode=$(next-version-code.sh "$URL" "$APPLICATION_ID" "$(git log -1 --format=%ct)")`.
|
||||
The script takes the timestamp as an argument rather than shelling out to
|
||||
git itself, which keeps it pure and testable -- the caller already knows
|
||||
its own commit. The URL form needs no credentials, since both this repo and
|
||||
the F-Droid repo it reads are public -- the point of weblib-viewer#16's
|
||||
"buildable when CI is down". Fails loudly (non-zero exit, nothing on
|
||||
stdout) if the index cannot be read; an application id with no entry at all
|
||||
in the index is not an error -- that is simply the first-ever publish for a
|
||||
channel, and yields the bare (clamped) candidate.
|
||||
|
||||
## Publish ordering and safety (fdroid-repo-ci#5, fdroid-repo-ci#6)
|
||||
|
||||
@@ -288,7 +324,7 @@ here needs, resolved through the same pin.
|
||||
bash run_tests.sh
|
||||
```
|
||||
|
||||
91 assertions across seven suites offline (~20s on a warm nix store, 4
|
||||
98 assertions across seven suites offline (~20s on a warm nix store, 4
|
||||
deliberately skipped -- see below), plus 13 more that only run against a
|
||||
real bucket:
|
||||
|
||||
@@ -298,12 +334,23 @@ real bucket:
|
||||
entries.
|
||||
* `test_next_version_code.py` -- pure standard library too, testing
|
||||
`next_version_code.py` as a subprocess against fixture index-v2.json files
|
||||
written to a temp dir: no prior build, a lower/equal/higher prior (the
|
||||
clock-went-backwards case), several versions at once, a version entry with
|
||||
no versionCode, an application id missing from the index entirely (not an
|
||||
written to a temp dir: a normal commit yields its own minute, a commit
|
||||
older than the published maximum still yields published+1 (backwards --
|
||||
rebase/amend/cherry-pick), a future-dated commit within tolerance passes
|
||||
through untouched, one beyond tolerance is clamped to roughly now *and
|
||||
warns* (forwards -- a bad clock or a malicious/typo'd date), a commit
|
||||
dated near the int32 ceiling (year 2999) does not produce a near-int32
|
||||
result, several published versions at once, a version entry with no
|
||||
versionCode, an application id missing from the index entirely (not an
|
||||
error), and an unreadable/malformed index (fails loudly, nothing on
|
||||
stdout). "now" is pinned via an undocumented env var seam so the expected
|
||||
candidate is a fixed number.
|
||||
stdout). A dedicated property test also checks, across every one of those
|
||||
scenarios in one place, that the result always strictly exceeds every
|
||||
versionCode already in the fixture -- the property that actually matters,
|
||||
which a future refactor could break while each individual case still
|
||||
looks right. Both the commit's committer date and "now" (used only for
|
||||
the forwards clamp) are pinned independently, the latter via an
|
||||
undocumented env var seam, so every expected number is fixed rather than
|
||||
depending on whatever minute the suite happens to run in.
|
||||
* `test_with_nixpkgs.sh` -- offline; every way the pinning wrapper is
|
||||
supposed to refuse rather than silently degrade (missing pin, malformed
|
||||
pin, too few arguments). Does not exercise the happy path of actually
|
||||
@@ -316,11 +363,14 @@ real bucket:
|
||||
and `fdroid update`. Needs the pinned toolchain to be resolvable (locally
|
||||
cached, or fetched once over the network); nothing beyond that.
|
||||
* `test_next_version_code.sh` -- the real `next-version-code.sh` CLI, through
|
||||
`with-nixpkgs.sh`: argument count, a local index file end to end, a
|
||||
missing application id, a missing/malformed local file, and an
|
||||
unreachable URL (`http://127.0.0.1:1`, the same closed-port trick
|
||||
`test_publish.sh` uses) -- proving the wrapper's argv/exit-code/stdout
|
||||
contract, not just the Python module's arithmetic.
|
||||
`with-nixpkgs.sh`: argument count (now three: index, application id,
|
||||
committer timestamp), a local index file end to end, a first-ever publish
|
||||
for an application id, a far-future committer date clamped with a warning
|
||||
on stderr, a non-numeric committer timestamp refused rather than coerced,
|
||||
a missing/malformed local file, and an unreachable URL
|
||||
(`http://127.0.0.1:1`, the same closed-port trick `test_publish.sh` uses)
|
||||
-- proving the wrapper's argv/exit-code/stdout contract, not just the
|
||||
Python module's arithmetic.
|
||||
* `test_publish.sh` -- argument/credential validation, `--dry-run` command
|
||||
construction (both the default two-command form, files then index, and
|
||||
each of `--files-only`/`--index-only` printing exactly one command, plus
|
||||
|
||||
Reference in New Issue
Block a user