Align executables by unambiguous label basename

SKILL.md

@@ -52,13 +52,23 @@ 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
+  - **Link consistency** — every name-aligned executable links with
     equivalent link flags. 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
 
@@ -70,17 +80,21 @@ Bazel aquery jsonproto    ──extract_bazel.py───────┘
   path**, so names and grouping don't matter — a rename or object-library
   fold-in converges with no mapping. Only **external** link deps are checked;
   internal dep names are noise.
-- **Executables** align by **name** (a missing exe is a real gap); use
-  `target_map` for renames.
+- **Executables** align by **name** (a missing exe is a real gap), or with the
+  one Bazel executable whose label basename matches (`BinRec` ↔ `gems:BinRec`;
+  see `aligned_by_basename` in `diff.json`). Use `target_map` for renames and
+  for a basename two packages share.
 
 ### Canonicalization
 
 `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`
 
@@ -112,12 +126,17 @@ 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 to mirror the
+  real build (see [Extract the Bazel action graph](#extract-bazel-graph)), e.g.
+  `["--config=release"]` or `["-c", "opt"]`; never a command
+  (`["aquery", "mnemonic(...)", ...]` is wrong). Recorded so the comparison is
+  reproducible; `diff.py` refuses an entry that is neither a flag nor a flag's
+  value.
 - **`target_map`** — `reference_name → bazel_name` for intentionally renamed
-  **executables**. Bazel targets are keyed by full label, so the value is
-  usually `:name` (e.g. `"bssl": ":bssl"`). Libraries need no mapping.
+  **executables**, or when two Bazel packages share the basename. The value is
+  the label without `//`: `:name` at the root, `pkg:name` elsewhere (e.g.
+  `"bssl": "tool:bssl"`). Same-named executables align without an entry;
+  libraries need no mapping.
 - **`dep_map`** — `reference_dep_name → bazel_dep_name` for an **external link
   dep** spelled differently per build (e.g. `Catch2Main` vs `catch2_main`). An
   explicit, recorded rename — not a fuzzy match — so a residual `missing_dep` is
@@ -283,7 +302,13 @@ python3 scripts/diff.py model.<frontend>.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 `<libraries>`, `<external>` (and `<tests>` when
@@ -293,14 +318,16 @@ 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. The WARN `configured_files_not_compared`
+does need action: byte-compare each listed file against the one your Bazel
+build generates.
 
 #### Triage findings {#triage}
 

docs/REFERENCE-cmake.md

@@ -40,6 +40,8 @@ python3 "$SKILL_DIR/scripts/extract_cmake.py" <build> <repo_root> 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.

scripts/canonicalize.py

@@ -183,12 +183,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,14 +231,16 @@ 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
     """
     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 +261,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 +301,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

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
+    # (diff._diff_variants; extra Bazel defines stay benign WARN under the
+    # asymmetric-subset rule). 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,12 @@ 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
+    # The FLAGS appended to `bazel build` / `bazel aquery` so the Bazel side
+    # mirrors the real build (e.g. "--config=release", or "-c" "opt"), recorded
     # so the comparison is reproducible. Read by the skill/operator, not by the
-    # diff itself (the diff consumes the already-extracted model).
+    # diff itself (the diff consumes the already-extracted model); validated at
+    # load (see validate_bazel_args) because anything that consumes it splices
+    # it into a bazel command line.
     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
@@ -161,6 +165,38 @@ def target_excluded(self, name: str) -> bool:
         return name in self.exclude_targets
 
 
+def validate_bazel_args(args) -> tuple:
+    """Check that `bazel_args` is a list of flags, and return it as a tuple.
+
+    Each entry must be a flag (starts with '-') or the value of the flag just
+    before it, when that flag has no '=' (so ["-c", "opt"] and
+    ["--config", "release"] are fine). Anything else -- most often a whole
+    command such as ["aquery", "mnemonic(...)", "--output=jsonproto"] -- is
+    refused, naming the entry: spliced into `bazel aquery <args> <query>` it
+    becomes a second query expression and breaks the command. A boolean flag
+    followed by a stray word slips through; the check is a guard against the
+    command-in-the-field mistake, not a Bazel option parser."""
+    if isinstance(args, str) or not isinstance(args, (list, tuple)):
+        raise ValueError(
+            f"bazel_args must be a list of flags, got {type(args).__name__}")
+    value_allowed = False
+    for a in args:
+        if not isinstance(a, str):
+            raise ValueError(f"bazel_args entry {a!r} is not a string")
+        if a.startswith("-"):
+            value_allowed = "=" not in a
+            continue
+        if value_allowed:
+            value_allowed = False
+            continue
+        raise ValueError(
+            f"bazel_args entry {a!r} is not a flag or a flag's value. "
+            "bazel_args holds only the flags appended to `bazel build` / "
+            "`bazel aquery` to mirror the real build (e.g. [\"--config=release\"] "
+            "or [\"-c\", \"opt\"]), never the command or query itself.")
+    return tuple(args)
+
+
 def load(path: str) -> MigrationConfig:
     """Load config from an explicit file path, or return empty if absent."""
     if not path or not os.path.isfile(path):
@@ -182,7 +218,7 @@ def load(path: str) -> MigrationConfig:
         source_map=tuple(
             (e["from"], e["to"]) for e in ig.get("source_map", [])),
         exclude_targets=set(obj.get("exclude_targets", [])),
-        bazel_args=tuple(obj.get("bazel_args", [])),
+        bazel_args=validate_bazel_args(obj.get("bazel_args", [])),
         include_tests=bool(obj.get("include_tests", False)),
     )