mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-26 16:26:42 +00:00
fix(paths): implement cross-platform absolute path detection for stored paths
This commit is contained in:
committed by
safishamsi
parent
26245b0841
commit
717b7b464a
+17
-10
@@ -31,6 +31,7 @@ from pathlib import Path
|
||||
import networkx as nx
|
||||
from .ids import make_id, normalize_id as _normalize_id
|
||||
from .paths import default_graph_json as _default_graph_json
|
||||
from .paths import is_absolute_any_platform as _is_abs
|
||||
from .validate import validate_extraction
|
||||
|
||||
|
||||
@@ -282,7 +283,7 @@ def _norm_source_file(p: str | None, root: str | None = None) -> str | None:
|
||||
if not p:
|
||||
return p
|
||||
p = p.replace("\\", "/")
|
||||
if root and os.path.isabs(p):
|
||||
if root and _is_abs(p):
|
||||
try:
|
||||
p = Path(p).relative_to(root).as_posix()
|
||||
except ValueError:
|
||||
@@ -314,7 +315,7 @@ def _abs_identity(p: str | None, root: str | None = None) -> str | None:
|
||||
return None
|
||||
q = p.replace("\\", "/")
|
||||
pp = Path(q)
|
||||
if not pp.is_absolute() and root:
|
||||
if not _is_abs(q) and root:
|
||||
pp = Path(root) / q
|
||||
try:
|
||||
return pp.resolve().as_posix()
|
||||
@@ -486,7 +487,7 @@ def _derive_prune_root(prune_sources: "list[str]", stored_sfs: "set[str]") -> "s
|
||||
rel_sfs = [
|
||||
sf.replace("\\", "/")
|
||||
for sf in stored_sfs
|
||||
if sf and isinstance(sf, str) and not os.path.isabs(sf.replace("\\", "/"))
|
||||
if sf and isinstance(sf, str) and not _is_abs(sf.replace("\\", "/"))
|
||||
]
|
||||
if not rel_sfs:
|
||||
return None
|
||||
@@ -495,7 +496,7 @@ def _derive_prune_root(prune_sources: "list[str]", stored_sfs: "set[str]") -> "s
|
||||
if not p or not isinstance(p, str):
|
||||
continue
|
||||
q = p.replace("\\", "/")
|
||||
if not os.path.isabs(q):
|
||||
if not _is_abs(q):
|
||||
continue
|
||||
hits = {q[: -len(s) - 1] for s in rel_sfs if q.endswith("/" + s)}
|
||||
if len(hits) > 1:
|
||||
@@ -608,8 +609,13 @@ def _semantic_id_remap(nodes: list, root: str | None) -> dict:
|
||||
continue
|
||||
sf_norm = _norm_source_file(str(sf), root) or str(sf)
|
||||
rel = Path(sf_norm)
|
||||
if rel.is_absolute():
|
||||
continue # can't relativize (no/failed root) — leave id untouched
|
||||
if _is_abs(sf_norm):
|
||||
# Can't relativize (no/failed root) — leave the id untouched rather
|
||||
# than bake an on-disk path into it. Tested for BOTH platforms: a
|
||||
# graph built on Linux/CI carries POSIX-absolute source_files that
|
||||
# WindowsPath.is_absolute() calls relative, which leaked the whole
|
||||
# build directory into node IDs when updated on Windows (#2618).
|
||||
continue
|
||||
if not rel.name:
|
||||
# source_file equals the scan root, so _norm_source_file relativized it
|
||||
# to Path('.') — a project-level node with no per-file identity to remap.
|
||||
@@ -642,7 +648,7 @@ def _semantic_id_remap(nodes: list, root: str | None) -> dict:
|
||||
# id registration. It is the longest form, so it goes first (greedy
|
||||
# prefix stripping, same ordering rule as _old_file_stems).
|
||||
sf_raw = str(sf).replace("\\", "/")
|
||||
if sf_raw != sf_norm and os.path.isabs(sf_raw):
|
||||
if sf_raw != sf_norm and _is_abs(sf_raw):
|
||||
abs_stem = make_id(_file_stem(Path(sf_raw)))
|
||||
if abs_stem and abs_stem != new_stem and abs_stem not in old_forms:
|
||||
old_forms.insert(0, abs_stem)
|
||||
@@ -685,8 +691,9 @@ def graph_has_legacy_ids(nodes: list, root: str | Path | None = None, sample: in
|
||||
sf = node.get("source_file")
|
||||
if not nid or not isinstance(nid, str) or not sf:
|
||||
continue
|
||||
rel = Path(_norm_source_file(str(sf), _r) or str(sf))
|
||||
if rel.is_absolute():
|
||||
sf_norm = _norm_source_file(str(sf), _r) or str(sf)
|
||||
rel = Path(sf_norm)
|
||||
if _is_abs(sf_norm):
|
||||
continue
|
||||
if not rel.name:
|
||||
continue # source_file == scan root -> Path('.'), no file stem (#1618)
|
||||
@@ -1047,7 +1054,7 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
|
||||
if not sf:
|
||||
continue
|
||||
rel = Path(str(sf))
|
||||
if rel.is_absolute():
|
||||
if _is_abs(str(sf)):
|
||||
continue
|
||||
new_stem = make_id(_fs(rel))
|
||||
if str(attrs.get("label", "")) == rel.name:
|
||||
|
||||
+36
-1
@@ -21,7 +21,7 @@ import os
|
||||
import re
|
||||
import stat
|
||||
import tempfile
|
||||
from pathlib import Path, PurePosixPath
|
||||
from pathlib import Path, PurePosixPath, PureWindowsPath
|
||||
|
||||
GRAPHIFY_OUT = os.environ.get("GRAPHIFY_OUT", "graphify-out")
|
||||
|
||||
@@ -303,6 +303,41 @@ def default_graph_json() -> str:
|
||||
return str(out_path("graph.json"))
|
||||
|
||||
|
||||
def is_absolute_any_platform(p: "str | Path | None") -> bool:
|
||||
"""Whether *p* is absolute under POSIX **or** Windows rules.
|
||||
|
||||
``Path.is_absolute()`` and ``os.path.isabs()`` answer for the HOST os only,
|
||||
which is the wrong question for a path that was *stored* — a ``source_file``
|
||||
in ``graph.json``, a ``prune_sources`` entry, a cache key. Those travel
|
||||
between machines (build in Docker/CI, update on a Windows workstation, or
|
||||
the reverse), so the host's rules do not describe the string in hand:
|
||||
|
||||
- On Windows, ``WindowsPath("/home/ci/repo/docs/a.md").is_absolute()`` is
|
||||
False — no drive letter — so a Linux-built graph's absolute paths read as
|
||||
relative and get baked into node IDs or joined under the scan root (#2618).
|
||||
- On POSIX, ``PosixPath("C:/Users/u/a.md").is_absolute()`` is False for the
|
||||
mirror-image reason (#2197, #1789).
|
||||
|
||||
``os.path.isabs`` is additionally not stable across supported interpreters:
|
||||
Python 3.13 changed ``ntpath.isabs`` so a path starting with a single slash
|
||||
is no longer absolute, where 3.10–3.12 said it was. The project supports
|
||||
>=3.10, so a guard written on it silently means different things per version.
|
||||
|
||||
Answering for both platforms is the conservative choice for stored paths:
|
||||
treating a path as absolute at worst declines to relativize it (the string is
|
||||
kept as-is), whereas treating an absolute path as relative corrupts identity.
|
||||
Covers drive-letter, UNC, and POSIX-root forms with either separator.
|
||||
|
||||
NOTE: this is for STORED/portable paths. Code resolving a path against the
|
||||
real local filesystem (``cli``, ``detect``, ``hooks``) must keep using
|
||||
``Path.is_absolute()`` — there the host's rules are exactly right.
|
||||
"""
|
||||
if not p:
|
||||
return False
|
||||
s = str(p)
|
||||
return PurePosixPath(s).is_absolute() or PureWindowsPath(s).is_absolute()
|
||||
|
||||
|
||||
def nfc(s: str) -> str:
|
||||
"""NFC-normalize a path string.
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import json
|
||||
from pathlib import Path
|
||||
import networkx as nx
|
||||
import pytest
|
||||
from networkx.readwrite import json_graph
|
||||
from graphify.build import build_from_json, build, build_merge, edge_data, edge_datas, dedupe_edges, dedupe_nodes
|
||||
|
||||
@@ -1427,3 +1428,65 @@ def test_build_from_json_prunes_dangling_hyperedge_members(capsys):
|
||||
assert set(hes) == {"he_partial"}, "an all-dangling hyperedge must be dropped"
|
||||
assert hes["he_partial"]["nodes"] == ["alpha", "beta"]
|
||||
assert "he_all_ghost" in capsys.readouterr().err
|
||||
|
||||
|
||||
# --- foreign-absolute source_file must not leak into IDs --------------------
|
||||
# A graph.json is portable: built in Docker/Linux CI, updated on a Windows
|
||||
# workstation, or the reverse. Every guard that asks "is this stored path
|
||||
# absolute?" therefore has to answer for BOTH platforms. Host-only tests
|
||||
# (Path.is_absolute / os.path.isabs) silently call the other platform's
|
||||
# absolute paths relative, which bakes a build directory into node IDs (the
|
||||
# mirror of #2197, reported as #2618) or joins it under the scan root.
|
||||
#
|
||||
# os.path.isabs is doubly unsafe here: Python 3.13 changed ntpath.isabs so a
|
||||
# single leading slash is no longer absolute, where 3.10-3.12 said it was, so
|
||||
# the same guard meant different things across supported interpreters.
|
||||
|
||||
FOREIGN_ABSOLUTE_SOURCE_FILES = [
|
||||
"/home/ci/build/repo/docs/api/README.md", # POSIX-absolute (Linux/Docker build)
|
||||
"C:/Users/u/repo/docs/api/README.md", # Windows-absolute, forward slashes
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("sf", FOREIGN_ABSOLUTE_SOURCE_FILES)
|
||||
def test_semantic_rekey_skips_absolute_from_either_platform(sf):
|
||||
"""#2618: an absolute source_file is left alone whichever OS wrote it."""
|
||||
from graphify.build import _semantic_id_remap
|
||||
nodes = [{"id": "api_readme", "source_file": sf, "type": "document"}]
|
||||
assert _semantic_id_remap(nodes, None) == {}, (
|
||||
f"{sf!r} leaked its on-disk path into the node ID"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("sf", FOREIGN_ABSOLUTE_SOURCE_FILES)
|
||||
def test_graph_has_legacy_ids_skips_absolute_from_either_platform(sf):
|
||||
"""The legacy-ID probe derives a stem from source_file, so it must skip an
|
||||
absolute path rather than mint a stem out of the whole build directory."""
|
||||
from graphify.build import graph_has_legacy_ids
|
||||
nodes = [{"id": "api_readme", "source_file": sf,
|
||||
"type": "document", "source_location": "L1"}]
|
||||
assert graph_has_legacy_ids(nodes, root=None) is False
|
||||
|
||||
|
||||
def test_norm_source_file_relativizes_a_posix_absolute_path():
|
||||
"""A Linux-built graph's absolute source_file must relativize against the
|
||||
matching root regardless of the host running the update."""
|
||||
from graphify.build import _norm_source_file
|
||||
assert _norm_source_file(
|
||||
"/home/ci/build/repo/docs/api/README.md", "/home/ci/build/repo"
|
||||
) == "docs/api/README.md"
|
||||
|
||||
|
||||
def test_derive_prune_root_recovers_root_from_posix_absolute_prune_sources():
|
||||
"""The prune-root recovery skips any prune source it thinks is relative.
|
||||
|
||||
With a host-only absoluteness test, every POSIX-absolute prune source was
|
||||
skipped on Windows, the root came back None, and prune/replace silently
|
||||
no-opped — the failure mode of #1151 / #2446 / #2012, reached from the other
|
||||
direction.
|
||||
"""
|
||||
from graphify.build import _derive_prune_root
|
||||
stored = {"docs/a.md", "/home/ci/build/repo/docs/b.md"}
|
||||
assert _derive_prune_root(
|
||||
["/home/ci/build/repo/docs/a.md"], stored
|
||||
) == "/home/ci/build/repo"
|
||||
|
||||
@@ -97,3 +97,50 @@ def test_disambiguate_path_proximity_same_dir() -> None:
|
||||
"pkg/a/caller.py",
|
||||
)
|
||||
assert winner == "near"
|
||||
|
||||
|
||||
# --- cross-platform absoluteness for STORED paths ---------------------------
|
||||
# Path.is_absolute()/os.path.isabs() answer for the host OS, which is the wrong
|
||||
# question for a path read out of graph.json: graphs are built in Docker/CI and
|
||||
# updated on Windows (and vice versa), so the string in hand may follow the
|
||||
# other platform's rules. Getting this wrong bakes an on-disk path into node IDs
|
||||
# (#2197, #1789) or joins an absolute path under the scan root.
|
||||
|
||||
@pytest.mark.parametrize("path", [
|
||||
"/home/ci/build/repo/docs/api/README.md", # POSIX absolute
|
||||
"/", # POSIX root
|
||||
"C:/Users/u/repo/docs/a.md", # Windows drive, forward slashes
|
||||
r"C:\Users\u\repo\docs\a.md", # Windows drive, backslashes
|
||||
r"\\server\share\docs\a.md", # UNC
|
||||
"//server/share/docs/a.md", # UNC, forward slashes
|
||||
])
|
||||
def test_is_absolute_any_platform_accepts_both_conventions(path):
|
||||
from graphify.paths import is_absolute_any_platform
|
||||
assert is_absolute_any_platform(path) is True, (
|
||||
f"{path!r} is absolute on some platform and must be treated as such"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", [
|
||||
"docs/api/README.md",
|
||||
r"docs\api\README.md",
|
||||
"README.md",
|
||||
".",
|
||||
"",
|
||||
None,
|
||||
r"\foo", # Windows root-relative (no drive) — not absolute anywhere
|
||||
])
|
||||
def test_is_absolute_any_platform_rejects_relative(path):
|
||||
from graphify.paths import is_absolute_any_platform
|
||||
assert is_absolute_any_platform(path) is False
|
||||
|
||||
|
||||
def test_is_absolute_any_platform_is_host_independent():
|
||||
"""The whole point: the answer must not depend on which OS is running.
|
||||
|
||||
Both spellings are absolute somewhere, so both must return True on every
|
||||
host — that is exactly what Path.is_absolute() fails to do.
|
||||
"""
|
||||
from graphify.paths import is_absolute_any_platform
|
||||
assert is_absolute_any_platform("/home/ci/x.md")
|
||||
assert is_absolute_any_platform("C:/Users/u/x.md")
|
||||
|
||||
Reference in New Issue
Block a user