From b63b6d49768879ebf5807da90c632464f55b183d Mon Sep 17 00:00:00 2001 From: abhay-codes07 Date: Mon, 31 Aug 2026 13:25:22 +0530 Subject: [PATCH] fix(build): make definition_file portable, like its sibling source_file (#3223) The #2990 decl/def merge stamps definition_file from the implementation's then-absolute source_file, and nothing ever relativized it: graph.json shipped "source_file": "src/Foo.h" beside "definition_file": "/home/ci/build/.../src/Foo.cpp" - a path no other machine can open, leaking the build host's layout into every consumer including MCP get_node's "Defined in:" line. definition_file now goes through the same _norm_source_file normalization as source_file at both build sites (fresh nodes, and loaded-graph attrs - which also heals a graph written before this fix on its next rebuild), and the cache portability round-trip relativizes and re-anchors it beside source_file. Out-of-root paths keep their absolute form, exactly as source_file does. --- graphify/build.py | 11 +++ graphify/cache.py | 66 +++++++++-------- tests/test_definition_file_portability.py | 89 +++++++++++++++++++++++ 3 files changed, 136 insertions(+), 30 deletions(-) create mode 100644 tests/test_definition_file_portability.py diff --git a/graphify/build.py b/graphify/build.py index bb03fe1f5..87d75b429 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -973,6 +973,13 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat continue if "source_file" in node: node["source_file"] = _norm_source_file(node["source_file"], _root) + # definition_file names a file inside the scanned tree exactly like + # source_file (the #2990 decl/def merge stamps it from the impl's + # source_file BEFORE this normalization runs), so it must be made + # portable the same way - it used to ship absolute, leaking the + # build host's layout into graph.json and MCP get_node (#3223). + if "definition_file" in node: + node["definition_file"] = _norm_source_file(node["definition_file"], _root) G.add_node(node["id"], **{k: v for k, v in node.items() if k != "id"}) node_set = set(G.nodes()) @@ -1202,6 +1209,10 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat ) if "source_file" in attrs: attrs["source_file"] = _norm_source_file(attrs["source_file"], _root) + if attrs.get("definition_file"): + # Same portability rule as source_file (#3223); heals a graph + # written before the fix on its next rebuild. + attrs["definition_file"] = _norm_source_file(attrs["definition_file"], _root) # Drop cross-language phantom edges — the same short names (render, parse, # time, ...) recur across language boundaries, so an unresolved target can # bind to a same-named node in another language. The extraction spec forbids diff --git a/graphify/cache.py b/graphify/cache.py index 622ba6cff..d6060ed2b 100644 --- a/graphify/cache.py +++ b/graphify/cache.py @@ -606,31 +606,35 @@ def _relativize_source_files_in(payload: dict, root: Path) -> None: # source_file the same way nodes/edges/hyperedges do, so it needs the same # portable-path treatment for cache entries to round-trip correctly across # machines/checkout directories. + # definition_file (#2990) is a path into the scanned tree exactly like + # source_file; a cache entry keeping it absolute replayed the build host's + # layout on every warm hit (#3223). for bucket in ("nodes", "edges", "hyperedges", "raw_calls"): for item in payload.get(bucket, []): if not isinstance(item, dict): continue - source = item.get("source_file") - if not source: - continue - sp = Path(source) - if not sp.is_absolute(): - # os.path.abspath is lexical (no symlink resolution), matching - # the symbolic relativization below. - cwd_form = Path(os.path.abspath(sp)) - try: - if cwd_form == root_resolved / sp or not cwd_form.exists(): - continue # already root-relative, or a ghost path - except OSError: + for key in ("source_file", "definition_file"): + source = item.get(key) + if not source: continue - sp = cwd_form - try: - rel = os.path.relpath(sp, root_resolved) - except (ValueError, OSError): - continue # out-of-root (e.g. Windows cross-drive) - if rel == ".." or rel.startswith(".." + os.sep) or rel.startswith("../"): - continue # escaped root — keep absolute - item["source_file"] = rel.replace(os.sep, "/") + sp = Path(source) + if not sp.is_absolute(): + # os.path.abspath is lexical (no symlink resolution), + # matching the symbolic relativization below. + cwd_form = Path(os.path.abspath(sp)) + try: + if cwd_form == root_resolved / sp or not cwd_form.exists(): + continue # already root-relative, or a ghost path + except OSError: + continue + sp = cwd_form + try: + rel = os.path.relpath(sp, root_resolved) + except (ValueError, OSError): + continue # out-of-root (e.g. Windows cross-drive) + if rel == ".." or rel.startswith(".." + os.sep) or rel.startswith("../"): + continue # escaped root — keep absolute + item[key] = rel.replace(os.sep, "/") def _normalize_source_file_value(src: "str | Path", root_resolved: Path) -> str: @@ -911,16 +915,18 @@ def _absolutize_source_files_in(payload: dict, root: Path) -> None: for item in payload.get(bucket, []): if not isinstance(item, dict): continue - source = item.get("source_file") - if not source: - continue - sp = Path(source) - if sp.is_absolute(): - continue - try: - item["source_file"] = str(root_resolved / sp) - except (TypeError, OSError): - continue + # Mirror of the relativize side: definition_file re-anchors too (#3223). + for key in ("source_file", "definition_file"): + source = item.get(key) + if not source: + continue + sp = Path(source) + if sp.is_absolute(): + continue + try: + item[key] = str(root_resolved / sp) + except (TypeError, OSError): + continue def cache_dir(root: Path = Path("."), kind: str = "ast", diff --git a/tests/test_definition_file_portability.py b/tests/test_definition_file_portability.py new file mode 100644 index 000000000..a1648fe19 --- /dev/null +++ b/tests/test_definition_file_portability.py @@ -0,0 +1,89 @@ +"""`definition_file` is portable, like its sibling `source_file` (#3223). + +The #2990 decl/def merge stamps `definition_file` from the implementation's +then-absolute `source_file` — and nothing ever relativized it. graph.json +shipped `"source_file": "src/Foo.h"` beside +`"definition_file": "/home/ci/build/.../src/Foo.cpp"`: a path no other +machine can open, leaking the build host's layout into every consumer, +including MCP `get_node`'s "Defined in:" line. +""" +from __future__ import annotations + +import io +import tempfile +from contextlib import redirect_stdout +from pathlib import Path + +from graphify.build import build_from_json +from graphify.cache import _absolutize_source_files_in, _relativize_source_files_in +from graphify.extract import extract + +FOO_H = "#pragma once\n\nclass Foo {\npublic:\n int Bar(int x);\n};\n" +FOO_CPP = '#include "Foo.h"\n\nint Foo::Bar(int x) {\n return x + 1;\n}\n' + + +def _decl_def_graph(tmp_path): + src = tmp_path / "src" + src.mkdir() + (src / "Foo.h").write_text(FOO_H, encoding="utf-8") + (src / "Foo.cpp").write_text(FOO_CPP, encoding="utf-8") + with redirect_stdout(io.StringIO()): + r = extract([src / "Foo.h", src / "Foo.cpp"], cache_root=Path(tempfile.mkdtemp()), + root=tmp_path, parallel=False) + G = build_from_json({"nodes": r["nodes"], "edges": r["edges"], "hyperedges": []}, + root=str(tmp_path)) + return G + + +def test_the_issues_repro_yields_a_relative_definition_file(tmp_path): + G = _decl_def_graph(tmp_path) + carriers = [(n, d) for n, d in G.nodes(data=True) if d.get("definition_file")] + assert carriers, "the decl/def pair must produce a definition_file carrier" + for _n, d in carriers: + df = str(d["definition_file"]).replace("\\", "/") + assert df == "src/Foo.cpp", df + assert str(d.get("source_file", "")).replace("\\", "/") == "src/Foo.h" + + +def test_build_normalizes_a_prebuilt_absolute_definition_file(tmp_path): + impl = tmp_path / "src" / "Foo.cpp" + impl.parent.mkdir() + impl.write_text(FOO_CPP, encoding="utf-8") + G = build_from_json({ + "nodes": [{"id": "n", "label": "Bar", "file_type": "code", + "source_file": str(tmp_path / "src" / "Foo.h"), + "definition_file": str(impl)}], + "edges": [], "hyperedges": [], + }, root=str(tmp_path)) + d = G.nodes["n"] + assert str(d["definition_file"]).replace("\\", "/") == "src/Foo.cpp" + + +def test_an_out_of_root_definition_file_is_left_alone(tmp_path): + outside = tmp_path.parent / "elsewhere.cpp" + G = build_from_json({ + "nodes": [{"id": "n", "label": "Bar", "file_type": "code", + "source_file": "src/Foo.h", + "definition_file": str(outside)}], + "edges": [], "hyperedges": [], + }, root=str(tmp_path)) + # out-of-root stays absolute; separators are normalized like source_file's + assert Path(G.nodes["n"]["definition_file"]) == outside + + +def test_cache_round_trip_keeps_definition_file_portable(tmp_path): + root = tmp_path / "proj" + (root / "src").mkdir(parents=True) + f = root / "src" / "Foo.cpp" + f.write_text(FOO_CPP, encoding="utf-8") + payload = {"nodes": [{"id": "n", "source_file": str(root / "src" / "Foo.h"), + "definition_file": str(f)}], + "edges": []} + _relativize_source_files_in(payload, root) + stored = payload["nodes"][0] + assert stored["definition_file"] == "src/Foo.cpp" + assert stored["source_file"] == "src/Foo.h" + _absolutize_source_files_in(payload, root) + restored = payload["nodes"][0] + assert Path(restored["definition_file"]) == root / "src" / "Foo.cpp" + assert Path(restored["source_file"]) == root / "src" / "Foo.h"