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),