fix: force LF on shell scripts so shebangs don't break (CRLF) - #2
Conversation
A CRLF in a checked-out shell script makes its shebang resolve to the interpreter "/bin/bash\r", which does not exist, so the script fails with "bad interpreter". core.autocrlf normalizes only on commit, not checkout; an explicit eol=lf attribute forces LF in the working tree on every checkout regardless of each user's global git config. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @d-morrison's task in 1m 41s —— View job Code Review — PR #2: force LF on shell scripts
Overall verdict: looks good — approve when ready. The fix is correct, well-scoped, and the PR description is thorough. The Checklist:
One optional suggestion posted as an inline comment on |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| # Shell scripts must use LF line endings — a CRLF on the shebang line makes | ||
| # the kernel look for "/bin/bash\r" and fail with "bad interpreter". | ||
| *.sh text eol=lf |
There was a problem hiding this comment.
Optional enhancement: Consider prepending a * text=auto rule before the *.sh line. It's the standard first line in .gitattributes — it tells git to auto-detect text files and normalize their line endings on commit. Combined with the explicit *.sh text eol=lf override, this gives blanket protection to all text files (YAML, R, Quarto, etc.) while still guaranteeing LF in working-tree shell scripts.
| # Shell scripts must use LF line endings — a CRLF on the shebang line makes | |
| # the kernel look for "/bin/bash\r" and fail with "bad interpreter". | |
| *.sh text eol=lf | |
| # Auto detect text files and perform LF normalization | |
| * text=auto | |
| # Shell scripts must use LF line endings — a CRLF on the shebang line makes | |
| # the kernel look for "/bin/bash\r" and fail with "bad interpreter". | |
| *.sh text eol=lf |
Not blocking — the current rule already solves the stated problem. The only downside to text=auto is it may trigger renormalization of other tracked files on first checkout for contributors; run git add --renormalize . locally to see if anything would change before merging.
There was a problem hiding this comment.
Adopted, with one tweak: used * text=auto eol=lf rather than bare * text=auto, so LF is actually forced in the working tree on checkout (bare text=auto leaves native EOL, i.e. CRLF on Windows) — this matches the repo owner's global gitattributes. Verified zero renormalization churn: git grep -lI $'\r' origin/main returns nothing and git add --renormalize . stages only .gitattributes. Worth noting this repo's working tree already has 19 CRLF files including 6 Python/R scripts whose shebangs would break the same way the .sh did, so the blanket rule is the better fix here.
Prepend `* text=auto eol=lf` so every text file (not just shell scripts) is forced to LF in the working tree on checkout. qmt's working tree already accumulates CRLF in non-shell scripts (.github/scripts/*.py, .lua, .css, .yml); a CRLF Python script breaks its shebang exactly like the .sh case did. No committed blobs change (verified zero renormalization churn). Addresses the reviewer's optional suggestion, using eol=lf (matching the repo owner's global gitattributes) rather than bare text=auto so LF is actually forced on checkout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @d-morrison's task in 49s —— View job Code Review — PR #2 (updated): broaden
|
| Rule | LF in repo (on commit) | LF in working tree (on checkout) |
|---|---|---|
* text=auto |
✅ | ❌ — native EOL (CRLF on Windows) |
* text=auto eol=lf |
✅ | ✅ — forced LF everywhere |
Since the original problem was CRLF in the working tree breaking shebangs, forcing eol=lf globally is the correct fix — and it was already verified to cause zero renormalization churn.
On the *.sh text eol=lf line being kept:
It's not redundant — it's belt-and-suspenders in a useful way. The bare text attribute (no =auto) means git unconditionally treats .sh files as text regardless of content heuristics, whereas text=auto applies only when git's heuristic agrees. Keeping the explicit override is good documentation and slightly stricter protection for shell scripts specifically.
File as merged looks good:
* text=auto eol=lf ← forces LF in working tree for all auto-detected text files
*.sh text eol=lf ← unconditionally marks .sh as text + LF (belt-and-suspenders)
No issues. Ready to merge.
|
1 participant
Closes #1.
A shell script checked out with Windows CRLF line endings gets a
#!/bin/bash\rshebang; the kernel then looks for the interpreter/bin/bash\r(shown as/bin/bash^M) and fails withbad interpreter: No such file or directory.core.autocrlfnormalizes CRLF only on commit, never on checkout, so a working tree that picks up CRLF stays broken.This adds
.gitattributeswith*.sh text eol=lf, which forces LF in the working tree on every checkout regardless of each user's global git config. No committed shell scripts in this repo currently have CRLF, so this is purely preventive — no file renormalization.