Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update cargo_build_script to work without runfiles support. #2887

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

UebelAndre
Copy link
Collaborator

@UebelAndre UebelAndre commented Sep 19, 2024

Partially addresses #1156

This pull-request implements an additional change to enable this behavior which aggregates all transitive BuildInfo.compile_data into Rustc actions. 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_script will 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 using CARGO_MANIFEST_DIR (@windows_x86_64_msvc//build.rs) to introduce un-tracked dependencies into dependent Rustc actions. This then leads to build failures if the windows_x86_64_msvc crate is ever a remote cache hit for the dependents as runfiles (or .cargo_runfiles in 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.script targets are aggregated into Rustc actions.

@UebelAndre UebelAndre force-pushed the cargo_runfiles branch 2 times, most recently from 7db0298 to 31cdc0f Compare September 19, 2024 07:43
@UebelAndre UebelAndre marked this pull request as ready for review September 19, 2024 07:51
@UebelAndre
Copy link
Collaborator Author

@scentini @krasimirgg would love your thoughts on this as well if you have the time.

@UebelAndre UebelAndre added this pull request to the merge queue Sep 19, 2024
Merged via the queue into bazelbuild:main with commit 59464fe Sep 19, 2024
4 checks passed
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.

2 participants