Repository navigation
test: stop autocrlf from poisoning the embedded templates - #657
Conversation
On a Windows clone with core.autocrlf=true, all 49 golden tests fail on a clean checkout. The goldens were innocent: src/templates/*.txt are checked out with CRLF, @embedfile bakes those bytes into the assembler, and the generated output becomes a mix -- template-derived lines ending \r\n, writer-emitted lines ending \n -- compared byte-for-byte against LF goldens. Diagnosing this was its own lesson: piping the test output through anything that normalizes newlines (a text-mode subprocess capture, most terminals) shows "expected" and "found" as character-identical, because the \r is invisible precisely where it matters. The templates' byte counts were the tell: build_zig.txt carries 424 CRs on disk against 0 in the blob. `* -text` disables translation in both directions for the whole repo. The committed blobs are already LF, so nothing rewrites; checkouts stop manufacturing platform-dependent bytes in files whose bytes are load-bearing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pbwN1ytAa8tctZMaTMjAk
📝 WalkthroughWalkthroughThe pull request adds a ChangesLine-ending preservation
Estimated code review effort: 1 (Trivial) | ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitattributes:
- Line 10: Update the .gitattributes rule from "* -text" to enforce LF
normalization with "* text eol=lf", adding explicit binary exceptions for files
that must retain raw bytes. Alternatively, add a CI validation that rejects CRLF
bytes in fixtures and golden files.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| # on a CLEAN checkout -- 49 of them -- for one platform and nobody else. A | ||
| # red that reproduces only for one developer teaches everyone that red is | ||
| # normal, which is worse than a real failure. | ||
| * -text |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
if git grep -Il $'\r' HEAD -- .; then
echo "Tracked text files contain CRLF bytes."
exit 1
fiRepository: labelle-toolkit/labelle-assembler
Length of output: 171
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- .gitattributes ---'
cat -n .gitattributes
printf '%s\n' '--- related policy references ---'
rg -n -i --hidden --glob '!.git' 'CRLF|LF|line.?ending|golden|fixture|template' .gitattributes README* .github 2>/dev/null || true
printf '%s\n' '--- effective attributes for representative paths ---'
git check-attr -a -- \
.gitattributes \
README.md \
2>/dev/null || true
printf '%s\n' '--- tracked files containing CR bytes ---'
git grep -Il $'\r' HEAD -- . || trueRepository: labelle-toolkit/labelle-assembler
Length of output: 3448
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
cd "$tmp"
git init -q
git config user.name verifier
git config user.email verifier@example.invalid
printf '* -text\n' > .gitattributes
printf 'one\r\ntwo\r\n' > fixture.txt
git add .gitattributes fixture.txt
printf '%s\n' '--- -text staged blob bytes ---'
git cat-file blob :fixture.txt | od -An -tx1
printf '%s\n' '--- -text effective attributes ---'
git check-attr -a -- fixture.txt
git commit -qm initial
rm fixture.txt
git checkout -q -- fixture.txt
printf '%s\n' '--- -text checked-out bytes ---'
od -An -tx1 fixture.txt
printf '%s\n' '--- LF-normalizing policy ---'
printf '* text eol=lf\n' > .gitattributes
git add .gitattributes
git commit -qm policy
printf 'one\r\ntwo\r\n' > fixture.txt
git add fixture.txt
git cat-file blob :fixture.txt | od -An -tx1
git check-attr -a -- fixture.txtRepository: labelle-toolkit/labelle-assembler
Length of output: 244
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
cd "$tmp"
git init -q
git config user.name verifier
git config user.email verifier@example.invalid
show_bytes() {
python3 - "$1" <<'PY'
import sys
print(sys.argv[1], open(sys.argv[1], "rb").read().hex(" "))
PY
}
printf '* -text\n' > .gitattributes
printf 'one\r\ntwo\r\n' > fixture.txt
git add .gitattributes fixture.txt
printf '%s\n' '--- -text staged blob bytes ---'
git cat-file blob :fixture.txt > blob.bin
show_bytes blob.bin
printf '%s\n' '--- -text effective attributes ---'
git check-attr -a -- fixture.txt
git commit -qm initial
rm fixture.txt
git checkout -q -- fixture.txt
printf '%s\n' '--- -text checked-out bytes ---'
show_bytes fixture.txt
printf '%s\n' '--- LF-normalizing policy ---'
printf '* text eol=lf\n' > .gitattributes
git add .gitattributes
git commit -qm policy
printf 'one\r\ntwo\r\n' > fixture.txt
git add fixture.txt
git cat-file blob :fixture.txt > blob.bin
show_bytes blob.bin
git check-attr -a -- fixture.txtRepository: labelle-toolkit/labelle-assembler
Length of output: 587
Enforce LF for future fixtures.
* -text preserves CRLF bytes in the index and working tree. It does not enforce LF. A CRLF template or golden can therefore reintroduce clean-checkout test failures. Use * text eol=lf with explicit binary exceptions, or add a CI check that rejects CRLF bytes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.gitattributes at line 10, Update the .gitattributes rule from "* -text" to
enforce LF normalization with "* text eol=lf", adding explicit binary exceptions
for files that must retain raw bytes. Alternatively, add a CI validation that
rejects CRLF bytes in fixtures and golden files.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3be1ee0dd2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # on a CLEAN checkout -- 49 of them -- for one platform and nobody else. A | ||
| # red that reproduces only for one developer teaches everyone that red is | ||
| # normal, which is worse than a real failure. | ||
| * -text |
There was a problem hiding this comment.
When a Windows contributor edits a template with an editor that saves CRLF, this rule also disables clean-time normalization, so Git stores those CR bytes in the blob rather than merely preventing checkout conversion. A subsequent edit to src/templates/*.txt therefore makes @embedFile consume CRLF on every platform and recreates the mixed-ending golden failures this file is intended to eliminate. Git documents unset text as disabling conversion “upon checkin or checkout”; use an LF-enforcing rule such as * text=auto eol=lf so checkouts remain LF without accepting new CRLF blobs. Git gitattributes documentation
Useful? React with 👍 / 👎.
| # on a CLEAN checkout -- 49 of them -- for one platform and nobody else. A | ||
| # red that reproduces only for one developer teaches everyone that red is | ||
| # normal, which is worse than a real failure. | ||
| * -text |
There was a problem hiding this comment.
In an existing checkout already populated with core.autocrlf=true, pulling this attributes-only commit does not rewrite unchanged tracked files. Reproducing a checkout from the parent to this change left all 424 CR bytes in src/templates/build_zig.txt, so the same golden failures persist; once Git refreshes the index, the newly unset text attribute also makes the CRLF copies of hundreds of tracked files appear modified. Include a one-time migration that safely forces affected working trees to re-check out the LF blobs rather than relying on the attribute alone.
Useful? React with 👍 / 👎.
1 participant
On a Windows clone with
core.autocrlf=true(the default), all 49 golden tests fail on a clean checkout. After this: zero. The 33 remaining suite failures are E2E tests needing external toolchains (crystal, cargo, quickjs runners) and fail identically on main — verified by diffing the failure sets.The mechanism — and why the goldens were innocent
src/templates/*.txtare@embedFile'd into the assembler. autocrlf checks them out with CRLF (build_zig.txt: 424 CRs on disk against 0 in the blob), so generated output becomes a mix — template-derived lines ending\r\n, writer-emitted lines ending\n— byte-compared against LF goldens.Diagnosing it was its own lesson, recorded in the commit: anything that normalizes newlines while displaying the test output (a text-mode subprocess capture, most terminals) shows "expected" and "found" as character-identical, because the
\ris invisible exactly where it matters. I chased line endings through the goldens twice before measuring the templates' raw bytes.The fix
* -text— no translation, repo-wide. The committed blobs are already LF, so nothing rewrites and non-Windows checkouts are untouched; Windows checkouts stop manufacturing platform-dependent bytes in files whose bytes are load-bearing.Repo-wide rather than scoped to
templates/+goldens/deliberately: this repo's committedexamples/**/.labelletrees are also byte-compared (regeneration no-op checks), and any future embedded fixture would silently rejoin the trap.A red that reproduces for one developer and nobody else is worse than a real failure — it teaches that red is normal. That's how 49 phantom failures were sitting on top of 33 real (environmental) ones without anyone able to tell them apart.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit