fix(update): the parked-branch cherry refinement never lazy-fetches

`_assess_parked_branch_switch` still ran `git cherry origin/<target>` whenever the
commit-graph count found parked-only commits. On a tree:0 partial clone that
patch-id walk lazy-fetches a tree batch per upstream commit, and nothing bounded
it: the #131444 trace identifies exactly this child (`git cherry origin/main`,
6 h 45 m, 332 packs / 180 GiB) as the writer behind the silent disk runaway.

Run the refinement with GIT_NO_LAZY_FETCH=1 (the NO_LAZY_FETCH_ENV every other
read-only updater probe already uses). A missing object now fails `git cherry`
fast and the conservative rev-list count stands in, so the guard can never turn
into a fetch loop; full clones keep the patch-equivalence refinement.

Test: a reachable promisor remote, a parked branch one commit ahead — the guard
must not add a single pack. Red on origin/main and on #124777 alone (4 packs
after vs 2 before), green here.

Refs #131444. Supersedes the bound/offline variants in #125529, #126693, #128335.
This commit is contained in:
teknium1
2026-10-03 11:10:36 -07:00
committed by Teknium
parent 85aa4b32d2
commit 3bcbfdce34
2 changed files with 42 additions and 10 deletions

View File

@@ -6,6 +6,7 @@ test patches on ``update_cmd`` stay effective).
"""
import logging
import os
from contextlib import suppress
import subprocess
import sys
@@ -13,7 +14,7 @@ from datetime import datetime, timedelta, timezone
from pathlib import Path
from typing import Optional
from hermes_cli._subprocess_compat import windows_hide_flags
from hermes_cli._subprocess_compat import NO_LAZY_FETCH_ENV, windows_hide_flags
logger = logging.getLogger("hermes_cli.update_cmd") # log-record parity with the origin module
@@ -34,12 +35,12 @@ def _git_ok(git_cmd, args, cwd, **kw) -> bool:
return _git_stdout(git_cmd, args, cwd, **kw) is not None
def _git_run(git_cmd, args, cwd=None, *, check=False):
def _git_run(git_cmd, args, cwd=None, *, check=False, env=None):
"""Run ``git_cmd + args`` and return the CompletedProcess.
The updater's git runner: capture all output and decode as UTF-8 regardless of the
Windows ANSI code page (#52649). ``check=True`` raises on non-zero exit. The spawn
always hides its console window (#117781).
always hides its console window (#117781). ``env`` replaces the child's environment.
"""
return subprocess.run(
git_cmd + list(args),
@@ -48,6 +49,7 @@ def _git_run(git_cmd, args, cwd=None, *, check=False):
text=True, encoding="utf-8", errors="replace",
check=check,
creationflags=windows_hide_flags(),
env=env,
)
@@ -200,7 +202,12 @@ def _assess_parked_branch_switch(git_cmd: list[str], cwd: Path, current_branch:
ahead_count = int(ahead.stdout.strip())
if ahead_count == 0:
return True, ""
cherry = _git_run(git_cmd, ["cherry", f"origin/{target_branch}"], cwd)
# The patch-id refinement must never fetch: on a tree:0 clone `git cherry` lazy-fetches
# a tree batch per commit and, with nothing bounding it, one such walk wrote 332 packs /
# 180 GiB over 7 h on Windows (#131444). With lazy fetch off a missing object fails the
# command fast and the commit count below stands in.
cherry = _git_run(git_cmd, ["cherry", f"origin/{target_branch}"], cwd,
env={**os.environ, **NO_LAZY_FETCH_ENV})
if cherry.returncode != 0:
# Patch-equivalence only refines the count for rebase/squash-merged branches;
# a partial clone whose lazy fetch failed must not block a clean checkout,

View File

@@ -175,11 +175,12 @@ def test_missing_origin_ref_is_unverifiable(repo_pair):
assert reason == "unverifiable"
def _treeless_repo_pair(tmp_path):
def _treeless_repo_pair(tmp_path, *, promisor_reachable=False):
"""A treeless clone parked on ``old-feature`` cut from c1, with origin/main
two commits ahead whose commits are local but whose trees are not, and a
promisor remote that can no longer satisfy a lazy fetch — the #124767
shape on a real partial clone.
two commits ahead whose commits are local but whose trees are not — the
#124767 shape on a real partial clone. By default the promisor remote can
no longer satisfy a lazy fetch; ``promisor_reachable=True`` keeps it live
(the #131444 shape, where every lazy fetch succeeds and nothing bounds them).
Returns ``(clone,)``. Local commits/trees are all present; only the
upstream-side trees are missing, which is exactly what ``git cherry``'s
@@ -209,12 +210,18 @@ def _treeless_repo_pair(tmp_path):
_git(origin, "add", "b.txt")
_git(origin, "commit", "-qm", "c3")
# c2/c3 arrive as commits only (no trees); from here every lazy fetch fails.
# c2/c3 arrive as commits only (no trees).
_git(clone, "fetch", "-q", "--filter=tree:0", "origin", "main")
_git(clone, "remote", "set-url", "origin", f"file://{tmp_path / 'nowhere'}")
if not promisor_reachable:
_git(clone, "remote", "set-url", "origin", f"file://{tmp_path / 'nowhere'}")
return clone
def _pack_count(clone) -> int:
"""Packfiles in the clone — every lazy fetch from the promisor adds one (#131444)."""
return len(list((clone / ".git" / "objects" / "pack").glob("*.pack")))
def test_treeless_clone_verifies_merged_parked_branch_from_commit_graph(tmp_path):
"""Fully merged parked branch on a tree:0 clone with an unreachable promisor
remote: ``git cherry`` needs origin/main's trees and its lazy fetch fails,
@@ -245,6 +252,24 @@ def test_treeless_cherry_failure_degrades_to_conservative_unmerged(tmp_path):
assert reason == "unmerged:1"
def test_parked_branch_guard_never_lazy_fetches_from_a_live_promisor(tmp_path):
"""Clean parked branch with a local commit on a tree:0 clone whose promisor
remote IS reachable: ``git cherry`` would lazy-fetch a tree batch per
upstream commit, and with nothing bounding that walk one such assessment
wrote 332 packs / 180 GiB in 7 h on Windows (#131444). The guard must not
add a single pack and settles for the commit-graph count."""
clone = _treeless_repo_pair(tmp_path, promisor_reachable=True)
(clone / "feature.txt").write_text("unmerged work\n")
_git(clone, "add", "feature.txt")
_git(clone, "commit", "-qm", "feature work")
packs_before = _pack_count(clone)
safe, reason = update_cmd._assess_parked_branch_switch(
GIT, clone, "old-feature", "main"
)
assert (safe, reason) == (True, "unmerged:1")
assert _pack_count(clone) == packs_before
# ---------------------------------------------------------------------------
# Skip warning content
# ---------------------------------------------------------------------------