diff --git a/SKILL.md b/SKILL.md index 62ad5c0..b9e48d8 100644 --- a/SKILL.md +++ b/SKILL.md @@ -52,13 +52,24 @@ 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 - equivalent link flags. The per-binary stage: the *artifacts* agree, not just - the TUs. + - **Link consistency** — every name-aligned executable links with + 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 - 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. + + **`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 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 @@ -76,11 +87,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` @@ -95,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"], @@ -112,9 +126,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 (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. @@ -126,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); @@ -223,7 +245,15 @@ python3 scripts/extract_.py <…> model..json Note the platform and options the reference build was configured with; the Bazel side must mirror them. -### Create initial BUILD.bazel files +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 {#create-build-files} Read `model..json`. For each production target emit a `cc_library` / `cc_binary` with `srcs`, `hdrs`, `copts`, `defines`, `includes`, `deps`. Library @@ -237,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} @@ -255,12 +288,22 @@ 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 > 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 +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=...] \ @@ -283,7 +326,13 @@ 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, +`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 reference, absent in Bazel". Synthetic target names ``, `` (and `` when @@ -293,14 +342,18 @@ 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` / `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. `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} @@ -337,7 +390,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`). 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/docs/REFERENCE-cmake.md b/docs/REFERENCE-cmake.md index 49ed665..db36825 100644 --- a/docs/REFERENCE-cmake.md +++ b/docs/REFERENCE-cmake.md @@ -40,10 +40,29 @@ 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. +## 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 @@ -53,7 +72,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/canonicalize.py b/scripts/canonicalize.py index f2b8c66..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", @@ -183,12 +198,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 +246,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 +255,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 +276,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 +316,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 @@ -273,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. @@ -289,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/config.py b/scripts/config.py index fd17cbd..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(). @@ -100,9 +101,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 diff --git a/scripts/diff.py b/scripts/diff.py index d602dcb..77bf20a 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 @@ -32,14 +33,18 @@ from __future__ import annotations import json -from dataclasses import asdict, dataclass +import re +from collections import Counter +from dataclasses import asdict, dataclass, field 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 reconstruct import TargetView, reconstruct +from model import (BuildSystem, CanonicalModel, TargetKind, TargetRole, + TranslationUnit) +from reconstruct import (TargetView, _is_header_processing, + _source_from_compile_args, reconstruct) def _display_bazel_only(flags) -> list: @@ -69,7 +74,9 @@ 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 + # 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 @@ -78,6 +85,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 @@ -91,6 +100,31 @@ 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. 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, cfg: "MigrationConfig") -> List[Discrepancy]: out: List[Discrepancy] = [] @@ -153,24 +187,72 @@ 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") -> List[Discrepancy]: - """Asymmetric link-flag subset for one aligned executable/shared-lib: every + cfg: "MigrationConfig", + notes: Optional[DiffNotes] = None) -> List[Discrepancy]: + """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. 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 a_lf - b_lf: - return [Discrepancy( + if notes is not None: + notes.bazel_only_link_flags.update(b_lf - a_lf) + 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: @@ -199,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 @@ -220,24 +313,114 @@ 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} +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", + 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. The Bazel-only flags of each chosen pair are recorded in + `notes`.""" + out: List[Discrepancy] = [] + for a in a_vs: + 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)) + 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 @@ -262,7 +445,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) @@ -291,14 +474,23 @@ 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() # 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) @@ -330,7 +522,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_tu("", 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): @@ -343,8 +536,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 @@ -357,12 +552,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_tu(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 @@ -399,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", @@ -410,12 +610,34 @@ 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)) + + # ---- 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) -> 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 @@ -430,12 +652,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() @@ -453,7 +675,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_tu("", 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): @@ -481,21 +704,147 @@ 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))} + + +# 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} + + +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]))} + + +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, and these defines only it has are " + "only a WARN. 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), @@ -506,6 +855,15 @@ 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 @@ -520,4 +878,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/extract_cmake.py b/scripts/extract_cmake.py index f070f1f..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) @@ -146,17 +147,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]) @@ -173,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", []): @@ -184,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: @@ -191,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/scripts/triage.py b/scripts/triage.py index 78729fd..b479ec8 100644 --- a/scripts/triage.py +++ b/scripts/triage.py @@ -82,6 +82,9 @@ 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"), + "levels": diff.get("levels"), + "ndebug": diff.get("ndebug"), } @@ -95,6 +98,45 @@ 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', {}))}") + 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; " + 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()) + 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 f50ef31..78ac76d 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) @@ -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"] @@ -189,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 @@ -417,6 +559,395 @@ 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=1"], 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=1"], []), (["LIB2=1"], [])], 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_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_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 _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_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. @@ -487,6 +1018,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)) @@ -508,10 +1111,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"] @@ -600,6 +1205,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 diff --git a/tests/test_extractors.py b/tests/test_extractors.py index fd282b7..1de9275 100644 --- a/tests/test_extractors.py +++ b/tests/test_extractors.py @@ -139,6 +139,52 @@ 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_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) @@ -255,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 diff --git a/tests/test_triage.py b/tests/test_triage.py index cb36bcb..6ae065e 100644 --- a/tests/test_triage.py +++ b/tests/test_triage.py @@ -75,6 +75,51 @@ 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)) + + +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 + + +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_")]