Update cargo_build_script to always render custom runfiles dirs.

[UebelAndre]

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 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.

[UebelAndre]

If this is going to exist long term then perhaps this directory could have symlinks generated into it whenever possible and the files from DefaultInfo.default_runfiles.files can be tracked in addition to the output directory. This would probably match runfiles directories more directly but I'm concerned about how remote caches will handle a directory of symlinks so will save such an optimization for another time.

[UebelAndre]

After soaking this change, I'm finding it leads to additional issues within the CargoBuildScriptRunfilesDir action. The actions will error with:

Failed to copy file 'bazel-out/**/_bs-.exe -> bazel-out/**/_bs.cargo_runfiles\*\_bs-.exe`
Os { code: 5, kind: PermissionDenied, message: "Access is denied." }

The error is somewhat confusing but when I manually inspect the filesystem on the worker I see very strangely:

dir bazel-out\**\

Mode            LastWriteTime          Length Name
----            -------------          ------ ----
d----l     9/19/2024 11:00 PM                 _bs-.exe
d----      ...
d----      ...
...

It appears the way symlinks are created confuse Windows into believing these are directories. When I inspect the target of the symlink I can see it's to the original build script:

(Get-Item _bs-.exe).Target

C:\bzl\**\_bs_.exe

However, the script is not on the machine. This is extra strange to me because while investigating I'd explicitly added the real build script to the runfiles of the cargo_build_script_runfiles rule and confirmed it's in script[DefaultInfo].default_runfiles.files but the file is never pulled from the remote cache for this action.

diff --git a/cargo/private/cargo_build_script.bzl b/cargo/private/cargo_build_script.bzl
index 28f91000..c6647ebe 100644
--- a/cargo/private/cargo_build_script.bzl
+++ b/cargo/private/cargo_build_script.bzl
@@ -48,7 +48,7 @@ def _cargo_build_script_runfiles_impl(ctx):

     # Tools are ommitted here because they should be within the `script`
     # attribute's runfiles.
-    runfiles = ctx.runfiles(files = ctx.files.data)
+    runfiles = ctx.runfiles(files = [script] + ctx.files.data)

     return [
         DefaultInfo(

This very much feels like a bug to me. If a depset of files is passed to an action, all files should be downloaded and made available for the action.

I'm not sure what to do with this change. It still feels more correct but I do not know how to handle this case and the failure mode of the CargoBuildScriptRunfilesDir action is a lot more frequent then the broken linking from what I've gathered so far.

[UebelAndre]

I'm not sure what to do with this change. It still feels more correct but I do not know how to handle this case and the failure mode of the CargoBuildScriptRunfilesDir action is a lot more frequent then the broken linking from what I've gathered so far.

I've decided to introduce an incompatibility flag to enable this behavior by default and a new flag for pruning the generated CARGO_MANIFEST_DIR to try and reduce the negative impact of always passing the directory on to downstream actions. This feels like the right thing to do given the state of runfiles.

[illicitonion]

This feels quite complicated, but I can see how we ended up here... Thanks for tracking it down!