Rebase onto main: main moved the repack into git_commit_graph.run_bounded_git, a niced git in a
process group of its own (process_group=0) that the timeout and atexit kill. A SIGKILLed or
OOM-killed owner ran neither, so git repack and its pack-objects kept packing for as long as they
took; the detached-spawn ratchet flagged the new spawn. It now starts through spawn_owned (same
own group, plus the owner-death supervisor) and releases the group once it is empty; the timeout,
atexit and SIGTERM-first kill are unchanged. spawn_commit_graph_refresh is meant to be detached
("must neither delay the update nor die with it"), so its Popen gets a marker with that reason.
Test: a git-maintenance case in the kill -9 owner matrix (a `git` that execs the child, the way
repack forks pack-objects): the SIGKILLed owner's tree dies, the live owner's survives. Fails
with the plain process_group spawn.
Review finding (#144647, P2): the object-store lock sweep removed a live
writer's lock. _git_proc_running matched only processes named exactly
`git` (git-core's dashed executables, e.g. a `git-commit-graph` writer,
ran unseen) and answered "no git running" whenever pgrep was missing or
failed. A writer stopped after taking commit-graph.lock, its lock aged
past ten minutes, lost the lock to the sweep; once resumed the repo was
left with no commit graph while Hermes had stamped it fresh.
The guard now fails closed: on POSIX it scans with psutil (pgrep with a
`^git(-.*|\.exe)?$` pattern in the dependency-free bootstrap), on Windows
it reads every tasklist image name; both match git, git-<sub> and .exe
through _early_recovery._git_program. A probe that cannot tell (pgrep
missing or erroring, tasklist failing, a psutil read denied) keeps every
lock. This covers every sweep that shares the guard (update-path
shallow/index/HEAD locks, tmp-pack debris), not only the graph refresh.
The freshness stamp is written only when a graph is on disk after a
successful write (a split chain whose layers all exist, or a monolithic
graph): a write that exits 0 with no graph, as in a commitless repo,
no longer marks the repo fresh.
Review round on the commit-graph refresh (Greptile + re-review):
- Global graph setting is ignored: core.commitGraph=false is read with the
user's global/system git config, which the writer's isolated env blanks.
- Expired slots allow duplicate writers: check and claim of the maintenance
slot run under one flock/msvcrt lock on the slot file, which is never
unlinked; a failed spawn gives back only a claim holding its own pid.
- Linked worktrees and submodules (a .git file) were skipped: the common git
dir is resolved once (update_lock._git_common_dir) and used for the object
store, slot, debris sweep and fresh stamp, on the update spawn and on the
`hermes -w` pass alike.
- Timeouts were silent: the kill is logged at WARNING, with the timeout
unrounded.
- kill_process_tree(grace>0) returned once the leader exited on SIGTERM,
orphaning a group member that ignores it: the rest of the group it led is
SIGKILLed. grace=0 is unchanged.
- The child's failure line was lost when <home>/logs did not exist: the log
dir is created at failure time via mkdir_under_hermes_home (a deleted
profile is not resurrected), and a log that cannot be opened no longer
raises out of the logger.
Review round on #144647 (head d016f04694d):
- CI routing: the refresh (slot claim, bounded runner, stale check, detached spawn) moves to
hermes_cli/git_commit_graph.py, routed to e2e_upgrade with the update pipeline. The update path
imports it instead of all of worktree_ops; worktree_ops keeps its repack seams and imports the
shared slot claim and runner from it (no cycle).
- W1: the -w pass runs in whatever repo it was launched from, so its refresh sweeps only the
object-store locks (clear_stale_git_locks(names=OBJECT_STORE_LOCK_NAMES)); index.lock / HEAD.lock
there belong to the user's git. The install checkout's update sweep is unchanged.
- W2: timeout/exit reaping SIGTERMs the git tree first and SIGKILLs only after a 5 s grace
(kill_process_tree(grace=), default unchanged for every other caller), so git deletes
commit-graph-chain.lock and tmp_graph_*. The age-gated tmp sweep now also takes
objects/info/commit-graphs/tmp_graph_*.
- N1: a breakaway-forbidding Windows job gets the no-breakaway retry, and a spawn that fails gives
the 6 h slot back.
- N2: the detached child logs a failed write (git's fatal line) to the home's agent.log/errors.log.
- core.commitGraph=false in the target repo skips the write.
- N3: a successful no-op write stamps hermes-commit-graph.fresh, so an idle repo is not rewritten
every slot.
- N4: test_update_finish's completion child and every in-process hermes_cli test stub the
detached spawn; no writer outlives a test run.
- Greptile: the refresh test now runs on a treeless clone over a Bloom graph and asserts the
write adds no pack (red when --no-changed-paths is dropped).
kshitijk4poor