perf(ci): use stable test ldflags for Go test cache#19394
Draft
perf(ci): use stable test ldflags for Go test cache#19394
Conversation
|
Skipping CI for Draft Pull Request. |
Contributor
|
Images are ready for the commit at 14b6d2b. To use with deploy scripts, first |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19394 +/- ##
=======================================
Coverage 49.68% 49.69%
=======================================
Files 2700 2700
Lines 203278 203297 +19
=======================================
+ Hits 100999 101024 +25
+ Misses 94753 94749 -4
+ Partials 7526 7524 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0dfd5e7 to
830929b
Compare
Go includes -X ldflags in the link ActionID (exec.go:1404), so per-commit values (MainVersion, GitShortSha) change the test binary ID, causing test cache misses on every commit. Fix: for go test, replace per-commit MainVersion with the stable base version tag (from git describe --tags --abbrev=0) and skip GitShortSha. Builds continue using full per-commit ldflags unchanged. Also adds -buildvcs=false which removes unused VCS stamping that Go 1.18+ silently enabled (no code reads debug.ReadBuildInfo). This is the minimal change to enable test caching — just go-tool.sh. The existing XDef/status.sh mechanism is preserved for builds. Partially generated by AI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
830929b to
14b6d2b
Compare
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.
Description
Enable Go build cache reuse across commits by stabilizing the
-Xldflagsthat Go includes in build ActionIDs.
The problem
scripts/go-tool.shinjects per-commit version values via-Xldflags:-X MainVersion=4.11.x-317-gabcdef(changes every commit — includes commit count + SHA)-X GitShortSha=abcdef(changes every commit)Go includes
-Xldflags in the link ActionID (exec.go:1404: linkflags %q).When ldflags change, Go computes a new ActionID for every package that gets
linked, causing cache misses for:
-Xvalues post-link, so the build cost is minimal ~1-2s)to miss the cache and re-run from scratch every commit
The fix
Split ldflags into two sets:
4.11.xfromgit describe --tags --abbrev=0),GitShortShaskippedThis preserves the correct version string in all built binaries while giving
test binaries a stable ActionID across commits.
-buildvcs=falseGo 1.18 (adopted September 2022) silently enabled
buildvcs=auto, whichstamps
vcs.revision,vcs.time,vcs.modifiedinto every binary via thelinker. No code in the repository reads this data — version stamping is done
independently via the
-Xldflags fromstatus.sh. The buildvcs data wasnever intentionally enabled; it appeared as an unnoticed side effect of the
Go 1.18 upgrade. The only time it was previously noticed was July 2024
(PR #11935) when it broke a scanner CI workflow.
Adding
-buildvcs=falseremoves this unused overhead.What changes
One file:
scripts/go-tool.shldflagsintobuild_ldflags(full version) andtest_ldflags(stable base tag)-X MainVersion=4.11.xinstead of-X MainVersion=4.11.x-317-gabcdef-X GitShortSha(not needed, changes per commit)-buildvcs=falseadded to both build and testWhat is preserved
//XDef:annotations — unchangedstatus.sh— unchanged-Xldflags for builds — unchanged (fullgit describeversion in binaries)version_data.go— unchangedBuild cache impact
Builds already benefit from GOCACHE for compilation (
.afiles are keyed bysource hashes, not ldflags). The
-Xldflags only affect linking, and Gopatches
-Xvalues post-link so there is minimal build cost (~1-2s).The main benefit of this PR is enabling test result caching when combined
with PR #19395 (file mtime stabilization). Together they achieve:
A combined test branch (
davdhacs/test-cache-combined) with only these twochanges is being tested to provide clean timing data.
Partially generated by AI.