Features from build dependencies aren't propagated to normal dependencies

[alexkirsz]

After #2877 (350d249) was merged, we've started seeing an error in our compilation pipeline where a crate's build script would be built with a feature, but the crate itself wouldn't.

Here's a minimal repro case: https://github.com/alexkirsz/rules_rust_build_dep_repro

If you run cargo build --target=aarch64-apple-ios --verbose, you'll see that cargo builds mime_guess on both the host platform (aarch64-apple-darwin in my case) and the target platform (aarch64-apple-ios) with the "rev-mappings" feature enabled, even though it is theoretically only needed at compile-time on the host platform.

If you run bazel build -s --platforms=//:ios_arm64 //:mime_guess_test --keep_going, you'll see that rules_rust builds the build script of mime_guess with the "rev-mappings" feature enabled, but on the target platform, it builds mime_guess without the feature, resulting in a compilation error.

Instead, it should either:

1. Copy Cargo's behavior and always build with the complete set of features from normal and build deps; or
2. Build the build script twice for the host platform, once with the feature, and once without.

[sthornington]

I believe we are seeing this as well. I've been hacking away on a little fix for #2938 and noticed that, e.g., building something with a crate dependency on tokio was failing for lack of parking_lot, so it seems like the feature propagation might not be working not only for build scripts but also general dependencies?

[sthornington]

In my case I was able to fix the problem by changing a dependency on tokio to use the "full" feature set, but I am not sure why it was trying to build parts of it that needed parking_lot if that feature was not present ...

[cerisier]

After a bit of investigation, I believe #2877 is doing the right thing but now showcase something missing in cargo_bazel generate.

I had a look at the BUILD.bazel file generated for the mime_guess crate in the repro example and I believe this is where the problem comes from:

rust_library(
    name = "mime_guess",
    deps = [
        "@crates__mime_guess-2.0.5//:build_script_build",
        # ...
    ],
    crate_features = select({
        "@rules_rust//rust/platform:aarch64-apple-darwin": [
            "rev-mappings",  # aarch64-apple-darwin
        ],
        "//conditions:default": [],
    }),
    target_compatible_with = select({
        "@rules_rust//rust/platform:aarch64-apple-darwin": [],
        "@rules_rust//rust/platform:aarch64-apple-ios": [],
        "//conditions:default": ["@platforms//:incompatible"],
    }),
    # ...
)

cargo_build_script(
    name = "_bs",
    crate_features = select({
        "@rules_rust//rust/platform:aarch64-apple-darwin": [
            "rev-mappings",  # aarch64-apple-darwin
        ],
        "//conditions:default": [],
    }),
    # ...
)

alias(
    name = "build_script_build",
    actual = ":_bs",
    tags = ["manual"],
)

The crate_features of the build script is dependent on the target platform but the build script itself is always built in exec which will always give rev-mappings even when the build script is a dependency of mime_guess for aarch64-apple-ios.

The mime_guess is built twice, once for aarch64-apple-darwin and one for aarch64-apple-ios but the actual rust_binary target generated by the cargo_build_script wrapper macro, is being built with a cfg=exec via the cargo_build_script_runfiles rule.

rules_rust/cargo/private/cargo_build_script.bzl

Lines 95 to 106 in 7890b42

	implementation = _cargo_build_script_runfiles_impl,
	attrs = {
	"data": attr.label_list(
	doc = "Data required by the build script.",
	allow_files = True,
	),
	"script": attr.label(
	doc = "The binary script to run, generally a `rust_binary` target.",
	executable = True,
	mandatory = True,
	providers = [rust_common.crate_info],
	cfg = "exec",

So when building the mime_guess target for aarch64-apple-ios, its build_script_build target will always be built in exec (aarch64-apple-darwin) in our case and the crate_features select will select the features of aarch64-apple-darwin instead of aarch64-apple-ios.

One idea would be to have cargo bazel generate generate one build_script_build for each targets with the corresponding crate_features without a select, and use the select on platforms to select which build_script_build use.

cc @UebelAndre, if the explanation makes sense and the potential fix acceptable, I can try working on a PR following your guidance (unless it's an easy fix on your side 🙂).

[illicitonion]

We added fake resolve roots in #2749 for proc macros so that they would get resolved for each platform we may run on. We could do something similar for deps which have build scripts.

That said, your proposal to have separate targets for "exec-like" features and "target-like" features feels like it would be an improvement. A hazard here is that I believe, because of the possibility of cross-compiling, we'd need to be careful around the platform-specific-ness of these things...