Repository navigation
fix(repo): force LF for the lib snapshot inputs so Windows checkouts - #19
Conversation
…match The lib_bind.bin and lib_parse.bin snapshots key their sections by a compile-time hash of the bytes of their inputs: the bundled libs read through include_str!, and the parser/binder sources read through include_bytes! (SOURCES_HASH in frontend/parser/lib_parse_snapshot.rs and binder/lib_snapshot.rs). Those bytes are the working-tree text, so on a checkout that rewrites text files to CRLF (Windows with core.autocrlf=true, the default) every key misses and two tests fail: - binder::lib_snapshot::tests::snapshot_matches_live_bind - frontend::parser::lib_parse_snapshot::tests::snapshot_matches_live_parse Pin LF in the working tree for .rs and for the bundled libs. The index already holds LF for all of these files, so no committed content changes; this only stops a Windows checkout from rewriting them. Verified: with the rule, both tests pass (280 passed, up from 278); the three remaining lib-test failures are unrelated environment cases (a Windows temp-path test, a test that needs TS_GO_REPO, and a Linux-only /proc/self/exe test).
DeryFerd
changed the title
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review. WalkthroughThe ChangesLine-ending attributes
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The LF rules stabilize the relevant snapshot inputs without changing the intentional CRLF files, so no material merge risk is evident. Architecture SummaryArchitecture risk: 🔵 Low · up to The changed surface does not map to a changed system, dependency edge, entrypoint, or external dependency. Changed systems: None identified. Architecture concerns Review detailsBefore / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Note 🤖 Claude Opus 5.5 responding on behalf of Theo This is correct. I checked it against main (
One addition for the description: an existing Windows clone keeps its CRLF files after it pulls this change, because git does not rewrite files whose content did not change. A fresh clone fixes it. So does CI has not run yet. It is waiting for approval of a first-time contributor. |
|
Note 🤖 Claude Opus 5.5 responding on behalf of Theo Thank you, @DeryFerd. I checked this change again on the current
I merged |
…er, --singleThreaded tsc -b and watch carry ids, value_symbol_links newtype) followups38 is main 8ac70c6 plus 7 commits, so this merge also brings main's PR code (pingdotgg#8, pingdotgg#10, pingdotgg#19, pingdotgg#30) and the light flake fix bf57472. Conflict: crates/ts_goport/tests/early_emit.rs. - ours (int56, eeflake1): the full K2 test change; build_emit_only_solution uses Solution and waits with Solution::wait_for_a_later_mtime. - theirs (main bf57472): only the mtime wait (free fn wait_for_a_later_mtime) in the old build_emit_only_solution. Took ours, as state note main-flake-fix-2026-10-09 says (int56 carries the full eeflake1 change, which includes the mtime wait). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2 participants
Problem
On Windows the
lib_bind.binandlib_parse.binsnapshots never match, socargo test -p ts_goport --libfailssnapshot_matches_live_bindandsnapshot_matches_live_parseon every checkout, out of the box. The cause is line endings. Each snapshot keys its sections with a hash of the bytes of the files it reads at build time, and a default Windows checkout (core.autocrlf=true) rewrites those files to CRLF, so none of the keys match the blobs.What the keys hash
The keys are not a hash of the lib text alone.
crates/ts_goport/src/frontend/parser/lib_parse_snapshot.rsandcrates/ts_goport/src/binder/lib_snapshot.rsfold aSOURCES_HASHinto every key, andSOURCES_HASHis a compile-time hash of the parser, scanner, factory, store, node,core.rs,flags.rsand lib-name sources, read withinclude_bytes!. The lib text comes in separately throughinclude_str!incrates/ts_goport/src/frontend/bundled/embed.rs, and both the text hash and itsconst_hashgo into the same key. All of this reads the working tree rather than the index, so a checkout that rewrites text files to CRLF changes every key at once.The change
.gitattributesnow forces LF in the working tree for the files those keys read:*.rscrates/ts_goport/libs/*.d.tsThe index already holds LF for all of them, so this changes no committed bytes. It only stops a Windows checkout from rewriting them. The three files the repo keeps as CRLF on purpose are left alone:
crates/ts_goport/tests/fixtures/source_map_cut/case/parserSkippedTokens16.tsand the twolicenses/files.Why the libs alone are not enough
Pinning only
crates/ts_goport/libs/*.d.tsis not sufficient. It fixes thetextandtext_constparts of the key, sosnapshot_matches_live_bindpasses, butsnapshot_matches_live_parsestill fails on itssourcesfield, which hashes the.rssources. Both rules are needed before both tests pass.Verification
On Windows, with rustc stable and
core.autocrlf=true:cargo check -p ts_goport --libpasses.cargo fmt --all -- --checkpasses.cargo test -p ts_goport --libgoes from 278 passed / 5 failed to 280 passed / 3 failed. The two tests this fixes arebinder::lib_snapshot::tests::snapshot_matches_live_bindandfrontend::parser::lib_parse_snapshot::tests::snapshot_matches_live_parse.The three that still fail are unrelated to line endings and are not touched by this change:
execute::build::config_prefetch::tests::panic_after_parse_reaches_takebuilds a temp path with Windows backslashes.format::api_test::test_format_format_checker_tsneeds the pinned Go checkout (TS_GO_REPO), which is not present.project::logging::logger::tests::log_time_is_local_timere-runs itself withTZ=Etc/GMT+7and reads/proc/self/exe, so it is Linux-only.Compatibility
On Linux and macOS the checkouts are already LF, so the rules are a no-op there and CI is unaffected. The rules only guarantee the working tree stays LF regardless of a contributor's
core.autocrlfsetting.Note
Force LF line endings for lib snapshot input files
Adds LF end-of-line rules for Rust files and TypeScript declaration files under
crates/ts_goport/libsin .gitattributes. This keeps the byte representation stable for snapshot hash inputs on Windows checkouts. Existing custom merge drivers are unchanged.Macroscope summarized 3faa37f.
Summary by CodeRabbit