Key go-installed tool binaries on the Go toolchain that builds them - #708
Open
wallrj-cyberark wants to merge 2 commits into
Open
Key go-installed tool binaries on the Go toolchain that builds them#708wallrj-cyberark wants to merge 2 commits into
wallrj-cyberark wants to merge 2 commits into
Conversation
- Tools built with "go install" are cached at $(DOWNLOAD_DIR)/tools/<tool>@<version>_<os>_<arch>, a path which says nothing about the Go toolchain that built them. - CI persists that download directory between runs, so after a VENDORED_GO_VERSION bump the stale binary is restored and reused indefinitely, even when it can no longer parse the new standard library. - Include the Go version in the path of Go-built tools, so that a Go upgrade forces a rebuild, and depend on the VENDORED_GO_VERSION stamp file so the unversioned symlink is re-pointed. Refs: cert-manager/cert-manager#9174 Signed-off-by: Richard Wall <richard.wall@cyberark.com>
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
This was referenced Aug 20, 2026
There was a problem hiding this comment.
Pull request overview
Keys Go-installed tool binaries by vendored Go version, preventing stale cached binaries after toolchain upgrades.
Changes:
- Adds per-tool download path variables.
- Adds Go versions to Go-built binary paths.
- Relinks tools when the vendored Go version changes.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- When vendoring is disabled, tools are built with the system Go, so keying the download path on VENDORED_GO_VERSION mislabels binaries in the shared cache and a system Go upgrade never invalidates them. Key the path and the stamp file on the Go version actually used: "go env GOVERSION" for the system Go, go$(VENDORED_GO_VERSION) when vendoring. - Make the versioned binary a normal prerequisite of the unversioned symlink, so a rebuilt binary always re-points the symlink. Previously an existing symlink newer than the stamp files caused the binary to be rebuilt at the new path while the symlink kept pointing at the old one. In the steady state the symlink resolves to the same file as the prerequisite, so nothing is remade. - Update the LN comment for the new path format. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Richard Wall <richard@the-moon.net>
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.
Motivation
The Go 1.27 upgrade (#704) is breaking downstream repositories in a way which does not reproduce on a clean checkout — and which comes and goes between CI runs.
In cert-manager/cert-manager#9174 (the Renovate PR which vendors this module at 455529c),
make-verifyfailed because a cachedopenapi-genbinary, built by Go 1.26, can no longer parse the Go 1.27 standard library (build log):A tool built from source is cached at:
That path says nothing about the Go toolchain which produced the binary, so bumping
VENDORED_GO_VERSIONdoes not invalidate it. The download directory is deliberately persisted between CI runs — for cert-manager,preset-local-cachecopies_bin/downloadedforward from a hostPath cache shared per Prow node; for other repositories it is a GitLab CI or GitHub Actions cache.The failure is intermittent because it depends on which node a job lands on. Failed jobs do not write back their cache ("Local cache [update]: Job failed, not updating cache"), and cert-manager's master branch — still on Go 1.26.6, with
openapi-genat the same tool version — keeps re-priming node caches with a Go 1.26-built binary at the same path. A run scheduled on a master-primed node fails; a run on a node with a cold cache rebuilds the tool with Go 1.27 and passes (as the later runs of cert-manager/cert-manager#9174 did, where the job now fails only on the golangci-lint findings addressed by cert-manager/cert-manager#9175). The stale binaries cannot be cleared without purging the node caches by hand.Changes
Include the Go toolchain version in the download path of tools built with
go install:where
<goversion>is the version of the toolchain which actually builds the tool:go$(VENDORED_GO_VERSION)when Go is vendored, otherwise the system Go'sgo env GOVERSION. Keying on the actual toolchain means a system Go upgrade also invalidates the cache, a repository which does not vendor Go cannot mislabel binaries in the shared$HOME/.cache/makefile-modulescache, and such a repository does not rebuild every tool when a module bump changes aVENDORED_GO_VERSIONit never uses.To achieve this:
tool_defsnow exports a$(XXX_DOWNLOAD_PATH)variable instead of declaring the symlink rule directly;go_dependencyoverrides that variable for the tools it builds; and the symlink rule is generated afterwards, for every tool, from$(XXX_DOWNLOAD_PATH).$(bin_dir)/tools/xxxsymlink, so rebuilding the binary always re-points the symlink. In the steady state the symlink resolves to that same file, so nothing is remade. Go-built tools additionally depend on a$(bin_dir)/scratch/GO_TOOLCHAIN_VERSIONstamp file, which catches reverting to an older, already-cached toolchain, where file modification times cannot be trusted.Downloaded (non-Go) tools are unaffected: their paths and their learned checksums are unchanged, so
make learn-tools-shasneeds no rerun.Known limitations:
_go*_binaries would thrash the$HOMEcache shared between repositories on different toolchains.go.modcarries atoolchaindirective which upgrades beyond the invoking Go, the label understates the truth. That does not affect the problem being fixed here, which is that a Go upgrade must invalidate previously built binaries.Testing
Verified locally against the two repositories where the breakage was observed, by copying the patched module over their vendored copy:
_bin/tools/openapi-genand rebuilt: the binary was rebuilt atopenapi-gen@v0.0.0-20260721132016-d427ff9ee9ad_go1.27.0_linux_amd64andmake generate-codegenthen produced no diff, i.e. themake-verifyfailure on that pull request is entirely the stale-binary problem.klone@v0.3.0_go1.27.0_linux_amd64; a repeat run was a no-op; settingVENDORED_GO_VERSION := 1.26.5rebuilt and relinked toklone@v0.3.0_go1.26.5_linux_amd64, and switching back relinked again without rebuilding.goonPATH: a fresh build cachedklone@v0.3.0_go1.24.5_linux_amd64— the system toolchain, notVENDORED_GO_VERSION; a repeat run was a no-op; a toolchain bump rebuilt and re-pointed the symlink; a downgrade re-pointed to the already-cached binary without rebuilding; and a pre-existing symlink whose modification time was newer than every stamp file was still re-pointed after the rebuild.helm@v4.2.4_linux_amd64, no suffix, no re-download.make test-e2epasses in this repository.scripts/learn_tools_shas.shwas exercised in dry-run mode for one Go tool and one downloaded tool, to confirm the helper makefile still evaluates cleanly under--warn-undefined-variables.with claude fable-5