mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-25 16:22:30 +00:00
fix(hooks): run the rebuild from the checkout's top level (#3258)
The rebuild bodies read graphify-out/ and its .graphify_root relative to the cwd, but git does not always start hooks at the top level. With GIT_DIR exported and no GIT_WORK_TREE (a nested git call from another hook or tool), git takes the caller's cwd as the work tree and `git rev-parse --show-toplevel` echoes that subdirectory. A stray graphify-out/ left there by an earlier run then roots the rebuild at the subdirectory; for .claude/worktrees/ that walks every linked worktree. The #3265 guard rejects a `..` marker, but its fallback is the same cwd. Add a shared _TOPLEVEL_ANCHOR snippet right after _WORKTREE_GUARD in all four hooks. In the standard layout it cds to the parent of the absolute common .git dir; with GIT_WORK_TREE or a separate git dir it keeps git's own --show-toplevel. GIT_DIR is rewritten to its absolute form so later git calls still resolve. The block already runs in a subshell, so the cd does not leak into chained hooks.
This commit is contained in:
1 parent
c588b72d93
commit
e6193ab816
2 files changed
+95
-4
No files matched your search
+27
-4
@@ -413,6 +413,29 @@ fi
|
||||
"""
|
||||
|
||||
|
||||
# Run the rest of each hook from the checkout's top level, shared by all hooks
|
||||
# and placed right after _WORKTREE_GUARD (it reuses _GFY_GITDIR/_GFY_COMMONDIR).
|
||||
# The rebuild reads graphify-out/ and its .graphify_root relative to the cwd,
|
||||
# but git does not always start hooks at the top: with GIT_DIR exported and no
|
||||
# GIT_WORK_TREE (a nested git call from another hook or tool), git takes the
|
||||
# caller's cwd as the work tree, and `--show-toplevel` echoes that same
|
||||
# subdirectory. A stray graphify-out/ left there by an earlier run then roots the
|
||||
# rebuild at the subdirectory - for .claude/worktrees/ that is every linked
|
||||
# worktree (#3258). In the standard layout the work tree is the parent of the
|
||||
# common .git dir; GIT_WORK_TREE and --separate-git-dir layouts keep git's own
|
||||
# answer. GIT_DIR is rewritten to its absolute form (not newly exported) so the
|
||||
# git calls below still resolve after the cd.
|
||||
_TOPLEVEL_ANCHOR = """\
|
||||
if [ -z "${GIT_WORK_TREE:-}" ] && [ "${_GFY_COMMONDIR##*/}" = ".git" ]; then
|
||||
_GFY_TOP=${_GFY_COMMONDIR%/*}
|
||||
else
|
||||
_GFY_TOP=$(git rev-parse --show-toplevel 2>/dev/null)
|
||||
fi
|
||||
[ -n "$_GFY_TOP" ] && cd "$_GFY_TOP" || exit 0
|
||||
[ -n "$_GFY_GITDIR" ] && GIT_DIR=$_GFY_GITDIR
|
||||
"""
|
||||
|
||||
|
||||
# Both hook bodies run inside a subshell `( ... )`. The generated block is
|
||||
# appended to whatever post-commit / post-checkout already exists, and other
|
||||
# tools chain their own logic after it; every skip condition in the block is a
|
||||
@@ -449,7 +472,7 @@ GIT_DIR=${GIT_DIR:-$(git rev-parse --git-dir 2>/dev/null)}
|
||||
[ -f "$GIT_DIR/MERGE_HEAD" ] && exit 0
|
||||
[ "${GRAPHIFY_SKIP_HOOK:-0}" = "1" ] && exit 0
|
||||
|
||||
""" + _WORKTREE_GUARD + """
|
||||
""" + _WORKTREE_GUARD + _TOPLEVEL_ANCHOR + """
|
||||
# CHERRY_PICK_HEAD can't be the skip signal: git leaves it on disk while post-commit
|
||||
# runs, so every pick was silenced (#4490, same trap as MERGE_HEAD in #4463). For a
|
||||
# range pick or revert, skip every step but the last and rebuild once from
|
||||
@@ -579,7 +602,7 @@ GIT_DIR=${GIT_DIR:-$(git rev-parse --git-dir 2>/dev/null)}
|
||||
# suppressed commit-triggered rebuilds but not branch-switch ones (#1809).
|
||||
[ "${GRAPHIFY_SKIP_HOOK:-0}" = "1" ] && exit 0
|
||||
|
||||
""" + _WORKTREE_GUARD + _PYTHON_DETECT + """
|
||||
""" + _WORKTREE_GUARD + _TOPLEVEL_ANCHOR + _PYTHON_DETECT + """
|
||||
_GRAPHIFY_LOG="${HOME}/.cache/graphify-rebuild.log"
|
||||
mkdir -p "$(dirname "$_GRAPHIFY_LOG")"
|
||||
export GRAPHIFY_REBUILD_LOG="$_GRAPHIFY_LOG"
|
||||
@@ -632,7 +655,7 @@ GIT_DIR=${GIT_DIR:-$(git rev-parse --git-dir 2>/dev/null)}
|
||||
# Honor the same opt out the commit and checkout hooks already respect.
|
||||
[ "${GRAPHIFY_SKIP_HOOK:-0}" = "1" ] && exit 0
|
||||
|
||||
""" + _WORKTREE_GUARD + _PYTHON_DETECT + """
|
||||
""" + _WORKTREE_GUARD + _TOPLEVEL_ANCHOR + _PYTHON_DETECT + """
|
||||
_GRAPHIFY_LOG="${HOME}/.cache/graphify-rebuild.log"
|
||||
mkdir -p "$(dirname "$_GRAPHIFY_LOG")"
|
||||
export GRAPHIFY_REBUILD_LOG="$_GRAPHIFY_LOG"
|
||||
@@ -684,7 +707,7 @@ fi
|
||||
# Honor the same opt out the commit and checkout hooks already respect.
|
||||
[ "${GRAPHIFY_SKIP_HOOK:-0}" = "1" ] && exit 0
|
||||
|
||||
""" + _WORKTREE_GUARD + _PYTHON_DETECT + """
|
||||
""" + _WORKTREE_GUARD + _TOPLEVEL_ANCHOR + _PYTHON_DETECT + """
|
||||
_GRAPHIFY_LOG="${HOME}/.cache/graphify-rebuild.log"
|
||||
mkdir -p "$(dirname "$_GRAPHIFY_LOG")"
|
||||
export GRAPHIFY_REBUILD_LOG="$_GRAPHIFY_LOG"
|
||||
|
||||
@@ -1213,6 +1213,74 @@ def test_worktree_guard_runs_on_primary_skips_linked(tmp_path):
|
||||
assert "RAN" not in r_linked.stdout, "guard failed to skip the linked worktree"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("name,script", _HOOK_SCRIPTS)
|
||||
def test_hooks_anchor_to_the_top_level_after_the_worktree_guard(name, script):
|
||||
"""Every hook cds to the checkout's top level before anything reads
|
||||
graphify-out/ or launches the rebuild (#3258), and does so after the
|
||||
worktree guard, whose _GFY_GITDIR/_GFY_COMMONDIR it reuses."""
|
||||
from graphify.hooks import _TOPLEVEL_ANCHOR, _WORKTREE_GUARD
|
||||
assert script.count(_TOPLEVEL_ANCHOR) == 1, f"{name} anchor not present exactly once"
|
||||
assert script.index(_WORKTREE_GUARD) < script.index(_TOPLEVEL_ANCHOR)
|
||||
# The cheap "is there a graph here?" early exits may stay ahead of it: at
|
||||
# worst they skip a rebuild. Choosing the interpreter and launching the
|
||||
# rebuild (which reads .graphify_root) must not.
|
||||
code = "\n".join(line for line in script.splitlines() if not line.lstrip().startswith("#"))
|
||||
anchor_at = code.index(_TOPLEVEL_ANCHOR.splitlines()[0])
|
||||
for later in ("GRAPHIFY_PYTHON", "_rebuild_code"):
|
||||
assert anchor_at < code.index(later), f"{name} uses {later} before anchoring"
|
||||
|
||||
|
||||
def test_toplevel_anchor_runs_from_the_top_when_git_starts_the_hook_in_a_subdir(tmp_path):
|
||||
"""End-to-end against a real repo (#3258): with GIT_DIR exported and no
|
||||
GIT_WORK_TREE, git runs hooks in the caller's cwd and `--show-toplevel`
|
||||
echoes that subdirectory. The anchored snippet must still run at the
|
||||
repository top, keep later git calls working, and leave a linked worktree
|
||||
to the guard."""
|
||||
if shutil.which("git") is None: # pragma: no cover
|
||||
pytest.skip("git not available")
|
||||
primary = tmp_path / "primary"
|
||||
primary.mkdir()
|
||||
|
||||
def _git(*args, cwd):
|
||||
subprocess.run(["git", *args], cwd=cwd, check=True,
|
||||
capture_output=True, text=True)
|
||||
|
||||
_git("init", "-q", ".", cwd=primary)
|
||||
_git("config", "user.email", "t@t.co", cwd=primary)
|
||||
_git("config", "user.name", "t", cwd=primary)
|
||||
(primary / "a.txt").write_text("x")
|
||||
_git("add", "-A", cwd=primary)
|
||||
_git("commit", "-qm", "init", cwd=primary)
|
||||
sub = primary / ".claude" / "worktrees"
|
||||
sub.mkdir(parents=True)
|
||||
linked = sub / "wt1"
|
||||
_git("worktree", "add", "-q", str(linked), "-b", "feature", cwd=primary)
|
||||
|
||||
from graphify.hooks import _TOPLEVEL_ANCHOR, _WORKTREE_GUARD
|
||||
snippet = (_WORKTREE_GUARD + _TOPLEVEL_ANCHOR
|
||||
+ 'git rev-parse --verify -q HEAD >/dev/null && echo GIT_OK\n'
|
||||
+ ': > anchored.txt\n')
|
||||
env = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")}
|
||||
|
||||
cases = [
|
||||
("top", primary, {}),
|
||||
("subdir", sub, {}),
|
||||
("subdir+GIT_DIR", sub, {"GIT_DIR": "../../.git"}),
|
||||
]
|
||||
for label, cwd, extra in cases:
|
||||
r = subprocess.run(["sh", "-c", snippet], cwd=cwd, env={**env, **extra},
|
||||
capture_output=True, text=True)
|
||||
assert (primary / "anchored.txt").exists(), f"{label}: did not run at the top ({r.stderr})"
|
||||
assert not (sub / "anchored.txt").exists(), f"{label}: ran in the subdirectory"
|
||||
assert "GIT_OK" in r.stdout, f"{label}: git broke after the cd ({r.stderr})"
|
||||
(primary / "anchored.txt").unlink()
|
||||
|
||||
r = subprocess.run(["sh", "-c", snippet], cwd=linked, env=env,
|
||||
capture_output=True, text=True)
|
||||
assert not (linked / "anchored.txt").exists() and not (primary / "anchored.txt").exists(), \
|
||||
"the linked worktree must still be skipped by the guard"
|
||||
|
||||
|
||||
# ── #1907: duplicate keys in .git/config must not trigger spurious warnings ──
|
||||
|
||||
def _append_duplicate_config_entries(repo: Path) -> None:
|
||||
|
||||
Reference in new issue
Block a user