Clippy aspect sometimes fails when run on an unsandboxed test and rustc_output_diagnostics is enabled

[william-smith-skydio]

I want to preface this by saying that the underlying issue isn't a rules_rust issue, but rather a bazel issue: bazelbuild/bazel#21474

If I have a setup with the following conditions:

• A rust target which is unsandboxed (e.g. a test with tags = ["no-sandbox"])
• --@rules_rust//:rustc_output_diagnostics=true is set
• --incompatible_allow_tags_propagation is enabled (default in bazel 7.x)

...and then I run the clippy aspect, I get spurious failures. I am currently seeing this in our internal CI and my temporary solution is to disable clippy for any unsandboxed tests.

Here's a gist with this setup: https://gist.github.com/william-smith-skydio/f7821eb732c539f878fdfb00986230db

If I build the test first, and then build the clippy target, I get a failure:

bazel build //:some_test
bazel build //:some_test.clippy

Error (with --verbose_failures):

ERROR: BUILD.bazel:4:10: Clippy test-383275658/some_test.clippy.ok failed: (Exit 1): process_wrapper failed: error executing Clippy command (from target //:some_test) 
  (cd /mnt/sdb1_data/bazel_cache/_bazel_williamsmith/0fa9b297dfe33f49fded1b1ed008f1ce/execroot/_main && \
  exec env - \
    CARGO_CFG_TARGET_ARCH=x86_64 \
    CARGO_CFG_TARGET_OS=linux \
    CARGO_CRATE_NAME=some_test \
    CARGO_MANIFEST_DIR='${pwd}' \
    CARGO_PKG_AUTHORS='' \
    CARGO_PKG_DESCRIPTION='' \
    CARGO_PKG_HOMEPAGE='' \
    CARGO_PKG_NAME=some_test \
    CARGO_PKG_VERSION=0.0.0 \
    CARGO_PKG_VERSION_MAJOR=0 \
    CARGO_PKG_VERSION_MINOR=0 \
    CARGO_PKG_VERSION_PATCH=0 \
    CARGO_PKG_VERSION_PRE='' \
    CLIPPY_CONF_DIR='${pwd}/external/rules_rust/tools/clippy' \
    ZERO_AR_DATE=1 \
  bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/rules_rust/util/process_wrapper/process_wrapper --subst 'pwd=${pwd}' --output-file bazel-out/k8-fastbuild/bin/test-383275658/some_test.rustc-output --touch-file bazel-out/k8-fastbuild/bin/test-383275658/some_test.clippy.ok -- bazel-out/k8-fastbuild/bin/external/rust_linux_x86_64__x86_64-unknown-linux-gnu__stable_tools/rust_toolchain/bin/clippy-driver some_test.rs '--crate-name=some_test' '--crate-type=bin' '--error-format=human' '--codegen=metadata=-383275658' '--codegen=extra-filename=-383275658' '--out-dir=bazel-out/k8-fastbuild/bin/test-383275658' '--codegen=opt-level=0' '--codegen=debuginfo=0' '--remap-path-prefix=${pwd}=' '--emit=dep-info,metadata' '--color=always' '--target=x86_64-unknown-linux-gnu' -L bazel-out/k8-fastbuild/bin/external/rust_linux_x86_64__x86_64-unknown-linux-gnu__stable_tools/rust_toolchain/lib/rustlib/x86_64-unknown-linux-gnu/lib '--edition=2021' '--sysroot=bazel-out/k8-fastbuild/bin/external/rust_linux_x86_64__x86_64-unknown-linux-gnu__stable_tools/rust_toolchain' --test -Dwarnings)
# Configuration: 18fb488c35bfbf2ad904ab0372587fb4b61e280579714ba9c4cf29a6d9ae274d
# Execution platform: @@local_config_platform//:host
Error: ProcessWrapperError("Unable to open output_file: Permission denied (os error 13)")
Target //:some_test.clippy failed to build

Thanks to the above-linked issue, in bazel 7.x, the action in the clippy aspect runs unsandboxed if the underlying target is unsandboxed. This in itself is not a problem, but it's not great. When combined with rustc_output_diagnostics (used for editor integration), the clippy aspect tries to write an output file (in this case bazel-out/k8-fastbuild/bin/test-383275658/some_test.rustc-output) which is also produced by the rustc compile action. That file is not a declared output of the action, so when sandboxing is enabled it doesn't cause any issues. But when sandboxing is disabled, if the file already exists (because the compile action already ran), writing the file will fail since bazel outputs have the write permission disabled.

I was able to work around this locally with the following patch to rules_rust:

diff --git a/rust/private/clippy.bzl b/rust/private/clippy.bzl
index 9fd9842c..6f5346f8 100644
--- a/rust/private/clippy.bzl
+++ b/rust/private/clippy.bzl
@@ -127,6 +127,7 @@ def _clippy_aspect_impl(target, ctx):
         build_flags_files = build_flags_files,
         emit = ["dep-info", "metadata"],
         skip_expanding_rustc_env = True,
+        no_rustc_output = True,
     )
 
     if crate_info.is_test:
diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl
index eff542eb..e26c562e 100644
--- a/rust/private/rustc.bzl
+++ b/rust/private/rustc.bzl
@@ -804,7 +804,8 @@ def construct_arguments(
         use_json_output = False,
         build_metadata = False,
         force_depend_on_objects = False,
-        skip_expanding_rustc_env = False):
+        skip_expanding_rustc_env = False,
+        no_rustc_output = False):
     """Builds an Args object containing common rustc flags
 
     Args:
@@ -945,9 +946,9 @@ def construct_arguments(
     if build_metadata:
         # Configure process_wrapper to terminate rustc when metadata are emitted
         process_wrapper_flags.add("--rustc-quit-on-rmeta", "true")
-        if crate_info.rustc_rmeta_output:
+        if crate_info.rustc_rmeta_output and not no_rustc_output:
             process_wrapper_flags.add("--output-file", crate_info.rustc_rmeta_output.path)
-    elif crate_info.rustc_output:
+    elif crate_info.rustc_output and not no_rustc_output:
         process_wrapper_flags.add("--output-file", crate_info.rustc_output.path)
 
     rustc_flags.add(error_format, format = "--error-format=%s")

But this doesn't seem like a particularly clean solution. I'm not sufficiently familiar with the code to know what a cleaner solution would look like. I understand that rustc_output_diagnostics is a pretty niche and unsupported feature at the moment, and I'm happy to keep using a workaround internally, but I figured I should share in case others run into the same issue.