Update cargo_build_script to work without runfiles support.
#2887
Conversation
This file contains 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
UebelAndre
force-pushed
the
cargo_runfiles
branch
2 times, most recently
from
7db0298 to
31cdc0f
Compare
|
@scentini @krasimirgg would love your thoughts on this as well if you have the time. |
illicitonion
approved these changes
Sep 19, 2024
UebelAndre
force-pushed
the
cargo_runfiles
branch
from
31cdc0f to
3d2b32b
Compare
This was referenced Sep 20, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 3, 2024
…2891) After #2887 I started finding similar linker errors in environments that supported runfiles and symlinks but have yet to see this issue in environments without them. I believe my understanding of how the [DefaultInfo.default_runfiles](https://bazel.build/rules/lib/providers/DefaultInfo#default_runfiles) objects worked and in the previous change had attempted to rely on it for a runfiles directory. However, there is no guarantee a directory will be rendered and no way to force it to be rendered for actions either (bazelbuild/bazel#15486). The consequence is that creating a custom runfiles directory is no longer a contingency and explicitly creating it is the correct thing to do to ensure `CARGO_MANIFEST_DIR` linker inputs are always available to the consuming actions. With the propagation of `CARGO_MANIFEST_DIR` (now an action output) into `Rustc` actions, a new flag is also introduced to prune unnecessary files out of the directory to reduce bloat and potential invalidation in downstream actions, `--@rules_rust//cargo/settings:cargo_manifest_dir_filename_suffixes_to_retain`. This flag accepts a list of path suffixes used to determine what (if anything) in `CARGO_MANIFEST_DIR` should be retained and passed to downstream actions. Note that the failure mode this addresses is hard to observe as it requires a crate which defines a linker path to something in `CARGO_MANIFEST_DIR` and then for a clean build to be done with a change to a dependent of said crate. If the runfiles directory for the build script is never rendered then there will be no file to link into the crate. --------- Co-authored-by: Daniel Wagner-Hall <dawagner@gmail.com>
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.
Partially addresses #1156
This pull-request implements an additional change to enable this behavior which aggregates all transitive
BuildInfo.compile_dataintoRustcactions. While this seems to bloat these actions with unnecessary data, it addresses a catastrophic flaw in how Windows works at all.As of Bazel 7.3.1, Windows does not run any actions in a sandbox (bazelbuild/bazel#18401), this means that references to the current working directory will be consistent since they always refer to the execroot. On top of this fact,
cargo_build_scriptwill assign CARGO_MANIFEST_DIR to a path within the runfiles directory of the build script. The combination of these two facts leads crates like windows_x86_64_msvc, which assign a linker path usingCARGO_MANIFEST_DIR(@windows_x86_64_msvc//build.rs) to introduce un-tracked dependencies into dependentRustcactions. This then leads to build failures if thewindows_x86_64_msvccrate is ever a remote cache hit for the dependents as runfiles (or.cargo_runfilesin the case of this PR) are not fetched and will not exist on the host at link time.This change addresses this issue of untracked dependencies by ensuring the runfiles of
cargo_build_script.scripttargets are aggregated intoRustcactions.