diff --git a/graphify/__main__.py b/graphify/__main__.py index d379a8e72..2b2f44dd7 100644 --- a/graphify/__main__.py +++ b/graphify/__main__.py @@ -994,7 +994,7 @@ def _run_cli() -> None: print(" (or set NEO4J_PASSWORD instead of --password to keep it off argv)") print(" export falkordb emit Cypher or push to FalkorDB [--graph PATH] [--push URI] [--user U] [--password P]") print(" (or set FALKORDB_PASSWORD instead of --password to keep it off argv)") - print(" hook install install post-commit/post-checkout/post-merge git hooks (all platforms)") + print(" hook install install post-commit/post-checkout/post-merge/post-rewrite git hooks (all platforms)") print(" hook uninstall remove git hooks") print(" hook status check if git hooks are installed") print( diff --git a/graphify/hooks.py b/graphify/hooks.py index 05d3b28d7..cd0727190 100644 --- a/graphify/hooks.py +++ b/graphify/hooks.py @@ -11,6 +11,8 @@ _CHECKOUT_MARKER = "# graphify-checkout-hook-start" _CHECKOUT_MARKER_END = "# graphify-checkout-hook-end" _MERGE_HOOK_MARKER = "# graphify-merge-hook-start" _MERGE_HOOK_MARKER_END = "# graphify-merge-hook-end" +_REWRITE_HOOK_MARKER = "# graphify-rewrite-hook-start" +_REWRITE_HOOK_MARKER_END = "# graphify-rewrite-hook-end" # __PINNED_PYTHON__ is replaced at install time with the absolute path of the # Python interpreter that ran `graphify hook install`. For uv-tool and pipx @@ -438,22 +440,55 @@ if [ -n "${WINDIR:-}" ] || [ -n "${MSYSTEM:-}" ]; then export GRAPHIFY_MAX_WORKERS="${GRAPHIFY_MAX_WORKERS:-1}" fi -# Skip during rebase/merge/cherry-pick to avoid blocking --continue with unstaged changes +# Skip during a rebase or merge to avoid blocking --continue with unstaged changes # git exports GIT_DIR to hooks; the rev-parse fallback only runs when invoked by # hand (each git exec costs 1s+ on AV-scanned Windows machines). GIT_DIR=${GIT_DIR:-$(git rev-parse --git-dir 2>/dev/null)} [ -d "$GIT_DIR/rebase-merge" ] && exit 0 [ -d "$GIT_DIR/rebase-apply" ] && exit 0 [ -f "$GIT_DIR/MERGE_HEAD" ] && exit 0 -[ -f "$GIT_DIR/CHERRY_PICK_HEAD" ] && exit 0 - [ "${GRAPHIFY_SKIP_HOOK:-0}" = "1" ] && exit 0 """ + _WORKTREE_GUARD + """ +# 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 +# sequencer/head, so a tracked graph.json isn't rewritten between steps. sequencer/ is +# trusted only when its first todo line is the commit just made; a stale one left by +# `git reset --hard` would otherwise silence every later commit. +_GFY_SEQ_BASE="" +if [ -f "$GIT_DIR/sequencer/todo" ]; then + IFS= read -r _GFY_FIRST < "$GIT_DIR/sequencer/todo" + _GFY_VERB=${_GFY_FIRST%% *} + _GFY_REST=${_GFY_FIRST#* } + case "$_GFY_REST" in + *" "*) _GFY_SUBJ=${_GFY_REST#* } ;; + *) _GFY_SUBJ="" ;; + esac + _GFY_SEQ_LIVE=0 + _GFY_Q='"' + case "$_GFY_VERB" in + pick) [ "$(git log -1 --format=%s 2>/dev/null)" = "$_GFY_SUBJ" ] && _GFY_SEQ_LIVE=1 ;; + revert) [ "$(git log -1 --format=%s 2>/dev/null)" = "Revert ${_GFY_Q}${_GFY_SUBJ}${_GFY_Q}" ] && _GFY_SEQ_LIVE=1 ;; + esac + if [ "$_GFY_SEQ_LIVE" = "1" ]; then + _GFY_PENDING=0 + while IFS= read -r _GFY_STEP || [ -n "$_GFY_STEP" ]; do + [ -n "$_GFY_STEP" ] && _GFY_PENDING=$((_GFY_PENDING + 1)) + done < "$GIT_DIR/sequencer/todo" + [ "$_GFY_PENDING" -gt 1 ] && exit 0 + [ -f "$GIT_DIR/sequencer/head" ] && IFS= read -r _GFY_SEQ_BASE < "$GIT_DIR/sequencer/head" + fi +fi + # core.quotePath=false: by default git prints a non-ASCII path as a quoted # octal-escaped string, which matches no file on disk, so the edit was silently # left out of the rebuild. Names git still quotes are decoded in the rebuild. CHANGED=$(git -c core.quotePath=false diff --name-only HEAD~1 HEAD 2>/dev/null || git -c core.quotePath=false diff --name-only HEAD 2>/dev/null) +# Last pick of a range: cover every commit picked since the sequence began, not only this one. +if [ -n "$_GFY_SEQ_BASE" ]; then + CHANGED=$(git -c core.quotePath=false diff --name-only "$_GFY_SEQ_BASE" HEAD 2>/dev/null) +fi if [ -z "$CHANGED" ]; then exit 0 fi @@ -608,6 +643,58 @@ echo "[graphify] Merge completed - launching background rebuild (log: $_GRAPHIFY """ +_REWRITE_SCRIPT = """\ +# graphify-rewrite-hook-start +# Auto-rebuilds the knowledge graph (code only) when a rebase finishes, including +# `git pull --rebase`. post-commit cannot cover it: it skips while rebase-merge/ +# exists, and every replayed commit is made under it. git runs post-rewrite once, +# after the last commit, with the tree in its final state (#4490). +# Installed by: graphify hook install +( + +# Deterministic clustering: networkx louvain iterates string-keyed sets whose +# order is randomized per-process by PYTHONHASHSEED, so community assignments +# churn run-to-run. Pinning it makes graphify-out reproducible. +export PYTHONHASHSEED=0 +__VIZ_LIMIT_EXPORT__ +# Git for Windows/MSYS hooks can inherit fragile pipe handles from GUI clients +# and agent shells. Keep hook-triggered rebuilds sequential by default there; +# explicit GRAPHIFY_MAX_WORKERS still wins for users who want parallelism. +if [ -n "${WINDIR:-}" ] || [ -n "${MSYSTEM:-}" ]; then + export GRAPHIFY_MAX_WORKERS="${GRAPHIFY_MAX_WORKERS:-1}" +fi + +# git writes the old/new sha pairs of the rewritten commits to stdin; drain them so +# git never blocks on a full pipe. Skipped when run by hand, where stdin is a terminal. +[ -t 0 ] || cat >/dev/null + +# $1 is "rebase" or "amend". An amend already rebuilt through post-commit. +[ "${1:-}" = "rebase" ] || exit 0 + +# Only run if the output dir exists (graph has been built before). Resolve it from +# GRAPHIFY_OUT like the rebuild body does (#1423), not a literal graphify-out/. +_GFY_OUT="${GRAPHIFY_OUT:-graphify-out}" +if [ ! -d "${_GFY_OUT%/}" ]; then + exit 0 +fi + +# No rebase-merge/ or rebase-apply/ check, unlike post-commit and post-checkout: git +# removes rebase-merge/ only after post-rewrite returns, so it is still on disk here +# and would skip every rebase (the same trap as MERGE_HEAD in post-merge, #4463). + +# Honor the same opt out the commit and checkout hooks already respect. +[ "${GRAPHIFY_SKIP_HOOK:-0}" = "1" ] && exit 0 + +""" + _WORKTREE_GUARD + _PYTHON_DETECT + """ +_GRAPHIFY_LOG="${HOME}/.cache/graphify-rebuild.log" +mkdir -p "$(dirname "$_GRAPHIFY_LOG")" +export GRAPHIFY_REBUILD_LOG="$_GRAPHIFY_LOG" +echo "[graphify] Rebase completed - launching background rebuild (log: $_GRAPHIFY_LOG)" +""" + _detached_launch(_REBUILD_BODY_CHECKOUT) + """) +# graphify-rewrite-hook-end +""" + + def _load_graphifyrc(root: Path) -> dict: """Load ``/.graphifyrc``; the parser lives in :mod:`graphify.rcfile`. @@ -1005,7 +1092,7 @@ def _user_hooks_dir(hooks_dir: Path) -> Path: def install(path: Path = Path(".")) -> str: - """Install graphify's post-commit, post-checkout, and post-merge hooks in the nearest git repo.""" + """Install graphify's post-commit, post-checkout, post-merge, and post-rewrite hooks in the nearest git repo.""" root = _git_root(path) if root is None: raise RuntimeError(f"No git repository found at or above {path.resolve()}") @@ -1027,22 +1114,26 @@ def install(path: Path = Path(".")) -> str: hook = _HOOK_SCRIPT.replace("__PINNED_PYTHON__", pinned).replace("__VIZ_LIMIT_EXPORT__", viz_export) checkout = _CHECKOUT_SCRIPT.replace("__PINNED_PYTHON__", pinned).replace("__VIZ_LIMIT_EXPORT__", viz_export) merge_hook = _MERGE_SCRIPT.replace("__PINNED_PYTHON__", pinned).replace("__VIZ_LIMIT_EXPORT__", viz_export) + rewrite_hook = _REWRITE_SCRIPT.replace("__PINNED_PYTHON__", pinned).replace("__VIZ_LIMIT_EXPORT__", viz_export) commit_msg = _install_hook(hooks_dir, "post-commit", hook, _HOOK_MARKER, _HOOK_MARKER_END) checkout_msg = _install_hook(hooks_dir, "post-checkout", checkout, _CHECKOUT_MARKER, _CHECKOUT_MARKER_END) post_merge_msg = _install_hook( hooks_dir, "post-merge", merge_hook, _MERGE_HOOK_MARKER, _MERGE_HOOK_MARKER_END ) + post_rewrite_msg = _install_hook( + hooks_dir, "post-rewrite", rewrite_hook, _REWRITE_HOOK_MARKER, _REWRITE_HOOK_MARKER_END + ) merge_msg = _register_merge_driver(root) return ( f"post-commit: {commit_msg}\npost-checkout: {checkout_msg}\n" - f"post-merge: {post_merge_msg}\nmerge driver: {merge_msg}" + f"post-merge: {post_merge_msg}\npost-rewrite: {post_rewrite_msg}\nmerge driver: {merge_msg}" ) def uninstall(path: Path = Path(".")) -> str: - """Remove graphify's post-commit, post-checkout, and post-merge hooks.""" + """Remove graphify's post-commit, post-checkout, post-merge, and post-rewrite hooks.""" root = _git_root(path) if root is None: raise RuntimeError(f"No git repository found at or above {path.resolve()}") @@ -1053,11 +1144,14 @@ def uninstall(path: Path = Path(".")) -> str: post_merge_msg = _uninstall_hook( hooks_dir, "post-merge", _MERGE_HOOK_MARKER, _MERGE_HOOK_MARKER_END ) + post_rewrite_msg = _uninstall_hook( + hooks_dir, "post-rewrite", _REWRITE_HOOK_MARKER, _REWRITE_HOOK_MARKER_END + ) merge_msg = _unregister_merge_driver(root) return ( f"post-commit: {commit_msg}\npost-checkout: {checkout_msg}\n" - f"post-merge: {post_merge_msg}\nmerge driver: {merge_msg}" + f"post-merge: {post_merge_msg}\npost-rewrite: {post_rewrite_msg}\nmerge driver: {merge_msg}" ) @@ -1126,11 +1220,12 @@ def status(path: Path = Path(".")) -> str: commit = _check("post-commit", _HOOK_MARKER, _HOOK_MARKER_END, _HOOK_SCRIPT) checkout = _check("post-checkout", _CHECKOUT_MARKER, _CHECKOUT_MARKER_END, _CHECKOUT_SCRIPT) post_merge = _check("post-merge", _MERGE_HOOK_MARKER, _MERGE_HOOK_MARKER_END, _MERGE_SCRIPT) + post_rewrite = _check("post-rewrite", _REWRITE_HOOK_MARKER, _REWRITE_HOOK_MARKER_END, _REWRITE_SCRIPT) merge = _merge_driver_status(root) res = ( f"post-commit: {commit}\npost-checkout: {checkout}\n" - f"post-merge: {post_merge}\nmerge driver: {merge}" + f"post-merge: {post_merge}\npost-rewrite: {post_rewrite}\nmerge driver: {merge}" ) if cfg_limit is not None: res += f"\nviz node limit: {cfg_limit}" diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 0411565f6..5693eb187 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -9,6 +9,7 @@ from pathlib import Path import pytest from graphify.hooks import ( install, uninstall, status, _hooks_dir, _HOOK_MARKER, _CHECKOUT_MARKER, _MERGE_HOOK_MARKER, + _REWRITE_HOOK_MARKER, ) @@ -165,6 +166,59 @@ def test_status_shows_post_merge_hook(tmp_path): assert "post-merge: not installed" in result +def test_install_creates_post_rewrite_hook(tmp_path): + # #4490: git runs post-rewrite once when a rebase (or pull --rebase) finishes; + # post-commit skips every replayed commit, so nothing else rebuilds after one. + repo = _make_git_repo(tmp_path) + install(repo) + hook = repo / ".git" / "hooks" / "post-rewrite" + assert hook.exists() + assert _REWRITE_HOOK_MARKER in hook.read_text() + + +def test_install_post_rewrite_is_executable(tmp_path): + repo = _make_git_repo(tmp_path) + install(repo) + hook = repo / ".git" / "hooks" / "post-rewrite" + if os.name == "nt": + assert hook.read_text(encoding="utf-8").startswith("#!/bin/sh\n") + else: + assert hook.stat().st_mode & 0o111 + + +def test_uninstall_removes_post_rewrite_hook(tmp_path): + repo = _make_git_repo(tmp_path) + install(repo) + uninstall(repo) + assert not (repo / ".git" / "hooks" / "post-rewrite").exists() + + +def test_status_shows_post_rewrite_hook(tmp_path): + repo = _make_git_repo(tmp_path) + install(repo) + assert "post-rewrite: installed" in status(repo) + + uninstall(repo) + assert "post-rewrite: not installed" in status(repo) + + +def test_install_appends_to_a_user_post_rewrite_hook_and_uninstall_keeps_it(tmp_path): + repo = _make_git_repo(tmp_path) + hook = repo / ".git" / "hooks" / "post-rewrite" + hook.write_text("#!/bin/sh\necho 'user rewrite hook'\n", encoding="utf-8") + hook.chmod(0o755) + + assert "appended to existing post-rewrite hook" in install(repo) + content = hook.read_text(encoding="utf-8") + assert "echo 'user rewrite hook'" in content + assert _REWRITE_HOOK_MARKER in content + + uninstall(repo) + content = hook.read_text(encoding="utf-8") + assert "echo 'user rewrite hook'" in content + assert _REWRITE_HOOK_MARKER not in content + + def test_hooks_dir_resolves_relative_git_hooks_path(tmp_path, monkeypatch): repo = _make_git_repo(tmp_path) @@ -272,6 +326,7 @@ from graphify.hooks import ( # noqa: E402 _HOOK_SCRIPT, _CHECKOUT_SCRIPT, _MERGE_SCRIPT, + _REWRITE_SCRIPT, _REBUILD_BODY_COMMIT, _REBUILD_BODY_CHECKOUT, _detached_launch, @@ -281,6 +336,7 @@ _HOOK_SCRIPTS = [ ("post-commit", _HOOK_SCRIPT), ("post-checkout", _CHECKOUT_SCRIPT), ("post-merge", _MERGE_SCRIPT), + ("post-rewrite", _REWRITE_SCRIPT), ] @@ -1067,7 +1123,10 @@ def test_hooks_reuse_git_dir_from_env(name, script): """git exports GIT_DIR to hooks, so the rev-parse fallback should only run when the script is invoked by hand — each extra git exec costs 1s+ on AV-scanned Windows machines and lands in the commit's foreground.""" - assert "GIT_DIR=${GIT_DIR:-" in script, f"{name} always re-runs git rev-parse" + # A script that never reads $GIT_DIR (post-rewrite) has no rev-parse fallback to guard. + assert "GIT_DIR=${GIT_DIR:-" in script or "$GIT_DIR" not in script, ( + f"{name} always re-runs git rev-parse" + ) @pytest.mark.parametrize("name,script", _HOOK_SCRIPTS) @@ -1644,6 +1703,153 @@ def test_checkout_hook_still_skips_when_no_graph_was_built(tmp_path): assert _LAUNCH_LINE not in result.stdout +@_SH_ONLY +def test_rewrite_hook_rebuilds_when_graphify_out_is_renamed(tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q") + (repo / "custom-out").mkdir() # the configured output dir; no graphify-out/ + result = _emitted_hook_run(repo, _REWRITE_SCRIPT, ["rebase"], {"GRAPHIFY_OUT": "custom-out"}) + assert result.returncode == 0, result.stderr + assert _LAUNCH_LINE in result.stdout, f"stdout={result.stdout!r}\nstderr={result.stderr!r}" + + +@_SH_ONLY +def test_rewrite_hook_ignores_amend(tmp_path): + """amend already rebuilt through post-commit; a second rebuild would double it.""" + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q") + (repo / "graphify-out").mkdir() + result = _emitted_hook_run(repo, _REWRITE_SCRIPT, ["amend"], {}) + assert result.returncode == 0, result.stderr + assert _LAUNCH_LINE not in result.stdout + + +@_SH_ONLY +def test_rewrite_hook_runs_while_rebase_merge_is_still_on_disk(tmp_path): + """git runs post-rewrite before it removes rebase-merge/, so that dir is not a skip signal.""" + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q") + (repo / "graphify-out").mkdir() + (repo / ".git" / "rebase-merge").mkdir() + result = _emitted_hook_run(repo, _REWRITE_SCRIPT, ["rebase"], {}) + assert _LAUNCH_LINE in result.stdout, f"stdout={result.stdout!r}\nstderr={result.stderr!r}" + + +def _repo_with_code_commits(tmp_path: Path, names: tuple[str, ...]) -> Path: + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q") + (repo / "graphify-out").mkdir() + for name in names: + (repo / name).write_text(f"def {name[0]}(): pass\n", encoding="utf-8") + _git(repo, "add", name) + _git(repo, "commit", "-q", "-m", name) + return repo + + +def _head(repo: Path, rev: str) -> str: + return subprocess.run( + ["git", "-C", str(repo), "rev-parse", rev], capture_output=True, text=True, check=True, + ).stdout.strip() + + +def _write_sequencer(repo: Path, todo: str, head: str | None = None) -> None: + seq = repo / ".git" / "sequencer" + seq.mkdir() + (seq / "todo").write_text(todo, encoding="utf-8") + if head is not None: + (seq / "head").write_text(head + "\n", encoding="utf-8") + + +def _changed_dump(tmp_path: Path, monkeypatch) -> Path: + """Make the emitted-hook stub record what the hook hands the rebuild launcher in + GRAPHIFY_CHANGED (it passes the find_spec probe like the stock stub; the probe runs + before GRAPHIFY_CHANGED is set, hence the guard).""" + dump = tmp_path / "changed.txt" + + def dumping_stub(where: Path) -> Path: + py = where / "stubpy" + py.write_text( + f'#!/bin/sh\n[ -n "$GRAPHIFY_CHANGED" ] && printf %s "$GRAPHIFY_CHANGED" > "{dump}"\nexit 0\n', + encoding="utf-8", newline="\n", + ) + py.chmod(0o755) + return py + + monkeypatch.setattr(sys.modules[__name__], "_stub_python", dumping_stub) + return dump + + +@_SH_ONLY +def test_commit_hook_does_not_skip_on_cherry_pick_head(tmp_path): + """git leaves CHERRY_PICK_HEAD on disk while post-commit runs, so it is not a skip signal (#4490).""" + repo = _repo_with_code_commits(tmp_path, ("a.py", "b.py")) + (repo / ".git" / "CHERRY_PICK_HEAD").write_text("0" * 40 + "\n", encoding="utf-8") + result = _emitted_hook_run(repo, _HOOK_SCRIPT, [], {}) + assert _LAUNCH_LINE in result.stdout, f"stdout={result.stdout!r}\nstderr={result.stderr!r}" + + +@_SH_ONLY +def test_commit_hook_skips_a_pick_while_more_picks_are_pending(tmp_path): + repo = _repo_with_code_commits(tmp_path, ("a.py", "b.py")) + # The first todo line names the commit just made (HEAD's subject, "b.py"), so it is live. + _write_sequencer(repo, "pick aaaaaaa b.py\npick bbbbbbb c.py\n") + result = _emitted_hook_run(repo, _HOOK_SCRIPT, [], {}) + assert _LAUNCH_LINE not in result.stdout + + +@_SH_ONLY +def test_commit_hook_skips_a_revert_step_while_more_steps_are_pending(tmp_path): + repo = _repo_with_code_commits(tmp_path, ("a.py",)) + (repo / "x.py").write_text("def x(): pass\n", encoding="utf-8") + _git(repo, "add", "x.py") + _git(repo, "commit", "-q", "-m", 'Revert "x"') + _write_sequencer(repo, "revert aaaaaaa x\nrevert bbbbbbb y\n") + result = _emitted_hook_run(repo, _HOOK_SCRIPT, [], {}) + assert _LAUNCH_LINE not in result.stdout + + +@_SH_ONLY +@pytest.mark.parametrize("todo", [ + "pick aaaaaaa some other commit\npick bbbbbbb c.py\n", # first step is not the commit just made + "revert aaaaaaa b.py\nrevert bbbbbbb c.py\n", # a revert step, but HEAD is not a Revert "..." commit + "squash aaaaaaa b.py\npick bbbbbbb c.py\n", # a verb the hook does not recognise + "", # empty todo +]) +def test_commit_hook_ignores_a_stale_sequencer(tmp_path, todo): + """An abandoned pick (git reset --hard instead of --abort) leaves sequencer/ behind; + trusting its pending steps would silence every later commit.""" + repo = _repo_with_code_commits(tmp_path, ("a.py", "b.py")) + _write_sequencer(repo, todo) + result = _emitted_hook_run(repo, _HOOK_SCRIPT, [], {}) + assert _LAUNCH_LINE in result.stdout, f"stdout={result.stdout!r}\nstderr={result.stderr!r}" + + +@_SH_ONLY +def test_commit_hook_rebuilds_on_the_last_pick_from_where_the_sequence_began(tmp_path, monkeypatch): + """On the last pick the changed set is everything since sequencer/head, not just HEAD~1..HEAD.""" + dump = _changed_dump(tmp_path, monkeypatch) + repo = _repo_with_code_commits(tmp_path, ("a.py", "b.py", "c.py")) + _write_sequencer(repo, "pick ccccccc c.py\n", head=_head(repo, "HEAD~2")) + result = _emitted_hook_run(repo, _HOOK_SCRIPT, [], {}) + assert _LAUNCH_LINE in result.stdout, f"stdout={result.stdout!r}\nstderr={result.stderr!r}" + assert set(dump.read_text(encoding="utf-8").split()) == {"b.py", "c.py"} + + +@_SH_ONLY +def test_commit_hook_ignores_the_base_of_a_stale_sequencer(tmp_path, monkeypatch): + """A stale single-line todo must not widen the changed set to everything since its head.""" + dump = _changed_dump(tmp_path, monkeypatch) + repo = _repo_with_code_commits(tmp_path, ("a.py", "b.py", "c.py")) + _write_sequencer(repo, "pick ccccccc not-the-last-commit\n", head=_head(repo, "HEAD~2")) + result = _emitted_hook_run(repo, _HOOK_SCRIPT, [], {}) + assert _LAUNCH_LINE in result.stdout, f"stdout={result.stdout!r}\nstderr={result.stderr!r}" + assert set(dump.read_text(encoding="utf-8").split()) == {"c.py"} + + @_SH_ONLY def test_commit_hook_skips_graph_only_commit_when_graphify_out_is_renamed(tmp_path): repo = _repo_with_graph_only_commit(tmp_path, "custom-out") diff --git a/tests/test_hooks_rewrite.py b/tests/test_hooks_rewrite.py new file mode 100644 index 000000000..4da7d9498 --- /dev/null +++ b/tests/test_hooks_rewrite.py @@ -0,0 +1,264 @@ +"""Hooks must rebuild once after every git operation that changes the tree (#4490). + +Each test counts the background-rebuild launch line in git's output (hooks +write to stderr). git leaves CHERRY_PICK_HEAD, sequencer/ and rebase-merge/ on +disk while post-commit and post-rewrite run, so cherry-pick and rebase never +rebuilt. Real-build tests wait for the detached rebuild before reading the graph. +""" +import json +import os +import subprocess +import sys +import time + +import pytest + +from graphify.hooks import install +from tests.test_hooks import _LAUNCH_LINE, _SH_ONLY + + +class Repo: + def __init__(self, path, home): + self.path = path + path.mkdir() + self.env = { + **os.environ, "HOME": str(home), + "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@t.co", + "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@t.co", + "GIT_EDITOR": "true", + } + self.quiet = {**self.env, "GRAPHIFY_SKIP_HOOK": "1"} + + def git(self, *args, quiet=False, check=True): + return subprocess.run( + ["git", *args], cwd=self.path, capture_output=True, text=True, + env=self.quiet if quiet else self.env, check=check, + ) + + def launches(self, *args, check=True): + out = self.git(*args, check=check) + return (out.stdout + out.stderr).count(_LAUNCH_LINE) + + def write(self, name, text): + (self.path / name).write_text(text, encoding="utf-8") + + def commit(self, name, text, msg): + self.write(name, text) + self.git("add", name) + self.git("commit", "-qm", msg, quiet=True) + + def build(self): + subprocess.run([sys.executable, "-m", "graphify", "update", "."], cwd=self.path, + env=self.env, check=True, capture_output=True) + + def labels(self): + graph = json.loads((self.path / "graphify-out" / "graph.json").read_text(encoding="utf-8")) + return {n["label"] for n in graph["nodes"]} + + +def _settle(repo, home, timeout=60): + """Wait until the detached rebuild is done: log stable, no lock or pending files.""" + log, out = home / ".cache" / "graphify-rebuild.log", repo.path / "graphify-out" + t0 = time.time() + while time.time() - t0 < 5 and not log.exists(): + time.sleep(0.1) + if not log.exists(): + return # nothing was launched; the caller's assertions say what was expected + last = (-1, 0.0) + while time.time() - t0 < timeout: + size = log.stat().st_size + busy = any((out / n).exists() for n in (".rebuild.lock", ".pending_changes", ".pending_full")) + if size != last[0]: + last = (size, time.time()) + elif not busy and time.time() - last[1] > 2: + return + time.sleep(0.2) + pytest.fail("the background rebuild did not settle") + + +@pytest.fixture +def repo(tmp_path): + r = Repo(tmp_path / "repo", tmp_path) + r.git("init", "-q", "-b", "main") + (r.path / "graphify-out").mkdir() + r.write("graphify-out/graph.json", "{}") + r.commit("a.py", "def a(): pass\n", "init") + install(r.path) + return r + + +def _built_repo(tmp_path): + """A repo with a real first build and the hooks installed.""" + r = Repo(tmp_path / "repo", tmp_path) + r.git("init", "-q", "-b", "main") + r.commit("base.py", "".join(f"def b{i}(x):\n return x\n\n" for i in range(6)), "init") + r.commit("shared.py", "def s(): return 0\n", "shared") + r.build() + install(r.path) + return r + + +@_SH_ONLY +def test_control_plain_commit_rebuilds_once(repo): + repo.write("b.py", "def b(): pass\n") + repo.git("add", "b.py") + assert repo.launches("commit", "-qm", "b") == 1 + + +@_SH_ONLY +def test_single_cherry_pick_rebuilds_once(repo): + repo.git("checkout", "-qb", "side", quiet=True) + repo.commit("cp.py", "def cp(): pass\n", "cp") + repo.git("checkout", "-q", "main", quiet=True) + assert repo.launches("cherry-pick", "side") == 1 + + +@_SH_ONLY +def test_range_cherry_pick_rebuilds_once(repo): + repo.git("checkout", "-qb", "side", quiet=True) + for i in range(3): + repo.commit(f"r{i}.py", f"def r{i}(): pass\n", f"r{i}") + repo.git("checkout", "-q", "main", quiet=True) + assert repo.launches("cherry-pick", "side~3..side") == 1 + + +@_SH_ONLY +def test_rebase_rebuilds_once(repo): + repo.git("checkout", "-qb", "feat", quiet=True) + repo.commit("f.py", "def f(): pass\n", "f") + repo.git("checkout", "-q", "main", quiet=True) + repo.commit("m.py", "def m(): pass\n", "m") + repo.git("checkout", "-q", "feat", quiet=True) + assert repo.launches("rebase", "main") == 1 + + +@_SH_ONLY +def test_pull_rebase_rebuilds_once(repo, tmp_path): + clone = Repo(tmp_path / "clone", tmp_path) + clone.git("clone", "-q", str(repo.path), ".") # into the empty dir Repo made + (clone.path / "graphify-out").mkdir() # the graph is untracked, so a clone has none + clone.write("graphify-out/graph.json", "{}") + install(clone.path) + repo.commit("up.py", "def up(): pass\n", "up") + clone.commit("lo.py", "def lo(): pass\n", "lo") + assert clone.launches("pull", "--rebase") == 1 + + +@_SH_ONLY +def test_amend_rebuilds_exactly_once(repo): + # amend a non-root commit: on a root commit the hook has no HEAD~1 to diff + repo.commit("b.py", "def b(): pass\n", "b") + repo.write("a.py", "def a(): return 1\n") + repo.git("add", "a.py") + assert repo.launches("commit", "-q", "--amend", "-m", "amended") == 1 + + +@_SH_ONLY +def test_conflicted_rebase_continues_and_rebuilds_once(repo): + repo.git("checkout", "-qb", "feat", quiet=True) + repo.commit("a.py", "def a(): return 'feat'\n", "feat edit") + repo.git("checkout", "-q", "main", quiet=True) + repo.commit("a.py", "def a(): return 'main'\n", "main edit") + repo.git("checkout", "-q", "feat", quiet=True) + assert repo.launches("rebase", "main", check=False) == 0 # paused on conflict + repo.write("a.py", "def a(): return 'both'\n") + repo.git("add", "a.py") + out = repo.git("rebase", "--continue") # must not be blocked by a hook + assert (out.stdout + out.stderr).count(_LAUNCH_LINE) == 1 + assert not (repo.path / ".git" / "rebase-merge").exists() + + +@_SH_ONLY +def test_range_cherry_pick_lands_every_new_function(tmp_path): + r = _built_repo(tmp_path) + r.git("checkout", "-qb", "side", quiet=True) + for i in (1, 2, 3): + r.commit(f"f{i}.py", f"def new_fn{i}(x):\n return x\n", f"c{i}") + r.git("checkout", "-q", "main", quiet=True) + r.git("cherry-pick", "side~3..side") + _settle(r, tmp_path) + assert {"new_fn1()", "new_fn2()", "new_fn3()"} <= r.labels() + + +@_SH_ONLY +def test_skip_mid_range_rebuilds_the_picks_that_landed(tmp_path): + r = _built_repo(tmp_path) + r.git("checkout", "-qb", "side", quiet=True) + r.commit("fs1.py", "def fs_one(): pass\n", "c1") + r.write("fs2.py", "def fs_two(): pass\n") + r.write("shared.py", "def s(): return 2\n") + r.git("add", ".") + r.git("commit", "-qm", "c2", quiet=True) + r.commit("fs3.py", "def fs_three(): pass\n", "c3") + r.git("checkout", "-q", "main", quiet=True) + r.commit("shared.py", "def s(): return 1\n", "main edit") + r.git("cherry-pick", "side~3..side", check=False) # pauses on c2 + assert r.launches("cherry-pick", "--skip") == 1 + _settle(r, tmp_path) + labels = r.labels() + assert {"fs_one()", "fs_three()"} <= labels and "fs_two()" not in labels + + +@_SH_ONLY +def test_revert_range_rebuilds_once_and_removes_the_functions(tmp_path): + """Pins one rebuild per revert range. This is not a v8 bug: v8 rebuilt after + each revert, three times. A revert range uses the same sequencer/ as a pick + range, so it follows the same skip-until-the-last-step rule.""" + r = Repo(tmp_path / "repo", tmp_path) + r.git("init", "-q", "-b", "main") + r.commit("base.py", "def base(): pass\n", "init") + for i, name in enumerate(("fr_one", "fr_two", "fr_three"), 1): + r.commit(f"fr{i}.py", f"def {name}(): pass\n", name) + r.build() + install(r.path) + assert r.launches("revert", "--no-edit", "HEAD~3..HEAD") == 1 + _settle(r, tmp_path) + assert not {"fr_one()", "fr_two()", "fr_three()"} & r.labels() + + +@_SH_ONLY +def test_tracked_graph_range_pick_with_conflict_completes(tmp_path): + """With graph.json tracked, a rebuild between picks would dirty it and git would + refuse the next pick that touches it ("local changes would be overwritten").""" + r = Repo(tmp_path / "repo", tmp_path) + r.git("init", "-q", "-b", "main") + (r.path / "graphify-out").mkdir() + r.write("graphify-out/graph.json", "{}") + r.commit("shared.py", "def s(): return 0\n", "init") + r.git("add", "-f", "graphify-out/graph.json") + r.git("commit", "-qm", "track", quiet=True) + install(r.path) + r.git("checkout", "-qb", "side", quiet=True) + r.commit("n1.py", "def n1(): pass\n", "c1") + r.commit("shared.py", "def s(): return 2\n", "c2") + r.write("graphify-out/graph.json", '{"teammate": true}') + r.git("add", "-f", "graphify-out/graph.json") + r.git("commit", "-qm", "c3", quiet=True) + r.git("checkout", "-q", "main", quiet=True) + r.commit("shared.py", "def s(): return 1\n", "main edit") + r.git("cherry-pick", "side~3..side", check=False) # pauses on c2 + r.write("shared.py", "def s(): return 3\n") + r.git("add", "shared.py") + out = r.git("cherry-pick", "--continue", check=False) + assert out.returncode == 0, out.stdout + out.stderr + assert not (r.path / ".git" / "sequencer").exists() + _settle(r, tmp_path) # cleanup only: lets the rebuild after the last pick finish before tmp_path goes + + +@_SH_ONLY +def test_abandoned_range_pick_does_not_silence_later_commits(repo): + """git reset --hard (not --abort) leaves sequencer/ behind. Trusting its pending steps + would skip every later commit's rebuild; the hook must see it is stale.""" + repo.git("checkout", "-qb", "side", quiet=True) + repo.commit("n1.py", "def n1(): pass\n", "c1") + repo.commit("a.py", "def a(): return 2\n", "c2") + repo.commit("n3.py", "def n3(): pass\n", "c3") + repo.git("checkout", "-q", "main", quiet=True) + repo.commit("a.py", "def a(): return 1\n", "main edit") + repo.git("cherry-pick", "side~3..side", check=False) # pauses on c2 + repo.git("reset", "-q", "--hard", "HEAD") + if not (repo.path / ".git" / "sequencer").exists(): + pytest.skip("this git removes sequencer/ on reset --hard, so there is nothing stale to test") + repo.write("z.py", "def z(): pass\n") + repo.git("add", "z.py") + assert repo.launches("commit", "-qm", "z") == 1