Skip to content

Compare link-interface flags from CMake's libraries fragments - #44

Open
anakanemison wants to merge 1 commit into
mschaller/u08-compile-only-link-flags-warningfrom
mschaller/u09-link-interface-flags
Open

anakanemison wants to merge 1 commit into
mschaller/u08-compile-only-link-flags-warningfrom
mschaller/u09-link-interface-flags

Conversation

@anakanemison

Copy link
Copy Markdown

Part 9 of 23 of a stacked series (tracking issue: #35). Based on U08-compile-only-link-flags-warning, the previous part: review and merge that first.

Kind: fix. Lab proposal: P25.

What the migration runs showed

Blender's -Xlinker -export-dynamic reaches its link line through a dependency's link interface, in the File API's libraries fragment, which the model did not read; the migration converged without it, and by the agent's report its Python modules then failed to load.

The change

The CMake File API reports a target's link line in fragments by role.
Flags that a dependency carries on its link interface (Python's
-Xlinker -export-dynamic, OpenMP's -fopenmp, Threads' -pthread) arrive in
the "libraries" fragments, not "flags". extract_cmake.py read "libraries"
only for dependency names, so these flags reached neither the dep closure
nor the link-flag comparison, and a Bazel binary missing one converged.

Observed in any2bazel-lab: on a Blender migration, the lab
verified that -Xlinker -export-dynamic sits only in the blender target's
libraries fragment. By the migrating agent's own report, the diff
converged without it and Python extension modules then failed to resolve
CPython symbols at run time, caught only by its functional test; the lab
did not reproduce the failure, because that tree was not kept.

A "libraries" fragment that is not a library and starts with "-" is now
added to the target's link argv, so it is compared like any link flag.
"-Xlinker X" pairs are folded into "-Wl,X" first, so the fragment is
classified by what it passes to the linker. Excluded, because they position
or locate the libraries around them or name the CMake build tree: -L,
-framework, rpaths in every spelling (-Wl,-rpath, -Wl,--rpath, -Wl,-R, and
-Xlinker -rpath -Xlinker

), the whole-archive, push/pop-state, group,
-Bstatic/-Bdynamic and as-needed toggles, and -force_load.
canonicalize_link_flags also folds "-Xlinker X" into "-Wl,X" on both sides,
before its noise filters, so the pair compares as one flag, and then joins
an rpath given as two linker arguments (-Wl,-rpath then -Wl,DIR) into
-Wl,-rpath,DIR, which is what the linker receives. So these three
spellings of one rpath compare equal: -Xlinker -rpath -Xlinker DIR (what
CMake's LINKER:-rpath,DIR becomes under Clang), -Wl,-rpath -Wl,DIR and
-Wl,-rpath,DIR. -Wl,-rpath=DIR, -Wl,--rpath,DIR and -Wl,-R,DIR
are not unified with them, as before. Bazel spells its own rpaths the
first way, as $ORIGIN/, $EXEC_ORIGIN/ or @loader_path/ paths into its
solib directories; on the Bazel side exactly those (an anchored
match) are toolchain link noise, otherwise they would appear as Bazel-only
link flags on every binary. A project's own rpath
(-Wl,-rpath,$ORIGIN/../lib, -Wl,-rpath=/opt/lib, -Wl,-rpath-link,...) is
kept and compared.

One limitation: any non-library "libraries" fragment becomes a link
flag the Bazel side must have, including one that carries a CMake
build-tree path Bazel cannot reproduce (-Wl,--version-script,/sym.map; this holds when the map file's basename does not start with
"lib", since a fragment ending in lib. is still read as the
external dependency , as before). A version script decides which
symbols are exported, so such a flag is not benign: its exact CMake
spelling may go in ignore.link_flags only after confirming the Bazel link
passes an equivalent script with the same content, and SKILL.md's
link_flags_diff row now says so. A multi-token fragment is classified as a
whole, as before, so -Wl,-Bstatic -lfoo -Wl,-Bdynamic in one fragment
still loses its -lfoo; splitting fragments per token is left out because
rpath pairs would then need their own pairing logic.

A behavior change: a migration that converged before may now show a
link_flags_diff for such a flag, most often -pthread (Threads::Threads with
THREADS_PREFER_PTHREAD_FLAG) or -fopenmp. That is a real difference in the
link command; add the flag to linkopts or record it in ignore.link_flags.
SKILL.md says these flags are compared.

No migration run exercised this change; it was checked on the stored
Blender models.

Testing

pytest tests/ at this commit: 195 passed (130 on main).

🤖 Generated with Claude Code

The CMake File API reports a target's link line in fragments by role.
Flags that a dependency carries on its link interface (Python's
-Xlinker -export-dynamic, OpenMP's -fopenmp, Threads' -pthread) arrive in
the "libraries" fragments, not "flags". extract_cmake.py read "libraries"
only for dependency names, so these flags reached neither the dep closure
nor the link-flag comparison, and a Bazel binary missing one converged.

Observed in any2bazel-lab: on a Blender migration, the lab
verified that -Xlinker -export-dynamic sits only in the blender target's
libraries fragment. By the migrating agent's own report, the diff
converged without it and Python extension modules then failed to resolve
CPython symbols at run time, caught only by its functional test; the lab
did not reproduce the failure, because that tree was not kept.

A "libraries" fragment that is not a library and starts with "-" is now
added to the target's link argv, so it is compared like any link flag.
"-Xlinker X" pairs are folded into "-Wl,X" first, so the fragment is
classified by what it passes to the linker. Excluded, because they position
or locate the libraries around them or name the CMake build tree: -L,
-framework, rpaths in every spelling (-Wl,-rpath, -Wl,--rpath, -Wl,-R, and
-Xlinker -rpath -Xlinker <dir>), the whole-archive, push/pop-state, group,
-Bstatic/-Bdynamic and as-needed toggles, and -force_load.
canonicalize_link_flags also folds "-Xlinker X" into "-Wl,X" on both sides,
before its noise filters, so the pair compares as one flag, and then joins
an rpath given as two linker arguments (`-Wl,-rpath` then `-Wl,DIR`) into
`-Wl,-rpath,DIR`, which is what the linker receives. So these three
spellings of one rpath compare equal: `-Xlinker -rpath -Xlinker DIR` (what
CMake's LINKER:-rpath,DIR becomes under Clang), `-Wl,-rpath -Wl,DIR` and
`-Wl,-rpath,DIR`. `-Wl,-rpath=DIR`, `-Wl,--rpath,DIR` and `-Wl,-R,DIR`
are not unified with them, as before. Bazel spells its own rpaths the
first way, as $ORIGIN/, $EXEC_ORIGIN/ or @loader_path/ paths into its
_solib_<cpu> directories; on the Bazel side exactly those (an anchored
match) are toolchain link noise, otherwise they would appear as Bazel-only
link flags on every binary. A project's own rpath
(-Wl,-rpath,$ORIGIN/../lib, -Wl,-rpath=/opt/lib, -Wl,-rpath-link,...) is
kept and compared.

One limitation: any non-library "libraries" fragment becomes a link
flag the Bazel side must have, including one that carries a CMake
build-tree path Bazel cannot reproduce (-Wl,--version-script,<build
dir>/sym.map; this holds when the map file's basename does not start with
"lib", since a fragment ending in lib<name>.<ext> is still read as the
external dependency <name>, as before). A version script decides which
symbols are exported, so such a flag is not benign: its exact CMake
spelling may go in ignore.link_flags only after confirming the Bazel link
passes an equivalent script with the same content, and SKILL.md's
link_flags_diff row now says so. A multi-token fragment is classified as a
whole, as before, so `-Wl,-Bstatic -lfoo -Wl,-Bdynamic` in one fragment
still loses its -lfoo; splitting fragments per token is left out because
rpath pairs would then need their own pairing logic.

A behavior change: a migration that converged before may now show a
link_flags_diff for such a flag, most often -pthread (Threads::Threads with
THREADS_PREFER_PTHREAD_FLAG) or -fopenmp. That is a real difference in the
link command; add the flag to linkopts or record it in ignore.link_flags.
SKILL.md says these flags are compared.

No migration run exercised this change; it was checked on the stored
Blender models.

Lab-Proposal: P25

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant