From cdab44c8748f3caa8c971df52b81e8e68768f5e7 Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:27:14 -0400 Subject: [PATCH] Align executables by unambiguous label basename The CMake model keys executables by bare target name (BinRec), the Bazel model by package-qualified label (gems:BinRec). Executables align by exact name, so every executable in a non-root package needed a target_map entry even when nothing was renamed, which buries the entries that record real renames. Measured in any2bazel-lab: two graphicsgems migrations wrote 32 and 26 target_map entries, none of them a rename; every entry bridged a bare CMake name to its package-qualified Bazel label. After target_map is applied, each CMake executable that still has no exact-name counterpart aligns with the Bazel executable whose label basename equals its name, when exactly one unaligned Bazel executable has that basename. Two packages sharing the basename are left unaligned (missing_target plus extra_target, as before) for target_map to resolve; libraries, external-repo labels, and CMake names that are target_map keys (even when the mapped Bazel target does not exist) never align this way. The pairs are reported in diff.json as "aligned_by_basename" so the implicit matches stay reviewable. The alignment is part of _bazel_renames, the one rename decision the diff and every summary share, so an exclude_targets entry for an auto-aligned executable is honoured the same way by the comparison, the excluded list, the levels and the NDEBUG counts. SKILL.md now reserves target_map for renames and ambiguous basenames. In the lab's later graphicsgems run with this change, target_map was empty (the earlier run had 30 entries). Lab-Proposal: P3 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 12 ++++-- scripts/diff.py | 54 +++++++++++++++++++++--- tests/test_engine.py | 98 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+), 9 deletions(-) diff --git a/SKILL.md b/SKILL.md index 75ae9a4..627fbf8 100644 --- a/SKILL.md +++ b/SKILL.md @@ -80,8 +80,10 @@ Bazel aquery jsonproto ──extract_bazel.py───────┘ path**, so names and grouping don't matter — a rename or object-library fold-in converges with no mapping. Only **external** link deps are checked; internal dep names are noise. -- **Executables** align by **name** (a missing exe is a real gap); use - `target_map` for renames. +- **Executables** align by **name** (a missing exe is a real gap), or with the + one Bazel executable whose label basename matches (`BinRec` ↔ `gems:BinRec`; + see `aligned_by_basename` in `diff.json`). Use `target_map` for renames and + for a basename two packages share. ### Canonicalization @@ -131,8 +133,10 @@ Fields: reproducible; `diff.py` refuses an entry that is neither a flag nor a flag's value. - **`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. + **executables**, or when two Bazel packages share the basename. The value is + the label without `//`: `:name` at the root, `pkg:name` elsewhere (e.g. + `"bssl": "tool:bssl"`). Same-named executables align without an entry; + libraries need no mapping. - **`dep_map`** — `reference_dep_name → bazel_dep_name` for an **external link dep** spelled differently per build (e.g. `Catch2Main` vs `catch2_main`). An explicit, recorded rename — not a fuzzy match — so a residual `missing_dep` is diff --git a/scripts/diff.py b/scripts/diff.py index ab98cbd..3d434cc 100644 --- a/scripts/diff.py +++ b/scripts/diff.py @@ -121,6 +121,8 @@ class DiffNotes: 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 + # cmake_name -> bazel_name, executables aligned by unambiguous basename + aligned_by_basename: Dict[str, str] = field(default_factory=dict) def _diff_tu(target: str, a: TranslationUnit, b: TranslationUnit, @@ -256,14 +258,50 @@ def _apply_target_map(views: Dict[str, TargetView], return renamed +def _label_basename(name: str) -> str: + """'gems:BinRec' -> 'BinRec'; ':bssl' -> 'bssl' (Bazel names are labels).""" + return name.rsplit(":", 1)[-1] + + +def _align_by_basename(a_views: dict, b_views: dict, + mapped: frozenset = frozenset()) -> Dict[str, str]: + """cmake_name -> bazel_name for every CMake executable with no exact-name + counterpart whose name is the basename of EXACTLY ONE unaligned Bazel + executable (CMake keys executables by bare name, Bazel by package-qualified + label, so without this every executable needed a target_map entry). Two + Bazel executables sharing the basename are ambiguous and left alone: that + case, and genuine renames, still need target_map. External-repo labels + ('@...') never align, and neither does a CMake name in `mapped` (a + target_map key): its explicit entry decides, even if it names a Bazel + target that does not exist. Values need only a `.kind`: model targets + and reconstructed views both work.""" + by_base: Dict[str, List[str]] = {} + for name, v in b_views.items(): + if (v.kind == TargetKind.EXECUTABLE and name not in a_views + and not name.startswith("@")): + by_base.setdefault(_label_basename(name), []).append(name) + return {name: by_base[name][0] for name, v in sorted(a_views.items()) + if v.kind == TargetKind.EXECUTABLE and name not in b_views + and name not in mapped and len(by_base.get(name, ())) == 1} + + 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} + diff renames: target_map entries first, then the automatic alignment of + executables by unambiguous label basename (see _align_by_basename), which + never overrides a target_map key. 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.""" + renames = {bazel: cmake for cmake, bazel in cfg.target_map.items() + if bazel in b.targets} + renamed = {renames.get(label, label): t for label, t in b.targets.items()} + label_of = {name: label for label, name in renames.items()} + for cmake, name in _align_by_basename(a.targets, renamed, + frozenset(cfg.target_map)).items(): + renames[label_of.get(name, name)] = cmake + return renames _LIBRARY_KINDS = {TargetKind.STATIC, TargetKind.SHARED, TargetKind.OBJECT, @@ -434,6 +472,11 @@ def diff_models(a: CanonicalModel, b: CanonicalModel, 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) + if notes is not None: + explicit = set(cfg.target_map.values()) + notes.aligned_by_basename.update( + {name: label for label, name in renames.items() + if label not in explicit}) a_names = _participating(a_views, cfg) b_names = _participating(b_views, cfg) @@ -802,6 +845,7 @@ def summarize(discs: List[Discrepancy], "link": _by_count(notes.bazel_only_link_flags), # flag -> executables "defines": _by_count(notes.bazel_only_defines), # define -> TU pairs } + result["aligned_by_basename"] = dict(sorted(notes.aligned_by_basename.items())) return result diff --git a/tests/test_engine.py b/tests/test_engine.py index 05eddea..4b83922 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -976,6 +976,104 @@ def test_executables_still_aligned_by_identity(): assert any(d.kind == "missing_target" for d in discs) +def test_executable_aligns_by_unique_label_basename(): + # CMake keys executables by bare name, Bazel by package-qualified label; + # an unambiguous basename needs no target_map entry. + a = _bs(CanonicalModel(), is_bazel=False) + b = _bs(CanonicalModel(), is_bazel=True) + for name in ("BinRec", "Hash3D"): + a.add(Target(name, TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw(f"gems/{name}.c", ["-DX"], False)])) + b.add(Target("gems:BinRec", TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("gems/BinRec.c", ["-DX"], True)])) + b.add(Target(":Hash3D", TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("gems/Hash3D.c", ["-DX"], True)])) + notes = DiffNotes() + discs = diff_models(a, b, notes=notes) + res = summarize(discs, notes=notes) + assert res["converged"] and discs == [], discs + assert res["aligned_by_basename"] == {"BinRec": "gems:BinRec", "Hash3D": ":Hash3D"} + + +def test_ambiguous_basename_is_not_aligned(): + a = _bs(CanonicalModel(), is_bazel=False) + a.add(Target("tool", TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("x/tool.c", [], False)])) + b = _bs(CanonicalModel(), is_bazel=True) + for pkg in ("x", "y"): + b.add(Target(f"{pkg}:tool", TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("x/tool.c", [], True)])) + kinds = sorted(d.kind for d in diff_models(a, b)) + assert kinds == ["extra_target", "extra_target", "missing_target"], kinds + # target_map resolves the ambiguity + discs = diff_models(a, b, MigrationConfig(target_map={"tool": "x:tool"})) + assert [d.kind for d in discs] == ["extra_target"], discs + + +def test_target_map_wins_over_basename_alignment(): + # A target_map entry consumes the Bazel executable, so it is no longer a + # basename candidate for the same-named CMake executable. + a = _bs(CanonicalModel(), is_bazel=False) + for name in ("tool", "tool_v2"): + a.add(Target(name, TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("x/tool.c", [], False)])) + b = _bs(CanonicalModel(), is_bazel=True) + b.add(Target("x:tool", TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("x/tool.c", [], True)])) + notes = DiffNotes() + discs = diff_models(a, b, MigrationConfig(target_map={"tool_v2": "x:tool"}), + notes) + assert [(d.kind, d.target) for d in discs] == [("missing_target", "tool")], discs + assert notes.aligned_by_basename == {} + + +def test_a_target_map_key_never_aligns_by_basename(): + # An entry naming a Bazel target that does not exist consumes nothing; + # the CMake name must still not fall back to basename alignment. + a = _bs(CanonicalModel(), is_bazel=False) + a.add(Target("tool", TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("x/tool.c", [], False)])) + b = _bs(CanonicalModel(), is_bazel=True) + b.add(Target("x:tool", TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("x/tool.c", [], True)])) + notes = DiffNotes() + cfg = MigrationConfig(target_map={"tool": "y:tool_renamed"}) + kinds = sorted(d.kind for d in diff_models(a, b, cfg, notes)) + assert kinds == ["extra_target", "missing_target"], kinds + assert notes.aligned_by_basename == {} + + +def test_auto_aligned_exclusion_is_shared_by_diff_and_summaries(): + # CMake app, Bazel pkg:app, aligned by basename; exclude_targets names + # app. The diff skips it, and so must the excluded list and the levels. + 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("app/m.cpp", raw, is_bazel)])) + return model + a, b = m("app", ["-O2"], False), m("pkg:app", ["-O0"], True) + cfg = MigrationConfig(exclude_targets={"app"}) + notes = DiffNotes() + res = summarize(diff_models(a, b, cfg, notes), a, b, cfg, notes) + assert res["discrepancies"] == [], res + assert res["aligned_by_basename"] == {"app": "pkg:app"}, res + assert res["excluded"]["bazel"] == {"config_excluded": ["pkg:app"]}, res + assert not res["levels"]["differs"], res["levels"] + assert res["ndebug"]["bazel"] == {"(none)": 1}, res["ndebug"] + + +def test_basename_never_aligns_a_library_or_an_exact_match(): + a = _bs(CanonicalModel(), is_bazel=False) + a.add(Target("app", TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("app.c", [], False)])) + b = _bs(CanonicalModel(), is_bazel=True) + b.add(Target("pkg:app", TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu_from_raw("app.c", [], True)])) + assert any(d.kind == "missing_target" for d in diff_models(a, b)) + + def _exe(name, link_raw, is_bazel, src="tool/m.cpp"): return Target(name, TargetKind.EXECUTABLE, role=TargetRole.PRODUCTION, actions=[tu_from_raw(src, ["-DFOO=1"], is_bazel=is_bazel),