From e1fdf003a668f97bf5a53d7675c1e70b1dcfec34 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 5 Oct 2026 00:56:58 -0700 Subject: [PATCH] fix(update): a clashing local tag or old git no longer leaves the conversion retrying forever Final-review findings on convert_treeless_checkout: - A local tag that the --tags refetch would clobber made git exit non-zero after the objects had landed, and check=True raised before the completeness check ran, so every update warned and stayed tree:0. The offline history check is now the verdict, not the fetch's exit status. - Git older than 2.36 has no --refetch; such a checkout is left as it is instead of printing git's usage text as a warning on every update. --- hermes_cli/gitlock.py | 21 +++++++++++++++----- tests/hermes_cli/test_update_shallow_heal.py | 6 ++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/hermes_cli/gitlock.py b/hermes_cli/gitlock.py index a53ec92c83..c46818d7b5 100644 --- a/hermes_cli/gitlock.py +++ b/hermes_cli/gitlock.py @@ -7,6 +7,7 @@ from __future__ import annotations import logging import os +import re import subprocess import time from pathlib import Path @@ -620,23 +621,26 @@ def convert_treeless_checkout(repo_root: Path, **run_kwargs) -> bool: release ref the refspec does not name), so its history is checked offline and refetched by commit when trees are still missing. The new filter is recorded only once that history is whole: a failed or partial conversion leaves the checkout treeless, so the next update - retries. Returns whether it converted; fetch failures raise subprocess errors. + retries. That check, not the fetch's exit status, is the verdict: a refused tag update (a + local tag that would be clobbered) fails the fetch after its objects have landed. Git before + 2.36 has no ``--refetch`` and is left as it is. Returns whether it converted; fetch failures + raise subprocess errors. """ run_kwargs["creationflags"] = run_kwargs.get("creationflags", 0) | windows_hide_flags() - if _partial_clone_filter(repo_root, **run_kwargs) != "tree:0": + if _partial_clone_filter(repo_root, **run_kwargs) != "tree:0" or _git_version(**run_kwargs) < (2, 36): return False # A fetch spawns a detached gc/maintenance that would repack the whole refetch outside the # update's time limit; the per-command keys leave the user's own settings alone. refetch = ["git", "-c", "gc.auto=0", "-c", "maintenance.auto=false", "fetch", "--quiet", "--refetch", "--filter=blob:none", "origin"] - fetch_kwargs = dict(cwd=str(repo_root), check=True, capture_output=True, text=True, + fetch_kwargs = dict(cwd=str(repo_root), capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=900, **run_kwargs) - subprocess.run([*refetch, "--tags"], **fetch_kwargs) + subprocess.run([*refetch, "--tags"], check=False, **fetch_kwargs) if not _history_trees_complete(repo_root, **run_kwargs): head = subprocess.run(["git", "rev-parse", "HEAD"], cwd=str(repo_root), check=True, capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=30, **run_kwargs).stdout.strip() - subprocess.run([*refetch, head], **fetch_kwargs) + subprocess.run([*refetch, head], check=True, **fetch_kwargs) if not _history_trees_complete(repo_root, **run_kwargs): raise subprocess.CalledProcessError(1, refetch, stderr="the checked-out history is still missing trees") # A refetch into an existing partial clone leaves its configured filter alone; record the @@ -648,6 +652,13 @@ def convert_treeless_checkout(repo_root: Path, **run_kwargs) -> bool: return True +def _git_version(**run_kwargs) -> tuple: + out = subprocess.run(["git", "--version"], capture_output=True, text=True, encoding="utf-8", + errors="replace", timeout=30, **run_kwargs).stdout + match = re.search(r"(\d+)\.(\d+)", out) + return (int(match[1]), int(match[2])) if match else (0, 0) + + def _history_trees_complete(repo_root: Path, **run_kwargs) -> bool: """Whether every tree in HEAD's history is local, checked without fetching (~3 s for this repo).""" env = dict(run_kwargs.pop("env", None) or noninteractive_git_env(), GIT_NO_LAZY_FETCH="1") diff --git a/tests/hermes_cli/test_update_shallow_heal.py b/tests/hermes_cli/test_update_shallow_heal.py index 3fe5b69d6b..70d5bd0a0a 100644 --- a/tests/hermes_cli/test_update_shallow_heal.py +++ b/tests/hermes_cli/test_update_shallow_heal.py @@ -88,6 +88,11 @@ def _check_out_tag_only_release(clone: Path) -> None: _git(clone, "checkout", "-q", "--detach", "v9") +def _check_out_release_under_a_clashing_local_tag(clone: Path) -> None: + _check_out_tag_only_release(clone) + _git(clone, "tag", "-f", "v9", "origin/main") # the tag refetch is now refused: would clobber + + def _check_out_branch_outside_the_refspec(clone: Path) -> None: _git(clone, "config", "remote.origin.fetch", "+refs/heads/main:refs/remotes/origin/main") _git(clone, "fetch", "-q", "origin", "side:side") @@ -97,6 +102,7 @@ def _check_out_branch_outside_the_refspec(clone: Path) -> None: @pytest.mark.parametrize("ref, check_out, history", [ ("refs/heads/side", _check_out_main, 5), ("refs/tags/v9", _check_out_tag_only_release, 7), + ("refs/tags/v9", _check_out_release_under_a_clashing_local_tag, 7), ("refs/heads/side", _check_out_branch_outside_the_refspec, 7), ]) def test_treeless_checkout_gets_its_whole_history_once_so_walks_stay_offline(tmp_path, ref, check_out, history):