mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-30 19:09:28 +00:00
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.
This commit is contained in:
parent
240afd0b70
commit
e65ff9625f
2 changed files with 286 additions and 0 deletions
|
|
@ -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)
|
||||
|
|
|
|||
199
tests/hermes_cli/test_update_eol_churn.py
Normal file
199
tests/hermes_cli/test_update_eol_churn.py
Normal file
|
|
@ -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 == ""
|
||||
Loading…
Add table
Add a link
Reference in a new issue