mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-22 03:41:30 +00:00
fix(hooks): rebuild after cherry-pick, rebase and pull --rebase (#4490)
post-commit skipped on CHERRY_PICK_HEAD, which git leaves on disk while the hook runs, so a clean pick never rebuilt. A rebase has no hook that fires once at the end, because post-commit and post-checkout skip while rebase-merge/ exists. Drop the CHERRY_PICK_HEAD check. For a range pick or revert, skip every step but the last and diff from sequencer/head, so one rebuild covers the range; ignore a stale sequencer/ left by git reset --hard. Add a post-rewrite hook that rebuilds on rebase.
This commit is contained in:
1 parent
10130e79ed
commit
47085dd742
4 files changed
+575
-10
No files matched your search
@@ -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(
|
||||
|
||||
+103
-8
@@ -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 ``<root>/.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}"
|
||||
|
||||
+207
-1
@@ -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")
|
||||
|
||||
@@ -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
|
||||
Reference in new issue
Block a user