diff --git a/graphify/detect.py b/graphify/detect.py index eec8aaf1d2..200fead8b9 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -2111,6 +2111,40 @@ def _nfc(s: str) -> str: return unicodedata.normalize("NFC", s) +def _anchor_under_root(key_path: Path, root_res: Path) -> "str | None": + """Relativize ``key_path`` against ``root_res`` by identity, not by text. + + ``os.path.relpath`` is lexical and case-sensitive. Windows paths are not, + and a default macOS volume is not either, so a stored key whose case + differs from the resolved root ("C:\\Users\\Foo" against "C:\\Users\\foo", + a parent directory renamed only in case) reads as out-of-root and stays + absolute for ever. An upgrade that changes the key convention then leaves + the old key beside the new one rather than replacing it, and the manifest + ends up describing the same tree twice (#3581). + + Walks the key's ancestors looking for the one that IS ``root_res`` + according to the filesystem, and joins the remainder with its original + casing intact. Returns None when no ancestor matches, which is the + genuinely out-of-root case. Only reached after the lexical attempt has + already failed, so the ordinary in-root path costs nothing. + """ + parts: list[str] = [] + current = key_path + while True: + parent = current.parent + if parent == current: + return None + parts.append(current.name) + current = parent + try: + if os.path.samefile(current, root_res): + return "/".join(reversed(parts)) + except OSError: + # An ancestor we cannot stat cannot be shown to be root; leave the + # key absolute, as it was before this fallback existed. + return None + + def _to_relative_for_storage(key: str, root: Path) -> str: """Return ``key`` as a forward-slash relative path from ``root``. @@ -2135,15 +2169,19 @@ def _to_relative_for_storage(key: str, root: Path) -> str: if not p.is_absolute(): return key try: - base = _nfc(str(Path(root).resolve())) - rel = os.path.relpath(_nfc(str(p)), base) + root_res = Path(root).resolve() + rel = os.path.relpath(_nfc(str(p)), _nfc(str(root_res))) except (ValueError, OSError): return key # outside root (e.g. Windows cross-drive) # ``os.path.relpath`` happily produces ``../foo`` for paths outside # root; mirror the prior ``relative_to``-raises-ValueError semantics by # keeping out-of-root entries in their absolute form. if rel == ".." or rel.startswith(".." + os.sep) or rel.startswith("../"): - return key + # Lexically out of root. Before accepting that, ask the filesystem: + # the key may denote a path under root that merely spells it + # differently (see :func:`_anchor_under_root`). + anchored = _anchor_under_root(p, root_res) + return anchored if anchored is not None else key return rel.replace(os.sep, "/") diff --git a/tests/test_detect.py b/tests/test_detect.py index 22099029a5..89e31d3bfd 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -2537,6 +2537,113 @@ def test_save_manifest_out_of_root_keeps_absolute(tmp_path): outside.unlink(missing_ok=True) +# --- #3581: a stored key that spells root differently is still in root ------- +# ``os.path.relpath`` compares text. A filesystem compares identity. When a +# stored absolute key denotes a path under ``root`` that merely spells it +# differently (a case-variant on Windows/APFS, a symlinked checkout), the +# lexical answer is ``../...`` and the key would stay absolute for ever, so an +# upgrade that changes the key convention leaves the stale key sitting beside +# the new relative one and the manifest describes the same tree twice. + +def test_save_manifest_relativizes_key_reached_through_symlinked_root( + requires_symlinks, tmp_path +): + """A legacy key naming the corpus through a symlink to ``root`` relativizes + to the same key the fresh scan produces, so the row is replaced rather than + duplicated. Symlinks make this reproducible on every filesystem; case + variants (the reported shape) only do so on a case-insensitive one.""" + import json + from graphify.detect import save_manifest + + root = tmp_path / "repo" + (root / "pkg").mkdir(parents=True) + mod = root / "pkg" / "mod.py" + mod.write_text("def x(): pass\n") + link = tmp_path / "checkout" + link.symlink_to(root, target_is_directory=True) + + manifest_path = tmp_path / "graphify-out" / "manifest.json" + manifest_path.parent.mkdir(parents=True) + manifest_path.write_text(json.dumps({ + str(link / "pkg" / "mod.py"): {"mtime": 0.0, "ast_hash": "stale", "semantic_hash": ""}, + }), encoding="utf-8") + + save_manifest({"code": [str(mod)]}, str(manifest_path), kind="ast", root=root) + + raw = json.loads(manifest_path.read_text(encoding="utf-8")) + assert set(raw) == {"pkg/mod.py"}, ( + f"the symlinked key names a file under root and must collapse onto the " + f"relative key, got {sorted(raw)}" + ) + + +def test_save_manifest_legacy_case_variant_key_does_not_duplicate(tmp_path): + """The reported shape: a manifest carried across an upgrade holds absolute + keys whose root differs only in case. On the filesystems the reporter runs + (Windows, default macOS) those name the very same files, so they must + collapse onto the new relative keys. On a case-sensitive filesystem they + are genuinely different files and must both survive, absolute. Assert the + one that actually applies here rather than skipping half the platforms.""" + import json + from graphify.detect import save_manifest + + root = tmp_path / "Repo" + (root / "pkg").mkdir(parents=True) + mod = root / "pkg" / "mod.py" + mod.write_text("def x(): pass\n") + variant_root = tmp_path / "repo" + variant_key = str(variant_root / "pkg" / "mod.py") + case_insensitive = variant_root.exists() + if not case_insensitive: + # On a case-sensitive filesystem the variant names a file that is simply + # absent, and save_manifest drops rows whose file is gone before it + # relativizes anything. Give it a real file, so what the assertion below + # measures is the anchoring decision rather than that prune. + (variant_root / "pkg").mkdir(parents=True) + (variant_root / "pkg" / "mod.py").write_text("def y(): pass\n") + + manifest_path = tmp_path / "graphify-out" / "manifest.json" + manifest_path.parent.mkdir(parents=True) + manifest_path.write_text(json.dumps({ + variant_key: {"mtime": 0.0, "ast_hash": "stale", "semantic_hash": ""}, + }), encoding="utf-8") + + save_manifest({"code": [str(mod)]}, str(manifest_path), kind="ast", root=root) + + raw = json.loads(manifest_path.read_text(encoding="utf-8")) + if case_insensitive: + assert set(raw) == {"pkg/mod.py"}, ( + f"the case variant names the same file and must not survive as a " + f"second, absolute key, got {sorted(raw)}" + ) + else: + assert set(raw) == {"pkg/mod.py", variant_key}, ( + f"on a case-sensitive filesystem the variant is a different file " + f"and must keep its absolute key, got {sorted(raw)}" + ) + + +def test_anchor_under_root_returns_none_when_no_ancestor_is_root(tmp_path): + """The fallback must only fire on a path it can prove lies under ``root``. + + Both out-of-root shapes are unit-tested here because ``save_manifest`` + cannot reach the second one: it prunes rows whose file no longer exists + before relativizing, so a key naming a vanished tree never gets this far. + """ + from graphify.detect import _anchor_under_root + + root = tmp_path / "repo" + (root / "pkg").mkdir(parents=True) + sibling = tmp_path / "other" / "pkg" / "mod.py" + sibling.parent.mkdir(parents=True) + sibling.write_text("pass\n") + + assert _anchor_under_root(sibling, root.resolve()) is None + assert _anchor_under_root(tmp_path / "gone" / "pkg" / "mod.py", root.resolve()) is None + # ...and a path that genuinely is under root still anchors. + assert _anchor_under_root(root / "pkg" / "mod.py", root.resolve()) == "pkg/mod.py" + + def test_detect_incremental_portable_across_paths(tmp_path): """End-to-end: a manifest written at one root must be readable from a different absolute prefix (the cross-machine case #777 is about).