Skip to content

fix: keep shell scripts on LF line endings on every checkout - #1411

Merged
xiaolai merged 2 commits into
mainfrom
fix/shell-script-lf-line-endings
Sep 15, 2026
Merged

xiaolai merged 2 commits into
mainfrom
fix/shell-script-lf-line-endings

Conversation

@xiaolai

@xiaolai xiaolai commented Sep 15, 2026

Copy link
Copy Markdown
Owner

Summary

  • Shell scripts and git hooks broke on any default Git for Windows checkout. core.autocrlf=true rewrites them to CRLF, and bash dies on the first real statement ($'\r': command not found, set: pipefail: invalid option name). This is why pnpm check:all could not run on Windows, as reported in feat: add native read aloud integration #1403.
  • .gitattributes now pins *.sh, *.bash, *.zsh and .githooks/* to eol=lf, which overrides autocrlf per path. Everything else keeps git's defaults.
  • A new gate-tier test keeps it that way.

Root cause

The repo had no .gitattributes, so line endings on checkout were decided by each machine's git config. The files that break are the 45 *.sh scripts (including every bash scripts/*.sh gate), the three extensionless hooks in .githooks/, and the terminal's shell-integration files src-tauri/resources/shell-integration/vmark.{bash,zsh}. The index already holds LF for all of them, so no file content changes.

Reproduced locally by converting scripts/lint-console.sh to CRLF and running it with bash, which failed with the two errors above.

What changed

  • .gitattributes: the four eol=lf rules, with the mechanism written next to them.
  • scripts/shell-script-eol.test.mjs (gates tier, runs in check:static):
    • Asks git check-attr about every tracked shell script, found by extension and by shebang, so the extensionless hooks count.
    • Checks probe paths, so a script that doesn't exist yet is already covered.
    • Fails if a shell script's committed blob already holds CRLF.
    • Asserts that discovery found the known scripts, so a scan that matched nothing cannot pass.

Validation

  • RED: without .gitattributes, 2 of the 4 tests fail and the message names all 50 unpinned scripts.
  • GREEN: with it, all 4 pass.
  • Detector check: a CRLF blob staged on purpose with git update-index --cacheinfo fails the committed-CRLF test.
  • scripts/check-scripts-parity.test.mjs passes, so the new test belongs to exactly one tier.
  • pnpm check:predelta: 44 of 44 gates pass, exit 0.
  • pnpm check:all: exit 0.

Not covered

  • CI's rust check will be red until the rustls advisory fix (RUSTSEC-2026-0285) lands in its own PR. The failure is on main and unrelated to this change.
  • Not verified on a real Windows checkout. The test checks the attribute git applies, which is what decides the checkout.

Type of Change

  • Bug fix
  • Feature

Git for Windows installs with core.autocrlf=true, which rewrites text
files to CRLF on checkout. Bash reads the trailing carriage return as
part of each line, so a CRLF script dies on its first real statement
("command not found", then "set: pipefail: invalid option name"). The
repo had no .gitattributes, so every bash gate script and all three git
hooks broke for anyone contributing from Windows, which is why pnpm
check:all could not run there (reported in #1403). The terminal's shell
integration files under src-tauri/resources would check out as CRLF too.

.gitattributes now pins *.sh, *.bash, *.zsh and .githooks/* to eol=lf,
which overrides autocrlf per path. Everything else keeps git defaults.

scripts/shell-script-eol.test.mjs asks git check-attr about every tracked
shell script, found by extension and by shebang so the extensionless
hooks are covered, plus probe paths for scripts not yet written. It also
fails on a shell script whose committed blob already holds CRLF. Without
the new file it fails and names all 50 scripts; a deliberately staged
CRLF blob is caught.
@xiaolai
xiaolai merged commit 14573aa into main Sep 15, 2026
16 checks passed
@xiaolai
xiaolai deleted the fix/shell-script-lf-line-endings branch September 15, 2026 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant