How to combine features, platform and dependencies ?

[davidB]

Hi,

I used crates where the same dependencies is defined with different "feature" for different target platform.
eg in Cargo.toml of bevy_winit

[features]
wayland = ["winit/wayland"]
x11 = ["winit/x11"]

[dependencies]
# bevy
bevy_app = { path = "../bevy_app", version = "0.4.0" }
bevy_ecs = { path = "../bevy_ecs", version = "0.4.0" }
bevy_input = { path = "../bevy_input", version = "0.4.0" }
bevy_math = { path = "../bevy_math", version = "0.4.0" }
bevy_window = { path = "../bevy_window", version = "0.4.0" }
bevy_utils = { path = "../bevy_utils", version = "0.4.0" }

# other
winit = { version = "0.24.0", default-features = false }

[target.'cfg(target_arch = "wasm32")'.dependencies]
winit = { version = "0.24.0", features = ["web-sys"], default-features = false }
wasm-bindgen = { version = "0.2" }
web-sys = "0.3"

become

rust_library(
    name = "bevy_winit",
    srcs = glob(["**/*.rs"]),
    aliases = {
    },
    crate_features = [
    ],
    crate_root = "src/lib.rs",
    crate_type = "lib",
    data = [],
    edition = "2018",
    rustc_flags = [
        "--cap-lints=allow",
    ],
    tags = [
        "cargo-raze",
        "manual",
    ],
    version = "0.4.0",
    # buildifier: leave-alone
    deps = [
        "@raze__bevy_app__0_4_0//:bevy_app",
        "@raze__bevy_ecs__0_4_0//:bevy_ecs",
        "@raze__bevy_input__0_4_0//:bevy_input",
        "@raze__bevy_math__0_4_0//:bevy_math",
        "@raze__bevy_utils__0_4_0//:bevy_utils",
        "@raze__bevy_window__0_4_0//:bevy_window",
        "@raze__winit__0_24_0//:winit",
    ] + selects.with_or({
        # cfg(target_arch = "wasm32")
        (
            "@io_bazel_rules_rust//rust/platform:wasm32-unknown-unknown",
            "@io_bazel_rules_rust//rust/platform:wasm32-wasi",
        ): [
            "@raze__wasm_bindgen__0_2_68//:wasm_bindgen",
            "@raze__web_sys__0_3_45//:web_sys",
            "@raze__winit__0_24_0//:winit",
        ],
        "//conditions:default": [],
    }),
)

So it failed because:

• on bazel side because winit is listed twice:

  ERROR: /home/david/.cache/bazel/_bazel_david/1570ea1cacebfda74002c263f99cf611/external/raze__bevy_winit__0_4_0/BUILD.bazel:33:13: Label '@raze__winit__0_24_0//:winit' is duplicated in the 'deps' attribute of rule 'bevy_winit'

• on runtime (I guess because expected features will not be enabled)

If anyone have a immediate solution, please share. Else I would like to know/discuss how this kind of behavior could be translated into bazel (and later implemented into cargo-raze, I could try to make a PR) ?

• idea: adding some kind of select into the generation of crate_features ?

[UebelAndre]

Hey, hope this hasn't been a blocker for you. I'm not entirely sure how to solve this. Have you had more time to think about the idea you had?

• idea: adding some kind of select into the generation of crate_features ?

Very interested in how this might be solved 😄

[davidB]

No I didn't find a good enough idea to translate this behavior into bazel, except maybe creating a custom function that generate the features & dependencies list instead of using select. (I'll see if it's doable after vacations, I'm new to bazel & starlack so I don't know if it's doable and how).

Currently in my "test" project I call cargo from bazel (far to be optimal :-(, because everything (code + deps) is rebuild each time)
extract from https://github.com/davidB/ld47_keep_inside/blob/main/game/BUILD.bazel

genrule(
    name = "wasm",
    srcs = glob([
        "Cargo.*",
        "src/*.rs",
    ]),
    outs = ["game.wasm"],
    cmd_bash = """
    pushd game
    cargo build --features web --target wasm32-unknown-unknown --release
    popd
    cp game/target/wasm32-unknown-unknown/release/game.wasm $@
    """,
)

[UebelAndre]

Could I get you to move this issue to rules_rust 🙏? I think it represents either a gap in functionality or a lack of clarity in how features are expected to interact with dependencies. If there's an expected pattern for these situation then I don't think it'll be too difficult to update cargo-raze to handle them.

[UebelAndre]

Oooooh, I think I see what's going on here. winit is a dependency of bevy_winit but when building for the cfg(target_arch = "wasm32") target, bevy_winit wants winit with the "web-sys" feature.

That is an interesting one for sure. When rendered, the winit crate does include that feature (though I haven't investigated why exactly)

rust_library(
    name = "winit",
    srcs = glob(["**/*.rs"]),
    aliases = {
        "@raze__web_sys__0_3_45//:web_sys": "web_sys",
    },
    crate_features = [
        "default",
        "mio",
        "mio-extras",
        "parking_lot",
        "percent-encoding",
        "sctk",
        "wasm-bindgen",
        "wayland",
        "wayland-client",
        "web-sys",
        "web_sys",
        "x11",
        "x11-dl",
    ],
    crate_root = "src/lib.rs",
    crate_type = "lib",
    data = [],
    edition = "2018",
    rustc_flags = [
        "--cap-lints=allow",
    ],
    tags = [
        "cargo-raze",
        "manual",
    ],
    version = "0.24.0",
    # buildifier: leave-alone
    deps = [
        "@raze__bitflags__1_2_1//:bitflags",
        "@raze__instant__0_1_9//:instant",
        "@raze__lazy_static__1_4_0//:lazy_static",
        "@raze__libc__0_2_81//:libc",
        "@raze__log__0_4_11//:log",
        "@raze__raw_window_handle__0_3_3//:raw_window_handle",
    ] + selects.with_or({
        # cfg(target_arch = "wasm32")
        (
            "@io_bazel_rules_rust//rust/platform:wasm32-unknown-unknown",
            "@io_bazel_rules_rust//rust/platform:wasm32-wasi",
        ): [
            "@raze__wasm_bindgen__0_2_68//:wasm_bindgen",
            "@raze__web_sys__0_3_45//:web_sys",
        ],
        "//conditions:default": [],
    }),
)

Now I'm wondering if this can be solved with what @davidB was saying in the first post, a select statement in crate_features or if it's correct/acceptable to collect all required features and discard these kinds of conditional dependencies when there's already one specified for the "default" dependencies in deps.

I think my earlier comment is irrelevant now. But I'm not sure how best to solve this and would love some input here. How does cargo handle this?

[davidB]

I did some search, and read the interesting thread Add cargo_crate repository rule · Issue #2 · bazelbuild/rules_rust . And I didn't found a response to the following question (that is also valid for a full mono-repo) :

How to express for a single package different result that correspond to the enabling of "features set" ?

Until we'll be able to identify a target that correspond to a package with features X,Y; we'll not be able to include it into a deps attributes (and solve the problem cleanly)

"cleany" because:

• features are made to allow to disable useless code & dependencies, so the "enable all" is not a solution.
• I guess today, code generated by cargo-raze only provide BUILD with the single target "rust_library" that enable "all" features and all dependencies, that can add issue (bug, security, size) into the final app (eg include useless, un-secure, experimental code & dependencies). and maybe build issue for code that doesn't follow the rules "every features could be enabled" and use features to select one implementation of a code vs an other (eg openssl vs rustls backend,...).

If you have pointer, idea, I'm interested in

[davidB]

If I understand correctly https://github.com/google/cargo-raze/blob/master/impl/src/rendering/templates/partials/build_script.template every features are enabled by only dependencies from "default" features are included. Maybe a bug ?

[UebelAndre]

* features are made to allow to disable useless code & dependencies, so the "enable all" is not a solution.

Maybe I misunderstand this but when cargo resolves a build graph and finds that there are multiple dependents that require different features of a specific dependencies, the dependency is built once with all those features enabled.

Other use cases can be found here: https://doc.rust-lang.org/cargo/reference/unstable.html#features

[UebelAndre]

If I understand correctly https://github.com/google/cargo-raze/blob/master/impl/src/rendering/templates/partials/build_script.template every features are enabled by only dependencies from "default" features are included. Maybe a bug ?

It looks like that list is from a Node in a resolved dependency graph which would only include necessary features:

cargo-raze/impl/src/planning/subplanners.rs

Line 388 in 00fc4bb

features: self.node.features.clone(),

[davidB]

the dependency is built once with all those features enabled.

Not all the features but the addition of features set requested, and cargo can choose to build a transitive crate several times, it allows to have several version of a the same crate in the dependency graph (vs other dependency resolver)

About the code resolved dependency graph, I'll take a deeper look, but cargo-raze seems to use cargo-metadata that do the resolution based on the list of requested top level features (so once again only have one deps for all features set on transitive deps will not work)

extract from cargo_metadata doc:

let _metadata = MetadataCommand::new()
    .manifest_path("./Cargo.toml")
    .features(CargoOpt::AllFeatures)
    .exec()
    .unwrap();

Maybe I'm wrong, I don't know the internal of cargo-raze (I'll look into), just intuition based on the output.

[UebelAndre]

Maybe I'm wrong, I don't know the internal of cargo-raze (I'll look into), just intuition based on the output.

It currently does use a list of crates for it's generation. There's #282 which tracks work on updating cargo-raze to instead use a resolve graph. In my comments before, I was specifically referring to the features vec being rendered into the BUILD files. Maybe completing that PR would also fix this issue 🤔

[davidB]

I'll take a look to this issue,
Anyway, I don't see how a single target/label can be used to generate multiple output (different features set). in my mind (and I guess in bazel spirit) different features set => differents output => differents target/label.

[UebelAndre]

I guess in bazel spirit) different features set => differents output => differents target/label.

This is something I've been meaning to dig into deeper after I opened bazelbuild/rules_rust#405. But in my experience with Bazel, I believe that statement is true in general. The way around this is likely related to transitions and some wrapper rule for your targets. But I may not be aware of better ways of doing this.

[ttiurani]

I bumped into this with value-bag which is a dependency of log and has a complex build.rs which

• For non-WASM, i.e. cargo build , generates a rustc with --cfg value_bag_capture_ctor
• But for WASM, i.e. cargo build --target=wasm32-unknown-unknown , generates --cfg value_bag_capture_fallback

Now I don't think this is possible to do with current cargo raze, so I did a quick hack showing how this could look like:

ttiurani@07c775b

Any chances to add these kinds of escape hatches to cargo raze? I'd presume this would be useful for features too.