Pin gobind alongside gomobile instead of running gomobile init - #240
Merged
Conversation
gomobile bind shells out to gobind, and gobind is the tool that actually generates the Java bindings and the JNI glue. CI cached and installed only gomobile and left the build script to call `gomobile init`, which installs gobind from @latest: the driver was pinned while the generator floated, so the generated API could change without a commit here. Take the revision from the go.mod the submodule already carries, so the two tools cannot drift apart and the cache key follows a submodule bump on its own, and have the build script check that the pair is present rather than reaching for gomobile init. The cache key names both tools now. Entries saved under the old key hold no gobind, so keeping it would hit the cache, skip the install and leave the build without a generator.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
theodorsm
requested changes
Aug 17, 2026
Comment on lines
+85
to
+91
| check_gomobile_tools() { | ||
| local missing="" | ||
| command -v gomobile >/dev/null || missing="gomobile" | ||
| command -v gobind >/dev/null || missing="${missing:+$missing }gobind" | ||
| # Explicit 0: a bare return would propagate the test's exit status and set -e | ||
| # would abort the build on the success path. | ||
| [ -n "$missing" ] || return 0 |
Contributor
There was a problem hiding this comment.
This would let us run a local build with a different gomobile version than defined in the subpackage go.mod, should we rather ensure that the installed gomobile/gobind version is equal to the go.mod? Currently we are only asked to installed the correct version if we don't have the commands installed previously (regardless of installed version).
I had version v0.0.0-20260529142300-ecb4cd65260a installed locally, but version v0.0.0-20251113184115-a159579294ab was defined in go.mod.
theodorsm
approved these changes
Aug 17, 2026
pappz
added a commit
that referenced
this pull request
Aug 17, 2026
* Add manual workflow to build a version-code-consistent APK for ad-hoc distribution (#236) * Add manual workflow to build a version-code-consistent APK for ad-hoc distribution Adds prepare-publication.yml (workflow_dispatch) that reuses the release signing key and computes version_code as the combined run count of itself and build-release.yml, so the two never collide. A shared concurrency group on both workflows prevents a race on that computation. * Draw the release version code from the shared run counter build-release.yml derived its version code from github.run_number, which counts only its own runs and is blind to prepare-publication.yml. With 25 release runs so far, the first ad-hoc build would take 25+1+40=66 and the next release would take 26+40=66 as well, then fall behind: 67 after 68 was already published. The concurrency group cannot fix this, because run_number is assigned when the run is queued, not when the step executes. Both workflows now sum the same two counters, so every run of either one advances the code by exactly one. Reading those counts needs actions: read, which an explicit permissions block otherwise withholds. * Name the manual workflow build-snapshot prepare-publication named a step in a process, while the workflows beside it name what they produce: build-debug, build-release. What this one produces is a release-signed build from an arbitrary commit with no tag behind it, which is what snapshot means. Not build-rc: release candidates already exist here as published pre-release tags (v0.6.0-rc.1, v0.3.3-rc.2) and are built by build-release.yml, so the name would claim a meaning the repository has already given away. The version code counter is keyed by workflow file name, so the rename is free only while the workflow has no runs yet. * Label snapshot builds snapshot- rather than ci- The version name travels to the management server as the peer's ui_version and is what the about screen shows, so it is the only thing telling support which build a peer is running. build-debug.yml already emits ci-<sha> from the same expression, which left an unsigned PR build and a release-signed hand-out looking identical in the peer list. The artifact keeps just the version name; prefixing it again read as snapshot-artifacts-snapshot-<sha>. * Resolve the Go version from the submodule's release tags in CI CI builds used the version only when the submodule sat exactly on a tag and fell back to ci-<sha> otherwise, which the management server rejects in NBVersionCheck posture checks: it treats ci- as a development build everywhere except there. Since the submodule is bumped more often than it is tagged, release builds effectively always shipped as ci-<sha>. CI now resolves the version by walking the pinned commit's ancestry back to the last stable release tag and appending the commit as SemVer build metadata, e.g. 0.77.0+f06b8c762. The server strips build metadata before every comparison, so this passes the same gates as a plain 0.77.0 while still naming the exact commit in the dashboard. Pre-release tags are skipped as a base: a suffix like -rc.2 lands in SemVer pre-release position, which the server compares differently from a release. Local builds now always produce dev-<sha>, which skips every server-side version gate; a developer who needs a real version passes it as the argument. The ancestry walk needs full history, but actions/checkout clones submodules shallow — the tags arrive without the commits between HEAD and the tag, and the walk would silently come up empty. The composite action therefore unshallows the submodule before building, guarded because --unshallow on a complete repository is a hard error. * Document the three build workflows and their differences * Fail the build when a run count cannot be fetched The zero fallback existed for one legitimate case: the runs endpoint returns 404 until a workflow has run or reached the default branch, and treating that as zero is what lets build-release compute a code before build-snapshot's first run. But it also swallowed every other failure — a network error or a revoked token minted a version code far below the published ones, silently for hand-distributed snapshots. Keep the 404-means-zero case and abort on everything else, including a non-numeric response, which bash arithmetic would otherwise fold to zero. * Pin gobind alongside gomobile instead of running gomobile init (#240) * Pin gobind alongside gomobile instead of running gomobile init gomobile bind shells out to gobind, and gobind is the tool that actually generates the Java bindings and the JNI glue. CI cached and installed only gomobile and left the build script to call `gomobile init`, which installs gobind from @latest: the driver was pinned while the generator floated, so the generated API could change without a commit here. Take the revision from the go.mod the submodule already carries, so the two tools cannot drift apart and the cache key follows a submodule bump on its own, and have the build script check that the pair is present rather than reaching for gomobile init. The cache key names both tools now. Entries saved under the old key hold no gobind, so keeping it would hit the cache, skip the install and leave the build without a generator. * Install gomobile and gobind automatically at the go.mod pinned revision * ci: install the JDK from temurin instead of adopt setup-java fails to resolve the 'adopt' distribution — AdoptOpenJDK was renamed to Eclipse Temurin and the old endpoints are going away — so the build died before installing a JDK. Same JDK, current name. (cherry picked from commit 8592de3) * Cache the Android NDK and Go modules in the build action
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
2 participants
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.
gomobile bind shells out to gobind, and gobind is the tool that actually generates the Java bindings and the JNI glue. CI cached and installed only gomobile and left the build script to call
gomobile init, which installs gobind from @latest: the driver was pinned while the generator floated, so the generated API could change without a commit here.Take the revision from the go.mod the submodule already carries, so the two tools cannot drift apart and the cache key follows a submodule bump on its own, and have the build script check that the pair is present rather than reaching for gomobile init.
The cache key names both tools now. Entries saved under the old key hold no gobind, so keeping it would hit the cache, skip the install and leave the build without a generator.
netbirdio/netbird#7229