build(android): fix version stamping (versionName + versionCode)#8865
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates local/sideload Android build recipes to derive and propagate an accurate app version (instead of relying on the stale hardcoded version: literal in pubspec.yaml), aligning Flutter’s versionName with the Go common.Version ldflag for telemetry/support diagnostics.
Changes:
- Outside GitHub Actions CI, default
APP_VERSIONfromscripts/ci/version.sh generate nightly(with pubspec fallback). - Pass
--build-name=$(APP_VERSION_PUBSPEC)to Android Flutter build targets so AndroidversionNamefollows the resolved version. - Add Makefile documentation explaining the CI vs local version derivation behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
myleshorton
changed the title
Local and sideload Android builds (`make android-apk-release` / `-aab-release`, which internal testers run directly) stamped their version straight from the hardcoded `version:` literal in pubspec.yaml — currently `9.0.28` — while go.mod resolves the actual dependencies (kindling, radiance, sing-box) to their current versions at build time. The result: builds containing current 9.1.x-era code were labeled `9.0.28`, mislabeling them in telemetry (SigNoz `service.version`) and in support tickets (Freshdesk `cf_client_version`), and conflating internal dev builds with real 9.0.28 production users. The version is only corrected in CI: release.yml rewrites pubspec.yaml via scripts/ci/version.sh before building. Any build that doesn't go through that rewrite — i.e. every local/sideload build — trusted the stale literal. Because `version.sh generate` only increments from the highest existing tag (and `validate` refuses to go backwards), a successful overwrite could never produce the hardcoded `9.0.28`; the value persisted precisely because the overwrite never ran on these paths. Fix: - Outside CI, default APP_VERSION from `version.sh generate nightly` (real line + short SHA + timestamp), falling back to the pubspec read only if that yields nothing (e.g. a checkout with no tags). Gated on GITHUB_ACTIONS so CI keeps trusting the rewritten pubspec — CI behavior is byte-for-byte unchanged. - Pass `--build-name=$(APP_VERSION_PUBSPEC)` to the three Android Flutter recipes (android-debug, android-apk-release, android-aab-release) so Flutter's versionName comes from the resolved version instead of reading pubspec directly. This makes both version channels consistent for local builds — Flutter versionName (-> Freshdesk cf_client_version) and the Go common.Version ldflag (-> SigNoz service.version and the X-Lantern-App-Version header) — and makes sideload builds self-identifying via the embedded commit SHA. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tructor `new Date(2015, 1, 1)` is the deprecated java.util.Date(year, month, day) constructor: `year` is years-since-1900 and `month` is 0-indexed, so it actually meant Feb 1 of year 3915, not 2015-01-01. `now - start` was therefore a large negative number that wrapped on the (int) cast to a positive ~5.3e8. It stayed monotonic by luck (start is constant, so it still ticks +1/sec), so versionCode kept increasing and upgrades worked — but the value was meaningless and relied on signed-int overflow. Replace with whole seconds since a fixed epoch (no deprecated Date). Origin is 2009-01-01, chosen so today's value (~5.5e8) stays just above the previously shipped sequence — a true 2015-01-01 origin would compute ~3.6e8, LOWER than shipped codes, which Play/Android would reject and break upgrades — with ~50 years of headroom before 2^31. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
myleshorton
force-pushed
the
fisk/android-version-stamping-fix
branch
from
e9ff534 to
2e15bfc
Compare
… (PR review) Addresses Copilot review on PR #8865: - `APP_VERSION ?=` left the variable recursively expanded, so every $(APP_VERSION) reference re-ran $(shell …). Since version.sh embeds a timestamp, the value could drift between recipe invocations separated in wall-clock time (e.g. the deb/rpm/arch packages built sequentially in linux-release-ci). Freeze it once with a `:=` self-assignment after the fallback chain resolves. - The version.sh derivation used a bash script + POSIX `2>/dev/null`, neither of which works under cmd.exe. Move it into the non-Windows branch so Windows devs fall through to the existing PowerShell pubspec read instead. Verified: APP_VERSION is now stable across references (no timestamp drift even with a sleep between recipe lines); local still derives from version.sh, CI and Windows still fall back to pubspec. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…utput Addresses Copilot review on PR #8865. The reviewer flagged that version.sh's `sort -V` could fail on macOS BSD sort and silently regress. On modern macOS the stock `/usr/bin/sort` (Apple sort 2.3) does support `-V`, so that specific case doesn't occur — but verifying it surfaced a worse, real bug: `APP_VERSION ?= $(shell ./scripts/ci/version.sh …)` does NOT fall through when the shell returns empty. `?=` *defines* APP_VERSION as the empty string, which makes the subsequent pubspec `?=` a no-op and trips the "APP_VERSION_PUBSPEC is empty" $(error) — i.e. any environment where version.sh produces nothing (no git tags, shallow clone, a sort without -V, missing date) would hard-fail the build rather than fall back. Fix: capture the derivation into LANTERN_DERIVED_VERSION and only assign APP_VERSION when it's non-empty; otherwise emit a $(warning) and fall through to the pubspec read. Verified: normal local builds derive as before (no warning), CI/Windows use pubspec, and a failing version.sh now warns + falls back instead of erroring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…upplied Addresses Copilot review on PR #8865. The version.sh derivation (and its fallback $(warning)) ran unconditionally in the non-CI branch, so a developer who supplies APP_VERSION via env or command line still paid for a version.sh subprocess and could see a misleading "falling back to pubspec" warning even though their override is what gets used. Wrap the derivation in `ifndef APP_VERSION` so it only runs when the caller hasn't already provided a value — honoring the documented precedence (environment > pubspec) and suppressing the spurious cost/warning. Verified: normal local builds still derive (no warning); CI/Windows use pubspec; `APP_VERSION=9.2.0+1 make …` uses the override with no version.sh call and no warning. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
This PR fixes two related Android version-stamping bugs found while triaging a Sri Lanka ticket (#178331) whose logs claimed version
9.0.28but embedded June-11 kindling + sing-box 1.12.22 — a build that can't legitimately be9.0.28.Bug 1 —
versionName: local/sideload builds report a stale hardcoded versionLocal and sideload builds —
make android-apk-release/android-aab-release, the APKs internal testers actually run — stamped their version from the hardcodedversion:literal inpubspec.yaml(currently9.0.28), whilego.modresolves the real deps (kindling, radiance, sing-box) to their current versions at build time.Result: builds with current 9.1.x-era code were labeled
9.0.28, which:service.version(from the Gocommon.Versionldflag)cf_client_version(from Flutterpackage_info)9.0.28production users, corrupting version-based analysis (e.g. ticket triage).Root cause
The version is only corrected in CI:
release.ymlrewritespubspec.yamlviascripts/ci/version.shbefore building, thenbuild-android.ymlconsumes it. Any build that skips that rewrite — every local/sideload build — trusts the stale literal. And it can only be the stale literal:version.sh generateincrements from the highest tag (v9.1.9-beta→9.1.10) andvalidaterefuses to go backwards, so a successful overwrite could never produce9.0.28. The value persists precisely because the overwrite never runs on these paths — the Makefile's Android targets had no version-derivation step.Fix
APP_VERSIONfromversion.sh generate nightly(real line + short SHA + timestamp), falling back to the existing pubspec read only if that yields nothing (e.g. a checkout with no tags). Gated onGITHUB_ACTIONS, so CI keeps trusting the rewritten pubspec — CI behavior is byte-for-byte unchanged.--build-name=$(APP_VERSION_PUBSPEC)to the three Android Flutter recipes (android-debug,android-apk-release,android-aab-release).This makes both version channels consistent for local builds — Flutter
versionName(→ Freshdeskcf_client_version) and the Gocommon.Versionldflag (→ SigNozservice.versionand theX-Lantern-App-Versionheader to/v1/config-new) — and makes sideload builds self-identifying via the embedded commit SHA.Verification (
make -n)--build-name/common.Version9.1.10-3e943e2-20260615T…ZGITHUB_ACTIONS=true)make android-release-ciBug 2 —
versionCode: computed from a misusedDateconstructorandroid/app/build.gradlecomputedversionCodeas:new Date(2015, 1, 1)is the deprecatedjava.util.Date(year, month, day)constructor:yearis years-since-1900 (→3915) andmonthis 0-indexed (→ February). Sostartis Feb 1, year 3915 — ~1,900 years in the future.now - startis a large negative that wraps on the(int)cast to a positive ~5.3e8. It stayed monotonic by luck (constantstart→ still ticks +1/sec), so versionCode kept increasing and upgrades worked — but the value is meaningless and relies on signed-int overflow.Fix
Compute whole seconds since a fixed epoch, no deprecated
Date:Critical constraint:
versionCodemust never decrease (Play/Android reject lower codes on upgrade). A "correct" 2015-01-01 origin would yield3.6e8today — lower than already-shipped codes (5.3e8) — and break upgrades. The2009-01-01origin keeps today's value at ~5.5e8(just above the shipped sequence) with ~50 years ofintheadroom.🤖 Generated with Claude Code