From a24e61cdbd5df28ed1d1a3cb9371b4d8e13d530b Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Fri, 16 Oct 2026 01:32:45 -0400 Subject: [PATCH] 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 f77bed0..dfeb8f4 100644 --- a/scripts/diff.py +++ b/scripts/diff.py @@ -283,6 +283,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 @@ -626,10 +637,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 0fc56d0..b3089b1 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