From ccf83d5bc917c9d2bf1219533840e2ffaba69523 Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Mon, 2 Nov 2026 10:32:01 -0500 Subject: [PATCH 01/17] Check bazel_args with Bazel's own flag parser bazel_args is meant for the flags that make aquery mirror the real build (--config=release, -c opt). Anything that consumes it splices it into a bazel command line, but nothing checked its shape, and the SKILL.md text sits next to a literal `bazel aquery ...` line that is easy to paste in. Measured in any2bazel-lab: on a QGIS migration the agent recorded the whole aquery command as bazel_args. Its own diff never read the field, so the migration looked converged, while a consumer that spliced the field into `bazel aquery` failed on a second query expression and produced no parity result. SKILL.md now says the field holds flags, with one right and one wrong example, and runs `bazel canonicalize-flags --for_command=aquery` on it just before aquery. Bazel refuses anything that is not an aquery option, including a target pattern and a stray word after a boolean flag (`--keep_going true`), which a hand-written check would have to special-case. The one-liner hands the list to Bazel as argv (os.execvp), so a value with a space in it is not split. It does not check that a --config exists; aquery fails on one that does not. Checked against Bazel 9.3.1: it refused a whole aquery command and `--keep_going true`, and accepted `--copt=-DMSG="a b"`, `-c opt`, `--config=release` and a config with no bazel_args. The lab's later QGIS run, made with an earlier version of this change (a Python check), recorded only valid flags; this version has not been through a migration run. Lab-Proposal: P22 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 11 ++++++++--- scripts/config.py | 8 +++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/SKILL.md b/SKILL.md index 62ad5c0..d841c8b 100644 --- a/SKILL.md +++ b/SKILL.md @@ -112,9 +112,12 @@ whether its frontend reads it: ``` Fields: -- **`bazel_args`** — the extra args aquery must run with to mirror the real - build (see [Extract the Bazel action graph](#extract-bazel-graph)). Recorded so - the comparison is reproducible; read by the operator, not the diff. +- **`bazel_args`** — the extra **flags** aquery must run with to mirror the + real build (see [Extract the Bazel action graph](#extract-bazel-graph)), e.g. + `["--config=release"]` or `["-c", "opt"]`; never a command + (`["aquery", "mnemonic(...)", ...]` is wrong). Recorded so the comparison is + reproducible; check it with Bazel before running aquery (see + [Extract the Bazel action graph](#extract-bazel-graph)). - **`target_map`** — `reference_name → bazel_name` for intentionally renamed **executables**. Bazel targets are keyed by full label, so the value is usually `:name` (e.g. `"bssl": ":bssl"`). Libraries need no mapping. @@ -261,6 +264,8 @@ instead of being a warning nobody reads. > always point to files that are either stale, or do not exist at all. ```bash +# bazel_args, as Bazel parses them; it refuses anything that is not an aquery flag +python3 -c 'import json,os,sys; os.execvp("bazel", ["bazel", "canonicalize-flags", "--for_command=aquery", "--", *json.load(open(sys.argv[1])).get("bazel_args", [])])' any2bazel.json bazel aquery 'mnemonic("CppCompile|ObjcCompile|CppLink|CppArchive", //...)' \ --features=-compiler_param_file --features=-linker_param_file \ [--config=] [--copt=... --cxxopt=...] \ diff --git a/scripts/config.py b/scripts/config.py index fd17cbd..cd52739 100644 --- a/scripts/config.py +++ b/scripts/config.py @@ -100,9 +100,11 @@ class MigrationConfig: # Unlike `ignore` (flags/defines), this is the only lever for missing_tu / # missing_target on whole subtrees. Each entry is a reviewable decision. exclude_targets: Set[str] = field(default_factory=set) - # The bazel aquery invocation this migration is compared against, recorded - # so the comparison is reproducible. Read by the skill/operator, not by the - # diff itself (the diff consumes the already-extracted model). + # The FLAGS appended to `bazel aquery` so the Bazel side mirrors the real + # build (never the command itself), recorded so the comparison is + # reproducible. Read by the skill/operator, not by the diff itself (the + # diff consumes the already-extracted model); SKILL.md has Bazel parse + # them before aquery. bazel_args: tuple = () # Opt-in: also diff TEST targets (role=test). OFF by default because test # diffing requires BOTH models to be extracted with tests enabled and the From 57aed8b09466d0ec084594191dcd0469895e6dc1 Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 18:52:57 -0400 Subject: [PATCH 02/17] Decide target_map renames in one place, and list renamed exclusions exclude_targets holds CMake names. The diff applies target_map before deciding participation, so a Bazel target mapped to an excluded CMake name is skipped. excluded_summary, which reports what was skipped and why, checked the raw Bazel label instead, so that target appeared nowhere in `excluded.bazel`: skipped, but not on the record. A new helper, _bazel_renames, returns each Bazel label with the name the diff compares it under (today, the target_map entries whose label exists), and is the one place that decision is made. diff_models renames the Bazel views with it and, as before, also applies every explicit target_map entry to dependency references (a map entry can align an external dependency that has no target node). excluded_summary checks exclusion on the renamed name while still listing the target under its Bazel label. The per-side summaries later in this series use the same helper, so they count exactly the targets the diff compares. Co-Authored-By: Claude Opus 5.5 --- scripts/diff.py | 26 ++++++++++++++++++++++---- tests/test_engine.py | 26 ++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/scripts/diff.py b/scripts/diff.py index d602dcb..7d26d31 100644 --- a/scripts/diff.py +++ b/scripts/diff.py @@ -220,6 +220,16 @@ def _apply_target_map(views: Dict[str, TargetView], return renamed +def _bazel_renames(a: CanonicalModel, b: CanonicalModel, + cfg: "MigrationConfig") -> Dict[str, str]: + """Bazel label -> the name it is compared under, for every Bazel target the + diff renames (target_map). The ONE place that decision is made: the + comparison and the summaries apply it the same way, so an exclude_targets + entry (a CMake name) reaches a renamed Bazel target everywhere.""" + return {bazel: cmake for cmake, bazel in cfg.target_map.items() + if bazel in b.targets} + + _LIBRARY_KINDS = {TargetKind.STATIC, TargetKind.SHARED, TargetKind.OBJECT, TargetKind.INTERFACE} @@ -298,7 +308,12 @@ def diff_models(a: CanonicalModel, b: CanonicalModel, # Reconstruct raw actions into comparable views (TUs + link flags + deps), # build-system-specific interpretation keyed on each model's tag. a_views = reconstruct(a) - b_views = _apply_target_map(reconstruct(b), cfg.target_map) + renames = _bazel_renames(a, b, cfg) + # Target renames come from the shared decision. Explicit target_map entries + # also rename DEPENDENCY references, even with no target node to rename. + view_map = dict(cfg.target_map) + view_map.update({name: label for label, name in renames.items()}) + b_views = _apply_target_map(reconstruct(b), view_map) a_names = _participating(a_views, cfg) b_names = _participating(b_views, cfg) @@ -481,15 +496,18 @@ def excluded_summary(a: CanonicalModel, b: CanonicalModel, if cfg.include_tests: participating.add(TargetRole.TEST) - def by_reason(model: CanonicalModel) -> dict: + def by_reason(model: CanonicalModel, renames: Dict[str, str]) -> dict: + # exclusion is decided on the name the diff compares under; the entry + # lists the target's own name (a Bazel label keeps its label) out: Dict[str, List[str]] = {} for name, t in sorted(reconstruct(model).items()): - if cfg.target_excluded(name): + if cfg.target_excluded(renames.get(name, name)): out.setdefault("config_excluded", []).append(name) elif t.role not in participating: out.setdefault(t.role.value, []).append(name) return out - return {"cmake": by_reason(a), "bazel": by_reason(b)} + return {"cmake": by_reason(a, {}), + "bazel": by_reason(b, _bazel_renames(a, b, cfg))} def summarize(discs: List[Discrepancy], diff --git a/tests/test_engine.py b/tests/test_engine.py index f50ef31..ec902e2 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -600,6 +600,32 @@ def test_exclude_targets_suppresses_structural_diffs(): assert "benchmark" in res["excluded"]["cmake"]["config_excluded"] +def test_renamed_bazel_target_is_listed_as_excluded(): + # target_map renames pkg:app to app; exclude_targets names app. The diff + # skips it, so the excluded summary must list it on the Bazel side too. + a = _bs(CanonicalModel(), is_bazel=False); a.add(_exe("app", [], False)) + b = _bs(CanonicalModel(), is_bazel=True); b.add(_exe("pkg:app", [], True)) + cfg = MigrationConfig(target_map={"app": "pkg:app"}, exclude_targets={"app"}) + res = summarize(diff_models(a, b, cfg), a, b, cfg) + assert res["discrepancies"] == [], res + assert res["excluded"]["bazel"] == {"config_excluded": ["pkg:app"]}, res + assert res["excluded"]["cmake"] == {"config_excluded": ["app"]}, res + + +def test_target_map_still_renames_a_dependency_with_no_target_node(): + # An external dep spelled foo by CMake and pkg:foo by Bazel, aligned by a + # target_map entry; no pkg:foo target exists. The entry must still rename + # the dependency reference, as it always has. + def m(dep, is_bazel): + model = _bs(CanonicalModel(), is_bazel) + model.add(Target("lib", TargetKind.STATIC, role=TargetRole.PRODUCTION, + deps=[Dependency(dep, external=True)], + actions=[tu_from_raw("src/a.cpp", [], is_bazel)])) + return model + a, b = m("foo", False), m("pkg:foo", True) + assert diff_models(a, b, MigrationConfig(target_map={"foo": "pkg:foo"})) == [] + + def test_config_loads_all_fields_from_json(): # The skill drives the diff via a any2bazel.json on disk, so loading must # populate every lever. Guards against a field added to the dataclass but From 8fdabc2e62c3e18430bf0487089156eb35623a60 Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:23:01 -0400 Subject: [PATCH 03/17] Compare every variant of a multi-variant source, deterministically _union_tus pooled library TUs into one source-keyed map with dict.setdefault while iterating a set of target names. When one source is compiled into several libraries with different flags (a full and a lite build of the same file, a hardened variant), only one variant per side was compared, and which one depended on set iteration order, that is, on PYTHONHASHSEED. The verdict was not reproducible. Measured in any2bazel-lab: the same MySQL Server models gave 4,715 to 4,817 errors over 10 hash seeds, a different error set each time, and the final models of a Blender migration converged under 2 of 8 seeds. Projects without multi-variant sources were unaffected (50 recorded verdicts identical under 20 seeds). Pooling now keeps every distinct variant per source, visiting targets in sorted order. Each CMake variant is compared with its best-matching Bazel variant: fewest errors, then warnings, then the smallest symmetric difference of the two TUs' define, flag and include sets, then a stable text order. Every CMake variant must be matched by some Bazel variant and nothing is suppressed; an extra Bazel-only variant is tolerated, like an extra Bazel flag. The distance term keeps a tie from choosing a variant that only adds flags (-w, -Wno-error=...) over an exact one. A source with one variant per side takes exactly the old path, and on every single-variant verdict the lab has recorded, the result is unchanged. Executables and the opt-in test union use the same pairing, and the Java source union is visited in sorted order too. One consequence for source_map users: a token that collapses several CMake TUs (an AUTOMOC bundle, say) now yields one comparison, and so possibly one discrepancy, per distinct CMake TU, so the worklist can grow. The source_map comment in config.py says so. In the lab's later QGIS run with this change, the differ gave the same verdict under three seeds. Lab-Proposal: P14 Co-Authored-By: Claude Opus 5.5 --- scripts/config.py | 7 ++-- scripts/diff.py | 88 +++++++++++++++++++++++++++++++++------ tests/test_engine.py | 99 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+), 15 deletions(-) diff --git a/scripts/config.py b/scripts/config.py index cd52739..31a10fb 100644 --- a/scripts/config.py +++ b/scripts/config.py @@ -88,9 +88,10 @@ class MigrationConfig: # bundles every moc_*.cpp into one mocs_compilation.cpp TU, while Bazel # compiles each moc_*.cpp separately. Mapping both spellings to one token # (the CMake bundle path AND the Bazel moc/ dir -> "@moc") reconciles them: - # the pooled TU maps collapse to the shared token, so presence matches and - # the representative TU's flags are still compared (extra Bazel defines stay - # benign WARN under the asymmetric-subset rule). Only ever use for codegen + # the pooled TU maps collapse to the shared token, so presence matches, and + # every mapped TU is still compared against its best-matching counterpart + # (extra Bazel defines stay benign WARN under the asymmetric-subset + # rule in `diff._diff_variants()`). Only ever use for codegen # whose per-TU flags are uniform; never to paper over a real missing source. # Unlike include_map, the token REPLACES the whole path (the remainder is # dropped) so N generated files collapse to 1 -- see map_source(). diff --git a/scripts/diff.py b/scripts/diff.py index 7d26d31..cc13c11 100644 --- a/scripts/diff.py +++ b/scripts/diff.py @@ -234,17 +234,79 @@ def _bazel_renames(a: CanonicalModel, b: CanonicalModel, TargetKind.INTERFACE} +def _add_variants(out: Dict[str, List[TranslationUnit]], tus, + cfg: "MigrationConfig") -> None: + """Add TUs to a source-keyed map of DISTINCT variants (see _union_tus).""" + for tu in tus: + variants = out.setdefault(cfg.map_source(tu.key()), []) + if tu not in variants: + variants.append(tu) + + def _union_tus(views: Dict[str, TargetView], names, - cfg: "MigrationConfig") -> Dict[str, TranslationUnit]: + cfg: "MigrationConfig") -> Dict[str, List[TranslationUnit]]: """Pool TUs of the given target views into one source-keyed map (the TU-SET comparison): grouping/renames/fold-ins don't matter -- every compiled source 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.""" - out: Dict[str, TranslationUnit] = {} - for n in names: - for tu in views[n].tus: - out.setdefault(cfg.map_source(tu.key()), tu) + AUTOMOC bundle vs Bazel's per-header moc_*.cpp) collapse to one token. + + Each key holds every DISTINCT variant of that source: one source compiled + into several libraries with different flags (a lite and a full build of the + same file, a hardened variant) is several TUs, and _diff_variants compares + all of them. Names are visited in sorted order so the result never depends + on set iteration order (i.e. on PYTHONHASHSEED).""" + out: Dict[str, List[TranslationUnit]] = {} + for n in sorted(names): + _add_variants(out, views[n].tus, cfg) + return out + + +def _tu_distance(a: TranslationUnit, b: TranslationUnit, + cfg: "MigrationConfig") -> int: + """Size of the symmetric difference of two TUs' define, flag and include + sets, after the reviewer's ignores. Unlike the discrepancies, this counts + flags only one side has, so an exact variant beats a noisy one (-w, + -Wno-error=...) that happens to produce the same discrepancies.""" + ad = {d for d in a.defines if not cfg.define_ignored(d)} + bd = {d for d in b.defines if not cfg.define_ignored(d)} + af = {f for f in a.flags if not cfg.flag_ignored(f)} + bf = {f for f in b.flags if not cfg.flag_ignored(f)} + ai = set(_norm_includes(a.includes, cfg)) + bi = set(_norm_includes(b.includes, cfg)) + return len(ad ^ bd) + len(af ^ bf) + len(ai ^ bi) + + +def _rank(discs: List[Discrepancy], distance: int) -> tuple: + """Order candidate pairings: fewest errors, then fewest warnings, then the + smallest distance between the two TUs (see _tu_distance), then a stable + text form so ties never depend on iteration order.""" + errors = sum(1 for d in discs if d.severity == Severity.ERROR.value) + return (errors, len(discs) - errors, distance, + json.dumps([asdict(d) for d in discs], sort_keys=True)) + + +def _best_match(target: str, a: TranslationUnit, b_vs: List[TranslationUnit], + cfg: "MigrationConfig"): + """(discrepancies, chosen Bazel TU) for CMake TU `a` against its + best-ranked Bazel variant (see _rank).""" + return min(((_diff_tu(target, a, b, cfg), b) for b in b_vs), + key=lambda pair: _rank(pair[0], _tu_distance(a, pair[1], cfg))) + + +def _diff_variants(target: str, a_vs: List[TranslationUnit], + b_vs: List[TranslationUnit], + cfg: "MigrationConfig") -> List[Discrepancy]: + """Compare every CMake variant of one source against its BEST-matching Bazel + variant (fewest errors, then warnings, then distance; see _rank). + Every CMake variant must be matched by some Bazel variant, so nothing is + suppressed: a variant Bazel lacks shows up as that variant's discrepancies + against its closest match. An extra Bazel-only variant is tolerated, like + any extra Bazel flag. With one variant per side this is exactly one + _diff_tu call.""" + out: List[Discrepancy] = [] + for a in a_vs: + out.extend(_best_match(target, a, b_vs, cfg)[0]) return out @@ -272,7 +334,7 @@ def _union_java_sources(views: Dict[str, TargetView], names) -> Dict[str, str]: (Maven's 2 groups vs Bazel's 1) is irrelevant; the SOURCE SET is what must match. First occurrence wins for the displayed original path.""" out: Dict[str, str] = {} - for n in names: + for n in sorted(names): for g in views[n].compile_groups: for src in g.sources: out.setdefault(_java_src_key(src), src) @@ -345,7 +407,7 @@ def diff_models(a: CanonicalModel, b: CanonicalModel, out.append(Discrepancy(Kind.EXTRA_TU.value, Severity.WARN.value, "", "source compiled in bazel but not cmake", tu=src)) for src in sorted(set(a_union) & set(b_union)): - out.extend(_diff_tu("", a_union[src], b_union[src], cfg)) + out.extend(_diff_variants("", a_union[src], b_union[src], cfg)) # ---- executables: identity-aligned, own TUs + own flags ---------------- for name in sorted(a_exes - b_exes): @@ -358,8 +420,10 @@ def diff_models(a: CanonicalModel, b: CanonicalModel, ta, tb = a_views[name], b_views[name] # TU keys go through cfg.map_source (see _union_tus) so codegen grouping # asymmetries collapse to a shared token on both sides. - amap = {cfg.map_source(k): v for k, v in ta.tu_map().items()} - bmap = {cfg.map_source(k): v for k, v in tb.tu_map().items()} + amap: Dict[str, List[TranslationUnit]] = {} + bmap: Dict[str, List[TranslationUnit]] = {} + _add_variants(amap, ta.tus, cfg) + _add_variants(bmap, tb.tus, cfg) # Presence is judged against the whole other side (a_all/b_all), not just # this exe's own TUs: a source this exe compiles may live in a library on # the other side (or vice versa) -- that's a grouping difference, not a @@ -372,7 +436,7 @@ def diff_models(a: CanonicalModel, b: CanonicalModel, out.append(Discrepancy(Kind.EXTRA_TU.value, Severity.WARN.value, name, "source compiled in bazel but not cmake", tu=src)) for src in sorted(set(amap) & set(bmap)): - out.extend(_diff_tu(name, amap[src], bmap[src], cfg)) + out.extend(_diff_variants(name, amap[src], bmap[src], cfg)) # link flags: asymmetric subset, same policy as compile flags -- every # CMake link flag must be present on the Bazel side; extra Bazel link # flags tolerated (toolchain link noise already stripped). Reviewer @@ -468,7 +532,7 @@ def _diff_tests(a_views: Dict[str, TargetView], b_views: Dict[str, TargetView], out.append(Discrepancy(Kind.EXTRA_TEST_TU.value, Severity.WARN.value, "", "test source compiled in bazel but not cmake", tu=src)) for src in sorted(set(a_union) & set(b_union)): - out.extend(_diff_tu("", a_union[src], b_union[src], cfg)) + out.extend(_diff_variants("", a_union[src], b_union[src], cfg)) # 2. test-binary existence by count if len(a_tests) != len(b_tests): diff --git a/tests/test_engine.py b/tests/test_engine.py index ec902e2..656f88a 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -417,6 +417,105 @@ def test_source_map_reconciles_codegen_bundling_asymmetry(): assert summarize(diff_models(a, b, cfg))["converged"] +def _variant_models(cmake_libs, bazel_libs): + """One source (src/x.cc) compiled into several libraries per side, each + library a {name: [defines]} entry -- a multi-variant source.""" + def build(libs, is_bazel): + m = _bs(CanonicalModel(), is_bazel) + for name, defs in libs.items(): + raw = ["-D" + d for d in defs] + ["-std=c++17"] + m.add(Target(name, TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("src/x.cc", raw, is_bazel)])) + return m + return build(cmake_libs, False), build(bazel_libs, True) + + +def test_each_cmake_variant_is_paired_with_its_best_bazel_variant(): + # libprotobuf / libprotobuf-lite style: the same source, two flag sets, on + # both sides. Every variant has an exact counterpart -> converged. + a, b = _variant_models({"proto": ["FULL"], "proto_lite": ["LITE"]}, + {"pkg:proto": ["FULL"], "pkg:proto_lite": ["LITE"]}) + res = summarize(diff_models(a, b)) + assert res["converged"], res + + +def test_a_cmake_variant_missing_on_bazel_is_caught(): + a, b = _variant_models({"proto": ["FULL"], "proto_lite": ["LITE"]}, + {"pkg:proto_lite": ["LITE"]}) + discs = diff_models(a, b) + errs = [d for d in discs if d.severity == Severity.ERROR.value] + assert len(errs) == 1 and errs[0].kind == "defines_diff", errs + assert errs[0].cmake_only == ["FULL"], errs + + +def test_an_extra_bazel_variant_is_tolerated(): + a, b = _variant_models({"proto_lite": ["LITE"]}, + {"pkg:proto": ["FULL"], "pkg:proto_lite": ["LITE"]}) + discs = diff_models(a, b) + assert summarize(discs)["converged"], discs + assert discs == [], discs # the matching variant was chosen, not FULL + + +def test_tied_pairings_prefer_the_smaller_difference(): + # Three CMake bundles collapse to one source token via source_map, each + # with its own define; Bazel has a LIB0 TU and a TU with no define. LIB1 + # and LIB2 tie on errors against both; the pairing must be the one that + # does not report LIB0 (another variant's define) as bazel_only. + a, b = _bs(CanonicalModel(), False), _bs(CanonicalModel(), True) + a.add(Target("l", TargetKind.STATIC, role=TargetRole.PRODUCTION, actions=[ + tu_from_raw(f"gen/a{i}.cc", [f"-DLIB{i}"], False) for i in range(3)])) + b.add(Target("l", TargetKind.STATIC, role=TargetRole.PRODUCTION, actions=[ + tu_from_raw("gen/b0.cc", ["-DLIB0"], True), + tu_from_raw("gen/b1.cc", [], True)])) + discs = diff_models(a, b, MigrationConfig(source_map=(("gen/", "@gen"),))) + assert [(d.cmake_only, d.bazel_only) for d in discs] == \ + [(["LIB1"], []), (["LIB2"], [])], discs + + +def test_tied_pairings_prefer_the_exact_variant_over_a_noisy_one(): + # Both Bazel variants produce no discrepancy against CMake {-std=c++17}; + # the one that only ADDS flags (-w, -Wno-error=foo) must not be chosen. + from diff import _best_match + from reconstruct import reconstruct + a, b = _bs(CanonicalModel(), False), _bs(CanonicalModel(), True) + a.add(Target("l", TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("x.cc", ["-std=c++17"], False)])) + for name, raw in (("a:noisy", ["-std=c++17", "-w", "-Wno-error=foo"]), + ("b:exact", ["-std=c++17"])): + b.add(Target(name, TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("x.cc", raw, True)])) + ta = reconstruct(a)["l"].tus[0] + b_vs = [reconstruct(b)[n].tus[0] for n in ("a:noisy", "b:exact")] + discs, chosen = _best_match("", ta, b_vs, MigrationConfig()) + assert discs == [] and chosen.flags == ("-std=c++17",), chosen + + +def test_multi_variant_verdict_does_not_depend_on_hash_seed(): + # Regression: libraries used to be pooled with dict.setdefault while + # iterating a SET of names, so which variant got compared -- and the + # verdict -- changed with PYTHONHASHSEED. + import json + import subprocess + import tempfile + from serialize import dump_model + libs = {f"lib{i}": (["LITE"] if i % 2 else ["FULL"]) for i in range(8)} + a, b = _variant_models(libs, {"pkg:" + k: v for k, v in libs.items()}) + b.targets.pop("pkg:lib0") # FULL is still built by pkg:lib2, pkg:lib4, ... + with tempfile.TemporaryDirectory() as d: + pa, pb = os.path.join(d, "a.json"), os.path.join(d, "b.json") + dump_model(a, pa) + dump_model(b, pb) + script = os.path.join(os.path.dirname(__file__), "..", "scripts", "diff.py") + outs = set() + for seed in range(8): + env = dict(os.environ, PYTHONHASHSEED=str(seed)) + out = subprocess.run([sys.executable, script, pa, pb], env=env, + capture_output=True, text=True, check=True).stdout + outs.add(out) + assert len(outs) == 1, "diff output varies with PYTHONHASHSEED" + assert json.loads(outs.pop())["converged"] + + def test_ignore_flags_and_defines_suppress_diffs(): # A reviewer-approved flag/define difference is suppressed via config and # the diff converges; without the config it's an error. From f46ae8b66b49787fdf5047a7aaaf4954267d6593 Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:24:24 -0400 Subject: [PATCH 04/17] Report Bazel-only flags beside the verdict The flag comparison is an asymmetric subset: a CMake flag missing on the Bazel side is an error, and a flag only Bazel has is tolerated. That is the right policy, since Bazel's toolchain injects flags no CMake build has, but nothing in the output said so. "0 errors" reads as "both builds compile the same way" when it means "no CMake flag is missing on the Bazel side", and a flag the Bazel side added appeared nowhere unless that TU also had a real discrepancy. Measured in any2bazel-lab: one migration's Bazel side carried 15 distinct compiler warning flags against CMake's one, including ten -Wno-error= demotions the agent added to get past -Werror, and the diff reported 0 errors and 0 warnings with an empty ignore block. Those demotions were justified; an unjustified -w would have looked the same. diff_models takes an optional DiffNotes collector. For every compared TU pair (the chosen variant pair) it counts the compile flags only Bazel has, after the ignore lists and minus the toolchain defaults already hidden from bazel_only; for every aligned executable, it does the same for link flags. summarize() reports them as "bazel_only_flags" (flag -> compared TU pairs, or executables) with a note that they are not compared; triage.py prints a one-line summary with the top entries; SKILL.md says so where diff.json is described. Severity and `converged` are unchanged. Not done: raising warning-control flags the Bazel side adds (-w, -Wno-*, -Wno-error=*) to WARN. That changes what the tool counts, so it is left for the maintainers to decide. In the lab's later graphicsgems run with this change, the agent's report named its Bazel-only flags; the earlier run's report had not. Lab-Proposal: P17 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 8 +++- scripts/diff.py | 88 +++++++++++++++++++++++++++++++++++--------- scripts/triage.py | 13 +++++++ tests/test_engine.py | 73 +++++++++++++++++++++++++++++++++++- tests/test_triage.py | 16 ++++++++ 5 files changed, 178 insertions(+), 20 deletions(-) diff --git a/SKILL.md b/SKILL.md index d841c8b..0fe8a5b 100644 --- a/SKILL.md +++ b/SKILL.md @@ -58,7 +58,8 @@ Bazel aquery jsonproto ──extract_bazel.py───────┘ Done when both stages converge. The comparison is **asymmetric**: every correctness-relevant reference flag must be present on the Bazel side; extra - Bazel flags are tolerated. No artifact/symbol diff or test execution yet. + Bazel flags are tolerated (listed in `diff.json` as `bazel_only_flags`). No + artifact/symbol diff or test execution yet. ### How targets are matched @@ -288,7 +289,10 @@ python3 scripts/diff.py model..json model.bazel.json \ ``` `diff.json` has `converged` (⇔ zero `error` discrepancies), a `discrepancies` worklist (each with `kind`, `severity`, `target`, `tu`, `cmake_only`, -`bazel_only`), and an `excluded` map of non-participating targets by role. +`bazel_only`), an `excluded` map of non-participating targets by role, and +`bazel_only_flags`: flags only the Bazel side has. Those are never a +discrepancy (the comparison is one-directional), so a `-w` or `-Wno-error=` +you add converges silently; review the list before reporting. `cmake_only` is a historical name: for every frontend it means "present in the reference, absent in Bazel". Synthetic target names ``, `` (and `` when diff --git a/scripts/diff.py b/scripts/diff.py index cc13c11..78eacae 100644 --- a/scripts/diff.py +++ b/scripts/diff.py @@ -32,7 +32,8 @@ from __future__ import annotations import json -from dataclasses import asdict, dataclass +from collections import Counter +from dataclasses import asdict, dataclass, field from enum import Enum from typing import Dict, List, Optional @@ -91,6 +92,22 @@ class Discrepancy: bazel_only: Optional[list] = None # present in B, absent in A -> usually ok +@dataclass +class DiffNotes: + """What the diff observes but never counts as a discrepancy, reported in the + summary beside the verdict. Never affects `converged`. + + The flag comparison is one-directional (a CMake flag missing on Bazel is an + error; a flag only Bazel has is tolerated), so a converged diff can hide + flags the Bazel side ADDED -- a `-w` or `-Wno-error=` is never an error. + These counters make them visible: flag -> number of compared TU pairs + (compile; one pair per CMake TU variant, see _diff_variants) or aligned + executables (link) where Bazel has it and CMake does not, after the + reviewer's ignore lists and the toolchain's tolerated defaults.""" + bazel_only_flags: Counter = field(default_factory=Counter) + bazel_only_link_flags: Counter = field(default_factory=Counter) + + def _diff_tu(target: str, a: TranslationUnit, b: TranslationUnit, cfg: "MigrationConfig") -> List[Discrepancy]: out: List[Discrepancy] = [] @@ -154,13 +171,17 @@ def _diff_tu(target: str, a: TranslationUnit, b: TranslationUnit, def _diff_link_flags(name: str, ta: "TargetView", tb: "TargetView", - cfg: "MigrationConfig") -> List[Discrepancy]: + cfg: "MigrationConfig", + notes: Optional[DiffNotes] = None) -> List[Discrepancy]: """Asymmetric link-flag subset for one aligned executable/shared-lib: every CMake link flag must be present on the Bazel side; extra Bazel link flags - are tolerated. Reviewer-approved differences (cfg.link_flag_ignored) are - dropped from both sides first. Mirrors the compile-flag check.""" + are tolerated (and recorded in `notes`). Reviewer-approved differences + (cfg.link_flag_ignored) are dropped from both sides first. Mirrors the + compile-flag check.""" a_lf = {f for f in ta.link_flags if not cfg.link_flag_ignored(f)} b_lf = {f for f in tb.link_flags if not cfg.link_flag_ignored(f)} + if notes is not None: + notes.bazel_only_link_flags.update(b_lf - a_lf) if a_lf - b_lf: return [Discrepancy( kind=Kind.LINK_FLAGS_DIFF.value, @@ -295,18 +316,24 @@ def _best_match(target: str, a: TranslationUnit, b_vs: List[TranslationUnit], def _diff_variants(target: str, a_vs: List[TranslationUnit], - b_vs: List[TranslationUnit], - cfg: "MigrationConfig") -> List[Discrepancy]: + b_vs: List[TranslationUnit], cfg: "MigrationConfig", + notes: Optional[DiffNotes] = None) -> List[Discrepancy]: """Compare every CMake variant of one source against its BEST-matching Bazel variant (fewest errors, then warnings, then distance; see _rank). Every CMake variant must be matched by some Bazel variant, so nothing is suppressed: a variant Bazel lacks shows up as that variant's discrepancies against its closest match. An extra Bazel-only variant is tolerated, like any extra Bazel flag. With one variant per side this is exactly one - _diff_tu call.""" + _diff_tu call. The Bazel-only flags of each chosen pair are recorded in + `notes`.""" out: List[Discrepancy] = [] for a in a_vs: - out.extend(_best_match(target, a, b_vs, cfg)[0]) + discs, b = _best_match(target, a, b_vs, cfg) + out.extend(discs) + if notes is not None: + a_fl = {f for f in a.flags if not cfg.flag_ignored(f)} + b_fl = {f for f in b.flags if not cfg.flag_ignored(f)} + notes.bazel_only_flags.update(_display_bazel_only(b_fl - a_fl)) return out @@ -363,7 +390,11 @@ def _all_source_keys(views: Dict[str, TargetView], cfg: "MigrationConfig") -> se def diff_models(a: CanonicalModel, b: CanonicalModel, - cfg: Optional["MigrationConfig"] = None) -> List[Discrepancy]: + cfg: Optional["MigrationConfig"] = None, + notes: Optional[DiffNotes] = None) -> List[Discrepancy]: + """Compare the CMake model `a` with the Bazel model `b`. Returns the + discrepancy worklist; if `notes` is given, it is filled with what the diff + observed but does not count (see DiffNotes) for summarize() to report.""" out: List[Discrepancy] = [] if cfg is None: cfg = MigrationConfig() @@ -407,7 +438,8 @@ def diff_models(a: CanonicalModel, b: CanonicalModel, out.append(Discrepancy(Kind.EXTRA_TU.value, Severity.WARN.value, "", "source compiled in bazel but not cmake", tu=src)) for src in sorted(set(a_union) & set(b_union)): - out.extend(_diff_variants("", a_union[src], b_union[src], cfg)) + out.extend(_diff_variants("", a_union[src], b_union[src], + cfg, notes)) # ---- executables: identity-aligned, own TUs + own flags ---------------- for name in sorted(a_exes - b_exes): @@ -436,12 +468,12 @@ def diff_models(a: CanonicalModel, b: CanonicalModel, out.append(Discrepancy(Kind.EXTRA_TU.value, Severity.WARN.value, name, "source compiled in bazel but not cmake", tu=src)) for src in sorted(set(amap) & set(bmap)): - out.extend(_diff_variants(name, amap[src], bmap[src], cfg)) + out.extend(_diff_variants(name, amap[src], bmap[src], cfg, notes)) # link flags: asymmetric subset, same policy as compile flags -- every # CMake link flag must be present on the Bazel side; extra Bazel link # flags tolerated (toolchain link noise already stripped). Reviewer # ignores applied to both sides. - out.extend(_diff_link_flags(name, ta, tb, cfg)) + out.extend(_diff_link_flags(name, ta, tb, cfg, notes)) # ---- Java: project-wide source-SET comparison -------------------------- # Java compiles a whole source set per action; how those sources are grouped @@ -489,12 +521,13 @@ def diff_models(a: CanonicalModel, b: CanonicalModel, # ---- tests (opt-in) ---------------------------------------------------- if cfg.include_tests: - out.extend(_diff_tests(a_views, b_views, cfg, a_all, b_all)) + out.extend(_diff_tests(a_views, b_views, cfg, a_all, b_all, notes)) return out def _diff_tests(a_views: Dict[str, TargetView], b_views: Dict[str, TargetView], - cfg: "MigrationConfig", a_all: set, b_all: set) -> List[Discrepancy]: + cfg: "MigrationConfig", a_all: set, b_all: set, + notes: Optional[DiffNotes] = None) -> List[Discrepancy]: """Apply the COMPILE-PARITY stage to TEST targets. Two checks: 1. TU-SET union of all test sources (like libraries): same test .cc files @@ -532,7 +565,8 @@ def _diff_tests(a_views: Dict[str, TargetView], b_views: Dict[str, TargetView], out.append(Discrepancy(Kind.EXTRA_TEST_TU.value, Severity.WARN.value, "", "test source compiled in bazel but not cmake", tu=src)) for src in sorted(set(a_union) & set(b_union)): - out.extend(_diff_variants("", a_union[src], b_union[src], cfg)) + out.extend(_diff_variants("", a_union[src], b_union[src], + cfg, notes)) # 2. test-binary existence by count if len(a_tests) != len(b_tests): @@ -574,10 +608,22 @@ def by_reason(model: CanonicalModel, renames: Dict[str, str]) -> dict: "bazel": by_reason(b, _bazel_renames(a, b, cfg))} +def _by_count(c: Counter) -> dict: + """Counter -> dict ordered by count (desc), then value: stable JSON.""" + return {k: n for k, n in sorted(c.items(), key=lambda kv: (-kv[1], kv[0]))} + + +BAZEL_ONLY_NOTE = ("Flag comparison is one-directional: every CMake flag must be " + "on the Bazel side, but these flags only the Bazel side has " + "are never a discrepancy. Review them; converged does not " + "cover them.") + + def summarize(discs: List[Discrepancy], a: Optional[CanonicalModel] = None, b: Optional[CanonicalModel] = None, - cfg: Optional["MigrationConfig"] = None) -> dict: + cfg: Optional["MigrationConfig"] = None, + notes: Optional[DiffNotes] = None) -> dict: errors = [d for d in discs if d.severity == Severity.ERROR.value] result = { "total": len(discs), @@ -588,6 +634,12 @@ def summarize(discs: List[Discrepancy], } if a is not None and b is not None: result["excluded"] = excluded_summary(a, b, cfg or MigrationConfig()) + if notes is not None: + result["bazel_only_flags"] = { + "note": BAZEL_ONLY_NOTE, + "compile": _by_count(notes.bazel_only_flags), # flag -> TU pairs + "link": _by_count(notes.bazel_only_link_flags), # flag -> executables + } return result @@ -602,4 +654,6 @@ def summarize(discs: List[Discrepancy], a = load_model(sys.argv[1]) b = load_model(sys.argv[2]) cfg = config_mod.load(sys.argv[3]) if len(sys.argv) > 3 else MigrationConfig() - print(json.dumps(summarize(diff_models(a, b, cfg), a, b, cfg), indent=2)) + notes = DiffNotes() + discs = diff_models(a, b, cfg, notes) + print(json.dumps(summarize(discs, a, b, cfg, notes), indent=2)) diff --git a/scripts/triage.py b/scripts/triage.py index 78729fd..4c76842 100644 --- a/scripts/triage.py +++ b/scripts/triage.py @@ -82,6 +82,7 @@ def triage(diff: dict, only_kind: Optional[str] = None) -> dict: "warnings": diff.get("warnings"), "kinds": out_kinds, "excluded": diff.get("excluded", {}), + "bazel_only_flags": diff.get("bazel_only_flags"), } @@ -95,6 +96,18 @@ def render(t: dict, top: int = 25) -> str: lines = [] status = "CONVERGED" if t["converged"] else "NOT converged" lines.append(f"{status} errors={t['errors']} warnings={t['warnings']}") + extra = t.get("bazel_only_flags") + if extra: + comp, link = extra.get("compile") or {}, extra.get("link") or {} + lines.append(f"bazel-only flags (never a discrepancy; review): " + f"{len(comp)} compile, {len(link)} link") + for label, hist in (("compile (TU pairs)", comp), ("link (executables)", link)): + if not hist: + continue + items = list(hist.items()) + lines.append(f" {label}: " + ", ".join( + f"{v} x{n}" for v, n in items[:top]) + + (f", ... +{len(items) - top} more" if len(items) > top else "")) for k in t["kinds"]: lines.append(f"\n[{k['severity']}] {k['kind']} x{k['count']}") for label, hist, cap in ( diff --git a/tests/test_engine.py b/tests/test_engine.py index 656f88a..631c627 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -29,7 +29,7 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "scripts")) from config import MigrationConfig -from diff import Severity, diff_models, summarize +from diff import DiffNotes, Severity, diff_models, summarize from model import (Action, BuildSystem, CanonicalModel, Dependency, Target, TargetKind, TargetRole) @@ -516,6 +516,77 @@ def test_multi_variant_verdict_does_not_depend_on_hash_seed(): assert json.loads(outs.pop())["converged"] +def test_bazel_only_flags_are_reported_but_never_block(): + # The comparison is one-directional: flags only Bazel has never produce a + # discrepancy, so a converged diff must still SAY they exist. + cmake_raw = ["-std=c++17", "-Werror"] + bazel_raw = ["-std=c++17", "-Werror", "-Wno-error=parentheses", "-w", + "-fstack-protector"] # the last: tolerated toolchain default + a = _model(tu_from_raw("src/a.cpp", cmake_raw, is_bazel=False), is_bazel=False) + b = _model(tu_from_raw("src/a.cpp", bazel_raw, is_bazel=True), is_bazel=True) + notes = DiffNotes() + discs = diff_models(a, b, notes=notes) + res = summarize(discs, a, b, notes=notes) + assert res["converged"] and discs == [], res + assert res["bazel_only_flags"]["compile"] == {"-Wno-error=parentheses": 1, "-w": 1} + assert res["bazel_only_flags"]["link"] == {} + + +def test_bazel_only_flags_respect_ignore_and_count_per_tu(): + cmake_raw = ["-std=c++17"] + bazel_raw = ["-std=c++17", "-fno-strict-aliasing", "-Wno-unused"] + a, b = _bs(CanonicalModel(), False), _bs(CanonicalModel(), True) + for m, raw, bz in ((a, cmake_raw, False), (b, bazel_raw, True)): + m.add(Target("lib", TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu_from_raw(f"src/{n}.cpp", raw, bz) + for n in ("a", "b", "c")])) + notes = DiffNotes() + summarize(diff_models(a, b, MigrationConfig(ignore_flags={"-Wno-unused"}), notes)) + assert dict(notes.bazel_only_flags) == {"-fno-strict-aliasing": 3} + + +def test_bazel_only_link_flags_are_reported(): + a = _bs(CanonicalModel(), is_bazel=False); a.add(_exe("app", ["-pthread"], False)) + b = _bs(CanonicalModel(), is_bazel=True) + b.add(_exe("app", ["-pthread", "-Wl,--export-dynamic"], True)) + notes = DiffNotes() + res = summarize(diff_models(a, b, notes=notes), notes=notes) + assert res["converged"], res + assert res["bazel_only_flags"]["link"] == {"-Wl,--export-dynamic": 1} + + +def test_bazel_only_flags_are_collected_from_tests_when_opted_in(): + a, b = _bs(CanonicalModel(), False), _bs(CanonicalModel(), True) + a.add(Target("t_test", TargetKind.EXECUTABLE, role=TargetRole.TEST, + actions=[tu_from_raw("test/t.cpp", ["-std=c++17"], False)])) + b.add(Target("t:t_test", TargetKind.EXECUTABLE, role=TargetRole.TEST, + actions=[tu_from_raw("test/t.cpp", ["-std=c++17", "-w"], True)])) + notes = DiffNotes() + discs = diff_models(a, b, MigrationConfig(include_tests=True), notes) + assert summarize(discs)["converged"], discs + assert dict(notes.bazel_only_flags) == {"-w": 1} + + +def test_bazel_only_flags_come_from_the_exact_variant_when_one_exists(): + # Two Bazel variants of x.cc, one exact and one adding -w; the notes must + # reflect the chosen (exact) pair, not the noisy one. + a, b = _bs(CanonicalModel(), False), _bs(CanonicalModel(), True) + a.add(Target("l", TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("x.cc", ["-std=c++17"], False)])) + for name, raw in (("a:noisy", ["-std=c++17", "-w", "-Wno-error=foo"]), + ("b:exact", ["-std=c++17"])): + b.add(Target(name, TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("x.cc", raw, True)])) + notes = DiffNotes() + assert diff_models(a, b, notes=notes) == [] + assert dict(notes.bazel_only_flags) == {} + + +def test_summary_without_notes_has_no_bazel_only_field(): + a = _model(tu_from_raw("src/a.cpp", ["-O2"], is_bazel=False), is_bazel=False) + assert "bazel_only_flags" not in summarize(diff_models(a, a)) + + def test_ignore_flags_and_defines_suppress_diffs(): # A reviewer-approved flag/define difference is suppressed via config and # the diff converges; without the config it's an error. diff --git a/tests/test_triage.py b/tests/test_triage.py index cb36bcb..1f41aef 100644 --- a/tests/test_triage.py +++ b/tests/test_triage.py @@ -75,6 +75,22 @@ def test_render_caps_bazel_only_tail(): assert "-std=gnu++17" in text +def test_render_shows_bazel_only_flags_even_when_converged(): + diff = {"converged": True, "errors": 0, "warnings": 0, "discrepancies": [], + "excluded": {}, + "bazel_only_flags": {"note": "...", + "compile": {"-Wno-error=parentheses": 24, "-w": 3}, + "link": {}}} + text = render(triage(diff)) + assert "bazel-only flags (never a discrepancy; review): 2 compile, 0 link" in text + assert "compile (TU pairs): -Wno-error=parentheses x24" in text + + +def test_render_without_bazel_only_flags_is_unchanged(): + # diff.json written before the field existed renders as before + assert "bazel-only" not in render(triage(DIFF)) + + if __name__ == "__main__": import traceback fns = [v for k, v in sorted(globals().items()) if k.startswith("test_")] From 6dca07c0019297c8ce2681fc7d61814b5c79887f Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:25:32 -0400 Subject: [PATCH 05/17] Report each side's optimization and debug level in the diff summary canonicalize.py drops -O* and -g* from both sides before comparing. That is the right rule for a per-TU diff, since the level is a build-wide setting, but nothing else looked at it, so the tool reported parity for builds compiled at a different optimization level than the reference, and SKILL.md told the agent the flags were "never your concern". Measured in any2bazel-lab across its first 35 judged migrations, with and without the skill: seven are of pono, whose reference compiles all 94 TUs at -O3, and six of those seven compile at -O2, each reported as 0 errors. lensfun's reference sets -g on all 40 TUs, and every migration lost it. On MySQL Server one migration flattened the reference's 201 -O3 TUs to -O2. summarize() now adds "levels": per side and per language (C, CXX, OBJC, OBJCXX, by source extension), histograms of the optimization level, the debug level and the debug format each compared compile action carries. The last flag of each kind wins, and "(none)" means absent. Level and format are kept apart because they vary independently: `-g3 -gdwarf-4` and `-g1 -gdwarf-4` share a format, and only -g3 emits macro information. A -gdwarf-N with no level flag counts as -g, since it enables debug info. "differs" is set when, for a language both sides compile, any of the three differs. It reads the raw argv the models already hold, over exactly the targets the diff compares (participating roles, minus exclude_targets on the name the diff compares under, minus Bazel header-processing actions), so no new extraction is needed. It never affects errors or `converged`. triage.py prints the levels, and SKILL.md says where to look instead of calling the flags no concern. In the lab's later runs with this series, pono kept -O3 (8 of 9 earlier pono migrations were at -O2), lensfun and openttd kept -g (no earlier lensfun migration had), and xbmc kept its reference's -O3. Lab-Proposal: P16 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 21 +++++----- scripts/diff.py | 81 +++++++++++++++++++++++++++++++++++++- scripts/triage.py | 14 +++++++ tests/test_engine.py | 92 ++++++++++++++++++++++++++++++++++++++++++++ tests/test_triage.py | 14 +++++++ 5 files changed, 212 insertions(+), 10 deletions(-) diff --git a/SKILL.md b/SKILL.md index 0fe8a5b..7c234b8 100644 --- a/SKILL.md +++ b/SKILL.md @@ -77,11 +77,13 @@ Bazel aquery jsonproto ──extract_bazel.py───────┘ ### Canonicalization `canonicalize.py` strips noise before comparing flags. Universal mechanics -(driver/wrapper paths, `-c`/`-o`, sysroot, reproducibility defines, `-O*`/`-g*`) -are dropped in code — never your concern. Judgment calls (warning-set or cosmetic -differences) go in `any2bazel.json`'s `ignore`. Correctness flags (`-std=*`, -`-fno-exceptions`, `-fno-rtti`, …) must never be ignored — they surface as hard -errors by design. +(driver/wrapper paths, `-c`/`-o`, sysroot, reproducibility defines) are dropped +in code — never your concern. `-O*`/`-g*` are dropped from the per-TU +comparison too, but the level still matters: `levels` in `diff.json` reports +each side's, and `differs: true` there is real. Judgment calls (warning-set or +cosmetic differences) go in `any2bazel.json`'s `ignore`. Correctness flags +(`-std=*`, `-fno-exceptions`, `-fno-rtti`, …) must never be ignored — they +surface as hard errors by design. ## The migration config: `any2bazel.json` @@ -289,10 +291,11 @@ python3 scripts/diff.py model..json model.bazel.json \ ``` `diff.json` has `converged` (⇔ zero `error` discrepancies), a `discrepancies` worklist (each with `kind`, `severity`, `target`, `tu`, `cmake_only`, -`bazel_only`), an `excluded` map of non-participating targets by role, and -`bazel_only_flags`: flags only the Bazel side has. Those are never a -discrepancy (the comparison is one-directional), so a `-w` or `-Wno-error=` -you add converges silently; review the list before reporting. +`bazel_only`), an `excluded` map of non-participating targets by role, +`levels` (optimization/debug level per side), and `bazel_only_flags`: flags +only the Bazel side has. Those are never a discrepancy (the comparison is +one-directional), so a `-w` or `-Wno-error=` you add converges silently; +review the list before reporting. `cmake_only` is a historical name: for every frontend it means "present in the reference, absent in Bazel". Synthetic target names ``, `` (and `` when diff --git a/scripts/diff.py b/scripts/diff.py index 78eacae..5e81831 100644 --- a/scripts/diff.py +++ b/scripts/diff.py @@ -32,6 +32,7 @@ from __future__ import annotations import json +import re from collections import Counter from dataclasses import asdict, dataclass, field from enum import Enum @@ -40,7 +41,8 @@ from canonicalize import BAZEL_TOLERATED_FLAG_PREFIXES from config import MigrationConfig from model import (CanonicalModel, TargetKind, TargetRole, TranslationUnit) -from reconstruct import TargetView, reconstruct +from reconstruct import (TargetView, _is_header_processing, + _source_from_compile_args, reconstruct) def _display_bazel_only(flags) -> list: @@ -608,6 +610,82 @@ def by_reason(model: CanonicalModel, renames: Dict[str, str]) -> dict: "bazel": by_reason(b, _bazel_renames(a, b, cfg))} +# Optimization / debug level flags. canonicalize.py drops -O*/-g* from the +# per-TU comparison (build type is a build-wide setting, and diffing it per TU +# would be noise), so levels_summary reads them from the RAW argv instead. +_OPT_LEVEL = re.compile(r"^-O([0-3sgz]|fast)?$") +# Debug LEVEL (how much is emitted: -g1 has no locals, -g3 adds macros) and +# debug FORMAT (-gdwarf-N) are separate: `-g3 -gdwarf-4` and `-g1 -gdwarf-4` +# share a format and differ in level. +_DEBUG_LEVEL = re.compile(r"^-g([0-3]|gdb[0-3]?|line-tables-only)?$") +_DEBUG_FORMAT = re.compile(r"^-gdwarf(-[0-9]+)?$") +_COMPILE_LEVEL_MNEMONICS = {"CppCompile", "ObjcCompile"} +_LANG_BY_EXT = {".c": "C", ".m": "OBJC", ".mm": "OBJCXX"} + + +def _compared_compile_args(model: CanonicalModel, cfg: "MigrationConfig", + renames: Optional[Dict[str, str]] = None): + """Yield (language, raw argv) for each compile action the diff compares: + participating roles, minus config-excluded targets (judged on the name + the diff compares under, see _bazel_renames), minus Bazel + header-processing actions.""" + renames = renames or {} + roles = set(PARTICIPATING_ROLES) + if cfg.include_tests: + roles.add(TargetRole.TEST) + for name, t in sorted(model.targets.items()): + if t.role not in roles or cfg.target_excluded(renames.get(name, name)): + continue + for act in t.actions: + if act.mnemonic not in _COMPILE_LEVEL_MNEMONICS: + continue + args = act.arguments + src = _source_from_compile_args(args) + if not src or _is_header_processing(args): + continue + ext = src[src.rfind("."):] if "." in src else "" + yield _LANG_BY_EXT.get(ext, "CXX"), args + + +def _compile_levels(model: CanonicalModel, cfg: "MigrationConfig", + renames: Optional[Dict[str, str]] = None) -> dict: + """language -> {"opt", "debug", "debug_format": {flag: n}} over the + compared compile actions. The LAST flag of each kind wins, as it does for + the compiler; "(none)" means no flag. A -gdwarf-N with no level flag + enables debug info at the default level, so it counts as "-g".""" + out: Dict[str, Dict[str, Counter]] = {} + for lang_name, args in _compared_compile_args(model, cfg, renames): + lang = out.setdefault(lang_name, {"opt": Counter(), "debug": Counter(), + "debug_format": Counter()}) + opt = [x for x in args if _OPT_LEVEL.match(x)] + dbg = [x for x in args if _DEBUG_LEVEL.match(x)] + fmt = [x for x in args if _DEBUG_FORMAT.match(x)] + lang["opt"][opt[-1] if opt else "(none)"] += 1 + lang["debug"][dbg[-1] if dbg else ("-g" if fmt else "(none)")] += 1 + lang["debug_format"][fmt[-1] if fmt else "(none)"] += 1 + return {lang: {k: _by_count(c) for k, c in h.items()} + for lang, h in sorted(out.items())} + + +LEVELS_NOTE = ("-O*/-g* are dropped from the per-TU comparison, so a different " + "optimization or debug level never shows up as a discrepancy. " + "This is what each side actually compiles at (TU counts).") + + +def levels_summary(a: CanonicalModel, b: CanonicalModel, + cfg: "MigrationConfig") -> dict: + """Optimization level, debug level and debug format of each side, per + language, with a `differs` flag: true when, for a language both sides + compile, the SET of values of any of the three differs (counts alone may + differ because TUs are missing). Only targets the diff compares count.""" + la = _compile_levels(a, cfg) + lb = _compile_levels(b, cfg, _bazel_renames(a, b, cfg)) + differs = any(set(la[lang][k]) != set(lb[lang][k]) + for lang in set(la) & set(lb) + for k in ("opt", "debug", "debug_format")) + return {"note": LEVELS_NOTE, "cmake": la, "bazel": lb, "differs": differs} + + def _by_count(c: Counter) -> dict: """Counter -> dict ordered by count (desc), then value: stable JSON.""" return {k: n for k, n in sorted(c.items(), key=lambda kv: (-kv[1], kv[0]))} @@ -634,6 +712,7 @@ def summarize(discs: List[Discrepancy], } if a is not None and b is not None: result["excluded"] = excluded_summary(a, b, cfg or MigrationConfig()) + result["levels"] = levels_summary(a, b, cfg or MigrationConfig()) if notes is not None: result["bazel_only_flags"] = { "note": BAZEL_ONLY_NOTE, diff --git a/scripts/triage.py b/scripts/triage.py index 4c76842..20554b3 100644 --- a/scripts/triage.py +++ b/scripts/triage.py @@ -83,6 +83,7 @@ def triage(diff: dict, only_kind: Optional[str] = None) -> dict: "kinds": out_kinds, "excluded": diff.get("excluded", {}), "bazel_only_flags": diff.get("bazel_only_flags"), + "levels": diff.get("levels"), } @@ -96,6 +97,19 @@ def render(t: dict, top: int = 25) -> str: lines = [] status = "CONVERGED" if t["converged"] else "NOT converged" lines.append(f"{status} errors={t['errors']} warnings={t['warnings']}") + lv = t.get("levels") + if lv: + def fmt(h): + return " ".join(f"{v} x{n}" for v, n in h.items()) + tag = "DIFFERS" if lv.get("differs") else "same" + lines.append(f"opt/debug levels ({tag}; not part of the diff):") + for lang in sorted(set(lv.get("cmake") or {}) | set(lv.get("bazel") or {})): + for side in ("cmake", "bazel"): + h = (lv.get(side) or {}).get(lang) + if h: + lines.append(f" {lang:6s} {side:5s} opt: {fmt(h['opt'])}" + f" | debug: {fmt(h['debug'])}" + f" | format: {fmt(h.get('debug_format', {}))}") extra = t.get("bazel_only_flags") if extra: comp, link = extra.get("compile") or {}, extra.get("link") or {} diff --git a/tests/test_engine.py b/tests/test_engine.py index 631c627..2667bd5 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -587,6 +587,98 @@ def test_summary_without_notes_has_no_bazel_only_field(): assert "bazel_only_flags" not in summarize(diff_models(a, a)) +def test_levels_reported_when_diff_is_blind_to_them(): + # CMake Release compiles at -O3; Bazel -c opt at -O2. -O* is dropped from + # the per-TU diff, so the diff converges -- the levels summary must not. + a = _model(tu_from_raw("src/a.cpp", ["-O3", "-DNDEBUG"], is_bazel=False), + is_bazel=False) + b = _model(tu_from_raw("src/a.cpp", ["-O2", "-g0", "-DNDEBUG"], is_bazel=True), + is_bazel=True) + res = summarize(diff_models(a, b), a, b) + assert res["converged"], res + lv = res["levels"] + assert lv["differs"] + assert lv["cmake"] == {"CXX": {"opt": {"-O3": 1}, "debug": {"(none)": 1}, + "debug_format": {"(none)": 1}}} + assert lv["bazel"] == {"CXX": {"opt": {"-O2": 1}, "debug": {"-g0": 1}, + "debug_format": {"(none)": 1}}} + + +def test_levels_last_flag_wins_and_split_by_language(): + a, b = _bs(CanonicalModel(), False), _bs(CanonicalModel(), True) + a.add(Target("lib", TargetKind.STATIC, role=TargetRole.PRODUCTION, actions=[ + tu_from_raw("src/a.c", ["-g", "-O2", "-O0"], False), + tu_from_raw("src/b.cc", ["-g"], False)])) + b.add(Target("lib", TargetKind.STATIC, role=TargetRole.PRODUCTION, actions=[ + tu_from_raw("src/a.c", ["-O0", "-g"], True), + tu_from_raw("src/b.cc", ["-g"], True), + # a Bazel header-processing action is not a TU and is not counted + Action("CppCompile", ("-xc++-header", "-fsyntax-only", "-O2", + "-c", "src/b.h"))])) + lv = summarize(diff_models(a, b), a, b)["levels"] + assert lv["cmake"]["C"] == {"opt": {"-O0": 1}, "debug": {"-g": 1}, "debug_format": {"(none)": 1}} + assert lv["cmake"]["CXX"] == {"opt": {"(none)": 1}, "debug": {"-g": 1}, "debug_format": {"(none)": 1}} + assert lv["bazel"] == lv["cmake"] + assert not lv["differs"] + + +def test_levels_language_on_one_side_only_does_not_set_differs(): + # `differs` compares levels for languages BOTH sides compile; a language + # only one side has is a missing-TU matter, reported but not a level diff. + a = _model(tu_from_raw("src/a.c", ["-O2"], is_bazel=False), is_bazel=False) + b = _model(tu_from_raw("src/a.cpp", ["-O3"], is_bazel=True), is_bazel=True) + lv = summarize(diff_models(a, b), a, b)["levels"] + assert set(lv["cmake"]) == {"C"} and set(lv["bazel"]) == {"CXX"}, lv + assert lv["differs"] is False + + +def test_debug_level_and_format_are_tracked_separately(): + # -g3 emits macro information that -g1 does not; sharing -gdwarf-4 must + # not hide that. A format flag alone enables debug info at level -g. + a = _model(tu_from_raw("src/a.cpp", ["-g3", "-gdwarf-4"], is_bazel=False), + is_bazel=False) + b = _model(tu_from_raw("src/a.cpp", ["-g1", "-gdwarf-4"], is_bazel=True), + is_bazel=True) + lv = summarize(diff_models(a, b), a, b)["levels"] + assert lv["cmake"]["CXX"]["debug"] == {"-g3": 1} + assert lv["bazel"]["CXX"]["debug"] == {"-g1": 1} + assert lv["cmake"]["CXX"]["debug_format"] == {"-gdwarf-4": 1} + assert lv["differs"] + c = _model(tu_from_raw("src/a.cpp", ["-gdwarf-5"], is_bazel=True), is_bazel=True) + lc = summarize(diff_models(a, c), a, c)["levels"]["bazel"]["CXX"] + assert (lc["debug"], lc["debug_format"]) == ({"-g": 1}, {"-gdwarf-5": 1}) + + +def test_levels_skip_a_bazel_target_excluded_under_its_mapped_name(): + # target_map renames pkg:tool to tool, which is excluded: the diff skips + # it, so the levels must not count it either. + def m(name, raw, is_bazel): + model = _bs(CanonicalModel(), is_bazel) + model.add(Target("lib", TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("src/a.cpp", ["-O2"], is_bazel)])) + model.add(Target(name, TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("tool/m.cpp", raw, is_bazel)])) + return model + a, b = m("tool", ["-O2"], False), m("pkg:tool", ["-O0"], True) + cfg = MigrationConfig(target_map={"tool": "pkg:tool"}, exclude_targets={"tool"}) + lv = summarize(diff_models(a, b, cfg), a, b, cfg)["levels"] + assert lv["bazel"]["CXX"]["opt"] == {"-O2": 1}, lv + assert not lv["differs"], lv + + +def test_levels_count_only_compared_targets(): + a = _bs(CanonicalModel(), False) + a.add(Target("lib", TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("src/a.cpp", ["-O2"], False)])) + a.add(Target("vendored", TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("third_party/z.c", ["-O3"], False)])) + a.add(Target("t_test", TargetKind.EXECUTABLE, role=TargetRole.TEST, + actions=[tu_from_raw("test/t.cpp", ["-O0"], False)])) + lv = summarize([], a, a, MigrationConfig(exclude_targets={"vendored"}))["levels"] + assert lv["cmake"] == {"CXX": {"opt": {"-O2": 1}, "debug": {"(none)": 1}, + "debug_format": {"(none)": 1}}} + + def test_ignore_flags_and_defines_suppress_diffs(): # A reviewer-approved flag/define difference is suppressed via config and # the diff converges; without the config it's an error. diff --git a/tests/test_triage.py b/tests/test_triage.py index 1f41aef..10ad42c 100644 --- a/tests/test_triage.py +++ b/tests/test_triage.py @@ -91,6 +91,20 @@ def test_render_without_bazel_only_flags_is_unchanged(): assert "bazel-only" not in render(triage(DIFF)) +def test_render_shows_levels(): + diff = {"converged": True, "errors": 0, "warnings": 0, "discrepancies": [], + "excluded": {}, + "levels": {"note": "...", "differs": True, + "cmake": {"CXX": {"opt": {"-O3": 94}, "debug": {"(none)": 94}, + "debug_format": {"(none)": 94}}}, + "bazel": {"CXX": {"opt": {"-O2": 94}, "debug": {"-g0": 94}, + "debug_format": {"(none)": 94}}}}} + text = render(triage(diff)) + assert "opt/debug levels (DIFFERS; not part of the diff):" in text + assert "opt: -O3 x94" in text and "opt: -O2 x94" in text + assert "format: (none) x94" in text + + if __name__ == "__main__": import traceback fns = [v for k, v in sorted(globals().items()) if k.startswith("test_")] From bbefda3813e7cf180ebbdb5944c0fd84e140e67a Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 15:27:15 -0400 Subject: [PATCH 06/17] Apply -U in flag canonicalization: the last -D/-U wins canonicalize_flags collected every -D into the define set and passed -U through as an ordinary flag, so an undefine never removed anything. A Bazel build that keeps assertions on with `build --copt=-UNDEBUG` after -c opt's -DNDEBUG was reported as defining NDEBUG (a WARN on every TU), and one that turned assertions back on against a reference compiled with -DNDEBUG converged with 0 errors. Later commits in this series report NDEBUG state and recommend exactly that -UNDEBUG, so it has to be read correctly. - canonicalize_flags applies `-U NAME` / `-UNAME` as the compiler does: it removes every NAME define collected so far, and a later -D defines it again. A -D likewise replaces every earlier definition of its name, so each side ends with the last word per macro: `-DF(x)=1 -DF(x)=2` and `-DF(x)=1 -UF -DF(x)=2` both leave F(x)=2 and compare equal. `-UNAME` is also kept as a comparable flag iff the -U is the last word on NAME (a later -D clears it), so whether it survives never depends on what either side's toolchain passed before it. Bazel's -c opt preamble (-D_FORTIFY_SOURCE=1 -DNDEBUG) followed by a project's `-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2` now compares equal to the reference (before this change it produced a discrepancy, in either direction). A last-word -U still compares a -U of a compiler-predefined macro (-U_FORTIFY_SOURCE alone). A define is matched by its macro name, the identifier before any `(` or `=`, so `-UF` cancels a function-like `-DF(x)=x`, as `gcc -E` shows. - A define with no value means value 1 to the compiler, so `-DX` is spelled `X=1` (and `-DF(x)` is `F(x)=1`); `-DX -DX=1` and `-DX=1 -DX` compare equal. Define values in diff.json now show that spelling. - extract_cmake.py synthesizes each compile argv in the order CMake's generators emit it ($DEFINES $INCLUDES $FLAGS, as in CMake 3.28's Makefile and Ninja output) instead of flags first, so a -UNDEBUG in CMAKE_CXX_FLAGS lands after, and cancels, an NDEBUG compile definition, as it does in the real command. It does not cancel Release's -DNDEBUG, which comes later in the same flags fragment; only a compile definition is cancelled. Include order changes too: structured include directories now precede any -I from the flags fragments. That has no verdict impact, since includes are compared for presence, not order. Checked against the preprocessor: a differential test generates fixed-seed random -D/-U sequences over object-like and function-like names (joined and split forms, with and without values, repeats) and requires two sequences to canonicalize to the same define set exactly when `cc -E` expands a probe source identically; it is skipped when no C compiler is installed. A larger run of the same check (3,000 sequences, 4.5 million pairs, three seeds) found no disagreement in either direction. One class is documented instead of fixed: a function-like macro that differs only by parameter names (F(x)=x vs F(y)=y) preprocesses the same but reports a define difference. Trade-off: a reference compiled with `-DNDEBUG -UNDEBUG` against a Bazel side with neither now shows a cmake-only `-UNDEBUG` flag error. It is benign (both sides compile without NDEBUG); copy the flag into copts, or add it to ignore.flags (ignore.defines does not reach it: it is a flag). Tests cover valueless defines, last-word -U in both orders, redefinition with and without an intervening -U, function-like macros in both orders, the predefined-macro case, the fortify override against Bazel's opt preamble in both directions, both NDEBUG directions through the diff, and the CMake argv order. Co-Authored-By: Claude Opus 5.5 --- scripts/canonicalize.py | 53 +++++++++++++++-- scripts/extract_cmake.py | 9 ++- tests/test_engine.py | 120 ++++++++++++++++++++++++++++++++++++++- tests/test_extractors.py | 20 +++++++ 4 files changed, 192 insertions(+), 10 deletions(-) diff --git a/scripts/canonicalize.py b/scripts/canonicalize.py index f2b8c66..a6f5552 100644 --- a/scripts/canonicalize.py +++ b/scripts/canonicalize.py @@ -183,12 +183,44 @@ def _matches_any(flag: str, prefixes: Iterable[str]) -> bool: return any(flag == p or flag.startswith(p) for p in prefixes) +def _macro_name(define: str) -> str: + """The macro a define names: the identifier before any `(` (a + function-like macro, `F(x)=x`) or `=` (a value).""" + ends = [i for i in (define.find("("), define.find("=")) if i >= 0] + return define[:min(ends)] if ends else define + + +def _undefine(defines: List[str], undefined: List[str], name: str) -> None: + """Apply `-U name` as the compiler does: it cancels every `name` define + that came before it. `name` is also recorded in `undefined`, the names + whose LAST word is a -U: those are emitted as `-Uname` flags and still + compared, because the -U may target a compiler-predefined macro (e.g. + -U_FORTIFY_SOURCE). A later -D on `name` clears it (see _redefine), so + whether a -U survives never depends on what came before it.""" + defines[:] = [d for d in defines if _macro_name(d) != name] + if name not in undefined: + undefined.append(name) + + +def _redefine(defines: List[str], undefined: List[str], d: str) -> None: + """Apply `-D d` as the compiler does: it replaces every earlier definition + of the same macro name (so each side ends with the last word per name, + whether or not a -U came between), and a -U on that name is no longer the + last word.""" + key = _macro_name(d) + defines[:] = [x for x in defines if _macro_name(x) != key] + if key in undefined: + undefined.remove(key) + + def _add_define(defines: List[str], d: str) -> None: """Append a define unless it's a hardcoded reproducibility injection - (__DATE__/__TIME__/__TIMESTAMP__), matched by key before '='.""" + (__DATE__/__TIME__/__TIMESTAMP__), matched by key before '='. A define + with no value means value 1 to the compiler (`-DX` is `-DX=1`, `-DF(x)` + is `-DF(x)=1`), so it is spelled that way and the two compare equal.""" key = d.split("=", 1)[0] if key not in _DROP_DEFINE_KEYS: - defines.append(d) + defines.append(d if "=" in d else d + "=1") def canonicalize_flags( @@ -199,7 +231,8 @@ def canonicalize_flags( ) -> Tuple[Tuple[str, ...], Tuple[str, ...], Tuple[str, ...]]: """Return (defines, includes, other_flags) in canonical form. - - defines: sorted, deduped, '-D' stripped to 'KEY=VAL' + - defines: sorted, deduped, '-D' stripped to 'KEY=VAL'; a later -U removes + them (see _undefine) - includes: repo-relative, ORDER PRESERVED, dedup-adjacent only - other_flags: correctness-relevant only (ignorable + bazel-default dropped), sorted for stable comparison @@ -207,6 +240,7 @@ def canonicalize_flags( defines: List[str] = [] includes: List[str] = [] other: List[str] = [] + undefined: List[str] = [] # names whose last word is -U (see _undefine) i = 0 n = len(raw) @@ -227,9 +261,16 @@ def canonicalize_flags( # -Dfoo / -D foo if tok == "-D" and i + 1 < n: - _add_define(defines, raw[i + 1]); i += 2; continue + _redefine(defines, undefined, raw[i + 1]); _add_define(defines, raw[i + 1]) + i += 2; continue if tok.startswith("-D"): - _add_define(defines, tok[2:]); i += 1; continue + _redefine(defines, undefined, tok[2:]); _add_define(defines, tok[2:]) + i += 1; continue + # -Ufoo / -U foo: cancels the defines collected so far (last wins) + if tok == "-U" and i + 1 < n: + _undefine(defines, undefined, raw[i + 1]); i += 2; continue + if tok.startswith("-U"): + _undefine(defines, undefined, tok[2:]); i += 1; continue # force-include flavors: -include FILE / -imacros FILE. The argument is # a FILE path (not a search dir), which each build system may spell @@ -260,6 +301,8 @@ def canonicalize_flags( other.append(tok); i += 1 + # a -U that is the last word on its name stays a comparable flag + other.extend("-U" + name for name in undefined) # defines: order-insensitive canon_defines = tuple(sorted(set(defines))) # includes: order preserved, drop only consecutive dups diff --git a/scripts/extract_cmake.py b/scripts/extract_cmake.py index f070f1f..3ee6fe7 100644 --- a/scripts/extract_cmake.py +++ b/scripts/extract_cmake.py @@ -146,17 +146,20 @@ def _parse_target(tobj: dict, repo_root: str) -> Target: # 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 - # canonical compiler view: [, -D..., -I/-isystem ..., -c src]. + # canonical compiler view in the order CMake's generators emit it + # ($DEFINES $INCLUDES $FLAGS): [-D..., -I/-isystem ..., , + # -c src]. The order matters for -U: a -UNDEBUG in CMAKE_CXX_FLAGS comes + # after, and so cancels, a -DNDEBUG compile definition. actions: List[Action] = [] for cg in tobj.get("compileGroups", []): base: List[str] = [] - for frag in cg.get("compileCommandFragments", []): - base.extend(_split_fragment(frag.get("fragment", ""))) 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"]) + for frag in cg.get("compileCommandFragments", []): + base.extend(_split_fragment(frag.get("fragment", ""))) for idx in cg.get("sourceIndexes", []): src = sources[idx] argv = tuple(base + ["-c", src]) diff --git a/tests/test_engine.py b/tests/test_engine.py index 2667bd5..a8351a8 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -143,6 +143,122 @@ def test_missing_cmake_flag_is_caught(): assert "NEEDED=1" in dd.cmake_only +def test_undefine_cancels_earlier_defines_last_wins(): + from canonicalize import canonicalize_flags + def canon(raw): + d, _, f = canonicalize_flags(raw, REPO, is_bazel=False) + return d, f + # a -U cancels earlier defines, and stays a flag iff it is the last word + assert canon(["-DNDEBUG", "-UNDEBUG"]) == ((), ("-UNDEBUG",)) + assert canon(["-D", "X=1", "-U", "X", "-DY"]) == (("Y=1",), ("-UX",)) + assert canon(["-U_FORTIFY_SOURCE"]) == ((), ("-U_FORTIFY_SOURCE",)) + # a later -D clears it, whatever preceded the -U + assert canon(["-U_FORTIFY_SOURCE", "-D_FORTIFY_SOURCE=1"]) == \ + (("_FORTIFY_SOURCE=1",), ()) + assert canon(["-D_FORTIFY_SOURCE=1", "-U_FORTIFY_SOURCE", + "-D_FORTIFY_SOURCE=2"]) == (("_FORTIFY_SOURCE=2",), ()) + assert canon(["-UNDEBUG", "-DNDEBUG"]) == (("NDEBUG=1",), ()) + # function-like macros are matched by their name, in both orders + assert canon(["-DF(x)=x", "-UF"]) == ((), ("-UF",)) + assert canon(["-UF", "-DF(x)=x"]) == (("F(x)=x",), ()) + # a later -D replaces earlier definitions of the name, -U or not + assert canon(["-DF(x)=1", "-DF(x)=2"]) == (("F(x)=2",), ()) + assert canon(["-DF(x)=1", "-UF", "-DF(x)=2"]) == (("F(x)=2",), ()) + # a define with no value is value 1, for the compiler and for the diff + assert canon(["-DX=1", "-DX"]) == canon(["-DX", "-DX=1"]) == (("X=1",), ()) + assert canon(["-DF(x)"]) == (("F(x)=1",), ()) + assert canon(["-DX="]) == (("X=",), ()) # defined as empty: not 1 + + +def test_valueless_define_equals_value_one_across_sides(): + a = _model(tu_from_raw("src/a.cpp", ["-DX", "-DX=1"], is_bazel=False), + is_bazel=False) + b = _model(tu_from_raw("src/a.cpp", ["-DX=1", "-DX"], is_bazel=True), + is_bazel=True) + assert diff_models(a, b) == [] + + +def test_define_canonicalization_agrees_with_the_preprocessor(): + # Differential check against the C preprocessor, skipped when no compiler + # is installed: for fixed-seed random -D/-U sequences over object-like and + # function-like names (joined and split forms, with and without values, + # repeats), two sequences must canonicalize to the same define set exactly + # when `cc -E` expands a probe source identically. Known limitation, left + # out of the pool: a function-like macro that differs only by parameter + # names (F(x)=x vs F(y)=y) preprocesses the same but reports a difference. + import random + import shutil + import subprocess + from canonicalize import canonicalize_flags + cc = next((c for c in ("cc", "gcc", "clang") if shutil.which(c)), None) + if cc is None: + return + pool = [] + for n in ("A", "B"): + pool += [(f"-D{n}",), (f"-D{n}=1",), (f"-D{n}=2",), (f"-D{n}=",), + ("-D", n), ("-D", f"{n}=2"), (f"-U{n}",), ("-U", n)] + pool += [("-DF(x)",), ("-DF(x)=x",), ("-DF(x)=1",), ("-D", "F(x)=x+1"), + ("-DF=3",), ("-UF",), ("-U", "F")] + src = "".join(f"#ifdef {n}\n{n} is [{n}]\n#else\n{n} undef\n#endif\n" + for n in ("A", "B")) + src += "#ifdef F\nF is [F(7)] [F]\n#else\nF undef\n#endif\n" + rng = random.Random(7) + seqs = sorted({tuple(t for _ in range(rng.randint(0, 5)) + for t in rng.choice(pool)) for _ in range(300)}) + by_output, by_defines = {}, {} + for s in seqs: + out = subprocess.run([cc, "-E", "-P", "-x", "c", "-", *s], input=src, + capture_output=True, text=True, check=True).stdout + defines = canonicalize_flags(list(s), REPO, is_bazel=False)[0] + by_output.setdefault(" ".join(out.split()), set()).add(defines) + by_defines.setdefault(defines, set()).add(" ".join(out.split())) + split = {o: d for o, d in by_output.items() if len(d) > 1} + merged = {d: o for d, o in by_defines.items() if len(o) > 1} + assert not split, f"same preprocessing, different define sets: {split}" + assert not merged, f"same define set, different preprocessing: {merged}" + + +def test_redefinition_with_and_without_undefine_converges(): + # Both preprocess to F(x) == 2; only the Bazel side has the -U between. + a = _model(tu_from_raw("src/a.cpp", ["-DF(x)=1", "-DF(x)=2"], is_bazel=False), + is_bazel=False) + b = _model(tu_from_raw("src/a.cpp", ["-DF(x)=1", "-UF", "-DF(x)=2"], + is_bazel=True), is_bazel=True) + assert diff_models(a, b) == [] + + +def test_fortify_override_in_copts_converges_both_ways(): + # `-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2` copied into copts: under -c opt + # Bazel's toolchain has already passed -D_FORTIFY_SOURCE=1 -DNDEBUG. Both + # sides compile with exactly _FORTIFY_SOURCE=2, so nothing may differ. + override = ["-U_FORTIFY_SOURCE", "-D_FORTIFY_SOURCE=2"] + preamble = ["-D_FORTIFY_SOURCE=1", "-DNDEBUG"] + for cmake_raw, bazel_raw in ((["-DNDEBUG"] + override, preamble + override), + (preamble + override, ["-DNDEBUG"] + override)): + a = _model(tu_from_raw("src/a.cpp", cmake_raw, is_bazel=False), is_bazel=False) + b = _model(tu_from_raw("src/a.cpp", bazel_raw, is_bazel=True), is_bazel=True) + assert diff_models(a, b) == [], (cmake_raw, bazel_raw) + + +def test_bazel_undefine_after_opt_ndebug_matches_a_reference_without_it(): + # `build --copt=-UNDEBUG` after -c opt's -DNDEBUG: assertions on, like a + # reference that never defines NDEBUG. No define differs. + a = _model(tu_from_raw("src/a.cpp", ["-O2"], is_bazel=False), is_bazel=False) + b = _model(tu_from_raw("src/a.cpp", ["-O2", "-DNDEBUG", "-UNDEBUG"], + is_bazel=True), is_bazel=True) + assert diff_models(a, b) == [] + + +def test_bazel_undefine_is_caught_against_a_reference_with_ndebug(): + # The reverse: the reference turns assertions off, Bazel turns them back on. + a = _model(tu_from_raw("src/a.cpp", ["-DNDEBUG"], is_bazel=False), is_bazel=False) + b = _model(tu_from_raw("src/a.cpp", ["-DNDEBUG", "-UNDEBUG"], is_bazel=True), + is_bazel=True) + discs = diff_models(a, b) + assert [(d.kind, d.severity, d.cmake_only) for d in discs] == \ + [("defines_diff", "error", ["NDEBUG=1"])], discs + + def test_extra_bazel_define_is_warn_not_error(): cmake_raw = ["-DFOO=1"] bazel_raw = ["-DFOO=1", "-DEXTRA=1"] @@ -445,7 +561,7 @@ def test_a_cmake_variant_missing_on_bazel_is_caught(): discs = diff_models(a, b) errs = [d for d in discs if d.severity == Severity.ERROR.value] assert len(errs) == 1 and errs[0].kind == "defines_diff", errs - assert errs[0].cmake_only == ["FULL"], errs + assert errs[0].cmake_only == ["FULL=1"], errs def test_an_extra_bazel_variant_is_tolerated(): @@ -469,7 +585,7 @@ def test_tied_pairings_prefer_the_smaller_difference(): tu_from_raw("gen/b1.cc", [], True)])) discs = diff_models(a, b, MigrationConfig(source_map=(("gen/", "@gen"),))) assert [(d.cmake_only, d.bazel_only) for d in discs] == \ - [(["LIB1"], []), (["LIB2"], [])], discs + [(["LIB1=1"], []), (["LIB2=1"], [])], discs def test_tied_pairings_prefer_the_exact_variant_over_a_noisy_one(): diff --git a/tests/test_extractors.py b/tests/test_extractors.py index fd282b7..7ee7613 100644 --- a/tests/test_extractors.py +++ b/tests/test_extractors.py @@ -139,6 +139,26 @@ def test_full_pipeline_converges(): assert res["errors"] == 0, json.dumps(res, indent=2) +def test_cmake_flags_come_after_definitions_so_undefine_applies(): + # CMake emits $DEFINES $INCLUDES $FLAGS: a -UNDEBUG in CMAKE_CXX_FLAGS + # cancels a NDEBUG compile definition. + import extract_cmake as ec + tobj = dict(TARGET_MYLIB, compileGroups=[dict( + TARGET_MYLIB["compileGroups"][0], + compileCommandFragments=[{"fragment": "-std=c++17 -UNDEBUG"}], + defines=[{"define": "NDEBUG"}, {"define": "FOO=1"}])]) + tu = reconstruct(_model_of(ec._parse_target(tobj, REPO)))["mylib"].tus[0] + assert tu.defines == ("FOO=1",), tu + assert "-UNDEBUG" in tu.flags, tu # the last word on NDEBUG: still compared + + +def _model_of(target): + from model import BuildSystem, CanonicalModel + m = CanonicalModel(build_system=BuildSystem.CMAKE, repo_root=REPO) + m.add(target) + return m + + def test_cmake_extracts_canonical_flags(): with tempfile.TemporaryDirectory() as root: build = _write_cmake_fixture(root) From 264f18428b7c646bcb2673c1fe627d7af7736206 Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:59:21 -0400 Subject: [PATCH 07/17] Report Bazel-only defines and NDEBUG state beside the verdict Defines are compared both ways, but a define only the Bazel side has is a WARN on each TU, and no summary line showed it. The one that matters most is NDEBUG: Bazel's `-c opt` adds -DNDEBUG, so a migration that pins --compilation_mode=opt against a reference that keeps assertions turns them off everywhere and still converges. Measured in any2bazel-lab: four judged migrations compile with NDEBUG against a reference that passes none, and nothing in the differ's verdict or summary said so. Three migrations of CMake's own sources got it from `-c opt`, on every compile action; a GDAL migration passed -DNDEBUG itself, on 1,534 of 1,534 actions, at no optimization level. - bazel_only_flags gains "defines": each define only the Bazel side has, after ignore.defines, with the compared TU pairs it appears on. Its note says these are a WARN, not an error; triage prints the count and the top entries. - A new "ndebug" block sits next to "levels". Per side, it counts the compared compile actions that leave NDEBUG defined. With the diff's notes, "bazel_added" and "bazel_missing" count the compared TU pairs where only Bazel, or only CMake, defines it, and "differs" is true when either count is nonzero; a Bazel-only TU that is never compared cannot flip it. Both read the canonicalized defines, which apply -U in order (the previous commit), so a `build --copt=-UNDEBUG` after -c opt counts as assertions on. The counts ignore ignore.defines on purpose, so an accepted suppression cannot hide assertion state. triage.py prints one NDEBUG line after the levels. - SKILL.md mentions ndebug under "Diff against reference build". The guidance on opt's -DNDEBUG is in the build-type commit. Severity and `converged` are unchanged: a Bazel-only NDEBUG is still the existing WARN. Whether it should be an error, since it changes what the built program does, is a decision for the maintainers. In the lab's later CMake runs with this series, none added NDEBUG against the reference's none (three runs; two earlier runs had added it). Lab-Proposal: P17 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 6 ++- scripts/diff.py | 69 +++++++++++++++++++++++++-- scripts/triage.py | 19 +++++++- tests/test_engine.py | 110 +++++++++++++++++++++++++++++++++++++++++++ tests/test_triage.py | 15 ++++++ 5 files changed, 210 insertions(+), 9 deletions(-) diff --git a/SKILL.md b/SKILL.md index 7c234b8..dcc27ca 100644 --- a/SKILL.md +++ b/SKILL.md @@ -292,8 +292,10 @@ python3 scripts/diff.py model..json model.bazel.json \ `diff.json` has `converged` (⇔ zero `error` discrepancies), a `discrepancies` worklist (each with `kind`, `severity`, `target`, `tu`, `cmake_only`, `bazel_only`), an `excluded` map of non-participating targets by role, -`levels` (optimization/debug level per side), and `bazel_only_flags`: flags -only the Bazel side has. Those are never a discrepancy (the comparison is +`levels` (optimization/debug level per side), `ndebug` (assertion state per +side, and on how many compared TU pairs Bazel adds or lacks `NDEBUG`), and +`bazel_only_flags`: flags and defines only the Bazel side has. Those flags are +never a discrepancy and those defines only a WARN (the comparison is one-directional), so a `-w` or `-Wno-error=` you add converges silently; review the list before reporting. `cmake_only` is a historical name: for every frontend it means "present in the diff --git a/scripts/diff.py b/scripts/diff.py index 5e81831..48d4dff 100644 --- a/scripts/diff.py +++ b/scripts/diff.py @@ -38,9 +38,10 @@ from enum import Enum from typing import Dict, List, Optional -from canonicalize import BAZEL_TOLERATED_FLAG_PREFIXES +from canonicalize import BAZEL_TOLERATED_FLAG_PREFIXES, canonicalize_flags from config import MigrationConfig -from model import (CanonicalModel, TargetKind, TargetRole, TranslationUnit) +from model import (BuildSystem, CanonicalModel, TargetKind, TargetRole, + TranslationUnit) from reconstruct import (TargetView, _is_header_processing, _source_from_compile_args, reconstruct) @@ -105,9 +106,18 @@ class DiffNotes: These counters make them visible: flag -> number of compared TU pairs (compile; one pair per CMake TU variant, see _diff_variants) or aligned executables (link) where Bazel has it and CMake does not, after the - reviewer's ignore lists and the toolchain's tolerated defaults.""" + reviewer's ignore lists and the toolchain's tolerated defaults. Defines + only Bazel has are a WARN per TU, which no summary showed either; they are + counted the same way, per compared TU pair. + + NDEBUG is counted separately, in both directions and regardless of + ignore.defines, because it is assertion state: Bazel's `-c opt` adds it, so + pinning opt against a reference that keeps assertions turns them off.""" bazel_only_flags: Counter = field(default_factory=Counter) bazel_only_link_flags: Counter = field(default_factory=Counter) + bazel_only_defines: Counter = field(default_factory=Counter) + ndebug_bazel_added: int = 0 # compared TU pairs: Bazel defines, CMake not + ndebug_bazel_missing: int = 0 # compared TU pairs: CMake defines, Bazel not def _diff_tu(target: str, a: TranslationUnit, b: TranslationUnit, @@ -336,9 +346,21 @@ def _diff_variants(target: str, a_vs: List[TranslationUnit], a_fl = {f for f in a.flags if not cfg.flag_ignored(f)} b_fl = {f for f in b.flags if not cfg.flag_ignored(f)} notes.bazel_only_flags.update(_display_bazel_only(b_fl - a_fl)) + a_def = {d for d in a.defines if not cfg.define_ignored(d)} + b_def = {d for d in b.defines if not cfg.define_ignored(d)} + notes.bazel_only_defines.update(b_def - a_def) + a_nd, b_nd = _defines_ndebug(a.defines), _defines_ndebug(b.defines) + notes.ndebug_bazel_added += b_nd and not a_nd + notes.ndebug_bazel_missing += a_nd and not b_nd return out +def _defines_ndebug(defines) -> bool: + """NDEBUG (any value) among canonicalized defines, which already apply -U + in order (the last -D/-U wins; see canonicalize._undefine).""" + return any(d.split("=", 1)[0] == "NDEBUG" for d in defines) + + # Java source roots: a .java path is keyed by its PACKAGE-rooted form (the path # below the source root), which is invariant across build systems that disagree # on the repo prefix / source-root layout (Bazel 'guava/src/...' vs Maven @@ -686,6 +708,41 @@ def levels_summary(a: CanonicalModel, b: CanonicalModel, return {"note": LEVELS_NOTE, "cmake": la, "bazel": lb, "differs": differs} +NDEBUG_NOTE = ("Assertion state. NDEBUG is part of what a build type means, and " + "Bazel's -c opt adds it: pinning opt against a reference without " + "NDEBUG turns assertions off. An extra Bazel define is only a WARN, " + "so this does not block convergence. Counts ignore ignore.defines.") + + +def ndebug_summary(a: CanonicalModel, b: CanonicalModel, cfg: "MigrationConfig", + notes: Optional[DiffNotes] = None) -> dict: + """NDEBUG on each side over the compared compile actions (their + canonicalized defines, so -U applies in order), plus, when `notes` is + given, the compared TU pairs where only Bazel defines it (`bazel_added`: + assertions off that the reference keeps) or only CMake does + (`bazel_missing`: assertions on that the reference turns off).""" + def count(model, renames=None): + c = Counter() + is_bazel = model.build_system == BuildSystem.BAZEL + for _, args in _compared_compile_args(model, cfg, renames): + defines = canonicalize_flags(list(args), model.repo_root, + is_bazel=is_bazel)[0] + c["NDEBUG" if _defines_ndebug(defines) else "(none)"] += 1 + return _by_count(c) + ca, cb = count(a), count(b, _bazel_renames(a, b, cfg)) + out = {"note": NDEBUG_NOTE, "cmake": ca, "bazel": cb} + if notes is not None: + # The compared pairs decide: a Bazel-only TU that is never compared + # must not flip the verdict while both pair counts are zero. + out["bazel_added"] = notes.ndebug_bazel_added + out["bazel_missing"] = notes.ndebug_bazel_missing + differs = bool(notes.ndebug_bazel_added or notes.ndebug_bazel_missing) + else: + differs = bool(ca) and bool(cb) and set(ca) != set(cb) + out["differs"] = differs + return out + + def _by_count(c: Counter) -> dict: """Counter -> dict ordered by count (desc), then value: stable JSON.""" return {k: n for k, n in sorted(c.items(), key=lambda kv: (-kv[1], kv[0]))} @@ -693,8 +750,8 @@ def _by_count(c: Counter) -> dict: BAZEL_ONLY_NOTE = ("Flag comparison is one-directional: every CMake flag must be " "on the Bazel side, but these flags only the Bazel side has " - "are never a discrepancy. Review them; converged does not " - "cover them.") + "are never a discrepancy, and these defines only it has are " + "only a WARN. Review them; converged does not cover them.") def summarize(discs: List[Discrepancy], @@ -713,11 +770,13 @@ def summarize(discs: List[Discrepancy], if a is not None and b is not None: result["excluded"] = excluded_summary(a, b, cfg or MigrationConfig()) result["levels"] = levels_summary(a, b, cfg or MigrationConfig()) + result["ndebug"] = ndebug_summary(a, b, cfg or MigrationConfig(), notes) if notes is not None: result["bazel_only_flags"] = { "note": BAZEL_ONLY_NOTE, "compile": _by_count(notes.bazel_only_flags), # flag -> TU pairs "link": _by_count(notes.bazel_only_link_flags), # flag -> executables + "defines": _by_count(notes.bazel_only_defines), # define -> TU pairs } return result diff --git a/scripts/triage.py b/scripts/triage.py index 20554b3..b479ec8 100644 --- a/scripts/triage.py +++ b/scripts/triage.py @@ -84,6 +84,7 @@ def triage(diff: dict, only_kind: Optional[str] = None) -> dict: "excluded": diff.get("excluded", {}), "bazel_only_flags": diff.get("bazel_only_flags"), "levels": diff.get("levels"), + "ndebug": diff.get("ndebug"), } @@ -110,12 +111,26 @@ def fmt(h): lines.append(f" {lang:6s} {side:5s} opt: {fmt(h['opt'])}" f" | debug: {fmt(h['debug'])}" f" | format: {fmt(h.get('debug_format', {}))}") + nd = t.get("ndebug") + if nd: + def side(h): + return " ".join(f"{v} x{n}" for v, n in (h or {}).items()) or "-" + tag = "DIFFERS" if nd.get("differs") else "same" + line = (f"NDEBUG / assertions ({tag}): cmake {side(nd.get('cmake'))}" + f" | bazel {side(nd.get('bazel'))}") + if "bazel_added" in nd: + line += (f"; bazel adds it on {nd['bazel_added']} compared TU pairs," + f" lacks it on {nd['bazel_missing']}") + lines.append(line) extra = t.get("bazel_only_flags") if extra: comp, link = extra.get("compile") or {}, extra.get("link") or {} + defs = extra.get("defines") or {} lines.append(f"bazel-only flags (never a discrepancy; review): " - f"{len(comp)} compile, {len(link)} link") - for label, hist in (("compile (TU pairs)", comp), ("link (executables)", link)): + f"{len(comp)} compile, {len(link)} link; " + f"bazel-only defines (WARN only): {len(defs)}") + for label, hist in (("compile (TU pairs)", comp), ("link (executables)", link), + ("defines (TU pairs)", defs)): if not hist: continue items = list(hist.items()) diff --git a/tests/test_engine.py b/tests/test_engine.py index a8351a8..8494005 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -795,6 +795,116 @@ def test_levels_count_only_compared_targets(): "debug_format": {"(none)": 1}}} +def _ndebug_pair(cmake_raw, bazel_raw, cfg=None): + a = _model(tu_from_raw("src/a.cpp", cmake_raw, is_bazel=False), is_bazel=False) + b = _model(tu_from_raw("src/a.cpp", bazel_raw, is_bazel=True), is_bazel=True) + notes = DiffNotes() + discs = diff_models(a, b, cfg, notes) + return summarize(discs, a, b, cfg, notes) + + +def test_ndebug_added_by_bazel_opt_is_reported_though_converged(): + # -c opt adds -DNDEBUG; the reference keeps assertions. An extra Bazel + # define is only a WARN, so the diff converges -- the summary must say it. + res = _ndebug_pair(["-O2"], ["-O2", "-DNDEBUG"]) + assert res["converged"], res + nd = res["ndebug"] + assert nd["cmake"] == {"(none)": 1} and nd["bazel"] == {"NDEBUG": 1}, nd + assert (nd["bazel_added"], nd["bazel_missing"], nd["differs"]) == (1, 0, True) + assert res["bazel_only_flags"]["defines"] == {"NDEBUG=1": 1} + + +def test_ndebug_missing_on_bazel_is_reported_in_the_other_direction(): + res = _ndebug_pair(["-O3", "-DNDEBUG"], ["-O3"]) + assert not res["converged"] # a missing CMake define stays an error + nd = res["ndebug"] + assert (nd["bazel_added"], nd["bazel_missing"], nd["differs"]) == (0, 1, True) + + +def test_ndebug_undefined_again_counts_as_absent(): + # `build --copt=-UNDEBUG` beside -c opt keeps assertions on: last one wins. + res = _ndebug_pair([], ["-DNDEBUG", "-UNDEBUG"]) + nd = res["ndebug"] + assert nd["bazel"] == {"(none)": 1}, nd + assert (nd["bazel_added"], nd["differs"]) == (0, False), nd + assert res["bazel_only_flags"]["defines"] == {}, res # agrees with ndebug + + +def test_ndebug_pair_counts_honour_argv_order(): + # `build --copt=-UNDEBUG` followed by a rule-level -DNDEBUG leaves NDEBUG + # defined. The pair counts must agree with the per-side counts. + res = _ndebug_pair(["-DNDEBUG"], ["-UNDEBUG", "-DNDEBUG"]) + nd = res["ndebug"] + assert nd["cmake"] == {"NDEBUG": 1} and nd["bazel"] == {"NDEBUG": 1}, nd + assert (nd["bazel_added"], nd["bazel_missing"], nd["differs"]) == (0, 0, False), nd + + +def test_ndebug_turned_back_on_is_missing_and_an_error(): + # Reference -DNDEBUG; Bazel -DNDEBUG then -UNDEBUG: assertions on. + res = _ndebug_pair(["-DNDEBUG"], ["-DNDEBUG", "-UNDEBUG"]) + assert not res["converged"], res + nd = res["ndebug"] + assert nd["bazel"] == {"(none)": 1}, nd + assert (nd["bazel_added"], nd["bazel_missing"], nd["differs"]) == (0, 1, True) + + +def test_ndebug_and_defines_collected_through_executables(): + # The executable path pairs TUs per target, not through . + def exe(raw, is_bazel): + m = _bs(CanonicalModel(), is_bazel) + m.add(Target("app", TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("tool/m.cpp", raw, is_bazel)])) + return m + a, b = exe(["-O2"], False), exe(["-O2", "-DNDEBUG", "-DEXTRA"], True) + notes = DiffNotes() + res = summarize(diff_models(a, b, notes=notes), a, b, notes=notes) + assert res["converged"], res + assert res["bazel_only_flags"]["defines"] == {"EXTRA=1": 1, "NDEBUG=1": 1} + assert (res["ndebug"]["bazel_added"], res["ndebug"]["bazel"]) == (1, {"NDEBUG": 1}) + + +def test_ndebug_counts_ignore_the_ignore_list(): + # An accepted suppression may hide the WARN and the bazel-only define, but + # never the assertion-state line. + cfg = MigrationConfig(ignore_defines={"NDEBUG"}) + res = _ndebug_pair([], ["-DNDEBUG", "-DEXTRA=1"], cfg) + assert res["converged"] and res["warnings"] == 1, res # EXTRA=1 only + assert res["bazel_only_flags"]["defines"] == {"EXTRA=1": 1} + assert res["ndebug"]["bazel_added"] == 1 and res["ndebug"]["differs"] + + +def test_ndebug_differs_follows_the_compared_pairs(): + # An extra Bazel-only TU without NDEBUG changes the per-side key sets but + # is never compared; both pair counts are 0, so nothing differs. + a = _model(tu_from_raw("src/a.cpp", ["-DNDEBUG"], is_bazel=False), is_bazel=False) + b = _bs(CanonicalModel(), is_bazel=True) + b.add(Target("lib", TargetKind.STATIC, role=TargetRole.PRODUCTION, actions=[ + tu_from_raw("src/a.cpp", ["-DNDEBUG"], True), + tu_from_raw("src/extra.cpp", [], True)])) + notes = DiffNotes() + nd = summarize(diff_models(a, b, notes=notes), a, b, notes=notes)["ndebug"] + assert nd["bazel"] == {"NDEBUG": 1, "(none)": 1}, nd + assert (nd["bazel_added"], nd["bazel_missing"], nd["differs"]) == (0, 0, False) + + +def test_ndebug_skips_a_bazel_target_excluded_under_its_mapped_name(): + a = _model(tu_from_raw("src/a.cpp", [], is_bazel=False), is_bazel=False) + b = _model(tu_from_raw("src/a.cpp", [], is_bazel=True), is_bazel=True) + a.add(Target("tool", TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("tool/m.cpp", [], False)])) + b.add(Target("pkg:tool", TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("tool/m.cpp", ["-DNDEBUG"], True)])) + cfg = MigrationConfig(target_map={"tool": "pkg:tool"}, exclude_targets={"tool"}) + nd = summarize(diff_models(a, b, cfg), a, b, cfg)["ndebug"] + assert nd["bazel"] == {"(none)": 1}, nd + + +def test_ndebug_without_notes_reports_sides_only(): + a = _model(tu_from_raw("src/a.cpp", ["-DNDEBUG"], is_bazel=False), is_bazel=False) + nd = summarize(diff_models(a, a), a, a)["ndebug"] + assert "bazel_added" not in nd and nd["differs"] is False, nd + + def test_ignore_flags_and_defines_suppress_diffs(): # A reviewer-approved flag/define difference is suppressed via config and # the diff converges; without the config it's an error. diff --git a/tests/test_triage.py b/tests/test_triage.py index 10ad42c..6ae065e 100644 --- a/tests/test_triage.py +++ b/tests/test_triage.py @@ -105,6 +105,21 @@ def test_render_shows_levels(): assert "format: (none) x94" in text +def test_render_shows_ndebug_and_bazel_only_defines(): + diff = {"converged": True, "errors": 0, "warnings": 1534, "discrepancies": [], + "excluded": {}, + "ndebug": {"note": "...", "cmake": {"(none)": 1534}, + "bazel": {"NDEBUG": 1534}, "bazel_added": 1534, + "bazel_missing": 0, "differs": True}, + "bazel_only_flags": {"note": "...", "compile": {}, "link": {}, + "defines": {"NDEBUG": 1534}}} + text = render(triage(diff)) + assert ("NDEBUG / assertions (DIFFERS): cmake (none) x1534 | bazel NDEBUG x1534;" + " bazel adds it on 1534 compared TU pairs, lacks it on 0") in text, text + assert "bazel-only defines (WARN only): 1" in text + assert "defines (TU pairs): NDEBUG x1534" in text + + if __name__ == "__main__": import traceback fns = [v for k, v in sorted(globals().items()) if k.startswith("test_")] From e965f3767402f02c87268f5e55ca054d275dc3af Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:26:22 -0400 Subject: [PATCH 08/17] Warn in diff.json when configure-time generated files are not compared A configure_file() output never reaches a command line, so a value that differs only inside one (a config.h define, a version string) is invisible to the diff. The skill said so in a note beside the CMake extraction commands, but diff.json, the artifact people act on, said nothing, and `converged: true` read as covering them. Measured in any2bazel-lab: lensfun's generated config.h differed from the reference in 3 of 7 migrations with the skill, each with a converged diff. configured_files does populate (the lab saw it on another project); the empty lists on lensfun, which spells the command CONFIGURE_FILE, come from the uppercase bug fixed earlier in this series. diff_models now appends one WARN discrepancy, kind configured_files_not_compared, target "", when the CMake model records configured files: cmake_only lists them, and the detail gives the count and how many land on a compile include path. It never blocks `converged`. One aggregated entry rather than one per file keeps a large project's worklist readable (MySQL Server has about 200). All project-owned configured files are listed, not only those flagged as compile inputs, because that flag depends on the output sitting under a known include directory and can miss a header reached another way. A new end-to-end test covers the path from a trace through extract_cmake and the model to the diff. In the lab's later runs with this series, agents byte-compared the listed files on lensfun, CMake and QGIS; the QGIS agent wrote eleven Bazel tests that do it, and CMake's three configure headers that had differed from the reference came out identical in all three CMake runs. Lab-Proposal: P2 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 4 +++- docs/REFERENCE-cmake.md | 2 ++ scripts/diff.py | 23 +++++++++++++++++++++++ tests/test_engine.py | 17 +++++++++++++++++ tests/test_extractors.py | 26 ++++++++++++++++++++++++++ 5 files changed, 71 insertions(+), 1 deletion(-) diff --git a/SKILL.md b/SKILL.md index dcc27ca..2b1db10 100644 --- a/SKILL.md +++ b/SKILL.md @@ -314,7 +314,9 @@ are easiest to reason about once the TUs underneath agree. also emits **WARN-only** kinds that don't block `converged` — `extra_tu` / `extra_target` (compiled/built on the Bazel side but not the reference) and `extra_test_tu` — note them but they need no action unless they point to -something you didn't intend to add. +something you didn't intend to add. The WARN `configured_files_not_compared` +does need action: byte-compare each listed file against the one your Bazel +build generates. #### Triage findings {#triage} diff --git a/docs/REFERENCE-cmake.md b/docs/REFERENCE-cmake.md index 49ed665..35c6f80 100644 --- a/docs/REFERENCE-cmake.md +++ b/docs/REFERENCE-cmake.md @@ -40,6 +40,8 @@ python3 "$SKILL_DIR/scripts/extract_cmake.py" model.cmake.js > the content differ are TODO (see > [TODO-configure-time-generation.md](TODO-configure-time-generation.md)). This > is distinct from build-time codegen (genrules), which is also unmodeled. +> `diff.py` lists them in a `configured_files_not_compared` warning (see +> [Diff against reference build](../SKILL.md#diff)). The project flags passed to `cmake` define the platform/options the Bazel aquery must mirror. diff --git a/scripts/diff.py b/scripts/diff.py index 48d4dff..9ccb7bc 100644 --- a/scripts/diff.py +++ b/scripts/diff.py @@ -82,6 +82,8 @@ class Kind(str, Enum): MISSING_TEST_TU = "missing_test_tu" # test source compiled in cmake, not bazel EXTRA_TEST_TU = "extra_test_tu" # test source compiled in bazel, not cmake TEST_BINARY_COUNT = "test_binary_count" # differing number of test executables + # configure-time generated files exist but their CONTENTS are not compared: + CONFIGURED_FILES = "configured_files_not_compared" @dataclass @@ -546,9 +548,30 @@ def diff_models(a: CanonicalModel, b: CanonicalModel, # ---- tests (opt-in) ---------------------------------------------------- if cfg.include_tests: out.extend(_diff_tests(a_views, b_views, cfg, a_all, b_all, notes)) + + # ---- configure-time generated files: recorded, NOT compared ------------- + # configure_file() outputs never reach a command line, so a value that + # differs only inside one (a config.h define, a version string) is + # invisible to everything above. Say so in the worklist itself, where the + # reader of diff.json will see it, rather than only in the docs. + out.extend(_configured_files_warning(a)) return out +def _configured_files_warning(a: CanonicalModel) -> List[Discrepancy]: + files = a.configured_files + if not files: + return [] + names = sorted(files) + n_inputs = sum(1 for c in files.values() if c.is_compile_input) + return [Discrepancy( + Kind.CONFIGURED_FILES.value, Severity.WARN.value, "", + f"{len(names)} configure-time generated file(s), {n_inputs} on a compile " + "include path, are not covered by this comparison: byte-compare each " + "against the file the Bazel build generates", + cmake_only=names)] + + def _diff_tests(a_views: Dict[str, TargetView], b_views: Dict[str, TargetView], cfg: "MigrationConfig", a_all: set, b_all: set, notes: Optional[DiffNotes] = None) -> List[Discrepancy]: diff --git a/tests/test_engine.py b/tests/test_engine.py index 8494005..e6ea33d 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -905,6 +905,23 @@ def test_ndebug_without_notes_reports_sides_only(): assert "bazel_added" not in nd and nd["differs"] is False, nd +def test_configured_files_raise_a_non_blocking_warning(): + from model import ConfiguredFile + a = _model(tu_from_raw("src/a.cpp", ["-DX"], is_bazel=False), is_bazel=False) + b = _model(tu_from_raw("src/a.cpp", ["-DX"], is_bazel=True), is_bazel=True) + assert diff_models(a, b) == [] # nothing configured -> no warning + a.add_configured_file(ConfiguredFile("config.h", "/b/config.h", + is_compile_input=True)) + a.add_configured_file(ConfiguredFile("foo.pc", "/b/foo.pc")) + discs = diff_models(a, b) + res = summarize(discs) + assert res["converged"] and res["warnings"] == 1, res + w = discs[0] + assert w.kind == "configured_files_not_compared" and w.severity == "warn" + assert w.cmake_only == ["config.h", "foo.pc"] + assert w.detail.startswith("2 configure-time generated file(s), 1 on a") + + def test_ignore_flags_and_defines_suppress_diffs(): # A reviewer-approved flag/define difference is suppressed via config and # the diff converges; without the config it's an error. diff --git a/tests/test_extractors.py b/tests/test_extractors.py index 7ee7613..b5ce4e9 100644 --- a/tests/test_extractors.py +++ b/tests/test_extractors.py @@ -159,6 +159,32 @@ def _model_of(target): return m +def test_configured_files_reach_the_diff_end_to_end(): + # The trace -> extract_cmake -> model JSON -> diff path, with the command + # spelled in capitals as large projects write it. + from serialize import dump_model, load_model + with tempfile.TemporaryDirectory() as root: + build = _write_cmake_fixture(root) + trace = os.path.join(root, "trace.jsonl") + with open(trace, "w") as f: + f.write(json.dumps({"version": {"major": 1, "minor": 2}}) + "\n") + f.write(json.dumps({"cmd": "CONFIGURE_FILE", "file": REPO + "/CMakeLists.txt", + "line": 3, "args": [REPO + "/config.h.in", + build + "/config.h"]}) + "\n") + path = os.path.join(root, "model.cmake.json") + dump_model(extract_cmake.extract(build, REPO, trace), path) + a = load_model(path) + assert list(a.configured_files) == ["config.h"], a.configured_files + + aq_path = os.path.join(root, "aquery.json") + with open(aq_path, "w") as f: + json.dump(AQUERY, f) + res = summarize(diff_models(a, extract_bazel.extract(aq_path, REPO))) + assert res["converged"], res + kinds = [d["kind"] for d in res["discrepancies"]] + assert kinds == ["configured_files_not_compared"], kinds + + def test_cmake_extracts_canonical_flags(): with tempfile.TemporaryDirectory() as root: build = _write_cmake_fixture(root) From 3aea0a3cf705a3921a4f6732755a7567839aead1 Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:29:21 -0400 Subject: [PATCH 09/17] State the convergence boundary where convergence is defined SKILL.md said that configure_file outputs are "recorded but not yet diffed" only in a note beside the CMake extraction commands. The paragraph that defines done ("Done when both stages converge") carried no such caveat, so a reader who lands there reads converged: true as covering the whole build. Measured in any2bazel-lab: a lensfun migration converged with its generated config.h differing from the reference (a GLIB_VERSION_MIN_REQUIRED value that appears in no command line), and the same divergence recurred in later runs, each alongside a converged diff. Adds a short "what converged does not cover" list directly under the definition of done, starting with generated file contents and the instruction to byte-compare them. Later commits add the other boundaries to the same list. Lab-Proposal: P1 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/SKILL.md b/SKILL.md index 2b1db10..badf111 100644 --- a/SKILL.md +++ b/SKILL.md @@ -61,6 +61,12 @@ Bazel aquery jsonproto ──extract_bazel.py───────┘ Bazel flags are tolerated (listed in `diff.json` as `bazel_only_flags`). No artifact/symbol diff or test execution yet. + **`converged: true` is parity of command lines, nothing more.** Check + separately what it cannot see: + - **Generated file contents.** A value that reaches only a generated file + (`configure_file` output, genrule output) never appears on a command line. + Byte-compare each generated file against the reference build's. + ### How targets are matched - **Roles:** each target gets a `role` (`production`, `test`, `dashboard`, From d6822a8e6c4e3e10f80e1b5cb6fc524403a7e073 Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:29:40 -0400 Subject: [PATCH 10/17] Say that shared-library link lines are not compared SKILL.md described link consistency as covering "every name-aligned executable/shared-lib". diff.py does not do that: shared libraries are in the library TU union, only executables reach the link-flag comparison, and the external-dependency check is a project-wide union, so a dependency one .so is missing passes if any other target links it. A clean diff says nothing about how a shared library links. Measured in any2bazel-lab: a pono migration converged while libpono.so lacked its MPFR link, and the Bazel-built binary crashed at load. The agent found it only by running the binary. Corrects the two places in SKILL.md and the four comments in scripts/diff.py (the module docstring, LINK_FLAGS_DIFF, _diff_link_flags and _diff_tests) that claimed shared-library link parity, and adds the boundary to the "what converged does not cover" list with the remedy: run a binary that loads each shared library. Comparing shared libraries' link lines is a larger design change (they would need identity alignment like executables) and is left to the maintainers. No later run exercised this change on the project that motivated it. On lensfun, an agent following it loaded the built shared library, found it missing a GLib symbol, and fixed the link. Lab-Proposal: P8 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 9 ++++++--- scripts/diff.py | 21 +++++++++++---------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/SKILL.md b/SKILL.md index badf111..791ed6b 100644 --- a/SKILL.md +++ b/SKILL.md @@ -52,7 +52,7 @@ Bazel aquery jsonproto ──extract_bazel.py───────┘ - **Compile parity** — every TU compiled with equivalent flags/defines/ includes (project-wide TU-set), and the external link-dependency closure matches. - - **Link consistency** — every name-aligned executable/shared-lib links with + - **Link consistency** — every name-aligned executable links with equivalent link flags. The per-binary stage: the *artifacts* agree, not just the TUs. @@ -66,6 +66,9 @@ Bazel aquery jsonproto ──extract_bazel.py───────┘ - **Generated file contents.** A value that reaches only a generated file (`configure_file` output, genrule output) never appears on a command line. Byte-compare each generated file against the reference build's. + - **How shared libraries link.** A shared library is compared by its TUs + only; its link flags and the libraries it links are not checked (the + external-dep check is project-wide). Run a binary that loads each one. ### How targets are matched @@ -313,8 +316,8 @@ comparisons. The diff reports both parity stages at once: **compile-parity** kinds (`missing_tu`, `defines_diff`, `includes_diff`, `flags_diff`, `missing_dep`, `missing_target`) and the **link-consistency** kind (`link_flags_diff`, one per -name-aligned executable/shared library). Fix compile parity first — link flags -are easiest to reason about once the TUs underneath agree. +name-aligned executable). Fix compile parity first — link flags are easiest to +reason about once the TUs underneath agree. `error`-severity kinds (above) block convergence and are what you fix. The diff also emits **WARN-only** kinds that don't block `converged` — `extra_tu` / diff --git a/scripts/diff.py b/scripts/diff.py index 9ccb7bc..62cb04a 100644 --- a/scripts/diff.py +++ b/scripts/diff.py @@ -19,8 +19,9 @@ Parity has two stages, both reported here: COMPILE PARITY -- per-TU flag/define/include equivalence (project-wide TU-set) + external link-dependency closure. - LINK CONSISTENCY -- per name-aligned executable/shared-lib, equivalent link - flags (link_flags_diff). + LINK CONSISTENCY -- per name-aligned executable, equivalent link flags + (link_flags_diff). Shared libraries are compared by + their TUs only; their link lines are not checked. ASYMMETRY: a flag present in CMake but missing in Bazel is a discrepancy (under-specified Bazel target -> likely wrong build). A flag present only in @@ -73,7 +74,7 @@ class Kind(str, Enum): DEFINES_DIFF = "defines_diff" INCLUDES_DIFF = "includes_diff" FLAGS_DIFF = "flags_diff" - LINK_FLAGS_DIFF = "link_flags_diff" # executable/shared-lib link flags + LINK_FLAGS_DIFF = "link_flags_diff" # executable link flags MISSING_DEP = "missing_dep" # Java (CompileGroup) source-set comparison: MISSING_JAVA_SRC = "missing_java_src" # .java compiled in A but not B @@ -187,7 +188,7 @@ def _diff_tu(target: str, a: TranslationUnit, b: TranslationUnit, def _diff_link_flags(name: str, ta: "TargetView", tb: "TargetView", cfg: "MigrationConfig", notes: Optional[DiffNotes] = None) -> List[Discrepancy]: - """Asymmetric link-flag subset for one aligned executable/shared-lib: every + """Asymmetric link-flag subset for one aligned executable: every CMake link flag must be present on the Bazel side; extra Bazel link flags are tolerated (and recorded in `notes`). Reviewer-approved differences (cfg.link_flag_ignored) are dropped from both sides first. Mirrors the @@ -589,12 +590,12 @@ def _diff_tests(a_views: Dict[str, TargetView], b_views: Dict[str, TargetView], sources compile but are never linked into runnable tests. SCOPE: tests get the COMPILE-PARITY stage only. LINK CONSISTENCY (per-binary - link flags and link-dep closure) runs for PRODUCTION executables/shared - libs, not for test binaries: tests align by source-set union, not by binary - identity, so there's no per-binary link command to compare. So with tests - on, "converged" means production artifacts are link-consistent AND all test - SOURCES compile equivalently -- not that each test binary links identically. - Extending link consistency to per-test-binary is a natural future increment. + link flags) runs for PRODUCTION executables, not for test binaries: tests + align by source-set union, not by binary identity, so there's no + per-binary link command to compare. So with tests on, "converged" means + production artifacts are link-consistent AND all test SOURCES compile + equivalently -- not that each test binary links identically. Extending + link consistency to per-test-binary is a natural future increment. """ out: List[Discrepancy] = [] a_tests = {n for n, t in a_views.items() From 306d4f1b1a312289a4e10dbca36acc3884a5b709 Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:27:55 -0400 Subject: [PATCH 11/17] Report compile-only flags on CMake link lines as a warning CMake passes CMAKE__FLAGS to the link line as well as the compile line, so warning flags, -pedantic and -D/-U defines (including the -DNDEBUG of Release builds) appear in the File API's link flags. The driver ignores them when linking, and Bazel's link lines do not carry them, so each one was a link_flags_diff error that every migration had to suppress by hand in ignore.link_flags. Measured in any2bazel-lab: every converged migration of three small projects suppressed the same kinds of flag, each agent rediscovering and justifying them in its own words (-Wall -Wextra -Wformat-security -pedantic on one, -Wno-format-truncation on another, -DNDEBUG on a third). Agents also suppressed front-end flags by hand: -fexceptions on lensfun, -w and -fno-common on CMake's sources, -fvisibility=hidden on GDAL and QGIS. A hand suppression is also where a reviewer can wave a real difference through. _diff_link_flags now sends these missing CMake link flags to a new WARN kind, link_compile_only_flags, which does not block `converged`: - warnings and -Wp, preprocessor pass-throughs (-W..., but not -Wl, or -Wa,, the latter because an LTO link runs the assembler), -w and -pedantic*; - -D and -U; - front-end -f flags that steer only the compiler, listed one by one: -fexceptions, -fno-exceptions, -fcommon, -fno-common, -fvisibility-inlines-hidden and -fvisibility=. Everything else stays a link_flags_diff error. Most -f flags matter at link time, so the list is explicit rather than a prefix, and -fopenmp, -pthread, -static*, -fsanitize* and -flto* are not in it. When an -flto* flag is on either side of a link, warning controls and the front-end flags stay errors, because LTO runs code generation, and diagnostics such as -Werror=lto-type-mismatch, at link time. ignore.link_flags still silences any of them entirely. SKILL.md lists the new kind, and the front-end flags, with the other no-action WARNs. The upstream test that used -fvisibility=hidden as its example of a link error now uses -Wl,--no-undefined. A built-in list is a design choice the maintainers may prefer to own; this one is reported rather than dropped, so nothing disappears silently. Not done: raising an unreferenced _EXPORTS define only when a source reads it, which would need the differ to read source files. In the lab's runs with this change, the first version of the list (without the front-end flags) still left agents suppressing -fexceptions on lensfun and -w and -fno-common on CMake. With the list above, neither project's next run suppressed a link flag by hand. Lab-Proposal: P11 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 10 +++--- docs/REFERENCE-cmake.md | 4 ++- scripts/diff.py | 56 ++++++++++++++++++++++++++--- tests/test_engine.py | 78 +++++++++++++++++++++++++++++++++++++++-- 4 files changed, 136 insertions(+), 12 deletions(-) diff --git a/SKILL.md b/SKILL.md index 791ed6b..77f2ceb 100644 --- a/SKILL.md +++ b/SKILL.md @@ -323,9 +323,11 @@ reason about once the TUs underneath agree. also emits **WARN-only** kinds that don't block `converged` — `extra_tu` / `extra_target` (compiled/built on the Bazel side but not the reference) and `extra_test_tu` — note them but they need no action unless they point to -something you didn't intend to add. The WARN `configured_files_not_compared` -does need action: byte-compare each listed file against the one your Bazel -build generates. +something you didn't intend to add. `link_compile_only_flags` (warnings, +`-w`, `-pedantic`, `-D`/`-U`, and front-end flags such as `-fexceptions`, +`-fno-common` and `-fvisibility=` that CMake repeats on link lines) needs none +either. The WARN `configured_files_not_compared` does need action: byte-compare +each listed file against the one your Bazel build generates. #### Triage findings {#triage} @@ -362,7 +364,7 @@ correctness flags). | `defines_diff` | add each `cmake_only` define to `defines`, or `ignore.defines` it | | `includes_diff` | add the missing reference include root to `includes`; for a dep whose root is spelled differently each side, `ignore.include_map` it (preferred) or `ignore.include_prefixes` it | | `flags_diff` | add each `cmake_only` flag to `copts`, or `ignore.flags` it | -| `link_flags_diff`| add each `cmake_only` flag to the target's `linkopts`, or `ignore.link_flags` it if benign (e.g. a compile flag the reference repeats at link) | +| `link_flags_diff`| add each `cmake_only` flag to the target's `linkopts`, or `ignore.link_flags` it if benign (a compile flag the reference repeats at link that is not already reported as `link_compile_only_flags`) | | `missing_dep` | add the missing external/system dep to the target's `deps`/`linkopts`; if it's just a name spelled differently per build (`Catch2Main` vs `catch2_main`, `OpenSSL::SSL` vs `ssl`), add a `dep_map` entry. External deps are captured from both `-l` flags and archive-file inputs (e.g. `external/catch2+/libcatch2_main.a`) | | `missing_test_tu`| (tests on) add the test source to a `cc_test`, or `exclude_targets` if out of scope | | `test_binary_count` | (tests on, warning) differing number of test executables — investigate which side has the extra/missing binary | diff --git a/docs/REFERENCE-cmake.md b/docs/REFERENCE-cmake.md index 35c6f80..ce6227a 100644 --- a/docs/REFERENCE-cmake.md +++ b/docs/REFERENCE-cmake.md @@ -55,7 +55,9 @@ aquery must mirror. `catch2_main` / `ssl`. - **Link flags**: CMake repeats compile/codegen flags (`-fvisibility=hidden`, `-fno-common`) on the link line, where they're benign, while Bazel doesn't. - This is the common `ignore.link_flags` case. + The differ reports warnings, `-D`/`-U` and the common front-end `-f` flags + as `link_compile_only_flags`; any other benign one is an `ignore.link_flags` + case. ## Tests diff --git a/scripts/diff.py b/scripts/diff.py index 62cb04a..4177805 100644 --- a/scripts/diff.py +++ b/scripts/diff.py @@ -75,6 +75,8 @@ class Kind(str, Enum): INCLUDES_DIFF = "includes_diff" FLAGS_DIFF = "flags_diff" LINK_FLAGS_DIFF = "link_flags_diff" # executable link flags + # compile-only flags CMake repeats on a link line, absent on Bazel's (WARN) + LINK_COMPILE_ONLY_FLAGS = "link_compile_only_flags" MISSING_DEP = "missing_dep" # Java (CompileGroup) source-set comparison: MISSING_JAVA_SRC = "missing_java_src" # .java compiled in A but not B @@ -185,6 +187,35 @@ def _diff_tu(target: str, a: TranslationUnit, b: TranslationUnit, return out +# -f flags that only steer the compiler's front end or code generation for a +# translation unit, and that CMake repeats on link lines when they are in +# CMAKE__FLAGS. Listed one by one: most -f flags DO matter at link time +# (-fopenmp, -fsanitize*, -flto*, -fuse-ld=, -fprofile-*), so -f in general is +# not compile-only. +_COMPILE_ONLY_F_FLAGS = ("-fexceptions", "-fno-exceptions", "-fcommon", + "-fno-common", "-fvisibility-inlines-hidden") +_COMPILE_ONLY_F_PREFIXES = ("-fvisibility=",) + + +def _compile_only_on_link(flag: str, lto: bool = False) -> bool: + """A flag that only means something to the COMPILER, which CMake repeats on + link lines because CMAKE__FLAGS reach both: warnings (-W..., -w) and + -Wp, preprocessor pass-throughs (but not -Wl, or -Wa,), -pedantic*, -D/-U, + and the front-end -f flags listed above. The driver ignores them when + linking. -Wl, is the linker's own, and -Wa, is kept because an LTO link + runs the assembler. With `lto` (an -flto* flag on that link), warning + controls and the -f flags are kept too: code generation, and diagnostics + such as -Werror=lto-type-mismatch, happen at link time then. Flags that do + matter at link time (-fopenmp, -pthread, -static*, -fsanitize*, -flto*, + -f... in general) are not in this set and stay errors.""" + if flag.startswith(("-Wl,", "-Wa,")): + return False + is_f = flag in _COMPILE_ONLY_F_FLAGS or flag.startswith(_COMPILE_ONLY_F_PREFIXES) + if lto and ((flag.startswith("-W") and not flag.startswith("-Wp,")) or flag == "-w" or is_f): + return False + return is_f or flag == "-w" or flag.startswith(("-W", "-D", "-U", "-pedantic")) + + def _diff_link_flags(name: str, ta: "TargetView", tb: "TargetView", cfg: "MigrationConfig", notes: Optional[DiffNotes] = None) -> List[Discrepancy]: @@ -197,16 +228,31 @@ def _diff_link_flags(name: str, ta: "TargetView", tb: "TargetView", b_lf = {f for f in tb.link_flags if not cfg.link_flag_ignored(f)} if notes is not None: notes.bazel_only_link_flags.update(b_lf - a_lf) - if a_lf - b_lf: - return [Discrepancy( + out: List[Discrepancy] = [] + missing = a_lf - b_lf + lto = any(f.startswith("-flto") for f in a_lf | b_lf) + compile_only = {f for f in missing if _compile_only_on_link(f, lto)} + if missing - compile_only: + out.append(Discrepancy( kind=Kind.LINK_FLAGS_DIFF.value, severity=Severity.ERROR.value, target=name, detail="cmake link flags missing on bazel side", - cmake_only=sorted(a_lf - b_lf), + cmake_only=sorted(missing - compile_only), bazel_only=sorted(b_lf - a_lf), - )] - return [] + )) + if compile_only: + # Reported, not blocking: every CMake project that sets warning flags + # or -DNDEBUG in CMAKE__FLAGS has them on each link line. + out.append(Discrepancy( + kind=Kind.LINK_COMPILE_ONLY_FLAGS.value, + severity=Severity.WARN.value, + target=name, + detail="compile-only flags (warnings, -pedantic, -D/-U, front-end " + "-f flags) on the cmake link line; no effect when linking", + cmake_only=sorted(compile_only), + )) + return out def _norm_includes(includes, cfg) -> tuple: diff --git a/tests/test_engine.py b/tests/test_engine.py index e6ea33d..0a70582 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -992,6 +992,78 @@ def test_missing_link_flag_is_caught(): assert "-rdynamic" in lf[0].cmake_only +def test_compile_only_flags_on_cmake_link_line_are_a_warning(): + # CMAKE__FLAGS reach CMake's link line too; warnings, -pedantic and + # -D/-U mean nothing to the linker, so they must not block convergence -- + # but link-relevant flags (-fopenmp, -Wl,...) still must. + link = ["-Wall", "-Wno-format-truncation", "-pedantic", "-DNDEBUG", + "-UFOO", "-Wp,-D_FORTIFY_SOURCE=2", "-fopenmp", "-Wl,--as-needed", + "-Wa,--noexecstack"] + a = _bs(CanonicalModel(), is_bazel=False); a.add(_exe("app", link, False)) + b = _bs(CanonicalModel(), is_bazel=True); b.add(_exe("app", ["-fopenmp"], True)) + discs = diff_models(a, b) + by_kind = {d.kind: d for d in discs} + assert set(by_kind) == {"link_flags_diff", "link_compile_only_flags"}, discs + assert by_kind["link_flags_diff"].severity == "error" + assert by_kind["link_flags_diff"].cmake_only == ["-Wa,--noexecstack", + "-Wl,--as-needed"] + w = by_kind["link_compile_only_flags"] + assert w.severity == "warn" + assert w.cmake_only == ["-DNDEBUG", "-UFOO", "-Wall", "-Wno-format-truncation", + "-Wp,-D_FORTIFY_SOURCE=2", "-pedantic"] + + +def test_front_end_f_flags_and_w_on_cmake_link_line_are_a_warning(): + # Seen hand-suppressed in lab runs: -fexceptions (lensfun, 7 runs), -w and + # -fno-common (CMake), -fvisibility=hidden (GDAL, QGIS). They steer the + # compiler only. Other -f flags stay link errors. + link = ["-fexceptions", "-fno-common", "-w", "-fvisibility=hidden", + "-fvisibility-inlines-hidden", "-fopenmp", "-fsanitize=address", + "-fuse-ld=gold"] + a = _bs(CanonicalModel(), is_bazel=False); a.add(_exe("app", link, False)) + b = _bs(CanonicalModel(), is_bazel=True); b.add(_exe("app", [], True)) + by_kind = {d.kind: d for d in diff_models(a, b)} + assert by_kind["link_flags_diff"].cmake_only == ["-fopenmp", "-fsanitize=address", + "-fuse-ld=gold"] + assert by_kind["link_compile_only_flags"].cmake_only == [ + "-fexceptions", "-fno-common", "-fvisibility-inlines-hidden", + "-fvisibility=hidden", "-w"] + + +def test_front_end_f_flags_stay_link_errors_under_lto(): + a = _bs(CanonicalModel(), is_bazel=False) + a.add(_exe("app", ["-flto", "-fvisibility=hidden", "-w", "-fexceptions"], False)) + b = _bs(CanonicalModel(), is_bazel=True); b.add(_exe("app", ["-flto"], True)) + by_kind = {d.kind: d for d in diff_models(a, b)} + assert by_kind["link_flags_diff"].cmake_only == ["-fexceptions", "-fvisibility=hidden", "-w"] + assert "link_compile_only_flags" not in by_kind + + +def test_warning_controls_stay_link_errors_under_lto(): + # With -flto the compiler's middle and back end run at link time, and so + # do their diagnostics: a missing -Werror=lto-type-mismatch matters there. + a = _bs(CanonicalModel(), is_bazel=False) + a.add(_exe("app", ["-flto", "-Werror=lto-type-mismatch", "-DNDEBUG", + "-Wp,-D_FORTIFY_SOURCE=2", "-pedantic"], False)) + b = _bs(CanonicalModel(), is_bazel=True); b.add(_exe("app", ["-flto"], True)) + by_kind = {d.kind: d for d in diff_models(a, b)} + assert by_kind["link_flags_diff"].cmake_only == ["-Werror=lto-type-mismatch"] + # preprocessor and front-end flags still have no effect at an LTO link + assert by_kind["link_compile_only_flags"].cmake_only == \ + ["-DNDEBUG", "-Wp,-D_FORTIFY_SOURCE=2", "-pedantic"] + + +def test_only_compile_only_link_flags_missing_converges(): + a = _bs(CanonicalModel(), is_bazel=False) + a.add(_exe("app", ["-Wall", "-pedantic", "-fopenmp"], False)) + b = _bs(CanonicalModel(), is_bazel=True); b.add(_exe("app", ["-fopenmp"], True)) + res = summarize(diff_models(a, b)) + assert res["converged"] and res["warnings"] == 1, res + # and ignore.link_flags still silences them entirely + cfg = MigrationConfig(ignore_link_flag_prefixes=("-W", "-pedantic")) + assert diff_models(a, b, cfg) == [] + + def test_extra_bazel_link_flag_tolerated(): # Asymmetric: an extra link flag only on the bazel side must NOT fail. a = _bs(CanonicalModel(), is_bazel=False); a.add(_exe("app", ["-pthread"], False)) @@ -1013,10 +1085,12 @@ def test_link_toolchain_noise_stripped(): def test_ignore_link_flags_suppresses(): - a = _bs(CanonicalModel(), is_bazel=False); a.add(_exe("app", ["-fvisibility=hidden", "-pthread"], False)) + # (-fvisibility=hidden was the example here; it is now reported as a + # compile-only warning, so a link-relevant flag stands in for it.) + a = _bs(CanonicalModel(), is_bazel=False); a.add(_exe("app", ["-Wl,--no-undefined", "-pthread"], False)) b = _bs(CanonicalModel(), is_bazel=True); b.add(_exe("app", ["-pthread"], True)) assert not summarize(diff_models(a, b))["converged"] - cfg = MigrationConfig(ignore_link_flags={"-fvisibility=hidden"}) + cfg = MigrationConfig(ignore_link_flags={"-Wl,--no-undefined"}) assert summarize(diff_models(a, b, cfg))["converged"] From da2e00cb3499a53ce55438738698026e69ae1b88 Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:28:58 -0400 Subject: [PATCH 12/17] Compare link-interface flags from CMake's libraries fragments The CMake File API reports a target's link line in fragments by role. Flags that a dependency carries on its link interface (Python's -Xlinker -export-dynamic, OpenMP's -fopenmp, Threads' -pthread) arrive in the "libraries" fragments, not "flags". extract_cmake.py read "libraries" only for dependency names, so these flags reached neither the dep closure nor the link-flag comparison, and a Bazel binary missing one converged. Observed in any2bazel-lab: on a Blender migration, the lab verified that -Xlinker -export-dynamic sits only in the blender target's libraries fragment. By the migrating agent's own report, the diff converged without it and Python extension modules then failed to resolve CPython symbols at run time, caught only by its functional test; the lab did not reproduce the failure, because that tree was not kept. A "libraries" fragment that is not a library and starts with "-" is now added to the target's link argv, so it is compared like any link flag. "-Xlinker X" pairs are folded into "-Wl,X" first, so the fragment is classified by what it passes to the linker. Excluded, because they position or locate the libraries around them or name the CMake build tree: -L, -framework, rpaths in every spelling (-Wl,-rpath, -Wl,--rpath, -Wl,-R, and -Xlinker -rpath -Xlinker ), the whole-archive, push/pop-state, group, -Bstatic/-Bdynamic and as-needed toggles, and -force_load. canonicalize_link_flags also folds "-Xlinker X" into "-Wl,X" on both sides, before its noise filters, so the pair compares as one flag, and then joins an rpath given as two linker arguments (`-Wl,-rpath` then `-Wl,DIR`) into `-Wl,-rpath,DIR`, which is what the linker receives. So these three spellings of one rpath compare equal: `-Xlinker -rpath -Xlinker DIR` (what CMake's LINKER:-rpath,DIR becomes under Clang), `-Wl,-rpath -Wl,DIR` and `-Wl,-rpath,DIR`. `-Wl,-rpath=DIR`, `-Wl,--rpath,DIR` and `-Wl,-R,DIR` are not unified with them, as before. Bazel spells its own rpaths the first way, as $ORIGIN/, $EXEC_ORIGIN/ or @loader_path/ paths into its _solib_ directories; on the Bazel side exactly those (an anchored match) are toolchain link noise, otherwise they would appear as Bazel-only link flags on every binary. A project's own rpath (-Wl,-rpath,$ORIGIN/../lib, -Wl,-rpath=/opt/lib, -Wl,-rpath-link,...) is kept and compared. One limitation: any non-library "libraries" fragment becomes a link flag the Bazel side must have, including one that carries a CMake build-tree path Bazel cannot reproduce (-Wl,--version-script,/sym.map; this holds when the map file's basename does not start with "lib", since a fragment ending in lib. is still read as the external dependency , as before). A version script decides which symbols are exported, so such a flag is not benign: its exact CMake spelling may go in ignore.link_flags only after confirming the Bazel link passes an equivalent script with the same content, and SKILL.md's link_flags_diff row now says so. A multi-token fragment is classified as a whole, as before, so `-Wl,-Bstatic -lfoo -Wl,-Bdynamic` in one fragment still loses its -lfoo; splitting fragments per token is left out because rpath pairs would then need their own pairing logic. A behavior change: a migration that converged before may now show a link_flags_diff for such a flag, most often -pthread (Threads::Threads with THREADS_PREFER_PTHREAD_FLAG) or -fopenmp. That is a real difference in the link command; add the flag to linkopts or record it in ignore.link_flags. SKILL.md says these flags are compared. No migration run exercised this change; it was checked on the stored Blender models. Lab-Proposal: P25 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 7 +-- scripts/canonicalize.py | 50 ++++++++++++++++++-- scripts/extract_cmake.py | 40 +++++++++++++++- tests/test_extractors.py | 100 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 189 insertions(+), 8 deletions(-) diff --git a/SKILL.md b/SKILL.md index 77f2ceb..5bf689f 100644 --- a/SKILL.md +++ b/SKILL.md @@ -53,8 +53,9 @@ Bazel aquery jsonproto ──extract_bazel.py───────┘ includes (project-wide TU-set), and the external link-dependency closure matches. - **Link consistency** — every name-aligned executable links with - equivalent link flags. The per-binary stage: the *artifacts* agree, not just - the TUs. + equivalent link flags, including those a dependency adds through its link + interface (`-fopenmp`, `-Xlinker -export-dynamic`). The per-binary stage: + the *artifacts* agree, not just the TUs. Done when both stages converge. The comparison is **asymmetric**: every correctness-relevant reference flag must be present on the Bazel side; extra @@ -364,7 +365,7 @@ correctness flags). | `defines_diff` | add each `cmake_only` define to `defines`, or `ignore.defines` it | | `includes_diff` | add the missing reference include root to `includes`; for a dep whose root is spelled differently each side, `ignore.include_map` it (preferred) or `ignore.include_prefixes` it | | `flags_diff` | add each `cmake_only` flag to `copts`, or `ignore.flags` it | -| `link_flags_diff`| add each `cmake_only` flag to the target's `linkopts`, or `ignore.link_flags` it if benign (a compile flag the reference repeats at link that is not already reported as `link_compile_only_flags`) | +| `link_flags_diff`| add each `cmake_only` flag to the target's `linkopts`, or `ignore.link_flags` it if benign (a compile flag the reference repeats at link that is not already reported as `link_compile_only_flags`). A flag that differs only by a reference build-tree path (`-Wl,--version-script,/x.map`) is still a correctness flag: `ignore.link_flags` that exact spelling only after confirming the Bazel link passes an equivalent script with the same content | | `missing_dep` | add the missing external/system dep to the target's `deps`/`linkopts`; if it's just a name spelled differently per build (`Catch2Main` vs `catch2_main`, `OpenSSL::SSL` vs `ssl`), add a `dep_map` entry. External deps are captured from both `-l` flags and archive-file inputs (e.g. `external/catch2+/libcatch2_main.a`) | | `missing_test_tu`| (tests on) add the test source to a `cc_test`, or `exclude_targets` if out of scope | | `test_binary_count` | (tests on, warning) differing number of test executables — investigate which side has the extra/missing binary | diff --git a/scripts/canonicalize.py b/scripts/canonicalize.py index a6f5552..824cbaf 100644 --- a/scripts/canonicalize.py +++ b/scripts/canonicalize.py @@ -32,6 +32,7 @@ import os import posixpath +import re from typing import Iterable, List, Tuple # Flags Bazel's C++ toolchain injects that are PURE NOISE: they never have a @@ -115,6 +116,20 @@ "-Wl,-rpath,__BAZEL", # bazel sandbox rpaths ) + +def _is_bazel_solib_rpath(tok: str) -> bool: + """Bazel's own rpaths, into its `_solib_` directories (spelled + `-Xlinker -rpath -Xlinker $ORIGIN/.../_solib_k8/...`, which fold_xlinker + turns into one `-Wl,-rpath,`). Only those are toolchain noise; a + project's own rpath (-Wl,-rpath,$ORIGIN/../lib, -Wl,-rpath=/opt/lib, + -Wl,-rpath-link,...) is kept and compared, even one whose path happens to + contain `_solib_` elsewhere.""" + return _BAZEL_SOLIB_RPATH.match(tok) is not None + + +_BAZEL_SOLIB_RPATH = re.compile( + r"-Wl,-rpath,(\$ORIGIN|\$EXEC_ORIGIN|@loader_path)(/.*)?/_solib_[^/]*(/|$)") + # Link flags with no correctness meaning on either side (build-type / debug). IGNORABLE_LINK_PREFIXES = ( "-g", @@ -316,6 +331,30 @@ def canonicalize_flags( return canon_defines, tuple(canon_includes), canon_other +def fold_xlinker(tokens: List[str]) -> List[str]: + """`-Xlinker X` -> `-Wl,X`: the same linker argument in one token. Then an + rpath given as two linker arguments (`-Wl,-rpath` followed by `-Wl,DIR`, + whichever spelling produced them) is joined into `-Wl,-rpath,DIR`, which + is what the linker receives, so every spelling of one rpath compares + equal.""" + folded: List[str] = [] + i = 0 + while i < len(tokens): + if tokens[i] == "-Xlinker" and i + 1 < len(tokens): + folded.append("-Wl," + tokens[i + 1]); i += 2 + else: + folded.append(tokens[i]); i += 1 + out: List[str] = [] + i = 0 + while i < len(folded): + if (folded[i] == "-Wl,-rpath" and i + 1 < len(folded) + and folded[i + 1].startswith("-Wl,")): + out.append("-Wl,-rpath," + folded[i + 1][4:]); i += 2 + else: + out.append(folded[i]); i += 1 + return out + + def canonicalize_link_flags(raw: List[str], *, is_bazel: bool) -> Tuple[str, ...]: """Extract correctness-relevant LINK flags from a link command. @@ -332,16 +371,21 @@ def canonicalize_link_flags(raw: List[str], *, is_bazel: bool) -> Tuple[str, ... """ out: List[str] = [] i = 0 - n = len(raw) - if is_bazel and n and _is_link_input(raw[0]): + if is_bazel and raw and _is_link_input(raw[0]): i = 1 # leading linker/wrapper path + # `-Xlinker X` and `-Wl,X` pass the same argument to the linker; fold the + # two-token form first, so it compares as one flag on either side and goes + # through the same noise filters (Bazel's rpaths, below). + raw = list(raw[:i]) + fold_xlinker(list(raw[i:])) + n = len(raw) while i < n: tok = raw[i] if tok in _LINK_PAIR_FLAGS and i + 1 < n: i += 2; continue if _is_link_input(tok): i += 1; continue - if is_bazel and _matches_any(tok, BAZEL_DEFAULT_LINK_PREFIXES): + if is_bazel and (_is_bazel_solib_rpath(tok) + or _matches_any(tok, BAZEL_DEFAULT_LINK_PREFIXES)): i += 1; continue if _matches_any(tok, IGNORABLE_LINK_PREFIXES): i += 1; continue diff --git a/scripts/extract_cmake.py b/scripts/extract_cmake.py index 3ee6fe7..09c7d41 100644 --- a/scripts/extract_cmake.py +++ b/scripts/extract_cmake.py @@ -40,6 +40,7 @@ import sys from typing import Dict, List, Optional +from canonicalize import fold_xlinker from extract_configure import parse_configure_trace from model import (Action, BuildSystem, CanonicalModel, Dependency, Target, TargetKind, TargetRole) @@ -176,8 +177,9 @@ def _attach_deps(target: Target, tobj: dict, id_to_name: Dict[str, str]) -> None dep_name = id_to_name.get(dep["id"]) if dep_name: target.deps.append(Dependency(dep_name, external=False)) - # link fragments: role "libraries" -> external deps (annotation); role - # "flags" -> a CppLink Action's argv (the differ canonicalizes them). + # link fragments: role "libraries" -> external deps (annotation), plus any + # link-interface FLAGS among them; role "flags" -> a CppLink Action's argv + # (the differ canonicalizes them). link = tobj.get("link") or {} flag_tokens: List[str] = [] for frag in link.get("commandFragments", []): @@ -187,6 +189,8 @@ def _attach_deps(target: Target, tobj: dict, id_to_name: Dict[str, str]) -> None ext = _library_identity(fragment) if ext and not any(d.name == ext for d in target.deps): target.deps.append(Dependency(ext, external=True)) + elif ext is None: + flag_tokens.extend(_link_interface_flags(fragment)) elif role == "flags": flag_tokens.extend(_split_fragment(fragment)) if flag_tokens: @@ -194,6 +198,38 @@ def _attach_deps(target: Target, tobj: dict, id_to_name: Dict[str, str]) -> None arguments=tuple(flag_tokens))) +# Flags in a "libraries" fragment that position or locate the libraries around +# them rather than changing how the program links: they belong to the link +# closure (compared as deps), and rpaths name the CMake build tree. Matched +# after `-Xlinker X` pairs are folded to `-Wl,X` (see _link_interface_flags). +_LIBRARY_POSITIONING_PREFIXES = ( + "-L", "-framework", "-Wl,-rpath", "-Wl,--rpath", "-Wl,-R", + "-Wl,--whole-archive", + "-Wl,--no-whole-archive", "-Wl,-whole-archive", "-Wl,-no-whole-archive", + "-Wl,--push-state", "-Wl,--pop-state", "-Wl,--start-group", + "-Wl,--end-group", "-Wl,-(", "-Wl,-)", "-Wl,-Bstatic", "-Wl,-Bdynamic", + "-Wl,--as-needed", "-Wl,--no-as-needed", "-Wl,-force_load", +) + + +def _link_interface_flags(fragment: str) -> List[str]: + """The link FLAGS a non-library "libraries" fragment carries, or []. + + Such a fragment is a flag a dependency carries on its link interface (e.g. + Python's `-Xlinker -export-dynamic`, OpenMP's `-fopenmp`, Threads' + `-pthread`) rather than a library. These change how the program links, so + they are compared with the target's link flags; before, they were dropped + and a missing one could not show up in the diff. `-Xlinker` pairs are + folded first, so `-Xlinker -rpath -Xlinker ` is recognised as the + rpath it is; positioning flags and -l references yield [].""" + tokens = fold_xlinker(_split_fragment(fragment)) + if (not tokens or not tokens[0].startswith("-") + or tokens[0].startswith("-l") + or tokens[0].startswith(_LIBRARY_POSITIONING_PREFIXES)): + return [] + return tokens + + def _library_identity(fragment: str) -> Optional[str]: """Turn a link fragment into an abstract dep name. diff --git a/tests/test_extractors.py b/tests/test_extractors.py index b5ce4e9..1de9275 100644 --- a/tests/test_extractors.py +++ b/tests/test_extractors.py @@ -301,6 +301,106 @@ def test_bazel_extracts_link_flags_from_cpplink(): assert foo is not None and not foo.external, t.deps +def test_cmake_link_interface_flags_become_link_flags(): + # Flags a dependency carries on its LINK INTERFACE arrive in the File API's + # "libraries" fragments, not "flags". They must reach the link-flag + # comparison; library positioning/rpath flags must not. + import extract_cmake as ec + from model import Target, TargetKind + tobj = {"link": {"commandFragments": [ + {"fragment": "-rdynamic", "role": "flags"}, + {"fragment": "-Xlinker -export-dynamic", "role": "libraries"}, + {"fragment": "-fopenmp", "role": "libraries"}, + {"fragment": "-Wl,-rpath,/work/proj/build/lib", "role": "libraries"}, + {"fragment": "-Wl,--rpath,/work/proj/build/lib", "role": "libraries"}, + {"fragment": "-Xlinker -rpath -Xlinker /work/proj/build/lib", + "role": "libraries"}, + {"fragment": "-lz", "role": "libraries"}, + {"fragment": "/usr/lib/libpython3.12.so", "role": "libraries"}, + {"fragment": "-Wl,--whole-archive", "role": "libraries"}, + {"fragment": "lib/libplugins.a", "role": "libraries"}, + {"fragment": "-Wl,--no-whole-archive", "role": "libraries"}, + {"fragment": "-L/opt/lib", "role": "libraryPath"}, + # search paths and frameworks under the "libraries" role are not flags + {"fragment": "-L/opt/other/lib", "role": "libraries"}, + {"fragment": "-framework Cocoa", "role": "libraries"}, + ]}} + t = Target("blender", TargetKind.EXECUTABLE) + ec._attach_deps(t, tobj, {}) + from model import BuildSystem + from reconstruct import reconstruct_target + v = reconstruct_target(t, BuildSystem.CMAKE, REPO) + assert v.link_flags == ("-Wl,-export-dynamic", "-fopenmp", "-rdynamic"), v.link_flags + assert sorted(d.name for d in v.deps) == ["Cocoa", "plugins", "python3", "z"], \ + v.deps + + +def test_bazel_solib_rpaths_are_toolchain_noise(): + # A realistic Bazel CppLink argv: its -Xlinker rpath pairs fold to -Wl, + # tokens, which must not surface as Bazel-only link flags. + from canonicalize import canonicalize_link_flags + argv = ["external/toolchain/cc_wrapper.sh", "-o", "bazel-out/k8-opt/bin/app", + "-Xlinker", "-rpath", "-Xlinker", "$ORIGIN/_solib_k8/_U_S_Sfoo", + "-Xlinker", "-rpath", "-Xlinker", "$EXEC_ORIGIN/_solib_k8/_U_S_Sfoo", + "-Xlinker", "-rpath", "-Xlinker", + "@loader_path/../_solib_darwin_arm64/_U_S_Sfoo", + "-Lbazel-out/k8-opt/bin/_solib_k8/_U_S_Sfoo", "-lfoo", + "bazel-out/k8-opt/bin/_objs/app/main.o", "-Wl,-S", "-pthread", + "-Xlinker", "-export-dynamic"] + assert canonicalize_link_flags(argv, is_bazel=True) == \ + ("-Wl,-export-dynamic", "-pthread") + + +def test_project_rpaths_in_linkopts_are_kept_and_compared(): + # A project's own rpath flags copied into linkopts must survive on the + # Bazel side (only the toolchain's _solib_ rpaths are noise), in any + # spelling, or they become false cmake-only errors. + from canonicalize import canonicalize_link_flags + from config import MigrationConfig + from diff import _diff_link_flags + from reconstruct import TargetView + from model import TargetKind, TargetRole + preamble = ["cc_wrapper.sh", "-o", "bazel-out/k8-opt/bin/app", + "-Xlinker", "-rpath", "-Xlinker", "$ORIGIN/_solib_k8/_U_S_Sfoo"] + + def converges(cmake_flags, bazel_flags): + cmake = canonicalize_link_flags(cmake_flags, is_bazel=False) + bazel = canonicalize_link_flags(preamble + bazel_flags, is_bazel=True) + views = [TargetView("app", TargetKind.EXECUTABLE, TargetRole.PRODUCTION, + link_flags=lf) for lf in (cmake, bazel)] + return _diff_link_flags("app", views[0], views[1], MigrationConfig()) == [] + + project = ["-Wl,-rpath,$ORIGIN/../lib", "-Wl,-rpath=/opt/lib", + "-Wl,-rpath-link,/opt/lib"] + assert canonicalize_link_flags(preamble + project, is_bazel=True) == \ + tuple(sorted(project)) + assert converges(project, project) + # two linker arguments (CMake's LINKER:-rpath,DIR under Clang is exactly + # -Xlinker -rpath -Xlinker DIR), and the -Wl, pair spelling + for two in (["-Xlinker", "-rpath", "-Xlinker", "/opt/lib"], + ["-Wl,-rpath", "-Wl,/opt/lib"], + ["-Xlinker", "-rpath", "-Xlinker", "@executable_path/../Frameworks"]): + assert converges(two, two), two + # across spellings: one rpath, however it is written + assert converges(["-Xlinker", "-rpath", "-Xlinker", "/x"], ["-Wl,-rpath,/x"]) + assert converges(["-Wl,-rpath,/x"], ["-Wl,-rpath", "-Wl,/x"]) + # and a genuinely missing project rpath is still caught + assert not converges(["-Xlinker", "-rpath", "-Xlinker", "/opt/lib"], []) + # only Bazel's own $ORIGIN/.../_solib_/ rpaths are noise, not a + # project path that merely contains _solib_ + assert converges(["-Wl,-rpath,/opt/_solib_/lib"], ["-Wl,-rpath,/opt/_solib_/lib"]) + assert not converges(["-Wl,-rpath,/opt/_solib_/lib"], []) + + +def test_xlinker_pairs_fold_to_wl_on_both_sides(): + from canonicalize import canonicalize_link_flags + assert canonicalize_link_flags(["-Xlinker", "-export-dynamic"], is_bazel=False) \ + == ("-Wl,-export-dynamic",) + assert canonicalize_link_flags(["cc_wrapper.sh", "-o", "out", "-Xlinker", + "-export-dynamic"], is_bazel=True) \ + == ("-Wl,-export-dynamic",) + + def test_bazel_external_archive_is_external_dep(): # An archive linked by PATH from external/ (a bzlmod module, e.g. catch2) # must be captured as an EXTERNAL dep -- previously dropped (only -l deps From 98245b49a9c5048b3fda2cc5dfea4a692415915f Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Fri, 16 Oct 2026 01:32:45 -0400 Subject: [PATCH 13/17] Count link-only targets' external libraries in the link-closure check The differ checks external link dependencies (-lz, -lhdf5, ...) as a project-wide union over PRODUCTION targets. A target that compiles nothing of its own is an AGGREGATE and is left out, and in a Bazel migration nearly every cc_shared_library is one: its sources live in the cc_library targets it wraps, and the external libraries it links appear only on its own link line. So a library linked only by a shared library read as missing although it was there. Found by an agent migrating QGIS in any2bazel-lab: its own diff ended with seven missing_dep errors (crypto, curl, dl, hdf5, netcdf, pthread, sz), which it traced to provider_mdal, a cc_shared_library classified as an aggregate. The lab confirmed the seven are on that target's Bazel link line. Link-only targets now take part in the external-dependency union on both sides, unless exclude_targets drops them. Their translation units are still compared through the libraries they wrap. Run over the stored models of the 42 migrations the lab has judged, each with its agent's own config, this removes exactly those seven errors and changes nothing else. (One earlier QGIS model is unreadable to both versions.) The next QGIS migration, with this change, converged. Lab-Proposal: P26 Co-Authored-By: Claude Opus 5.5 --- scripts/diff.py | 20 ++++++++++++++++++-- tests/test_engine.py | 26 ++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/scripts/diff.py b/scripts/diff.py index 4177805..77bf20a 100644 --- a/scripts/diff.py +++ b/scripts/diff.py @@ -281,6 +281,17 @@ def _participating(views: Dict[str, TargetView], cfg: "MigrationConfig") -> set: if t.role in PARTICIPATING_ROLES and not cfg.target_excluded(n)} +def _link_only(views: Dict[str, TargetView], cfg: "MigrationConfig") -> set: + """Targets that compile nothing of their own but do link: a CMake library + assembled from object libraries, or (nearly always) a Bazel + cc_shared_library or cc_binary that wraps cc_library deps. Their TUs are + compared through the libraries they wrap, but the external libraries THEY + link are only on their own link lines, so the link-closure check must read + them too.""" + return {n for n, t in views.items() + if t.role == TargetRole.AGGREGATE and not cfg.target_excluded(n)} + + def _apply_target_map(views: Dict[str, TargetView], target_map: Dict[str, str]) -> Dict[str, TargetView]: """Rename Bazel-side target VIEWS into the CMake namespace using a declared @@ -583,10 +594,15 @@ def diff_models(a: CanonicalModel, b: CanonicalModel, # static libs, whose .a is repeated on the link line but which are not a # direct structural dependency). Such names are verified via the compile # TU-set of that target, so drop them from the external closure. + # Link-only targets (see _link_only) take part: in a Bazel migration every + # cc_shared_library is one, so without them a library linked only by a + # shared library read as missing although it is on that library's line. _internal_names = set(a_views) | set(b_views) - a_ext = {cfg.dep_map.get(d, d) for d in _external_deps(a_views, a_names) + a_link = a_names | _link_only(a_views, cfg) + b_link = b_names | _link_only(b_views, cfg) + a_ext = {cfg.dep_map.get(d, d) for d in _external_deps(a_views, a_link) if cfg.dep_map.get(d, d) not in _internal_names and d not in _internal_names} - b_ext = {d for d in _external_deps(b_views, b_names) if d not in _internal_names} + b_ext = {d for d in _external_deps(b_views, b_link) if d not in _internal_names} for dep in sorted(a_ext - b_ext): out.append(Discrepancy(Kind.MISSING_DEP.value, Severity.ERROR.value, "", "external link dependency missing on bazel side", diff --git a/tests/test_engine.py b/tests/test_engine.py index 0a70582..78ac76d 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -305,6 +305,32 @@ def test_missing_external_link_dep_is_error(): assert any(d.kind == "missing_dep" and d.severity == "error" for d in discs) +def test_external_dep_linked_only_by_a_bazel_shared_library_is_not_missing(): + # QGIS (any2bazel-lab run 089): provider_mdal is a CMake SHARED library with + # its own sources, linking hdf5; under Bazel it is a cc_shared_library that + # compiles nothing itself (an aggregate) and carries -lhdf5 on its own link + # line. That is the dep, present; it must not read as missing. + a = _bs(CanonicalModel(), is_bazel=False) + a.add(Target("provider_mdal", TargetKind.SHARED, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("src/mdal.cpp", ["-DFOO=1"], is_bazel=False)], + deps=[Dependency("hdf5", external=True)])) + b = _bs(CanonicalModel(), is_bazel=True) + b.add(Target("mdal_lib", TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("src/mdal.cpp", ["-DFOO=1"], is_bazel=True)])) + b.add(Target("provider_mdal.so", TargetKind.SHARED, role=TargetRole.AGGREGATE, + actions=[], deps=[Dependency("hdf5", external=True)])) + assert not [d for d in diff_models(a, b) if d.kind == "missing_dep"] + # ...and a dep that no target links on the Bazel side still is missing + b2 = _bs(CanonicalModel(), is_bazel=True) + b2.add(Target("mdal_lib", TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("src/mdal.cpp", ["-DFOO=1"], is_bazel=True)])) + b2.add(Target("provider_mdal.so", TargetKind.SHARED, role=TargetRole.AGGREGATE, actions=[])) + assert [d.cmake_only for d in diff_models(a, b2) if d.kind == "missing_dep"] == [["hdf5"]] + # ...and a config-excluded link-only target does not supply it + cfg = MigrationConfig(exclude_targets={"provider_mdal.so"}) + assert [d for d in diff_models(a, b, cfg) if d.kind == "missing_dep"] + + def test_external_dep_naming_an_in_project_target_is_not_missing(): # A dep flagged external=True whose name matches an in-project target is # NOT truly external -- the CMake File-API surfaces cyclically-linked static From 48188bc3d7b92facfdcfdf311d7319c03be3c75b Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Fri, 16 Oct 2026 02:11:26 -0400 Subject: [PATCH 14/17] Say what to do when the toolchain refuses to disable linker param files SKILL.md says to always call aquery with --features=-linker_param_file. The toolchain that rules_cc 0.2.22 autodetects on Linux implies that feature unconditionally, so analysis fails. In any2bazel-lab's migration runs, 8 of 10 agents hit the error and retried without the flag, one to three calls each, and none of the aquery outputs then held a param file reference (four runs checked). The text now names the error, says to drop the flag, and gives the check that keeps the instruction's purpose: no @....params entry in any action's arguments. Written after the runs, so no run has used it yet. Lab-Proposal: P29 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/SKILL.md b/SKILL.md index 5bf689f..7cd9996 100644 --- a/SKILL.md +++ b/SKILL.md @@ -275,6 +275,10 @@ instead of being a warning nobody reads. > `--features=-linker_param_file` to disable use of param files. Param files > are never generated during `aquery` calls, so when they're in use they'll > always point to files that are either stale, or do not exist at all. +> Some toolchains refuse this: analysis fails with "unconditionally implies +> feature 'linker_param_file'" (the toolchain rules_cc autodetects on Linux +> does). Then drop that flag, and check that no action's `arguments` in the +> output hold an `@….params` entry: if none does, nothing is hidden. ```bash # bazel_args, as Bazel parses them; it refuses anything that is not an aquery flag From 3d6dc887d36f1b1e072d12789a06893cffc8dca2 Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:30:11 -0400 Subject: [PATCH 15/17] Require parity for the default bazel invocation bazel_args records the extra args aquery runs with to mirror the real build. Nothing said where settings the migration itself introduces should live, so parity could be established for a named --config that a plain `bazel build //...` never applies: the claim then holds for a command no user runs. Measured in any2bazel-lab: one pono migration put the reference's Release settings behind --config=release, recorded it in bazel_args, and converged. A plain `bazel build //...` produced an assertion-enabled binary that crashes on an input where the reference prints an error and exits. The four other runs on the same project put the settings in .bazelrc's build lines and were equivalent. Adds one bullet to the "invoke aquery the way the project is built" list: settings the migration adds go in `.bazelrc` build lines, and parity must hold for the project's documented build command, which is a plain `bazel build //...` unless the project documents flags; only those documented flags go in bazel_args. The bazel_args field description now says the same, and that the example config (a project config plus the top-level -fno-exceptions BoringSSL expects) is that kind of entry. In the lab's later pono and lensfun runs with this change, bazel_args was empty and the settings were in .bazelrc build lines. Lab-Proposal: P10 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/SKILL.md b/SKILL.md index 7cd9996..b8ed773 100644 --- a/SKILL.md +++ b/SKILL.md @@ -125,12 +125,12 @@ whether its frontend reads it: ``` Fields: -- **`bazel_args`** — the extra **flags** aquery must run with to mirror the - real build (see [Extract the Bazel action graph](#extract-bazel-graph)), e.g. - `["--config=release"]` or `["-c", "opt"]`; never a command - (`["aquery", "mnemonic(...)", ...]` is wrong). Recorded so the comparison is - reproducible; check it with Bazel before running aquery (see - [Extract the Bazel action graph](#extract-bazel-graph)). +- **`bazel_args`** — the extra **flags** aquery must run with (see + [Extract the Bazel action graph](#extract-bazel-graph)): only what the + project's documented build passes, as in the example (BoringSSL expects + `-fno-exceptions` at top level). Never a command (`["aquery", ...]`); check + it with Bazel before running aquery. Recorded so the comparison is + reproducible. - **`target_map`** — `reference_name → bazel_name` for intentionally renamed **executables**. Bazel targets are keyed by full label, so the value is usually `:name` (e.g. `"bssl": ":bssl"`). Libraries need no mapping. @@ -271,6 +271,10 @@ instead of being a warning nobody reads. > - Use the **same platform/options** as the reference build in > [Extract the reference model](#extract-reference-model), or the two sides > aren't comparable. +> - For settings **you** add to mirror the reference (e.g. Release's level), +> use `.bazelrc` `build` lines, not a named `--config`. Parity must hold for +> the project's documented build command: a plain `bazel build //...` unless +> the project documents flags, and only those go in `bazel_args`. > - **Always** call aquery with `--features=-compiler_param_file` and > `--features=-linker_param_file` to disable use of param files. Param files > are never generated during `aquery` calls, so when they're in use they'll From 4f25e37ca2f483a30e1c6b78633b984d7e944ce6 Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:30:29 -0400 Subject: [PATCH 16/17] Pin the reference configuration and record its command Nothing in the procedure said which CMake configuration the reference should use, or that choosing one is a decision. An agent can turn options off before extracting the reference and then reach parity with that narrower build; both sides of the comparison move together, so no error count shows it. Measured in any2bazel-lab: a MySQL Server migration configured the reference with Router, NDB, the unit tests and the distribution's package compiler flags all turned off, recorded only in a comment in any2bazel.json. Against the project's default configuration the result showed 4,928 differences; against its own, 13. The reason given for the package flags was defensible; the problem was that the choice was silent. Agents redefined the reference with and without the skill, on small projects as well as large. "Extract the reference model" now says to configure with the project's documented default options, to surface any change as a scope decision, and to record the exact command in any2bazel.json as reference.configure_command (added to the example and the field list as record-only; config.load ignores it, like any unknown key) and never change it after the first diff. The optional test step reconfigures with tests on; that command is recorded separately, as reference.test_configure_command, so it does not count as changing the reference. In the lab's later runs with this change, every migrating agent recorded its configure command. Lab-Proposal: P13 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/SKILL.md b/SKILL.md index b8ed773..75291ef 100644 --- a/SKILL.md +++ b/SKILL.md @@ -108,6 +108,7 @@ whether its frontend reads it: "dep_map": { "Catch2Main": "catch2_main" }, "exclude_targets": ["benchmark", "some_tool"], "include_tests": false, + "reference": { "configure_command": "cmake -S . -B build -G Ninja" }, "ignore": { "defines": ["BORINGSSL_DISPATCH_TEST"], "flags": ["-fvisibility=hidden"], @@ -142,6 +143,11 @@ Fields: third-party/vendored code Bazel pulls as an external module, or tooling out of scope. The **only** lever for `missing_tu`/`missing_target` on whole subtrees. Excluded targets still appear under `excluded.config_excluded`. +- **`reference.configure_command`** — the exact configure command the + reference model came from (see + [Extract the reference model](#extract-reference-model)); + `reference.test_configure_command` holds the test-inclusive one + ([Diff tests](#diff-tests)). Recorded for reviewers; not read by the diff. - **`include_tests`** (default `false`) — opt in to also diffing **test** targets. OFF by default because it requires BOTH models to be extracted with tests enabled and the **same** test scope (symmetric configure + aquery); @@ -239,6 +245,14 @@ python3 scripts/extract_.py <…> model..json Note the platform and options the reference build was configured with; the Bazel side must mirror them. +Configure the reference build with the project's **documented default** +options: turning anything off (a product, the tests, distribution flags) +changes what "migrated" means, so surface it when +[confirming scope](#confirm-scope). Record the exact command as +`any2bazel.json`'s `reference.configure_command` and never change it after the +first diff; the test-inclusive configure for [Diff tests](#diff-tests) is +recorded apart, as `test_configure_command`. + ### Create initial BUILD.bazel files Read `model..json`. For each production target emit a `cc_library` / From 651c209fe2ad68da0ab9d6357599e16e76eac22f Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:30:42 -0400 Subject: [PATCH 17/17] Tell the agent to map the build type to flags, not to a compilation mode The skill never mentioned CMAKE_BUILD_TYPE or --compilation_mode, and its canonicalization text called -O*/-g* "never your concern" (reworded in the levels commit). Agents therefore map the concept, Release to `-c opt`, and lose the number: CMake's Release is -O3 -DNDEBUG, Bazel's opt is -O2; CMake's Debug is -g, Bazel's default fastbuild has neither. Measured in any2bazel-lab: all nine migrations of pono, with and without the skill, wrote `build --compilation_mode=opt` as the counterpart of Release, several saying so in a comment on the line above, and eight of the nine compile at -O2 against a reference at -O3; the ninth added `build --copt=-O3` and matched. lensfun's CMakeLists.txt defaults the build type to Debug when none is given; no lensfun migration set a compilation mode, so every one dropped -g from all 40 TUs. Adds a short "Build type" section to docs/REFERENCE-cmake.md, under a new "Writing the BUILD files" heading that "Create initial BUILD.bazel files" points to: the two mappings that differ, the reminder that a project can set its own default build type, where to read the reference's actual level (diff.json's levels, or the cache), and the two .bazelrc lines that pin it. It also says that opt adds -DNDEBUG and so turns off assertions the reference may keep (diff.json's ndebug shows it), and that `build --copt=-UNDEBUG` undoes only the toolchain's -DNDEBUG, not one a rule passes in its copts, which come later on the command line. In the lab's later runs with this change, pono kept -O3, lensfun kept -g, and CMake's sources, whose reference sets no build type, gained neither -O2 nor NDEBUG in three runs (two earlier runs had added both). Lab-Proposal: P18 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 5 ++++- docs/REFERENCE-cmake.md | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/SKILL.md b/SKILL.md index 75291ef..b9e48d8 100644 --- a/SKILL.md +++ b/SKILL.md @@ -253,7 +253,7 @@ changes what "migrated" means, so surface it when first diff; the test-inclusive configure for [Diff tests](#diff-tests) is recorded apart, as `test_configure_command`. -### Create initial BUILD.bazel files +### Create initial BUILD.bazel files {#create-build-files} Read `model..json`. For each production target emit a `cc_library` / `cc_binary` with `srcs`, `hdrs`, `copts`, `defines`, `includes`, `deps`. Library @@ -267,6 +267,9 @@ rather than recalled). Put `common --check_direct_dependencies=error` in the generated `.bazelrc` so a declared version that MVS overrides fails the build instead of being a warning nobody reads. +The reference guide covers what is specific to translating its build system +(for CMake: the build type). + ### Main migration loop {#main-migration-loop} #### Extract the Bazel action graph {#extract-bazel-graph} diff --git a/docs/REFERENCE-cmake.md b/docs/REFERENCE-cmake.md index ce6227a..db36825 100644 --- a/docs/REFERENCE-cmake.md +++ b/docs/REFERENCE-cmake.md @@ -46,6 +46,23 @@ python3 "$SKILL_DIR/scripts/extract_cmake.py" model.cmake.js The project flags passed to `cmake` define the platform/options the Bazel aquery must mirror. +## Writing the BUILD files + +What is specific to CMake when +[creating the BUILD files](../SKILL.md#create-build-files). + +### Build type + +`--compilation_mode` is not a translation of `CMAKE_BUILD_TYPE`: Release is +`-O3 -DNDEBUG` where Bazel's `opt` is `-O2 -DNDEBUG`, and Debug is `-g` where +Bazel's default `fastbuild` has neither. A project may set its own default +build type in `CMakeLists.txt` even when none is passed. Read the reference's +level (`levels` in `diff.json`, or `CMAKE__FLAGS_` in the cache) +and pin it in `.bazelrc`, e.g. `build -c opt` plus `build --copt=-O3`. `opt` +also turns assertions off where the reference keeps them (`ndebug` in +`diff.json`); `build --copt=-UNDEBUG` undoes the toolchain's `-DNDEBUG`, not +one in a rule's `copts`. + ## CMake-specific comparison notes - **Target names** in `target_map` and `exclude_targets` keys are CMake target