From 8f4486d5feb546ab5b33b2a02a86b0d52a98bf41 Mon Sep 17 00:00:00 2001 From: Christian Svensson Date: Sun, 27 Sep 2026 08:07:30 +0200 Subject: [PATCH 1/3] extract_cmake: key sources relative to repo_root, not the CMake source dir The File API spells sources relative to the top-level CMake source dir. When that dir is a subdirectory of the repo root (a vendored third_party/ package, or a monorepo holding several CMake projects) the CMake side keyed a TU as 'avl.c' while the Bazel side keyed the same file as 'third_party/libubox/avl.c', so every TU showed up as missing_tu/extra_tu and the diff was meaningless. Anchor each source on the codemodel's paths.source and re-relativize against repo_root; sources outside the repo stay absolute as before. Fixtures without paths.source keep the old behaviour. Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/extract_cmake.py | 29 ++++++++++++++++++++++++++--- tests/test_extractors.py | 17 +++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/scripts/extract_cmake.py b/scripts/extract_cmake.py index f070f1f..76d60b2 100644 --- a/scripts/extract_cmake.py +++ b/scripts/extract_cmake.py @@ -139,10 +139,31 @@ def _target_id_to_name(codemodel: dict) -> Dict[str, str]: return out -def _parse_target(tobj: dict, repo_root: str) -> Target: +def _source_path(path: str, source_dir: Optional[str], repo_root: str) -> str: + """Key a codemodel source by its path relative to repo_root. + + The File API spells a source relative to the CMake *top-level source dir* + (absolute only when it lives outside it). When the CMake project is not the + repo root -- a package vendored under third_party/, a monorepo with several + CMake projects -- that spelling never matches the Bazel side, which keys the + same file workspace-relative. Anchor it on the codemodel's `paths.source` + and re-relativize against repo_root; a source outside repo_root stays + absolute (the differ leaves those alone).""" + if source_dir and not os.path.isabs(path): + path = os.path.normpath(os.path.join(source_dir, path)) + if os.path.isabs(path) and repo_root: + rel = os.path.relpath(path, repo_root) + if not rel.startswith(".."): + return rel.replace(os.sep, "/") + return path.replace(os.sep, "/") + + +def _parse_target(tobj: dict, repo_root: str, + source_dir: Optional[str] = None) -> Target: name = tobj["name"] kind = _KIND.get(tobj.get("type", ""), TargetKind.UNKNOWN) - sources = [s["path"] for s in tobj.get("sources", [])] + sources = [_source_path(s["path"], source_dir, repo_root) + for s in tobj.get("sources", [])] # Synthesize one CppCompile Action per source: an argv the differ parses the # same way it parses Bazel's. CMake has no real command line, so we build the @@ -230,6 +251,8 @@ def extract(build_dir: str, repo_root: str, reply_dir = os.path.join(build_dir, ".cmake", "api", "v1", "reply") codemodel = _find_codemodel(reply_dir) id_to_name = _target_id_to_name(codemodel) + # top-level CMake source dir; sources are spelled relative to it + source_dir = (codemodel.get("paths") or {}).get("source") model = CanonicalModel(build_system=BuildSystem.CMAKE, repo_root=repo_root) include_dirs = set() @@ -237,7 +260,7 @@ def extract(build_dir: str, repo_root: str, for tref in cfg["targets"]: with open(os.path.join(reply_dir, tref["jsonFile"])) as f: tobj = json.load(f) - target = _parse_target(tobj, repo_root) + target = _parse_target(tobj, repo_root, source_dir) _attach_deps(target, tobj, id_to_name) model.add(target) for cg in tobj.get("compileGroups", []): diff --git a/tests/test_extractors.py b/tests/test_extractors.py index fd282b7..c502a72 100644 --- a/tests/test_extractors.py +++ b/tests/test_extractors.py @@ -139,6 +139,23 @@ def test_full_pipeline_converges(): assert res["errors"] == 0, json.dumps(res, indent=2) +def test_cmake_project_below_repo_root_keys_sources_repo_relative(): + """A CMake project vendored under the repo root (third_party/foo) must key + its sources the way the Bazel side does: relative to the REPO root, not to + the CMake source dir. The File API spells sources relative to `paths.source`.""" + with tempfile.TemporaryDirectory() as root: + build = _write_cmake_fixture(root) + reply = os.path.join(build, ".cmake", "api", "v1", "reply") + cm = dict(CODEMODEL) + cm["paths"] = {"source": os.path.join(REPO, "third_party", "proj"), + "build": build} + with open(os.path.join(reply, "codemodel.json"), "w") as f: + json.dump(cm, f) + a = extract_cmake.extract(build, REPO) + tu = _view(a, "mylib").tus[0] + assert tu.source == "third_party/proj/src/a.cpp", tu.source + + def test_cmake_extracts_canonical_flags(): with tempfile.TemporaryDirectory() as root: build = _write_cmake_fixture(root) From 7d6ed5cb171cf97b8012fa1ae02605c29d78bf6c Mon Sep 17 00:00:00 2001 From: Christian Svensson Date: Sun, 27 Sep 2026 08:07:30 +0200 Subject: [PATCH 2/3] diff: make the library TU-union representative deterministic _union_tus keeps the first TU seen per source, and the library names it walks come from a set comprehension, so when one source is compiled by two library targets with different flags (a shared/static twin: -Dfoo_EXPORTS, -fPIC) the representative -- and therefore the reported defines_diff/flags_diff -- flipped between runs with Python's per-process hash seed. Walk the names sorted. Found by the model during the libubox (OpenWrt) migration: the same diff run alternated between reporting ubox_EXPORTS as cmake-only and not at all. Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/diff.py | 6 +++++- tests/test_engine.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/scripts/diff.py b/scripts/diff.py index d602dcb..4ed8ee2 100644 --- a/scripts/diff.py +++ b/scripts/diff.py @@ -231,8 +231,12 @@ def _union_tus(views: Dict[str, TargetView], names, lands in one flat map keyed by repo-relative path. Keys are run through cfg.map_source so generated-source grouping asymmetries (e.g. CMake's single AUTOMOC bundle vs Bazel's per-header moc_*.cpp) collapse to one token.""" + # Deterministic: `names` is usually a set, and the first TU wins below, so + # a source compiled by two targets with different flags (a shared/static + # twin: -Dfoo_EXPORTS, -fPIC) would otherwise be represented by whichever + # target Python's hash seed enumerates first -- a diff that flips per run. out: Dict[str, TranslationUnit] = {} - for n in names: + for n in sorted(names): for tu in views[n].tus: out.setdefault(cfg.map_source(tu.key()), tu) return out diff --git a/tests/test_engine.py b/tests/test_engine.py index c8a83ea..470eb55 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -690,6 +690,41 @@ def test_nonparticipating_roles_are_excluded_not_diffed(): assert "Nightly" in res["excluded"]["cmake"]["dashboard"] +_TWIN_SCRIPT = r""" +import json, os, sys +sys.path.insert(0, sys.argv[1]) +from diff import diff_models, summarize +from model import Action, BuildSystem, CanonicalModel, Target, TargetKind, TargetRole +def tu(raw): + return Action(mnemonic="CppCompile", arguments=tuple(raw + ["-c", "foo/a.cpp"])) +a = CanonicalModel(build_system=BuildSystem.CMAKE, repo_root="/work/proj") +a.add(Target("foo", TargetKind.SHARED, role=TargetRole.PRODUCTION, + actions=[tu(["-Dfoo_EXPORTS", "-DX=1"])])) +a.add(Target("foo-static", TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu(["-DX=1"])])) +b = CanonicalModel(build_system=BuildSystem.BAZEL, repo_root="/work/proj") +b.add(Target(":foo", TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu(["-DX=1"])])) +print(json.dumps(summarize(diff_models(a, b)), sort_keys=True)) +""" + + +def test_shared_static_twin_representative_is_deterministic(): + # CMake builds the same source twice: as a SHARED lib (-Dfoo_EXPORTS) and + # as its STATIC twin (no define). The library TU-union keeps ONE + # representative per source; which one must not depend on target-name + # enumeration order (a set, so hash-seeded per process) or the reported + # defines_diff flips between runs. Run the diff under several hash seeds. + import subprocess + scripts = os.path.join(os.path.dirname(__file__), "..", "scripts") + outs = set() + for seed in range(8): + env = dict(os.environ, PYTHONHASHSEED=str(seed)) + outs.add(subprocess.check_output( + [sys.executable, "-c", _TWIN_SCRIPT, scripts], env=env).decode()) + assert len(outs) == 1, outs + + if __name__ == "__main__": import traceback fns = [v for k, v in sorted(globals().items()) if k.startswith("test_")] From 16fd2b185bcfd68e0c98d210ce2dd2607a590170 Mon Sep 17 00:00:00 2001 From: Christian Svensson Date: Sun, 27 Sep 2026 08:07:30 +0200 Subject: [PATCH 3/3] extract_cmake: resolve relative -I roots against the target's source dir A subdirectory CMakeLists that does ADD_DEFINITIONS(-I..) (libubox lua/, ubus lua/ and examples/, uci lua/) reaches the codemodel as the literal token `-I..` in compileCommandFragments. Stored verbatim, the CMake side carried `..` as an include root while the Bazel side spells the same directory `third_party/`, so every such package needed the same include_map entry (`..` -> `third_party/`) before it could converge. `..` only means something relative to the directory that spelled it. CMake itself defines a relative include_directories() entry as relative to CMAKE_CURRENT_SOURCE_DIR, and the raw -I.. is the hand-spelled version of that intent (it only names a real header root for in-source builds, which is OpenWrt's default: cmake.mk sets CMAKE_BINARY_DIR = CMAKE_SOURCE_DIR). So resolve relative -I/-isystem/-iquote/-idirafter roots -- joined or split, in fragments or in includes[].path -- against the target's source dir (codemodel paths.source joined with the target's paths.source), normalize (lua/.. -> package root) and re-relativize against repo_root, the same way the previous commit anchors sources. Absolute roots are untouched: the canonicalizer already normalizes those (`examples/..` from INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}/..) collapses there). Replies without codemodel paths keep the verbatim spelling. With this, libubox, ubus and uci converge with their include_map entries removed. The `.` -> `third_party/` entries those configs also carried were never load-bearing: `.` only ever appeared on the Bazel side (the toolchain's `-iquote .` for the workspace root), which the asymmetric include check already tolerates as bazel_only. Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/extract_cmake.py | 69 ++++++++++++++++++++++++++++++++++++++-- tests/test_extractors.py | 59 ++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 2 deletions(-) diff --git a/scripts/extract_cmake.py b/scripts/extract_cmake.py index 76d60b2..78ea77e 100644 --- a/scripts/extract_cmake.py +++ b/scripts/extract_cmake.py @@ -158,12 +158,75 @@ def _source_path(path: str, source_dir: Optional[str], repo_root: str) -> str: return path.replace(os.sep, "/") +# Include-root flags whose argument is a DIRECTORY that the compiler resolves +# against its CWD. Joined (-I..) or split (-I ..) forms both occur in fragments. +_INCLUDE_FLAGS = ("-I", "-isystem", "-iquote", "-idirafter") + + +def _include_root(path: str, target_source_dir: Optional[str], + repo_root: str) -> str: + """Spell a RELATIVE include root the way the Bazel side spells it. + + A subdirectory CMakeLists that does INCLUDE_DIRECTORIES(..) or + ADD_DEFINITIONS(-I..) yields a root spelled relative to that directory. + CMake defines a relative include dir as relative to + CMAKE_CURRENT_SOURCE_DIR; the raw -I.. flag is the same intent spelled by + hand (it only ever names a real header root for in-source builds, where + the compiler's CWD is the source tree -- OpenWrt's default). Stored + verbatim it can never match Bazel's workspace-relative `third_party/`, + and every such package needed an include_map entry for `..`. Resolve it + against the target's source dir, normalize (`lua/..` -> package root) and + re-relativize against repo_root, exactly as sources are keyed. Absolute + roots are left alone: canonicalize.py already normalizes those (and + `examples/..` collapses there).""" + if os.path.isabs(path) or not target_source_dir: + return path + return _source_path(path, target_source_dir, repo_root) + + +def _resolve_include_tokens(tokens: List[str], target_source_dir: Optional[str], + repo_root: str) -> List[str]: + """Rewrite the path of every -I/-isystem/-iquote/-idirafter token in a + compileCommandFragments argv through _include_root; everything else is kept + verbatim (the model stores raw argv -- only the SPELLING of a CWD-relative + path is fixed, since that path is meaningless outside its CWD).""" + out: List[str] = [] + i, n = 0, len(tokens) + while i < n: + tok = tokens[i] + if tok in _INCLUDE_FLAGS and i + 1 < n: + out.append(tok) + out.append(_include_root(tokens[i + 1], target_source_dir, repo_root)) + i += 2 + continue + for flag in _INCLUDE_FLAGS: + if tok.startswith(flag) and len(tok) > len(flag): + tok = flag + _include_root(tok[len(flag):], target_source_dir, + repo_root) + break + out.append(tok) + i += 1 + return out + + +def _target_source_dir(tobj: dict, source_dir: Optional[str]) -> Optional[str]: + """The directory whose CMakeLists defines this target: the codemodel's + top-level `paths.source` joined with the target's own `paths.source` + (`.` for the top level, `lua` for lua/CMakeLists.txt). None when the reply + carries no `paths` (older fixtures), which keeps relative roots verbatim.""" + if not source_dir: + return None + sub = (tobj.get("paths") or {}).get("source") or "." + return os.path.normpath(os.path.join(source_dir, sub)) + + def _parse_target(tobj: dict, repo_root: str, source_dir: Optional[str] = None) -> Target: name = tobj["name"] kind = _KIND.get(tobj.get("type", ""), TargetKind.UNKNOWN) sources = [_source_path(s["path"], source_dir, repo_root) for s in tobj.get("sources", [])] + target_source_dir = _target_source_dir(tobj, source_dir) # Synthesize one CppCompile Action per source: an argv the differ parses the # same way it parses Bazel's. CMake has no real command line, so we build the @@ -172,12 +235,14 @@ def _parse_target(tobj: dict, repo_root: str, for cg in tobj.get("compileGroups", []): base: List[str] = [] for frag in cg.get("compileCommandFragments", []): - base.extend(_split_fragment(frag.get("fragment", ""))) + base.extend(_resolve_include_tokens( + _split_fragment(frag.get("fragment", "")), + target_source_dir, repo_root)) for d in cg.get("defines", []): base.append("-D" + d["define"]) for inc in cg.get("includes", []): base.append("-isystem" if inc.get("isSystem") else "-I") - base.append(inc["path"]) + base.append(_include_root(inc["path"], target_source_dir, repo_root)) for idx in cg.get("sourceIndexes", []): src = sources[idx] argv = tuple(base + ["-c", src]) diff --git a/tests/test_extractors.py b/tests/test_extractors.py index c502a72..735affd 100644 --- a/tests/test_extractors.py +++ b/tests/test_extractors.py @@ -156,6 +156,65 @@ def test_cmake_project_below_repo_root_keys_sources_repo_relative(): assert tu.source == "third_party/proj/src/a.cpp", tu.source +def test_cmake_subdir_relative_include_roots_are_resolved(): + """A subdirectory CMakeLists (lua/) doing ADD_DEFINITIONS(-I..) or + INCLUDE_DIRECTORIES(..) yields roots that only mean something from the + compile's CWD. The File API hands them over verbatim -- `-I..` in a + compileCommandFragment, or as an `includes[]` path -- so stored as-is the + CMake side says `..` while the Bazel side says `third_party/proj`, and each + package needed an include_map entry to converge. Resolve them + against the target's source dir and re-relativize against the repo root.""" + with tempfile.TemporaryDirectory() as root: + build = _write_cmake_fixture(root) + reply = os.path.join(build, ".cmake", "api", "v1", "reply") + cm = dict(CODEMODEL) + cm["paths"] = {"source": os.path.join(REPO, "third_party", "proj"), + "build": build} + with open(os.path.join(reply, "codemodel.json"), "w") as f: + json.dump(cm, f) + tgt = json.loads(json.dumps(TARGET_MYLIB)) + tgt["paths"] = {"source": "lua", "build": "lua"} + tgt["sources"] = [{"path": "lua/a.cpp"}] + cg = tgt["compileGroups"][0] + cg["compileCommandFragments"] = [ + {"fragment": "-std=c++17 -I.. -isystem ../vendor -I ."}] + cg["includes"] = [{"path": ".."}, # relative + {"path": "/work/proj/third_party/proj/lua/.."}, + {"path": "/opt/sdk/include", "isSystem": True}] + with open(os.path.join(reply, "target-mylib.json"), "w") as f: + json.dump(tgt, f) + a = extract_cmake.extract(build, REPO) + # the raw argv is rewritten only in the SPELLING of the relative roots + raw = a.targets["mylib"].actions[0].arguments + assert "-Ithird_party/proj" in raw, raw + assert ("-isystem", "third_party/proj/vendor") in zip(raw, raw[1:]), raw + assert ".." not in raw and "." not in raw, raw + assert "-std=c++17" in raw + tu = _view(a, "mylib").tus[0] + assert tu.source == "third_party/proj/lua/a.cpp", tu.source + assert "third_party/proj" in tu.includes, tu.includes # -I.. / [..] + assert "third_party/proj/lua" in tu.includes, tu.includes # -I . + assert "third_party/proj/vendor" in tu.includes, tu.includes + assert "/opt/sdk/include" in tu.includes, tu.includes # abs, kept + assert not any(i.startswith(".") for i in tu.includes), tu.includes + + +def test_cmake_relative_include_roots_kept_without_paths(): + """A reply with no codemodel `paths` (older fixtures) has nothing to anchor + on; a relative root stays verbatim rather than being resolved against a + guessed directory.""" + with tempfile.TemporaryDirectory() as root: + build = _write_cmake_fixture(root) + reply = os.path.join(build, ".cmake", "api", "v1", "reply") + tgt = json.loads(json.dumps(TARGET_MYLIB)) + tgt["compileGroups"][0]["compileCommandFragments"] = [ + {"fragment": "-std=c++17 -I.."}] + with open(os.path.join(reply, "target-mylib.json"), "w") as f: + json.dump(tgt, f) + a = extract_cmake.extract(build, REPO) + assert "-I.." in a.targets["mylib"].actions[0].arguments + + def test_cmake_extracts_canonical_flags(): with tempfile.TemporaryDirectory() as root: build = _write_cmake_fixture(root)