From 946c01eb9d712e4cf4d57611005dd3d1e0307aa6 Mon Sep 17 00:00:00 2001 From: Som Date: Thu, 13 Aug 2026 03:22:05 +0000 Subject: [PATCH] fix: skip merge attribute for ignored graph output Contributor: Som Samantray --- .../fix-2595-ignored-graph-merge-attribute.md | 57 +++++++++++++++++++ graphify/hooks.py | 31 +++++++++- tests/test_hooks.py | 18 ++++++ 3 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 docs/plans/fix-2595-ignored-graph-merge-attribute.md diff --git a/docs/plans/fix-2595-ignored-graph-merge-attribute.md b/docs/plans/fix-2595-ignored-graph-merge-attribute.md new file mode 100644 index 000000000..f827a6d15 --- /dev/null +++ b/docs/plans/fix-2595-ignored-graph-merge-attribute.md @@ -0,0 +1,57 @@ +--- +artifact_contract: ce-unified-plan/v1 +artifact_readiness: implementation-ready +execution: code +product_contract_source: issue-2595 +--- + +# Fix #2595: Do not add a `.gitattributes` merge rule for ignored graph output + +## Problem + +`graphify hook install` registers the local `merge.graphify.*` driver and adds a `graphify-out/graph.json merge=graphify` rule to `.gitattributes`. When the graph output is ignored by Git, the attribute cannot affect merges but still dirties the repository's tracked worktree. + +## Scope + +- Keep registering the local Git merge driver regardless of whether the graph is ignored. +- Skip only the `.gitattributes` mutation when the configured graph path is ignored by Git. +- Preserve current behavior for tracked/non-ignored graph output, existing attributes, custom relative `GRAPHIFY_OUT`, and idempotent installation. +- Do not change uninstall behavior beyond making it safe when no attribute was added. + +## Implementation units + +### U1. Guard `.gitattributes` mutation for ignored graph output + +**Goal:** Avoid writing a merge attribute that Git will never use. + +**Files:** +- `graphify/hooks.py` +- `tests/test_hooks.py` + +**Approach:** +1. Derive the repository-relative graph path used by the existing merge-attribute line. +2. Ask Git whether that path is ignored using `git -C check-ignore -q `. +3. Treat a successful `check-ignore` result as ignored and return a clear status after the local merge driver has already been registered. +4. Treat missing `.gitignore`, non-ignored paths, and `check-ignore` failures as non-ignored so existing behavior remains intact. +5. Keep all filesystem writes and existing idempotence logic unchanged for non-ignored paths. + +**Test scenarios:** +- ignored default `graphify-out/graph.json` registers Git config but does not create or modify `.gitattributes`; +- non-ignored output still creates the attribute; +- existing unrelated attributes remain preserved; +- repeated installation remains idempotent; +- a configured relative `GRAPHIFY_OUT` is checked using the same repository-relative path. + +## Risks and mitigations + +- Git may be unavailable or return an error: fail open to current behavior, because the guard is a worktree-cleanliness optimization and must not block hook installation. +- Attribute path must remain repository-relative: reuse `_merge_attr_line()`'s path calculation rather than duplicating `GRAPHIFY_OUT` normalization. +- The local merge driver must still be configured for ignored output: perform the Git config writes before the optional attribute guard. + +## Verification + +Run the focused hook tests, then the full test suite if practical. Inspect the final diff and verify that the new regression test fails against the pre-fix behavior and passes after the change. + +## Contributor + +Som Samantray diff --git a/graphify/hooks.py b/graphify/hooks.py index 84d3e9b85..8c90f7d78 100644 --- a/graphify/hooks.py +++ b/graphify/hooks.py @@ -526,11 +526,38 @@ def _merge_attr_line() -> str: absolute output-dir override cannot be expressed there — fall back to the default name in that case. """ + return f"{_merge_attr_path()} merge=graphify" + + +def _merge_attr_path() -> str: + """Return the repository-relative graph path used by ``.gitattributes``.""" from graphify.paths import GRAPHIFY_OUT + out = GRAPHIFY_OUT if not out or Path(out).is_absolute() or "\\" in out: out = "graphify-out" - return f"{out.rstrip('/')}/graph.json merge=graphify" + return f"{out.rstrip('/')}/graph.json" + + +def _graph_path_is_ignored(root: Path) -> bool: + """Return whether Git ignores the graph path in ``root``. + + Failure is deliberately treated as ``False``. The check only prevents an + unnecessary worktree mutation; it must not prevent local merge-driver setup + when Git cannot answer the query. + """ + import subprocess as _sp + + try: + result = _sp.run( + ["git", "-C", str(root), "check-ignore", "-q", "--", _merge_attr_path()], + check=False, + capture_output=True, + text=True, + ) + except OSError: + return False + return result.returncode == 0 def _has_merge_attr(content: str) -> bool: @@ -579,6 +606,8 @@ def _register_merge_driver(root: Path) -> str: return f"not registered (git config failed: {exc})" line = _merge_attr_line() + if _graph_path_is_ignored(root): + return f"registered (graph output is ignored; skipped .gitattributes: {line})" attrs = root / ".gitattributes" if attrs.exists(): content = attrs.read_text(encoding="utf-8") diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 18c2b59ce..b3836dae3 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -641,6 +641,24 @@ def test_install_registers_merge_driver(tmp_path): assert "merge driver" in result +def test_install_skips_merge_attribute_for_ignored_graph(tmp_path): + """An ignored graph cannot participate in a merge, so installation must + not dirty the worktree with a useless .gitattributes entry (#2595).""" + repo = _make_git_repo(tmp_path) + (repo / ".gitignore").write_text("/graphify-out/\n", encoding="utf-8") + + result = install(repo) + + driver = subprocess.run( + ["git", "-C", str(repo), "config", "--get", "merge.graphify.driver"], + capture_output=True, + text=True, + ) + assert driver.returncode == 0 + assert "ignored" in result + assert not (repo / ".gitattributes").exists() + + def test_install_merge_driver_idempotent(tmp_path): """Running install twice must not duplicate the .gitattributes line.""" repo = _make_git_repo(tmp_path)