From e65ff9625fa4368b39c5b6df10df1fc21508e45e Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 29 Jul 2026 19:57:33 -0500 Subject: [PATCH] fix(update): repair managed checkouts still running core.autocrlf=true Git for Windows ships core.autocrlf=true in its system config, which renormalizes this repo's LF text files to CRLF in the working tree. install.ps1 pins core.autocrlf=false on the managed clone for that reason (#67730), but a checkout created before that landed never got the pin -- and cannot get it, because hermes-setup.exe resolves install.ps1 by an immutable build-time commit pin and reuses the cached script forever. A Windows install from May 2026 still runs the May install.ps1 no matter how many times it updates. `hermes update` ships with the checkout itself, so it is the only path left that reaches those installs. The pin and the cleanup have to be one operation. Under autocrlf=true git compares normalized content, so a CRLF working tree reads clean; pinning alone would expose every tracked text file as modified and hand the very next update an autostash and pop of the whole tree -- strictly worse than the state it set out to fix. So the tree is evaluated as it would look pinned (git -c, nothing persisted), the files whose only difference is the line ending are restored, and the pin is written only once that is verified clean. A checkout we cannot fully normalize is left exactly as it was found. Files still dirty under --ignore-cr-at-eol are never touched, so a real edit survives even when it also got renormalized. The restore takes its pathspec over stdin because a fully renormalized checkout is thousands of paths, well past the Windows command-line limit. --- hermes_cli/update_cmd.py | 87 ++++++++++ tests/hermes_cli/test_update_eol_churn.py | 199 ++++++++++++++++++++++ 2 files changed, 286 insertions(+) create mode 100644 tests/hermes_cli/test_update_eol_churn.py diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 5e70487698f..f286de0f59c 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -3046,6 +3046,89 @@ def _discard_lockfile_churn(git_cmd, repo_root): # Never let lockfile cleanup block an update. pass +def _normalize_managed_eol(git_cmd, repo_root): + """Take a managed checkout off ``core.autocrlf=true`` without leaving it dirty. + + Git for Windows ships ``core.autocrlf=true`` in its system config, which + renormalizes this repo's LF text files to CRLF in the working tree. That + breaks ``git checkout`` on update with "Your local changes would be + overwritten", so ``install.ps1`` pins ``core.autocrlf=false`` on the managed + clone (#67730). Checkouts created before that landed never got the pin and + cannot receive it — the bootstrap installer reuses its build-pinned + ``install.ps1`` forever — so ``hermes update``, which ships with the checkout + itself, is the only path left that can fix them. + + The pin and the cleanup are one operation. Under ``autocrlf=true`` git + compares normalized content, so a CRLF working tree reads clean; pinning + alone would expose every text file as modified and hand the update an + autostash of the whole tree. So the pin is written only after the tree is + verified clean under it, and a checkout we cannot fully normalize is left + exactly as it was. Best-effort: never blocks an update. + """ + # -c, not config: evaluate the tree as it WOULD look pinned, without + # persisting anything we might not be able to follow through on. + probe = git_cmd + ["-c", "core.autocrlf=false"] + + def _dirty(*extra): + out = subprocess.run( + probe + ["diff", "-z", "--name-only", *extra], + cwd=repo_root, + capture_output=True, + text=True, encoding="utf-8", errors="replace", + ) + if out.returncode != 0: + return None + return {p for p in out.stdout.split("\0") if p} + + def _eol_only(): + all_dirty, real_dirty = _dirty(), _dirty("--ignore-cr-at-eol") + if all_dirty is None or real_dirty is None: + return None + return all_dirty - real_dirty + + try: + effective = subprocess.run( + git_cmd + ["config", "--get", "core.autocrlf"], + cwd=repo_root, + capture_output=True, + text=True, encoding="utf-8", errors="replace", + ) + # Only "true" rewrites LF to CRLF on checkout. Unset, false, and input + # all leave the working tree alone, so there is nothing to repair. + if effective.stdout.strip().lower() != "true": + return + + eol_only = _eol_only() + if eol_only is None: + return + if eol_only: + # Pathspec over stdin, not argv: a fully renormalized checkout is + # thousands of paths, well past the Windows command-line limit. + subprocess.run( + probe + + ["checkout", "--pathspec-from-file=-", "--pathspec-file-nul", "--"], + cwd=repo_root, + input="\0".join(sorted(eol_only)), + capture_output=True, + text=True, encoding="utf-8", errors="replace", + check=False, + ) + if _eol_only(): + # Still dirty — persisting the pin here would only surface churn + # we failed to clear. Leave the checkout as we found it. + return + print(f"→ Normalized line-ending churn ({len(eol_only)} file(s))") + + subprocess.run( + git_cmd + ["config", "core.autocrlf", "false"], + cwd=repo_root, + capture_output=True, + check=False, + ) + except Exception: + # Never let line-ending cleanup block an update. + pass + def _cmd_update_impl(args, gateway_mode: bool): """Body of ``cmd_update`` — kept separate so the wrapper can always restore stdio even on ``sys.exit``.""" @@ -3173,6 +3256,10 @@ def _cmd_update_impl(args, gateway_mode: bool): # switches fragile. Restoring them first lets the common case (only # lockfile churn) update with a clean tree. _discard_lockfile_churn(git_cmd, _m().PROJECT_ROOT) + # Same rationale, different generator: line-ending churn is machine-made + # dirt on a managed checkout, so clear it (and stop generating it) before + # the stash/branch logic rather than autostashing the entire tree. + _normalize_managed_eol(git_cmd, _m().PROJECT_ROOT) # Detect if we're updating from a fork (before any branch logic) origin_url = _m()._get_origin_url(git_cmd, _m().PROJECT_ROOT) diff --git a/tests/hermes_cli/test_update_eol_churn.py b/tests/hermes_cli/test_update_eol_churn.py new file mode 100644 index 00000000000..fb742d8001d --- /dev/null +++ b/tests/hermes_cli/test_update_eol_churn.py @@ -0,0 +1,199 @@ +"""Regression: ``hermes update`` should take a managed checkout off autocrlf=true. + +Git for Windows ships ``core.autocrlf=true`` in its system config, which +renormalizes this repo's LF text files to CRLF in the working tree and breaks +``git checkout`` on update. ``install.ps1`` pins ``core.autocrlf=false`` on new +installs, but checkouts created before that landed cannot receive the fix, so +``hermes update`` has to repair them. + +The pin and the cleanup are one operation: under ``autocrlf=true`` git compares +normalized content, so the CRLF tree reads clean and pinning alone would expose +the whole tree as modified. These tests pin down that coupling. +""" + +from __future__ import annotations + +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +from hermes_cli.update_cmd import _normalize_managed_eol + +pytestmark = pytest.mark.skipif(shutil.which("git") is None, reason="needs git") + +GIT_CMD = ["git"] + + +def _git(cwd: Path, *args: str) -> subprocess.CompletedProcess: + return subprocess.run( + ["git", "-c", "user.email=t@t", "-c", "user.name=t", *args], + cwd=cwd, + check=True, + capture_output=True, + text=True, + ) + + +def _managed_repo(tmp_path: Path, files: dict[str, bytes]) -> Path: + """A checkout in the broken state: LF in the index, CRLF in the worktree, + ``core.autocrlf=true`` still set — i.e. what Git for Windows leaves behind. + + The CRLF is written by git's own checkout rather than by hand, which is how a + real managed install gets there. It matters: git's stat cache then records the + CRLF size, so the churn reads *clean* under ``autocrlf=true`` and is only + visible once the pin is evaluated. + """ + repo = tmp_path / "managed" + repo.mkdir() + _git(repo, "init") + for name, body in files.items(): + path = repo / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(body) + _git(repo, "-c", "core.autocrlf=false", "add", "-A") + _git(repo, "commit", "-m", "init") + _git(repo, "config", "core.autocrlf", "true") + for name in files: + (repo / name).unlink() + _git(repo, "checkout", "--", ".") + return repo + + +def _dirty(repo: Path) -> set[str]: + out = subprocess.run( + ["git", "-c", "core.autocrlf=false", "diff", "--name-only"], + cwd=repo, + capture_output=True, + text=True, + check=True, + ) + return {line for line in out.stdout.splitlines() if line} + + +def _autocrlf(repo: Path) -> str: + out = subprocess.run( + ["git", "config", "--local", "--get", "core.autocrlf"], + cwd=repo, + capture_output=True, + text=True, + ) + return out.stdout.strip() + + +def test_churn_invisible_under_autocrlf_true_is_still_found(tmp_path: Path) -> None: + """The reason the pin and the cleanup are one operation. + + A managed install's CRLF churn reads clean while ``autocrlf=true`` is set, so + an implementation that only looked at ``git status`` would pin the config and + hand the update a whole-tree autostash. Evaluating the tree as it would look + pinned is what makes the churn visible in time to clear it. + """ + repo = _managed_repo(tmp_path, {"a.py": b"x = 1\ny = 2\n"}) + as_git_sees_it = subprocess.run( + ["git", "status", "--porcelain"], cwd=repo, capture_output=True, text=True, check=True + ) + assert as_git_sees_it.stdout.strip() == "" + assert b"\r\n" in (repo / "a.py").read_bytes() + + _normalize_managed_eol(GIT_CMD, repo) + + assert _dirty(repo) == set() + assert b"\r\n" not in (repo / "a.py").read_bytes() + assert _autocrlf(repo) == "false" + + +def test_churn_is_cleared_and_the_pin_is_persisted(tmp_path: Path) -> None: + repo = _managed_repo(tmp_path, {"a.py": b"x = 1\n", "b.md": b"# Title\n\nbody\n"}) + + _normalize_managed_eol(GIT_CMD, repo) + + assert _dirty(repo) == set() + assert b"\r\n" not in (repo / "a.py").read_bytes() + assert _autocrlf(repo) == "false" + + +def test_real_edits_survive_even_when_line_endings_also_flipped(tmp_path: Path) -> None: + repo = _managed_repo(tmp_path, {"churn.py": b"y = 2\n", "both.py": b"z = 3\n"}) + # A genuine edit that ALSO got renormalized must not be discarded. + (repo / "both.py").write_bytes(b"z = 3\r\nz += 1\r\n") + + _normalize_managed_eol(GIT_CMD, repo) + + assert _dirty(repo) == {"both.py"} + assert (repo / "both.py").read_bytes() == b"z = 3\r\nz += 1\r\n" + assert _autocrlf(repo) == "false" + + +def test_pin_alone_is_written_when_there_is_no_churn(tmp_path: Path) -> None: + repo = _managed_repo(tmp_path, {"a.py": b"x = 1\n"}) + (repo / "a.py").write_bytes(b"x = 1\n") + + _normalize_managed_eol(GIT_CMD, repo) + + assert _dirty(repo) == set() + assert _autocrlf(repo) == "false" + + +@pytest.mark.skipif(sys.platform == "win32", reason="shim needs a POSIX shell") +def test_pin_is_withheld_when_the_churn_cannot_be_cleared(tmp_path: Path) -> None: + """If normalization can't finish, the checkout is left as found — pinning + anyway would surface churn we failed to clear.""" + repo = _managed_repo(tmp_path, {"a.py": b"x = 1\n"}) + # Real git everywhere except the restore, which fails. Stands in for any + # reason it cannot finish: a git too old for --pathspec-from-file, an + # unwritable working tree, a file locked by a running process. + shim = tmp_path / "git-no-checkout" + shim.write_text( + '#!/bin/sh\nfor a in "$@"; do [ "$a" = checkout ] && exit 1; done\nexec git "$@"\n' + ) + shim.chmod(0o755) + + _normalize_managed_eol([str(shim)], repo) + + assert _autocrlf(repo) == "true" + assert b"\r\n" in (repo / "a.py").read_bytes() + + +def test_already_pinned_checkout_is_untouched(tmp_path: Path) -> None: + repo = _managed_repo(tmp_path, {"a.py": b"x = 1\n"}) + _git(repo, "config", "core.autocrlf", "false") + + _normalize_managed_eol(GIT_CMD, repo) + + # Nothing to repair from this function's perspective: autocrlf is not true, + # so it must not start restoring files behind the user's back. + assert _dirty(repo) == {"a.py"} + + +def test_autocrlf_input_is_left_alone(tmp_path: Path) -> None: + repo = _managed_repo(tmp_path, {"a.py": b"x = 1\n"}) + _git(repo, "config", "core.autocrlf", "input") + + _normalize_managed_eol(GIT_CMD, repo) + + assert _autocrlf(repo) == "input" + + +def test_churn_across_more_files_than_fit_in_one_argv(tmp_path: Path) -> None: + """The pathspec goes over stdin, so a fully renormalized tree fits. + + Passing thousands of paths as arguments overflows the Windows command-line + limit; this asserts the batch is not argv-bound. + """ + files = {f"pkg/mod_{i:04d}.py": f"VALUE = {i}\n".encode() for i in range(1200)} + repo = _managed_repo(tmp_path, files) + assert len(_dirty(repo)) == len(files) + + _normalize_managed_eol(GIT_CMD, repo) + + assert _dirty(repo) == set() + assert _autocrlf(repo) == "false" + + +def test_non_repo_is_ignored(tmp_path: Path, capsys: pytest.CaptureFixture) -> None: + _normalize_managed_eol(GIT_CMD, tmp_path) + + assert capsys.readouterr().out == ""