fix(scripts): stop the large-file guard firing on text - #3475
Merged
Conversation
`docs/changelog.mdx` reached 512 KB and started failing the 500 KB pre-commit check, so `chore: release v0.8.14` could only be committed by passing `HF_MAX_NONLFS_KB`. Every release from here would need the same override, and the file grows a few KB each time. The check is for binaries. Its own error message says "large binaries are being committed to git instead of LFS", and its header explains why: an ONNX model, HDR-regression MP4s, demo clips, each of which lives in history forever and is paid for by every clone. That cost is specific to binaries. Git delta-compresses text, so release notes that grow a few KB per commit add a few KB to the pack, while a binary of the same size re-enters the pack whole on every edit. Text is now exempt regardless of size, detected with `grep -I` (a file with NUL bytes is binary), the same heuristic git uses for "Binary files differ". The binary rule, the `registry/` exemption, the LFS check and the size threshold are all unchanged. The alternative was an allowlist entry for the one file, which would leave the next legitimately growing text file to hit the same wall and get the same one-off exemption. `scripts/check-large-files.sh` had no test. It has one now, wired into `test:scripts` so it actually runs: an over-limit binary fails, an over-limit text file passes, an under-limit binary passes, and multiple offenders are all named. Verified the text case fails with the exemption removed, so the test pins the behaviour rather than describing it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The pre-commit large-file guard no longer fires on text files. The binary rule, the
registry/exemption, the LFS check and the 500 KB threshold are unchanged.Also adds the first test for
scripts/check-large-files.sh, wired intotest:scripts.Why
docs/changelog.mdxreached 512 KB, sochore: release v0.8.14could only be committed by passingHF_MAX_NONLFS_KB. Every release from here would need the same override, and the file grows a few KB each time.The check is for binaries. Its own error message says "large binaries are being committed to git instead of LFS", and its header explains the cost it exists to prevent: a 31 MB ONNX model, HDR-regression MP4s, demo clips, each living in history forever and paid for by every clone.
That cost is specific to binaries. Git delta-compresses text, so release notes growing a few KB per commit add a few KB to the pack. A binary of the same size re-enters the pack whole on every edit, which is how the history got its hundreds of megabytes in the first place.
How
One
grep -qIskip: a file containing NUL bytes is binary, the same heuristic git uses to decide "Binary files differ". Everything else in the script is untouched.The alternative was an allowlist entry for
docs/changelog.mdx. That leaves the next legitimately growing text file to hit the same wall and collect its own one-off exemption, and it does not address why a binary check was rejecting text.Worth flagging for the reviewer, since it is a real narrowing of the guard: a large generated text blob (a vendored bundle, a giant JSON fixture) now passes where it previously failed. I judged that acceptable because such a file is caught in review by being visible in the diff, whereas a binary is not, and because a text blob's pack cost is a fraction of a binary's. Say so if you would rather have a higher text threshold than a full exemption.
Test plan
scripts/check-large-files.shhad no test. It has four now, added to thetest:scriptslist so they actually run rather than sitting unreferenced:Verified the test bites: with the exemption line removed, "accepts a text file over the limit" fails and the other three still pass.
Also exercised through the real staged-set path the hook uses, rather than only the explicit-argument path the tests use:
Full
bun run test:scripts: 188 passed, 0 failed, plus the 42 vitest catalog tests it chains.