rustc: include proc-macro dep transitive rlibs in proc-macro link inputs#29
Conversation
This reverts commit 59a507e.
Add support for tier 3 targets bpfeb-unknown-none and bpfel-unknown-none (see https://github.com/rust-lang/rust/blob/f5e2df7/src/doc/rustc/src/platform-support.md?plain=1#L311-L312). This is modeled after bazelbuild#3507 and should probably be updated if/when bazelbuild/platforms#131 is merged. (please use rebase merge when landing this as the proper commit message is in the commit, rather than the PR description) /cc @avrabe
…elbuild#3829)" This reverts commit f198dde.
…() (bazelbuild#3816)" This reverts commit 9586468.
…walter-zeromatter#3) * 0 * Add rust analyzer test coverage
…hollow rlibs: the RustcMetadata action runs rustc to completion with -Zno-codegen, emitting a .rlib archive. This approach mirrors the one used by buck2 and avoids needing to kill rustc mid-output in order to produce metadata. While not fixing problems with SVH mismatches when non-determinism, this does simplify the codepath and uses a production tested technique that doesn't have any of the dangers associated with killing the rustc process while it's still active.
Port the sharding wrapper feature from bazelbuild#3774 into the hermeticbuild fork. The implementation wraps rust_test executables when experimental_enable_sharding is set while keeping rustc_compile_action's existing provider-list API for internal and extension callers. rust_test now scans the returned providers to replace DefaultInfo for the wrapper, so extensions such as prost and wasm-bindgen continue to consume rustc_compile_action without API churn. Co-authored-by: Brian Duff <bduff@linkedin.com> Co-authored-by: Codex <noreply@openai.com>
Rustc emits GNU-like Windows staticlibs as lib<crate>.a, but rules_rust was stripping the lib prefix for all Windows non-rlib library outputs. Keep the prefix for staticlib outputs when the target ABI is gnu or gnullvm so declared outputs match rustc.
bazelbuild#25) rust_shared_library links the cc toolchain's C++ runtime (libstdc++/libc++ and the unwinder) dynamically, so it becomes an external DT_NEEDED on the produced shared library. When shipping a self-contained .so to a consumer that uses a different C++ stdlib, that dynamic dependency is undesirable. Add a `cc_runtime_linkage` attribute on rust_shared_library (dynamic|static, default dynamic so existing behavior is unchanged). When static, the C++ runtime is linked via cc_toolchain.static_runtime_lib so it is embedded in the shared library and no libstdc++/libc++/libunwind DT_NEEDED is emitted. get_cc_toolchain_runtime_libs gains a `linkage` parameter resolved from the attribute. Non-shared crate types are unchanged.
Add documentation extraction targets for the public cargo and rust Starlark packages. Declare the bazel_features, selects, and cc_debug_helper_bzl dependencies required by those targets.
When building a proc-macro, rustc passes all transitively-reachable rlibs to the linker (clang++). This includes rlibs that come through proc-macro dependency chains, even though those rlibs are already embedded in their parent proc-macro .so files. Under --remote_download_outputs=minimal, Bazel only downloads declared action inputs. The missing rlibs cause clang++ to fail with "no such file or directory" because they weren't declared as action inputs. Add a new `transitive_proc_macro_dep_outputs` field to DepInfo. Populate it with the transitive_crate_outputs of proc-macro deps (the ones skipped by the existing `if not is_proc_macro` guard). In collect_inputs, include this depset in the action inputs when building a proc-macro target.
|
Pushed follow-up Validated with |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 729e90b38c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| transitive_crate_outputs.append(dep_info.transitive_crate_outputs) | ||
| else: | ||
| transitive_proc_macro_dep_outputs.append(dep_info.transitive_crate_outputs) | ||
| transitive_proc_macro_dep_outputs.append(dep_info.transitive_proc_macro_dep_outputs) |
There was a problem hiding this comment.
DepInfo is part of the public rust_common API for custom rules, so existing providers constructed before this new field will leave transitive_proc_macro_dep_outputs unset. When such a proc-macro dependency is collected, this appends None into the depset(transitive=...) built below and analysis fails even if the dependency has no nested proc-macro outputs; treat a missing/None value as an empty depset before appending it.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7aee4c9473
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| direct_crate_outputs.append(crate_info.output) | ||
| if not is_proc_macro: | ||
| transitive_crate_outputs.append(dep_info.transitive_crate_outputs) | ||
| transitive_crate_outputs.append(dep_info.transitive_crate_outputs) |
There was a problem hiding this comment.
For a chain like bin -> lib_a where lib_a only uses proc-macro M at compile time and M -> rlib_R, this line now stores rlib_R in lib_a's transitive_crate_outputs. Any downstream non-proc-macro target that depends on lib_a consumes that depset as action inputs, so hidden exec-configured macro implementation rlibs are built/downloaded and included in action keys even though they are not command-line deps and are not needed after lib_a was compiled. Track these proc-macro-closure outputs separately and merge them only for the proc-macro crate action that actually needs them.
Useful? React with 👍 / 👎.
|
Chatted offline - we think this is a bug in the remote executor |
9 participants
Problem
collect_depsstops normaltransitive_crate_outputspropagation at proc-macro dependency edges. That is important because ordinary downstream targets should not inherit implementation rlibs used only to build or load compile-time proc macros.However, when the crate currently being compiled is itself a proc macro, rustc may need the crate-output closure of its direct proc-macro dependencies available in the sandbox. Under
--remote_download_outputs=minimal, a CI worker only materializes files that are declared action inputs, so a hidden proc-macro dependency rlib can be remote-only unless it is declared on that proc-macro compile action.The original CI failure was reported as a missing
.rlibduring proc-macro linking. Attempts to reproduce that exact linker-consumption path have been mixed, so this PR should be understood as a scoped Bazel action-input fix for proc-macro crate actions under minimal remote download, not as proof that every hidden proc-macro-chain.rlibis consumed by every downstream linker action.Behavior
This PR preserves the important split:
--externarguments.bin -> lib_a -> proc_macro -> rlib_R.Fix
Keep
DepInfo.transitive_crate_outputsscoped to direct crate outputs plus crate outputs reachable through non-proc-macro dependency chains.When collecting inputs for the main
Rustcaction of a proc-macro crate, merge in each direct proc-macro dependency's existingdep_info.transitive_crate_outputs. This avoids persistent provider pollution while declaring the extra inputs only on the proc-macro action that may need them.Changes
rust/private/rustc.bzl: restore the proc-macro edge stop incollect_deps.rust/private/rustc.bzl: add a scopedcollect_inputsoption used only by the main Rustc path for proc-macro crates.rust/private/providers.bzl: document thattransitive_crate_outputsdoes not recursively cross proc-macro dependency chains.test/unit/proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl: add analysis-test coverage for both sides:test/unit/proc_macro/leaks_deps/cargo_artifact_closure.rsandtest/unit/proc_macro/leaks_deps/lib/a_user.rs: add fixtures for the normal-dependent no-leak cases.The earlier
transitive_proc_macro_dep_outputsprovider field and recursive proc-macro propagation have been removed.Validation
Passed locally:
Additional action-shape checks:
//test/unit/proc_macro:proc_macro_uses_proc_macrohaslibproc_macro_dep-*.rlibas a Rustc action input.//test/unit/proc_macro:cargo_artifact_closurehas no Rustc action input matchinglibproc_macro_dep.//test/unit/proc_macro:a_userhas no Rustc action input matchinglibb.