Propagate dependency features through build graph instead of relying on `crate_features`

[sin-ack]

Currently, the way features are enabled for the dependencies of a crate is to have them explicitly specified through crate_features. This is bad for multiple reasons:

• Crates will be built with unnecessary features, especially if a rules_rs-equipped Bazel module is used as a dependency of another.
• First-party crates must unconditionally enable all features that other crates depend on.
• A Bazel-declared crate not usable when it is consumed as a Bazel module dependency unless it enables all its features ahead-of-time, which makes compilation times worse for everything that doesn't use those features. (In our case we have a crate that can optionally enable egui-related features gated behind an "egui" feature, but this increases compilation times significantly for when that crate is used for a CLI tool, for example, slowing down iteration speed).

I think a better approach is:

1. During the lockfile resolution/repo generation phase, collect all features of a crate's dependencies into data.bzl.
2. Add something like dependency_features to rust_library which can be populated with a function like all_crate_features().

The only downside I can see usability-wise is that this necessitates a workspace-hack crate similar to regular Cargo if one wants to avoid multiple copies of the same crate with different features. Another problem is how to encode the final artifact since multiple artifacts can be generated from the same rust_library target with different feature combinations.

Since rust_library etc. machinery seems to be taken directly from rules_rust this might not be a trivial change (and it breaks parity with upstream, even if we're using a fork), so I understand if it's not feasible to do this at this stage.

[dzbarsky]

I've thought a bit in the past about potentially modeling these via transitions/flags, but haven't had any great ideas. I'm not sure I understood your proposal though, how is it different from the current set of features that are encoded into data.bzl?

Ultimately not doing feature unification does lead to an explosion in the build graph, I think whenever possible it's better to split crates instead of using features, at least for first-party code.

[csmoe]

I've run into a similar problem. My project is a workspace with nearly 5 million lines of code, 300+ sub-crates, and tons of features. The final artifacts are sdk-foo and sdk-bar, each compiled as binaries through their own feature combinations. Since Bazel mandates that the DAG must be fully determined before build execution, I was forced to hack rules_rs
by adding root_package and root_package_features to crate.from_cargo:

  crate.from_cargo(
      name = "crates_foo",
      cargo_lock = "//:Cargo.lock",
      cargo_toml = "//:Cargo.toml",
      root_package = "//:sdk-foo/Cargo.toml",
      root_package_features = ["a", "b", "c"],
  )
  crate.from_cargo(
      name = "crates_bar",
      cargo_lock = "//:Cargo.lock",
      cargo_toml = "//:Cargo.toml",
      root_package = "//:sdk-bar/Cargo.toml",
      root_package_features = ["a", "b"],
  )

Using rust_library("sdk-foo", crate_features = ["a", "b", "c"]) alone doesn't help, because by the time the features are opened at that point it's already too late — the features won't propagate into the DAG resolution phase.

As I dug deeper into using Bazel, I came to realize that — at least for Rust projects — Buck2's design philosophy is far superior to Bazel's. The phase-based build model causes the entire pipeline to completely lose its dynamism

[dzbarsky]

@csmoe I'm not very familiar with buck2, can you help me understand what it does differently here? The features attribute described at https://buck2.build/docs/prelude/rules/rust/rust_library/ makes it sound like they only control the rustc command line, not resolution?

[csmoe]

@dzbarsky EDIT: my bad.

By the way do you have any suggestions for my case? Multiple crate.from_cargo seems a bit slow given the huge workspace.

[dzbarsky]

@dzbarsky EDIT: my bad.

By the way do you have any suggestions for my case? Multiple crate.from_cargo seems a bit slow given the huge workspace.

I'm not sure what you are experiencing but if you can make a repro I'm happy to look. FWIW internally we have a few thousand first party crates and 20ish crate.from_cargo, with a populated lockfile the extension runs in a few hundred milliseconds. If you have a repro please file a separate issue so we don't hijack this one :)

[sin-ack]

how is it different from the current set of features that are encoded into data.bzl?

🤦 I'm not sure how I missed those, I think it's because I saw the crate_features encoded into the generated targets for third-party crates and assumed it was all like that. That makes for an even stronger argument for bringing in something like an all_crate_features() though, IMO.

Ultimately not doing feature unification does lead to an explosion in the build graph

That's true, and is quite unfortunate. The main issue that I'm trying to solve here is "open-ended" targets where a Bazel target in a module may be used by targets not declared within that module. I think an alternative approach could be to still do feature unification, but instead go for an approach like #144 and #145 (comment) where multiple modules could contribute to features enabled for a crate. That would solve the original problem in the issue :)

I think whenever possible it's better to split crates instead of using features, at least for first-party code.

I would like to agree, but due to orphaning rules this is sometimes not feasible (e.g. a feature enables a critical trait implementation that can only exist within that crate's CU), and it also breaks with Rust convention. An additional example is a crate that can work with no_std but default-enables a std feature; there's no good way to expose this with Bazel except for having manual targets of //my/crate:crate and //my/crate:crate_no_std.

[dzbarsky]

how is it different from the current set of features that are encoded into data.bzl?

🤦 I'm not sure how I missed those, I think it's because I saw the crate_features encoded into the generated targets for third-party crates and assumed it was all like that. That makes for an even stronger argument for bringing in something like an all_crate_features() though, IMO.

Ultimately not doing feature unification does lead to an explosion in the build graph

That's true, and is quite unfortunate. The main issue that I'm trying to solve here is "open-ended" targets where a Bazel target in a module may be used by targets not declared within that module. I think an alternative approach could be to still do feature unification, but instead go for an approach like #144 and #145 (comment) where multiple modules could contribute to features enabled for a crate. That would solve the original problem in the issue :)

Like mentioned in the other PR, I think it's better to consume external repos via git URL in Cargo.toml so cargo can do this properly :/

I would like to agree, but due to orphaning rules this is sometimes not feasible (e.g. a feature enables a critical trait implementation that can only exist within that crate's CU), and it also breaks with Rust convention. An additional example is a crate that can work with no_std but default-enables a std feature; there's no good way to expose this with Bazel except for having manual targets of //my/crate:crate and //my/crate:crate_no_std.

Yeah, that's totally fair. For no_std specifically, perhaps you want to make your deps/features conditioned on https://github.com/hermeticbuild/rules_rust/blob/8a2219c1fcf2070120c26dc67eb8aeacfc5a3819/rust/settings/settings.bzl#L335-L337 via select?

[marvin-hansen]

Version:

• rules_rs 0.0.107 (bundled rules_rust 9ec1223),
• Bazel 9.2.0

I ran into a variation of this issue while migrating the deep causality mono repository to the rules_rs.
As a result, affected targets need to manually declare their bazel dependencies as a work around.

If anyone knows of a better solution, please let me know.

Reproduction

# lib_a/Cargo.toml
[features]
default = []          # the gating feature is NOT default
extra = ["dep:lib_b"]

[dependencies]
lib_b = { path = "../lib_b", optional = true }

# lib_a/BUILD.bazel
rust_library(
    name = "lib_a",
    crate_features = ["extra"],           # enabled here, not in Cargo's default set
    deps = all_crate_deps(normal = True),
)

Actual behaviour

error[E0432]: unresolved import `lib_b`

The edge exists, but nothing links it:

$ bazel query 'kind("rust_library", deps(//lib_a:lib_a, 1))'
//lib_b:lib_b                                   # present

$ bazel aquery 'mnemonic("Rustc", //lib_a:lib_a)' | grep -c 'extern=lib_b'
0                                               # never linked

Declaring it explicitly is rejected:

ERROR: Label '//lib_b:lib_b' is duplicated in the 'deps' attribute of rule 'lib_a'

The dependency is therefore unusable: not linked, and not re-declarable. The only workaround is to
abandon all_crate_deps for that target and hand-write the full dependency list.

Expected behaviour

Either would be fine, but the two cases should agree:

1. Omit the label, as already happens for external optional deps. @crates//:libm behind a
   non-default feature is absent from all_crate_deps, so naming it alongside the call works.
2. Emit the --extern for it.

The asymmetry

External and first-party optional dependencies behave differently today:

Optional dep behind a non-default feature	In all_crate_deps?	--extern emitted?	Can be added manually?
External (@crates//:libm)	no	n/a	yes
First-party (//lib_b)	yes	no	no — duplicate label