Skip to content

hooks: skip rebuild in linked git worktrees#1806

Closed
Claude-Madera wants to merge 1 commit into
Graphify-Labs:v8from
Claude-Madera:fix/skip-linked-worktrees
Closed

hooks: skip rebuild in linked git worktrees#1806
Claude-Madera wants to merge 1 commit into
Graphify-Labs:v8from
Claude-Madera:fix/skip-linked-worktrees

Conversation

@Claude-Madera

Copy link
Copy Markdown
Contributor

Problem

The post-commit and post-checkout hooks rebuild graphify-out/ on every commit and branch switch. Inside a linked worktree (git worktree add) this misbehaves:

  • The canonical graphify-out/ belongs to the primary checkout; rebuilding it from every worktree is wasteful.

  • Deploy/CI flows that create a worktree, switch/checkout, then git clean it, race the detached background rebuild against the cleanup — producing:

    failed to remove graphify-out/: Directory not empty
    

This surfaced in a deploy "cockpit" that pins release worktrees of three repos: every pin fired a pointless full rebuild and intermittently failed the git clean.

Fix

Guard both hooks: a linked worktree has git-dir != git-common-dir, so short-circuit before launching the rebuild.

Key correctness detail: the compare uses absolute paths —

_GFY_GITDIR=$(git rev-parse --absolute-git-dir 2>/dev/null)
_GFY_COMMONDIR=$(cd "$(git rev-parse --git-common-dir 2>/dev/null)" 2>/dev/null && pwd)
if [ -n "$_GFY_COMMONDIR" ] && [ "$_GFY_GITDIR" != "$_GFY_COMMONDIR" ]; then
    exit 0
fi

Git exports GIT_DIR to hooks and it is sometimes absolute; comparing a raw $GIT_DIR against a relative --git-common-dir (.git) would false-positive and wrongly skip the primary checkout. Resolving both to absolute paths avoids that. Factored into a shared _WORKTREE_GUARD constant, mirroring the existing _PYTHON_DETECT pattern.

Tests

  • test_hooks_skip_linked_worktrees — both generated scripts contain the guard exactly once and use the absolute-path compare.
  • test_worktree_guard_skips_only_linked_worktree — end-to-end against a real git worktree: the guard runs through on the primary checkout and exits early in a linked worktree.

Both pass; no previously-passing tests affected.

🤖 Generated with Claude Code

The post-commit and post-checkout hooks rebuild graphify-out/ on every
commit / branch switch. In a linked worktree (`git worktree add`) that is
wasteful -- the canonical graph belongs to the primary checkout -- and
deploy/CI flows that `git clean` a worktree race the detached rebuild
against the cleanup, producing 'failed to remove graphify-out/: Directory
not empty'.

Guard both hooks: a linked worktree has git-dir != git-common-dir. Compare
ABSOLUTE paths (git rev-parse --absolute-git-dir vs a resolved
--git-common-dir) so the GIT_DIR env var git exports to hooks -- sometimes
absolute -- cannot false-positive and skip the primary checkout.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@safishamsi

Copy link
Copy Markdown
Collaborator

Thanks for this, @Claude-Madera — the worktree diagnosis and especially the absolute-path detail (raw $GIT_DIR can be absolute while --git-common-dir is the relative .git, which would false-positive the primary checkout) were exactly right, and I've carried that guard into 5f57c46 (shipping in 0.9.14).

I landed it directly rather than merging so the same commit could also close the two related gaps from #1809 that this PR didn't cover: post-checkout not honoring GRAPHIFY_SKIP_HOOK, and the .git/info/exclude bloat in #1810. A couple of small deltas from your version: the guard normalizes both paths with cd ... && pwd instead of git rev-parse --absolute-git-dir (works on older git without the newer flag), and I left the existing comment em-dashes alone to keep the diff focused on behavior. Credited you in the commit trailer. Really appreciate the PR.

@safishamsi safishamsi closed this Jul 12, 2026
safishamsi added a commit that referenced this pull request Jul 13, 2026
…1809)

#1810 — detection read only .gitignore/.graphifyignore, never
.git/info/exclude, which is where git records local-only excludes and
where `git worktree add` writes nested worktree paths. graphify walked
into those worktree copies and the graph exploded (one 5-worktree repo:
9.4k nodes/10MB -> 210k nodes/311MB, ~77% duplicate). detect now loads
info/exclude at lowest precedence (below every per-dir .gitignore, per
git, so a nearer `!` still wins) and resolves the linked-worktree /
submodule case where `.git` is a file to the shared common git dir.

#1809 — two git-hook gaps: (a) post-checkout never honored
GRAPHIFY_SKIP_HOOK, so the var stopped commit rebuilds but not
branch-switch ones; now checked in both. (b) with core.hooksPath shared
across worktrees, a commit in any linked worktree fired post-commit,
which wrote a rogue delta-only graph.json into it and raced deploy/CI
`git clean` against the detached rebuild. Both hooks now short-circuit
in a linked worktree (git-dir != git-common-dir), comparing ABSOLUTE
paths so the primary checkout (where --git-common-dir is the relative
".git") is never false-positived and skipped.

Adds regression tests: info/exclude honored + negation precedence;
both hooks honor the skip env and carry the worktree guard; and an
end-to-end guard check against a real `git worktree`.

Reported by @cdahl86-cyber (#1810, #1809); the worktree guard was
co-developed with @Claude-Madera's PR #1806.

Co-Authored-By: cdahl86-cyber <cdahl86-cyber@users.noreply.github.com>
Co-Authored-By: Claude-Madera <Claude-Madera@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants