diff --git a/SKILL.md b/SKILL.md index 9c1380d..effe05d 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 @@ -366,7 +367,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