fix(ci): check out Rust sources as LF on Windows too - #582
Conversation
main's CI failed after the v2.9.8 merge (0bd814d), on `test (windows-latest)` only, so Auto Release skipped and v2.9.8 has no tag yet. The failure: app::tests::every_load_path_installs_a_dual_system_cabinet `build_dual_cabinet` is called from `cabinet_for_image` only left: 3, right: 1 The cause is the checkout, not the code. The frontend's source-shape tests read app.rs with include_str! and cut off the test module at "\n#[cfg(test)]\nmod tests {". The windows-latest runner checks out with core.autocrlf, so the file is CRLF and that pattern never matches. The "production" text is then the whole file, and the test counts its own assertion strings: three occurrences of `self.build_dual_cabinet(` where production has one. Splits on the bare "\n#[cfg(test)]" survive CRLF, because the "\n" is still inside "\r\n"; only the two "...\nmod tests {" splits break, which is why one test failed and not six. Pull requests never saw it: the matrix gives non-release PRs Linux only, and Windows tests run on main and on release/* branches. This branch is named release/* so its PR runs Windows. Fix: `*.rs text eol=lf` in .gitattributes, next to the existing WGSL/GLSL rule that exists for the same reason (a byte-compared shader generator). Every .rs file is already LF in the index (`git ls-files --eol`: 523 of 523 i/lf), so no committed byte changes; only what a Windows checkout writes. Rust compiles LF sources on every platform. Not done: making each test split CRLF-tolerant. The attribute fixes the cause for every test that reads source, including future ones, which a per-test patch would not. Verified: converting app.rs to CRLF locally reproduces the failure exactly (left 3, right 1); restoring LF passes it. `git check-attr` reports eol=lf for app.rs. The Windows leg of this PR's CI is the real check. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 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 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe ChangesRust line endings
Priority: ⬆️ High Estimated code review effort: 1 (Trivial) | ~3 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change addresses the reported Windows line-ending issue, with no concrete merge-blocking risk 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 | ✅ 9✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Action performedReview finished.
|
Antigravity review (Gemini via Ultra)This PR enforces LF line endings for all Rust source files during checkout and updates the v2.9.8 release notes and changelog with bitstream build details. Blocking issuesNone found. Suggestions
NitpicksNone. Automated first-pass review by Earlier review rounds (newest first)Round reviewed at 2026-10-03 14:02 UTCAntigravity review (Gemini via Ultra)This PR enforces LF line endings for all Rust source files in Blocking issuesNone found. SuggestionsNone. Nitpicks
Automated first-pass review by |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The attribute directly addresses the verified CRLF-sensitive test behavior without changing source bytes.
Review effort: Balanced
Findings: None
What changed in this PR
Enforces LF endings for Rust sources so source-shape tests behave consistently on Windows.
Changes:
- Adds
*.rs text eol=lfto.gitattributes. - Documents the Windows-only CI failure and fix.
| File | Description |
|---|---|
.gitattributes |
Forces LF endings for checked-out Rust files. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The release notes promised bitstreams "compiled from the release commit at fitter seed 2, on the day of the release". That is no longer true. The release-day compile of the merged sibling (65d86a6, BUILD_DATE 261003) missed on-die setup by 0.162 ns on the pll_hdmi output counter (Slow 1100mV -40C), and release-rbf.sh refused it. The off-die build closed (+0.033 / +0.116 ns). At the maintainer's direction (2026-10-03, the v2.9.3 precedent), v2.9.8 ships the pair compiled at 261001. That compile came from hardware sources identical to the release commit: the diff of rtl/, sys/, *.qip and *.sdc between its tree (e2f88bd) and the merge is empty. Timing: on-die +0.656 / +0.105 ns, off-die +0.033 / +0.116 ns. Two clean compiles of each were byte-identical: on-die 132ec5c0..., off-die 995f8259.... The notes now say exactly that, carry both md5s (the old text promised to add them on attach), and name the datecoded on-die file, RustyNES_20261001.rbf. The sibling's releases/ change is its PR #50. CHANGELOG's Verification bullet records the same fact. The release audits (notes render, anchors, state prose) pass, and markdownlint is clean. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Answering Antigravity (no blocking items):
|
Fix the Windows test failure that blocked the v2.9.8 release
After #579 merged,
main's CI failed ontest (windows-latest)only, so Auto Release skipped v2.9.8.The cause is the checkout, not the code. The frontend's source-shape tests read
app.rswithinclude_str!and split off the test module at"\n#[cfg(test)]\nmod tests {". The Windows runner checks out CRLF, so the split never matches, andevery_load_path_installs_a_dual_system_cabinetcounts its own assertion strings (3 against 1).The fix is
*.rs text eol=lfin.gitattributes, beside the existing WGSL/GLSL rule that exists for the same reason. All 523.rsfiles are already LF in the index, so no committed byte changes.Verified: converting
app.rsto CRLF locally reproduces the failure exactly, and restoring LF passes it. This branch is namedrelease/*so that its CI runs the Windows leg, which PRs otherwise skip.Also in this PR (b31047a): the release notes and CHANGELOG now say that v2.9.8 ships the bitstream pair compiled on 2026-10-01. On release day the on-die compile missed timing by 0.162 ns, and the maintainer chose to ship the 261001 pair, which is timing-clean, byte-identical across two compiles, and built from the same hardware sources. Both md5s are now in the notes. The MiSTer repository carries the matching change.
Once this merges and
maingoes green, Auto Release publishes v2.9.8 from that commit.🤖 Generated with Claude Code