Conversation
…e dir The File API spells sources relative to the top-level CMake source dir. When that dir is a subdirectory of the repo root (a vendored third_party/ package, or a monorepo holding several CMake projects) the CMake side keyed a TU as 'avl.c' while the Bazel side keyed the same file as 'third_party/libubox/avl.c', so every TU showed up as missing_tu/extra_tu and the diff was meaningless. Anchor each source on the codemodel's paths.source and re-relativize against repo_root; sources outside the repo stay absolute as before. Fixtures without paths.source keep the old behaviour. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
_union_tus keeps the first TU seen per source, and the library names it walks come from a set comprehension, so when one source is compiled by two library targets with different flags (a shared/static twin: -Dfoo_EXPORTS, -fPIC) the representative -- and therefore the reported defines_diff/flags_diff -- flipped between runs with Python's per-process hash seed. Walk the names sorted. Found by the model during the libubox (OpenWrt) migration: the same diff run alternated between reporting ubox_EXPORTS as cmake-only and not at all. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A subdirectory CMakeLists that does ADD_DEFINITIONS(-I..) (libubox lua/,
ubus lua/ and examples/, uci lua/) reaches the codemodel as the literal
token `-I..` in compileCommandFragments. Stored verbatim, the CMake side
carried `..` as an include root while the Bazel side spells the same
directory `third_party/<pkg>`, so every such package needed the same
include_map entry (`..` -> `third_party/<pkg>`) before it could converge.
`..` only means something relative to the directory that spelled it.
CMake itself defines a relative include_directories() entry as relative to
CMAKE_CURRENT_SOURCE_DIR, and the raw -I.. is the hand-spelled version of
that intent (it only names a real header root for in-source builds, which
is OpenWrt's default: cmake.mk sets CMAKE_BINARY_DIR = CMAKE_SOURCE_DIR).
So resolve relative -I/-isystem/-iquote/-idirafter roots -- joined or
split, in fragments or in includes[].path -- against the target's source
dir (codemodel paths.source joined with the target's paths.source),
normalize (lua/.. -> package root) and re-relativize against repo_root,
the same way the previous commit anchors sources. Absolute roots are untouched: the
canonicalizer already normalizes those (`examples/..` from
INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}/..) collapses there).
Replies without codemodel paths keep the verbatim spelling.
With this, libubox, ubus and uci converge with their include_map entries
removed. The `.` -> `third_party/<pkg>` entries those configs also carried
were never load-bearing: `.` only ever appeared on the Bazel side (the
toolchain's `-iquote .` for the workspace root), which the asymmetric
include check already tolerates as bazel_only.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
CMake's shared/static twin (add_library(foo SHARED ...) + add_library( foo-static STATIC ...) from one source list) compiles every file twice with different argv: the shared objects carry -Dfoo_EXPORTS and -fPIC, and sometimes project defines of their own. The library TU-union kept ONE representative per source -- the first in sorted target order -- so the Bazel cc_library's single compile was compared against whichever twin happened to sort first. Migrations then worked around the tool: they suppressed the *_EXPORTS marker in ignore.defines, copied it onto the cc_library, or renamed Bazel targets so the comparison landed on the twin they match. _union_tus now keeps every variant of a source on both sides (TUVariant: the TU plus the target that compiled it; identical argv from two targets is one variant). A Bazel TU converges if its canonical defines/includes/ flags satisfy the existing asymmetric checks against at least one reference variant; each Bazel compile of a source is checked on its own. When none matches, the findings against the closest variant are reported -- fewest missing tokens, then fewest extra, then the twin of the same kind (PIC objects belong to the shared twin), then sorted target name -- and the detail names that CMake target and the Bazel target the TU came from. A one-to-one source keeps the bare detail text. missing_tu/extra_tu keep their meaning (compiled nowhere on the other side), and the test-TU union shares the code path. Output stays independent of PYTHONHASHSEED. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Three ways a plain host build (zlib, fmt, auto-configured gcc toolchain) made a faithful BUILD file look wrong, or a wrong one look converged: * Bazel's argv[0] is the driver whatever its spelling. The host toolchain passes `/usr/bin/gcc`, which was kept as a compile/link flag because only relative paths were recognised as the tool. `-pass-exit-codes` (the unix toolchain's link_flags) joins the link noise list. * `-std=` has last-wins semantics in the driver, so only the last spelling on a side counts: a toolchain -std=c++17 followed by the rule's -std=c++11 is C++11 and converges against a C++11 reference; the reverse order is a flags_diff (it previously passed, since the reference's spelling was present somewhere on the Bazel argv). * -I/-isystem/-idirafter roots are told apart from -iquote ones. Bazel puts the workspace root on -iquote for every compile, so a reference `-I<src>` root looked present while zlib's `#include <zconf.h>` fell through to /usr/include in the sandbox with the diff saying converged. Such a root is now an includes_diff naming -iquote. TranslationUnit.quote_only records the quote-only roots; canonicalize_flags keeps its three-tuple signature. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Under -c opt, a cc_library that feeds both an archive and a cc_shared_library is compiled twice (`.o` and `.pic.o`), and the two compiles differ by the PIC flag alone. Which of them the toolchain builds is not a BUILD-file decision, so the pair is now judged as one compile: the twin closest to the reference speaks for both. A shared-only reference (every TU -fPIC, e.g. fmt with BUILD_SHARED_LIBS=ON) no longer gets a `-fPIC missing` error for the archive compile, and a finding both twins share is reported once instead of twice. A second compile that differs by more than the PIC flag is still judged on its own. A defines_diff on CMake's `<target>_EXPORTS` DEFINE_SYMBOL marker with no static twin to match now says what the marker is and names its two fixes (local_defines, or ignore.defines), since every shared-only library meets exactly this finding. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #23. Only the last 3 commits are new here. After #23 merges I'll rebase. This PR and #25 also touch the same
SKILL.mdtable row, so whichever merges second needs a trivial rebase.Diff-correctness fixes. Each one made a faithful BUILD file look wrong, or made a wrong one look converged:
-Dfoo_EXPORTS -fPIConly on the shared copy. The diff compared Bazel's single compile against whichever twin came first. On zlib this gave 15 falsedefines_differrors (ZLIB_DLL), and migrations worked around it by ignoring*_EXPORTSor renaming targets. A Bazel TU now converges if it matches any variant. Otherwise it is reported against the closest variant, and both targets are named.-iquote, so a reference-I<src>looked present. zlib's#include <zconf.h>then failed in the sandbox while the diff said converged. That case is now anincludes_diffsaying the root is only reachable via-iquote.-std=wins. A Bazel argv of-std=c++11 … -std=c++17compiles as C++17 but passed against a C++11 reference. Only the last-std=on each side is compared now./usr/bin/gccas argv[0] was kept as a flag. It is now recognised as the tool.-c opt, acc_libraryfeeding acc_shared_librarycompiles both.oand.pic.o. Against a shared-only reference (fmt), the non-PIC copy gave a false-fPICerror and every finding was reported twice. The pair is now judged as one compile.<target>_EXPORTSdefines_diffnow explains the marker and its two fixes.Testing. 14 new tests are in
tests/test_engine.py, and all existing tests pass. We re-extracted and re-diffed saved zlib, fmt, spdlog, tinyxml2, jansson and expat builds with and without this change. fmt, spdlog and tinyxml2 stay converged and identical. zlib's falsedefines_differrors are gone. Theincludes_differrors left on zlib, jansson and expat are real: they are CMake build-dir roots holding generated headers, which need codegen support (not in this PR).🤖 Generated with Claude Code