fix(rustdoc): link doc tests against the cc_toolchain's runtime libs - #4220
Open
marvin-hansen wants to merge 5 commits into
Open
fix(rustdoc): link doc tests against the cc_toolchain's runtime libs#4220marvin-hansen wants to merge 5 commits into
marvin-hansen wants to merge 5 commits into
Conversation
…untime libs
Adds a failing analysis test for `rust_doc_test` under a cc_toolchain that
supplies its C++/unwind runtime through `static_runtime_lib`.
NOTE: this test fails on its own. It is committed separately to record the
reproduction; the fix follows in the next commit.
What breaks
-----------
On a toolchain enabling `static_link_cpp_runtimes`, doc tests fail to link:
ld.lld: error: undefined symbol: _Unwind_Resume
>>> referenced by alloc.rs:0 ... liballoc-*.rlib
... and the rest of the _Unwind_* family
`rust_library` / `rust_binary` / `rust_test` are unaffected, which makes the
failure look toolchain-specific when it is not.
The test extends the fixture added in bazelbuild#3741, which already declares a
cc_toolchain with `static_runtime_lib = ":dummy.a"` and the
`static_link_cpp_runtimes` feature enabled -- exactly the configuration needed
to observe this. It asserts that the doc test's rustdoc action names the
runtime lib on the command line (`-Clink-arg=-ldummy`).
Today it does not. The action's argv shows the search path arriving while the
library reference never does:
"-Lnative=test/unit/cc_toolchain_runtime_lib", <- present
... <- no -Clink-arg=-ldummy
That asymmetry is the bug: `add_native_link_flags` emits the runtime libs'
`-Lnative=` unconditionally but gates the matching `-lstatic=` behind
`include_link_flags`, which rustdoc sets to False (bazelbuild#2467). bazelbuild#4080 added a
compensating `-Clink-arg=-l` loop for doc tests, but it iterates only
`dep_info.transitive_noncrates`, which never contains the toolchain runtime
libs: Bazel injects those into C++ link actions, and rustdoc never runs one --
it drives the doc test link itself. The archives end up on the search path with
nothing referencing them.
A system GNU toolchain hides this, because the `-lgcc_s` rustc emits resolves
to a real libgcc_s carrying the `_Unwind_*` symbols. Toolchains shipping their
own unwinder tend to make `-lgcc_s` / `-lunwind` resolve to empty stub archives
so third-party build systems do not fail on a missing library, which is what
turns the omission into a link error.
Fixes the reproduction added in the previous commit: `//test/unit/cc_toolchain_runtime_lib:runtime_libs_test/doc_test` now passes. Two parts, both required. 1. `rustdoc.bzl` extends the bazelbuild#4080 `-Clink-arg=-l` loop to the cc_toolchain runtime libs, mirroring the crate-type split in `collect_inputs` so the libs it names are the ones that were added to the action inputs. Guarded on `cc_toolchain` being present, since bazelbuild#3665 made it optional. The libs are returned in the action struct for use by (2). 2. `rustdoc_test.bzl` adds those libs' root to the `--strip_substring` list. Without this the `-l` alone still fails with "unable to find library": the runtime libs are built in a different configuration than the crate outputs, so their root is not among the crate roots already collected, and the `-Lnative=` path stays an execroot path that does not exist in the runfiles tree the doc test runs from. Roots are only added when non-empty. Source-file runtime libs have an empty root and need no stripping -- their `-Lnative=` path is already workspace-relative -- and an empty root would emit `--strip_substring=/`. `rustdoc_test_writer` applies these as plain `str::replace`, so that would delete every `/` in every argument. The libs already reach the doc test's runfiles via `ctx.runfiles(transitive_files = action.inputs)`, thanks to bazelbuild#3741, so no additional wiring is needed there. Verified on macOS with `bazel test -- //... -//test/unit/remap_path_prefix:integration_test`: 569 tests pass and 35 are skipped, against 568/35 before this series. The only target that changed status is the new doc test. Every other action was a cache hit, so the change is a provable no-op for toolchains whose `static_runtime_lib` is empty.
Fixes the reproduction added in the previous commit: `//test/unit/cc_toolchain_runtime_lib:runtime_libs_test/doc_test` now passes. Two parts, both required. 1. `rustdoc.bzl` extends the bazelbuild#4080 `-Clink-arg=-l` loop to the cc_toolchain runtime libs, mirroring the crate-type split in `collect_inputs` so the libs it names are the ones that were added to the action inputs. Guarded on `cc_toolchain` being present, since bazelbuild#3665 made it optional. The libs are returned in the action struct for use by (2). 2. `rustdoc_test.bzl` adds those libs' root to the `--strip_substring` list. Without this the `-l` alone still fails with "unable to find library": the runtime libs are built in a different configuration than the crate outputs, so their root is not among the crate roots already collected, and the `-Lnative=` path stays an execroot path that does not exist in the runfiles tree the doc test runs from. Roots are only added when non-empty. Source-file runtime libs have an empty root and need no stripping -- their `-Lnative=` path is already workspace-relative -- and an empty root would emit `--strip_substring=/`. `rustdoc_test_writer` applies these as plain `str::replace`, so that would delete every `/` in every argument. The libs already reach the doc test's runfiles via `ctx.runfiles(transitive_files = action.inputs)`, thanks to bazelbuild#3741, so no additional wiring is needed there. Verified on macOS with `bazel test -- //... -//test/unit/remap_path_prefix:integration_test`: 569 tests pass and 35 are skipped, against 568/35 before this series. The only target that changed status is the new doc test. Every other action was a cache hit, so the change is a provable no-op for toolchains whose `static_runtime_lib` is empty.
…lib' into fix-rustdoc-test-static-runtime-lib
The doc test assertion added alongside the runtime-lib fix hardcoded
`-Clink-arg=-ldummy`, which fails on Windows CI:
error: expected '-Clink-arg=-ldummy' in the rustdoc test link args:
'[..., "-Clink-arg=dummy"]'
The fix itself works there -- `rustdoc.bzl` omits the `-l` prefix when the
target ABI is msvc, where link.exe takes bare library names, matching the
existing bazelbuild#4080 loop. Only the test was wrong.
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
marvin-hansen
added a commit
to marvin-hansen/deep_causality
that referenced
this pull request
Aug 12, 2026
Fix a broken MODULE.bazel
-------------------------
`toolchain.version(llvm_version = ...)` is not valid: the `toolchain`
extension only accepts `exec` and `target` tags. Every build failed analysis
with "does not have a tag class named version". The LLVM version belongs to the
separate `llvm` extension, so it moves to `llvm.version()`. 22.1.6 is a version
the module actually publishes.
Vendor the rule sources
-----------------------
Check the five registry modules into thirdparty/ behind local_path_override:
aspect_bazel_lib, bazel_skylib, platforms, toolchains_buildbuddy and llvm.
5.4 MB in total -- these are Starlark only.
Two things worth knowing about how they were vendored:
* Each tree is copied from the RESOLVED external repo, not from an upstream
release tarball. aspect_bazel_lib, toolchains_buildbuddy and llvm each
carry a BCR remote_patch; downloading from GitHub would silently drop it.
* The vendored trees are added to .bazelignore. Without that, `//...` tries
to load them as packages of the root module and fails -- bazel_skylib's
internal labels such as `//lib:unittest.bzl` resolve against the root
workspace rather than against the module.
build/scripts/vendor_modules.sh re-vendors them after a version bump, and
refuses to run while the overrides are active (they would otherwise resolve to
the tree being overwritten instead of to the registry).
This is deliberately limited to rule sources. The toolchains these rules
configure -- Rust ~1.8 GB, LLVM ~1 GB, Lean/Mathlib ~10 GB -- arrive through
module extensions, which local_path_override cannot vendor; those stay in the
CI repository cache. Vendoring rule sources removes ~54 MB of a ~12 GB cold
fetch, so the value here is not fetch time but not depending on the registry
being reachable to build at all.
rules_rust is not vendored: it is pinned to a fork through git_override while
bazelbuild/rules_rust#4220 is open, and the two override kinds are mutually
exclusive.
Also
----
The CI test step ran under --config=remote, so it never picked up the ROLE and
USER build metadata that --config=ci sets; both steps now use --config=ci and
BuildBuddy can tell CI runs from developer machines. Drops the .bazelignore
entry for the removed thirdparty/rules_astro.
Verified with `bazel test --config=ci //...`: 1153 tests pass, covering the
Lean proofs and all 27 doc tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.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
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.
rust_doc_testfails to link on any cc_toolchain that supplies its C++/unwindruntime via
static_runtime_lib(i.e. enablesstatic_link_cpp_runtimes).This happens when cross compiling with the hermetic-LLVM e.g.:
rust_library/rust_binary/rust_testare fine. Only doc tests fail.Evidence
The first commit adds only a test to reproduce the issue by extending the
fixture from #3741 (which already declares a cc_toolchain with
static_runtime_lib = ":dummy.a"); the second commit adds the fix://test/unit/cc_toolchain_runtime_lib:runtime_libs_test/doc_testtest(rustdoc): reproduce ...fix(rustdoc): link doc tests ...The failing action's argv shows the search path arriving while the library
reference never does:
Cause
add_native_link_flagsemits the runtime libs'-Lnative=unconditionally butgates the matching
-lstatic=behindinclude_link_flags, which rustdoc setsto False (#2467). #4080 added a compensating
-Clink-arg=-lloop for doc tests,but this does not work for a
static_runtime_lib.Alternatives
Injecting the flag from outside doesn't work. The archive is already in the
doc test's runfiles (via #3741), so
-Clink-arg=<its short_path>does fix doctests. However,
toolchain.extra_rustc_flagsapplies to every rustc invocation,and normal
rust_testbinaries link in the execroot, where thatrunfiles-relative path doesn't exist:
clang++: error: no such file or directory. Doc tests and normal links need different paths to the same file,No single toolchain-level flag satisfies both. Per-target
rustdoc_flagsdoes work, but only by hardcoding the canonical repo name and the runtime lib's
internal layout in every
rust_doc_test. However, this is unmaintainable in any major project.Changes
rustdoc.bzl— extend the Fix rust_doc_test linking with native libraries #4080 loop to the cc_toolchain runtime libs,mirroring the crate-type split in
collect_inputs. Guarded oncc_toolchainbeing present (Add support for optional cc_toolchain #3665).
rustdoc_test.bzl— add those libs' root to--strip_substring. Required:they're built in a different configuration than the crate outputs, so
otherwise the
-Lnative=path stays an execroot path that doesn't exist inthe runfiles tree the doc test runs from. Only non-empty roots are added —
source-file runtime libs need no stripping, and an empty root would emit
--strip_substring=/, whichrustdoc_test_writerapplies as a plainstr::replace.The libs already reach runfiles via
ctx.runfiles(transitive_files = action.inputs)thanks to #3741.
Testing
bazel test -- //... -//test/unit/remap_path_prefix:integration_teston macOS:569 passed / 35 skipped, against 568 / 35 before. The new doc test is
the only target that changed status; every other action was a cache hit, so
this is a provable no-op where
static_runtime_libis empty.The patch has also been verified against a repo that builds with hermetic-LLVM and all failing doc tests
passed.