Unable to skip cargo_build_script on incompatible platform

[gpanders]

Problem

The cargo_build_script macro does not respect target_compatible_with when build_manual_tests is enabled.

Description

In our workspace we have build --build_manual_tests set in our .bazelrc. We have a Rust target that is Linux only, that depends on files generated via a build script. This build script is also only supported on Linux.

Adding target_compatible_with = ["@platforms//os:linux"] to the main rust_binary target works to skip building the binary on non-Linux platforms (macOS in my case), but it does not work for the cargo_build_script rule. That is, the cargo_build_script target still runs (and fails) on macOS.

As far as I can tell what is happening is that the cargo_build_script rule creates its own rust_binary, but target_compatible_with is not forwarded to that binary. I've applied the following patch which seems to resolve the issue, though I'm not sure if it's the proper fix:

diff --git a/cargo/private/cargo_build_script_wrapper.bzl b/cargo/private/cargo_build_script_wrapper.bzl
index a7295b80d..38174f091 100644
--- a/cargo/private/cargo_build_script_wrapper.bzl
+++ b/cargo/private/cargo_build_script_wrapper.bzl
@@ -151,7 +151,7 @@ def cargo_build_script(
         rustc_env["CARGO_CRATE_NAME"] = name_to_crate_name(name_to_pkg_name(name))
 
     script_kwargs = {}
-    for arg in ("exec_compatible_with", "testonly"):
+    for arg in ("exec_compatible_with", "testonly", "target_compatible_with"):
         if arg in kwargs:
             script_kwargs[arg] = kwargs[arg]
 

I also tried setting exec_compatible_with on the cargo_build_script rule, but that fails for a different reason (Unable to find an execution platform for target platform @@platforms//host:host from available execution platforms []).

Below is a simplified version of our BUILD file. It's not an exact reproducer of course since it doesn't include a build.rs file. I can work on putting together a full reproduction, but first wanted to make sure that I wasn't missing something obvious about how to do this correctly, or if the patch above is indeed necessary.

Thanks!

cargo_build_script(
    name = "my_build_script",
    srcs = [
        "build.rs",
    ],
    target_compatible_with = ["@platforms//os:linux"],
    visibility = ["//visibility:private"],
    deps = all_crate_deps(
        build = True,
    ),
)

rust_binary(
    name = "my_binary",
    srcs = [
        # ...
    ],
    aliases = aliases(),
    proc_macro_deps = all_crate_deps(
        proc_macro = True,
    ),
    stamp = True,
    target_compatible_with = ["@platforms//os:linux"],
    visibility = ["//visibility:public"],
    deps = [
        ":my_build_script",
    ] + all_crate_deps(
        build = True,
        normal = True,
    ),
)

[gpanders]

Actually this seems to be caused by build --build_manual_tests in our .bazelrc file. Here is a reproduction https://github.com/gpanders/rules_rust_repro

[UebelAndre]

I think something like the following diff would solve the issue but I'm not confident about the impact of exec_compatible_with being dropped from the actual/internal cargo_build_script target.

diff --git a/cargo/private/cargo_build_script_wrapper.bzl b/cargo/private/cargo_build_script_wrapper.bzl
index a7295b80d..1591febdf 100644
--- a/cargo/private/cargo_build_script_wrapper.bzl
+++ b/cargo/private/cargo_build_script_wrapper.bzl
@@ -150,8 +150,10 @@ def cargo_build_script(
     if "CARGO_CRATE_NAME" not in rustc_env:
         rustc_env["CARGO_CRATE_NAME"] = name_to_crate_name(name_to_pkg_name(name))

+    exec_compatible_with = kwargs.pop("exec_compatible_with", None)
+
     script_kwargs = {}
-    for arg in ("exec_compatible_with", "testonly"):
+    for arg in ("testonly",):
         if arg in kwargs:
             script_kwargs[arg] = kwargs[arg]

@@ -160,6 +162,11 @@ def cargo_build_script(
         if arg in kwargs:
             wrapper_kwargs[arg] = kwargs[arg]

+    # The script is always run in an `exec` configuration so whenever the `cargo_build_script` is given
+    # an `exec_compatible_with`, for this target it is translated to `target_compatible_with` to match
+    # the `cfg = "exec"` consumption of the target.
+    script_kwargs["target_compatible_with"] = exec_compatible_with
+
     binary_tags = depset(
         (tags if tags else []) + ["manual"],
     ).to_list()

@krasimirgg or @fmeum are you familiar with exec_compatible_with or do you know of a good place to read about it's expectations?

[fmeum]

That's pretty smart, but in a good way. Are users likely to use exec_compatible_with to steer the execution of a cargo_build_script? This attribute just filters out exec platforms without these constraints for the toolchain resolution performed for the default exec group.

[UebelAndre]

That's pretty smart, but in a good way. Are users likely to use exec_compatible_with to steer the execution of a cargo_build_script? This attribute just filters out exec platforms without these constraints for the toolchain resolution performed for the default exec group.

Haha thanks <3. Do you know if there is a reason exec_compatible_with is not configurable? My first attempt was to set

exec_compatible_with = select({
    "@platforms//os:linux": [],
    "//conditions:default": ["@platforms//:incompatible],
})

For this I was expecting the select to be resolved behind the exec configuration and then skip building but instead it's not configurable and yeah, I ran into toolchain resolution errors when I just set

exec_compatible_with = ["@platforms//os:linux"]

[fmeum]

I don't think there's any fundamental limitation at play here, but I think that this decision was made as toolchain resolution already provides a way to limit the selection of execution platforms based on the target platform and configuration. Allowing the additional constraints for the exec platform to depend on the target configuration would introduce yet another layer of indirection.

[UebelAndre]

@gpanders if you wanna give that diff a try and find it works for you I'd be happy to review a pull-request

[gpanders]

I think something like the following diff would solve the issue but I'm not confident about the impact of exec_compatible_with being dropped from the actual/internal cargo_build_script target.

@UebelAndre Yes, applying this patch (and setting exec_compatible_with = ["@platforms//os:linux"] on the cargo_build_script) does work.