fix(build): .incbin escaped the checkout from a worktree — use --bin-include-dir (#116) - #117
Merged
Merged
Conversation
…include-dir (#116) ca65 resolves a relative `.incbin` operand against the CURRENT DIRECTORY first, falling back to the including source file's directory only on a miss. Every `.incbin` here was spelled `../../../<dir>/<file>` on the belief that source-relative was the rule. From the repo root the two interpretations coincide, so it worked — but a git worktree lives at `.claude/worktrees/<name>/`, exactly three levels down, so `../../../` climbed out of the worktree and into the PRIMARY checkout. Every worktree build therefore embedded the primary checkout's blob while make's dependency graph tracked the worktree's. Nothing diagnosed it, and nothing could: the two copies are normally byte-identical, so the PRG hash is unchanged and the entire existing evidence bar passes. It goes wrong the moment they can differ — bump the ip65 submodule on a branch and the PRG silently ships the old network stack; or run concurrent agents in sibling worktrees, where whoever last wrote the primary's copy decides what everyone else compiles. Fix, per the measurements on #116: CA65FLAGS += --bin-include-dir $(abspath $(IP65_BUILD)) CA65FLAGS += --bin-include-dir $(abspath build) with the `../` dropped from every operand. Note `-I` does NOT feed `.incbin` — that is the source-include path, and aiming it at a blob directory fails with "Cannot open include file" even though the file is sitting in it. Binary includes have their own search path. ABSOLUTE roots, because a relative one is cwd-relative and would re-acquire this exact bug the moment a recipe assembles from a subdirectory. FOUR sites, not one. #116 was filed against the ip65 blob; the same escape was in both overlay TUs: src/net/ip65/ip65_blob.s ../../../ip65-build/ip65-c64.bin src/crypto/shared/p256_overlay_blobs.s ../../../build/lib/nistcurves-p256-verify.bin src/crypto/shared/p384_overlay_blobs.s ../../../build/lib/overlay-p384-{sha384,curve}.bin The overlay ones are the worse of the three: `ip65-build/` is deterministic, but `build/` is branch- and flag-dependent, so a borrowed overlay image could have come from a different pin entirely. All three files also *documented* the wrong rule ("resolved by ca65 relative to this source file"), which is what made the `../../../` look deliberate and kept it alive. Replaced rather than left beside a corrected path. ACCEPTANCE GATE — a marker test, not a hash ------------------------------------------- Hashes cannot see this bug, so "the build is green and the hash matches" is not evidence. Planting distinguishable bytes in the two copies and checking which reaches the image is. Checked in the LINKED PRG, not the object: ca65 stores .incbin data in fragments (measured — only 8 bytes of the blob appear contiguously in the .o), so an object-level string search cannot see the marker even when the right file was read. worktree blob marked, both copies present: pre-fix -> PRG lacks the marker (read the PRIMARY's) FAIL post-fix -> PRG contains the WORKTREE marker PASS worktree blob absent: pre-fix -> assembles anyway (escaped) FAIL post-fix -> Cannot open include file 'ip65-c64.bin' PASS Gate exits 1 pre-fix and 0 post-fix, so it discriminates rather than merely passing. The primary checkout's blob is never written by the test. Verification ------------ all 5 backend x profile builds PASS, hashes IDENTICAL to master (d522e684 / b181ec08 / 66e37037 / 118241e9 / 211ad1bd) — output-neutral marker gate PASS post-fix, FAIL pre-fix overlay operands resolve with a stub .bin present, fail loudly when absent (both TUs, real flags, real defines) The two overlay-embed BUILDS remain blocked by pre-existing defects unrelated to this change, confirmed identical on master: P-256 dies at `LIB_NISTCURVES_P256_CODE overflows OVERLAY_REGION by 644 bytes` (and from clean, earlier, at `No rule to make target 'build/labels.txt'`), P-384 at its SHA-384 table overflow. Their `.incbin` lines are therefore never reached by a real build, which is why the operands are verified in isolation above rather than end-to-end. Stated plainly rather than implied by a green matrix. Refs #116. Co-Authored-By: Claude Opus 5 (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.
Fixes #116. Taking the implementation at LaneWaveBump's request — I have the resolution matrix from the issue and found the trap in their proposed fix, so reviewing my own patch does not require anyone to rebuild those measurements.
What was wrong
ca65 resolves a relative
.incbinoperand against the current directory first, falling back to the including source file's directory only on a miss. Every.incbinin the tree was spelled../../../<dir>/<file>on the belief that source-relative was the rule. From the repo root the two interpretations coincide — which is why it worked, and why the wrong rule survived in three separate comments.They stop coinciding inside a git worktree. One lives at
.claude/worktrees/<name>/, exactly three levels down, so../../../climbed out of the worktree and into the primary checkout. Every worktree build embedded the primary's blob while make's dependency graph tracked the worktree's.Nothing diagnosed it, and nothing could: the copies are normally byte-identical, so the PRG hash is unchanged and the whole existing evidence bar passes. It bites the moment they can differ — bump the ip65 submodule on a branch and the PRG silently ships the old network stack; or run concurrent agents in sibling worktrees, where whoever last wrote the primary's copy decides what everyone else compiles.
The fix
with the
../dropped from every operand.Two things that are easy to get wrong here, both measured on #116:
-Idoes not feed.incbin. That is the source-include path; aimed at a blob directory it fails withCannot open include fileeven though the file is sitting in it. Binary includes have their own search path. The fix originally proposed on the issue used-Iand would not have built.--bin-include-dirwhile leaving../../../in place still resolves to the primary checkout, exits 0, and changes nothing.Absolute roots rather than relative: a relative root is cwd-relative and would re-acquire this exact bug the moment a recipe assembles from a subdirectory — cwd sensitivity being the thing that caused this in the first place.
Four sites, not one
#116 was filed against the ip65 blob. The same escape was in both overlay TUs, and they are the worse case —
ip65-build/is deterministic, butbuild/is branch- and flag-dependent, so a borrowed overlay image could have come from a different pin entirely:src/net/ip65/ip65_blob.s../../../ip65-build/ip65-c64.binsrc/crypto/shared/p256_overlay_blobs.s../../../build/lib/nistcurves-p256-verify.binsrc/crypto/shared/p384_overlay_blobs.s../../../build/lib/overlay-p384-sha384.binsrc/crypto/shared/p384_overlay_blobs.s../../../build/lib/overlay-p384-curve.binAll three files also documented the wrong rule ("the
.incbinpath is resolved by ca65 relative to this source file"). That comment is what made../../../look deliberate and kept it alive across several reorganisations, so it is replaced rather than left sitting next to a corrected path.Acceptance gate — a marker test, not a hash
Per the criterion agreed on the issue. Hashes cannot see this bug, so "green build, matching hash" is not evidence here.
One wrinkle worth recording, because it invalidated my first attempt at the gate: checking the object does not work. ca65 stores
.incbindata in fragments — measured, only 8 bytes of the blob appear contiguously in the.o— so a marker never appears as a searchable string even when the correct file was read. My first run reported FAIL on a working fix for exactly that reason. The gate checks the linked PRG, where ld65 emits the blob raw.Cannot open include file 'ip65-c64.bin'PASSGate exits 1 pre-fix, 0 post-fix — it discriminates, rather than merely passing on the version I wrote. The primary checkout's blob is never written by the test (it is shared with other agents); only the worktree's copy is marked, and it is restored byte-for-byte.
Verification
d522e684/b181ec08/66e37037/118241e9/211ad1bd) — output-neutral.binpresent; fail loudly when absentWhat is not verified end-to-end, and why. Both overlay-embed builds are blocked by pre-existing defects unrelated to this change — confirmed identical on master by stashing the patch:
No rule to make target 'build/labels.txt'; after the documented bootstrap two-step,LIB_NISTCURVES_P256_CODE overflows OVERLAY_REGION by 644 bytesSo those
.incbinlines are never reached by a real build, and I verified the operands in isolation instead. Saying so plainly rather than letting a green build matrix imply coverage those two paths do not have.Refs #116.
🤖 Generated with Claude Code