build: CI owns the BPF objects — state the contract, make taking them cheap (#117) - #150
Merged
Merged
Conversation
#117 has been treated as "make two machines produce identical bytes". That is fighting a compiler bug with pinning. Clang assigns BTF type IDs to forward declarations from a pointer-keyed std::map, so they follow malloc addresses and therefore the build directory string; no flag removes it (-fdebug-compilation-dir was measured and does not), and bpf2go absolutizes the source path unconditionally, so the path cannot be made relative without patching it. What #87 actually established was trust: CI had never built from the committed objects and go generate silently replaced them. Byte-identity was the mechanism chosen to enforce that, not the requirement. So state the contract instead of chasing the bytes -- the committed objects are CI's artifacts, and a local regeneration differing is expected. CONTRIBUTING.md said to regenerate with `make generate` for BPF-touching changes, which is the instruction that produces uncommittable bytes. It now describes the round trip, and names the two things that have cost real time here: committing a local regeneration, and hand-reverting the changed objects, which waives the check per file and is how this hid for four rounds. adopt-ci-objects no longer needs RUN=: it finds the failed run for the current branch, and refuses with an explanation rather than a usage line when there is none. The guard and CI messages lead with it, and generate-container is demoted from source-of-truth to preview -- it reproduces CI for the command lines committed today, so a match is encouraging and a mismatch inconclusive. This does not fix #117. The bug is upstream in BTFDebug::endModule, and the real local fix would be normalising .BTF ordering before committing. Both are recorded there. Claude-Session: https://claude.ai/code/session_01P5889hA6CrX8ysnQkvv6im
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
1 participant
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.
#117 has been approached as "make two machines produce identical bytes". That is fighting a compiler bug with pinning, and every lever has now been measured and eliminated.
Why the bytes cannot be pinned
Clang assigns BTF type IDs to forward declarations by iterating a pointer-keyed
std::mapinBTFDebug::endModule, so they follow malloc addresses — which depend on the build directory string. Same source, same clang, different directory, different bytes.Eliminated by measurement, per the issue and one new check here:
-fdebug-compilation-dir/-ffile-prefix-mapCI_BUILD_PATH)actions/checkout's layout; a repo rename breaks it silentlycilium/ebpf@v0.21.0/cmd/bpf2go/main.go:192doesfilepath.Abs(args[1])unconditionallyWhat #87 actually asked for
Trust: CI had never built from the committed objects, and
go generatesilently replaced them. Byte-identity was the mechanism chosen to enforce that, not the requirement. So this states the contract rather than chasing the bytes.Changes
CONTRIBUTING.mdtold contributors to regenerate withmake generatefor BPF-touching changes — the exact instruction that produces uncommittable bytes. It now documents the contract and the round trip, and names the two things that have cost real time: committing a local regeneration, and hand-reverting the objects, which waives the check per file and is how this hid for four rounds.make adopt-ci-objectsneeds noRUN=. It finds the failed run for the current branch, and refuses with an explanation rather than a usage line when there is none. Verified: on a branch with no failed run it prints why and what to do instead.generate-containeris demoted from source-of-truth to preview — it reproduces CI for the command lines committed today, so a match is encouraging and a mismatch inconclusive.This does not fix #117
Stated plainly in the commit and left open. The real fixes are upstream (
FixupDerivedTypeskeyed by something stable — still pointer-keyed on LLVMmain) or normalising.BTFtype ordering before committing, which would make the object a function of the source. Both are recorded on the issue.No code changes; 36 packages still pass, both YAML files parse.
https://claude.ai/code/session_01P5889hA6CrX8ysnQkvv6im