Skip to content

Commit

Permalink
Update cargo_build_script to always render custom runfiles dirs. (#…
Browse files Browse the repository at this point in the history
…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>
  • Loading branch information
UebelAndre and illicitonion authored Oct 3, 2024
1 parent ab50fce commit 5cb6121
Show file tree
Hide file tree
Showing 9 changed files with 507 additions and 350 deletions.
8 changes: 6 additions & 2 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,10 @@ tasks:
platform: windows
build_flags:
- "--noenable_runfiles"
- "--//cargo/settings:incompatible_runfiles_cargo_manifest_dir=true"
test_flags:
- "--noenable_runfiles"
- "--//cargo/settings:incompatible_runfiles_cargo_manifest_dir=true"
build_targets: *default_windows_no_runfiles_targets
test_targets: *default_windows_no_runfiles_targets
ubuntu2004_split_coverage_postprocessing:
Expand Down Expand Up @@ -202,10 +204,12 @@ tasks:
- "--noenable_runfiles"
- "--config=rustfmt"
- "--config=clippy"
- "--//cargo/settings:incompatible_runfiles_cargo_manifest_dir=true"
test_flags:
- "--noenable_runfiles"
- "--config=rustfmt"
- "--config=clippy"
- "--//cargo/settings:incompatible_runfiles_cargo_manifest_dir=true"
build_targets: *default_windows_no_runfiles_targets
test_targets: *default_windows_no_runfiles_targets
windows_rolling_with_aspects:
Expand Down Expand Up @@ -647,7 +651,7 @@ tasks:
environment:
# This ndk version matches with rules_android_ndk repo's CI
# https://github.com/bazelbuild/rules_android_ndk/blob/877c68ef34c9f3353028bf490d269230c1990483/.bazelci/presubmit.yml#L37
# The ndk is installed by this script
# The ndk is installed by this script
# https://github.com/bazelbuild/continuous-integration/blob/ba56013373821feadd9f2eaa6b81eb19528795f0/macos/mac-android.sh
ANDROID_NDK_HOME: /opt/android-ndk-r25b
android_examples_macos:
Expand All @@ -662,7 +666,7 @@ tasks:
environment:
# This ndk version matches with rules_android_ndk repo's CI
# https://github.com/bazelbuild/rules_android_ndk/blob/877c68ef34c9f3353028bf490d269230c1990483/.bazelci/presubmit.yml#L42
# The ndk is installed by this script
# The ndk is installed by this script
# https://github.com/bazelbuild/continuous-integration/blob/ba56013373821feadd9f2eaa6b81eb19528795f0/macos/mac-android.sh
ANDROID_NDK_HOME: /Users/buildkite/android-ndk-r25b
ios_examples:
Expand Down
Loading

0 comments on commit 5cb6121

Please sign in to comment.