Make --codegen=linker and --sysroot path-mapping-aware for a cc_toolchain-provided linker - #4253
Open
yakkala-pooja wants to merge 1 commit into
Open
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
yakkala-pooja
force-pushed
the
fix-4250-path-map-linker-args
branch
4 times, most recently
from
September 8, 2026 04:29
152f6ab to
8825577
Compare
yakkala-pooja
marked this pull request as draft
September 8, 2026 04:34
yakkala-pooja
force-pushed
the
fix-4250-path-map-linker-args
branch
4 times, most recently
from
September 8, 2026 11:55
303544d to
d0de99c
Compare
…hain-provided linker cc_common.get_tool_for_action() returns the C++ toolchain's linker as a plain string, and cc_common.get_memory_inefficient_command_line() returns its link args (including a --sysroot= value) as plain strings too. Passing either straight into Args means Bazel's path mapping (--experimental_output_paths=strip) has nothing it can rewrite, since only a File threaded through Args is eligible -- so a hermetic C++ toolchain whose linker or sysroot is a generated Bazel artifact ends up with a stale, un-rewritten configuration segment in the rustc command line, and linking fails when the Rustc action runs against the stripped bazel-out/cfg layout. get_linker_and_args now recovers the backing File for the linker (searching cc_toolchain.all_files for the string cc_common.get_tool_for_action() returned, including the case where the tool lives inside a directory/tree artifact) and threads it through rustc_flags.add/add_all instead of the raw string, mirroring the map_each/format_each pattern this file already uses for the Rust toolchain's own sysroot. The Rust-toolchain-provided linker (toolchain.linker) already had a File on hand for free, so that branch is fixed the same way. link_args gets the same treatment for a --sysroot= entry, via a small wrapper that leaves every other entry, and any entry that fails to resolve, untouched. The resolution is skipped up front for any path that doesn't start with bazel-out/: a system-absolute path (the common, non-hermetic case), an external-repository path, and a plain source-tree path are all already configuration-independent, so path mapping has nothing to rewrite for them and the scan would be pure overhead. An exact match is tried first; a fallback matches on the part of the path after bazel-out/<config>/bin/ instead, for a version where cc_toolchain's own files are reported under a different configuration segment than cc_common.get_tool_for_action()'s string for the very same toolchain -- confirmed via a temporary debug dump against the repo's minimum-supported Bazel version (7.4.1) in CI, which showed the exact same toolchain's tool resolving to a target-config path from get_tool_for_action() but an exec-config path in cc_toolchain.all_files. Using the matched File's own path to render the flag either way, not the original string, means whichever configuration Bazel actually materializes that File under is what ends up on the command line, so the two disagreeing on the configuration segment does not matter. construct_arguments now also tracks the File(s) it resolved this way on the returned args struct as extra_action_inputs, and rustc_compile_action merges that into the Rustc/RustcMetadata actions' inputs. Passing a File through Args puts it on the command line but does not by itself register it as an action input, and collect_inputs' own cc_toolchain.linker_files() mechanism did not reliably do this in every configuration this PR was tested against. The whole linker_files() depset is pulled in explicitly too -- using the same hasattr(cc_toolchain, "_linker_files") version-compat check collect_inputs already uses for that same method -- but only when the linker or a --sysroot= entry actually needed the config-agnostic or directory-relative fallback match in _resolve_tool_file (its resolved File's own path landing on something other than the original string is exactly that signal), not on every hermetic-toolchain build: an exact match has proven sufficient on its own on every Bazel version this needed it, so the extra flatten is reserved for the specific case that isn't true. Added test/unit/path_mapped_linker: a minimal cc_toolchain whose linker and sysroot are genrule outputs (so their real location is a config-dependent bazel-out/... path, unlike a normal system toolchain), registered via --extra_toolchains with linker_preference forced to "cc". Several things only surfaced against real CI, not local testing, and are addressed: - analysis_test_transition refuses to set --experimental_* options, so the test cannot force --experimental_output_paths=strip on itself. The assertions check only that --codegen=linker= and the --sysroot= link-arg name the right file, not the mapped bazel-out/cfg/... prefix specifically; the repo's own "Path Mapping Linux/RBE/MacOS" CI jobs already run the whole suite, this test included, with the flag set, which is where that stronger check actually happens. Verified manually both ways: run normally the test passes trivially; run with --experimental_output_paths=strip on the command line, temporarily reverting just the resolution step in rustc.bzl makes it fail, showing the real, un-rewritten configuration segment in place of bazel-out/cfg/ for exactly the flag that was reverted -- confirmed independently for the linker and the sysroot. - The fake toolchain, registered via --extra_toolchains with no compatibility constraints, was a candidate for every C++ toolchain resolution in the build. Under bazel coverage specifically (which forces a fresh, uncached build of exec-configuration tools), it was being selected to "link" util/process_wrapper -- a Rust binary with no relation to this test -- and since the fake linker was a no-op script, that build failed with "output ... was not created" on both Linux/RBE and macOS CI. Fixed by giving fake_cc_toolchain a target_compatible_with constraint satisfied only by a dedicated fake_platform (extending the real host platform via parents), and transitioning --platforms to it alongside --extra_toolchains: the fake toolchain now only ever matches this test's own target configuration. - analysistest never executes the target-under-test's own actions (it only inspects the analysis-time action graph), but bazel coverage does -- so once the fake linker was actually the one selected for this test's own rust_binary (correctly, now that the previous leak is fixed), its being a pure no-op became a real problem: Bazel requires a declared output to actually exist, and the no-op script produced none. The fake linker (test/unit/path_mapped_linker/fake_gcc_template.sh, copied into place by a genrule so its exec path stays under bazel-out/) parses "-o <path>" and "/OUT:<path>" and creates whichever one it is given, via ": > \"\$out\"", a shell builtin, not the external touch: the sandbox PATH a linker is invoked under is minimal and does not reliably have it (observed directly as "touch: not found" in the linker's own stderr, from CI). - Fixing the toolchain leak then surfaced the bug this PR actually fixes, one level up: with the fake toolchain correctly scoped to just this test's own rust_binary, its Rustc action failed with "linker bazel-out/.../fake_gcc not found" under bazel coverage on Bazel 7.4.1 specifically -- the file was on the command line but not materialized in the sandbox. Root-caused (see above) to cc_toolchain resolving to a different configuration than get_tool_for_action()'s own string on that Bazel version; extra_action_inputs plus the config-agnostic suffix match together close the gap. Testing: reproduced the toolchain-leak scenario locally with bazel coverage over this test plus util/process_wrapper together and confirmed process_wrapper falls through to the real toolchain; verified the fake linker script's argument parsing directly in isolation, including with an empty PATH to simulate the restricted sandbox environment that surfaced the touch issue. A full end-to-end Rust build could not be exercised on the development machine at all (no MSVC installed, unrelated to this change), and swapping to Bazel 7.4.1 via bazelisk hit the same MSVC gap rather than reaching the Linux-specific failures, so the minimum-Bazel- version-specific issues above were diagnosed from real CI logs -- one via a temporary print-based debug dump added to a throwaway commit, run against CI, then removed once it gave the answer -- rather than independently reproduced end-to-end. Fixes bazelbuild#4250 Assisted-by: Claude (Anthropic)
yakkala-pooja
force-pushed
the
fix-4250-path-map-linker-args
branch
from
September 8, 2026 12:38
d0de99c to
0c282ca
Compare
yakkala-pooja
marked this pull request as ready for review
September 8, 2026 12:52
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.
Fixes #4250
cc_common.get_tool_for_action() returns the C++ toolchain's linker as a
plain string, and get_memory_inefficient_command_line() returns its link
args (including --sysroot=) as plain strings too. Bazel's path mapping
(--experimental_output_paths=strip) can only rewrite a path that reaches
Args as a File, so a hermetic C++ toolchain whose linker or sysroot is a
generated Bazel artifact ends up with a stale, un-rewritten configuration
segment on the rustc command line, and linking fails when the Rustc action
runs against the stripped bazel-out/cfg layout.
Change
cc_toolchain.all_files for the string cc_common.get_tool_for_action()
returned, including when the tool lives inside a directory/tree
artifact) and threads it through Args instead of the raw string,
mirroring the map_each/format_each pattern already used elsewhere in
this file for the Rust toolchain's own sysroot.
on hand for free, so that branch is fixed the same way.
wrapper that leaves every other entry (and any entry that fails to
resolve) untouched.
path after bazel-out//bin/ instead, for a Bazel version where
cc_toolchain's own files are reported under a different configuration
segment than get_tool_for_action()'s string for the very same toolchain.
The matched File's own path is always used to render the flag, so
whichever configuration Bazel actually materializes it under is what
ends up on the command line.
bazel-out/, since only a generated-artifact path is ever
configuration-dependent — this keeps the scan a no-op for the common
non-hermetic-toolchain case.
struct as extra_action_inputs, and rustc_compile_action merges that into
the Rustc/RustcMetadata actions' inputs, since passing a File through
Args doesn't by itself register it as an action input. Both this and
the config-agnostic fallback above only trigger when a resolution
actually needed them, so the exact-match path (sufficient on most Bazel
versions) pays no extra cost.
Testing
Added test/unit/path_mapped_linker: a minimal cc_toolchain whose linker
and sysroot are genrule outputs (so their real location is genuinely
config-dependent, unlike a normal system toolchain), registered via
--extra_toolchains with linker_preference forced to "cc" and scoped to
this test's own configuration via a dedicated platform +
target_compatible_with, so it can't be picked up by unrelated builds
elsewhere in the graph.
The assertions check that --codegen=linker= and the --sysroot= link-arg
name the right file. They don't check the mapped bazel-out/cfg/... prefix
directly, since a test can't force --experimental_output_paths=strip on
itself — this repo's own "Path Mapping Linux/RBE/MacOS" CI jobs already
run the whole suite with that flag set, which is where that stronger
check happens.
Verified against this repo's full CI matrix — Ubuntu, RBE, macOS ARM64,
Windows, and every supported Bazel version down to the minimum (7.4.1) —
all green. Also verified locally: the assertions fail as expected when
the resolution logic is temporarily reverted (both for the linker and the
sysroot), and the fake linker's argument parsing was checked directly in
isolation.
Assisted-by: Claude (Anthropic)