fix(update): detect EOL-only churn via numstat, not name-only - #76431
Merged
Conversation
_normalize_managed_eol isolated line-ending churn from real edits by diffing twice: all dirty files minus files still dirty under --ignore-cr-at-eol. But 'git diff --name-only --ignore-cr-at-eol' computes its file list from blob/stat differences BEFORE the CR filter is applied, so it still lists CR-only files. On git 2.48.1 the two name-only sets are therefore identical, _eol_only() is always empty, and a managed Windows checkout gets pinned to core.autocrlf=false with the whole CRLF tree left dirty — breaking the next 'git checkout' on update (the exact failure this function exists to prevent). Compute the real-edit set with 'git diff --numstat --ignore-cr-at-eol' instead: numstat honors the CR filter (a CR-only file produces no record), so eol-only files are correctly identified and cleared while genuine edits are preserved. Pin core.quotepath=false so non-ASCII paths parse. Verified at 1200 files: 1199 eol-only normalized, one real edit preserved, autocrlf pinned only after the tree reads clean. This was a pre-existing failure on main (test_update_eol_churn's test_churn_across_more_files_than_fit_in_one_argv failed deterministically on git 2.48.1), surfaced while landing unrelated file-tools PRs.
alt-glitch
added
type/bug
Collaborator
Contributor
૮ >ﻌ< ა ci reviewran on 7e89ed9 ℹ️ InfoDesktop E2E visual evidence · View test artifacts · View job3 visual diffs. inline evidence upload failed. Failed to upload diff-1508682a2ae8-boot-ready-diff.png with gh image (exit code 1): Error uploading /home/runner/work/_temp/e2e-evidence/diff-1508682a2ae8-boot-ready-diff.png: step 0 (get upload token): uploadToken not found on repo page — do you have write access to NousResearch/hermes-agent? (or, if NousResearch enforces SAML SSO, authorize at https://github.com/orgs/NousResearch/sso) |
13 tasks
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
2 participants
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.
Summary
hermes updateon a managed Windows checkout now actually clears line-ending churn before pinningcore.autocrlf=false, instead of pinning a still-dirty tree that breaks the next update.Root cause:
_normalize_managed_eolisolated EOL-only churn from real edits by diffing twice — all dirty files minus files still dirty under--ignore-cr-at-eol. Butgit diff --name-only --ignore-cr-at-eolcomputes its file list from blob/stat differences before the CR filter is applied, so it still lists CR-only files. On git 2.48.1 the two name-only sets are identical →_eol_only()is always empty → the checkout that would clear the churn is skipped, yetautocrlf=falseis pinned anyway, leaving the whole CRLF tree dirty (the exact breakage this function exists to prevent).Changes
hermes_cli/update_cmd.py: compute the real-edit set withgit diff --numstat --ignore-cr-at-eol(numstat honors the CR filter — a CR-only file produces no record) instead of--name-only. Pincore.quotepath=falseso non-ASCII paths parse.Validation
_eol_only()empty → tree left dirty,autocrlfpinned anywayREAL_EDITintact), only that file left dirty,autocrlf=falsepinned.scripts/run_tests.sh tests/hermes_cli/test_update_eol_churn.py— 9/9 pass (was 1 failing ontest_churn_across_more_files_than_fit_in_one_argv).Pre-existing failure on
main— the test failed deterministically on git 2.48.1; surfaced while landing unrelated file-tools PRs (#76394, #76399).Infographic