bazelbuild / rules_cc

C++ Rules for Bazel
https://bazel.build
Apache License 2.0
182 stars 90 forks source link

rule-based toolchains: Can't enable sentinel features #233

Closed armandomontanez closed 3 weeks ago

armandomontanez commented 1 month ago

In Pigweed, we have a feature like this:

# This is a sentinel feature defined by rules_rust. It is by definition
# unsupported: rules_rust will disable this feature when linking Rust code.
cc_feature(
    name = "rules_rust_unsupported_feature",
    feature_name = "rules_rust_unsupported_feature",
)

This is a feature that should default to enabled when added to the toolchain, and then dynamically be disabled by Rust rules to signal toolchain flags that should be flipped for Rust interop. When the enabled attribute was removed from cc_feature, the only way to enable a feature by default became via implies (which is what is used by cc_toolchain's enabled_features attribute), which produces the following error:

$ bazel build --override_module=rules_cc=~/development/projects/bazel/rules_cc/ //pw_status/...
ERROR: /Users/amontanez/development/projects/pigweed/pigweed/pw_toolchain/rust/BUILD.bazel:22:34: in rust_toolchain rule //pw_toolchain/rust:host_rust_toolchain_macos_x86_64_stable_rust_toolchain:
Traceback (most recent call last):
    File "/private/var/tmp/_bazel_amontanez/e724b21efc8bc19866072fbc72ee5907/external/rules_rust~/rust/toolchain.bzl", line 640, column 72, in _rust_toolchain_impl
        libstd_and_allocator_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, rust_std, ctx.attr.allocator_library, "std"),
    File "/private/var/tmp/_bazel_amontanez/e724b21efc8bc19866072fbc72ee5907/external/rules_rust~/rust/toolchain.bzl", line 155, column 60, in _make_libstd_and_allocator_ccinfo
        cc_toolchain, feature_configuration = find_cc_toolchain(ctx)
    File "/private/var/tmp/_bazel_amontanez/e724b21efc8bc19866072fbc72ee5907/external/rules_rust~/rust/private/utils.bzl", line 55, column 57, in find_cc_toolchain
        feature_configuration = cc_common.configure_features(
    File "/virtual_builtins_bzl/common/cc/cc_common.bzl", line 179, column 49, in _configure_features
Error in configure_features: The C++ toolchain '//pw_toolchain/host_clang:host_toolchain' unconditionally implies feature 'rules_rust_unsupported_feature', which is unsupported by this rule. This is most likely a misconfiguration in the C++ toolchain.
armandomontanez commented 1 month ago

Either:

I'm not sure I fully understand cc_toolchain.enabled_features. Since it unconditionally enables a feature with no escape hatch, wouldn't the right pattern be to express the flags in that feature directly in cc_toolchain.args?

matts1 commented 1 month ago

Adding something to cc_toolchain.enabled_features was intended to be functionally equivalent to cc_feature(..., enabled = True).

I used implies instead of changing cc_feature.enabled, because that was easier to implement, and I thought it was equivalent because I didn't consider this particular use case.

FWIW, this should be a relatively simple fix - it should just be a change to legacy_converter.bzl.