bazelbuild / bazelbuild/rules_cc
Misleading toolchain API documentation for requires_true, requires_false
- Dominant language
- Starlark
- Stars
- 247
- Forks
- 196
- PR merge metrics
- No merged PRs in 30d
Description
While configuring a toolchain, we were trying to override the legacy `fully_static_link` feature to always be enabled, except when the variable `is_cc_test` is true or if the target needs runtime libraries.
The [documentation](https://github.com/bazelbuild/rules_cc/blob/01a1eced1d61670e3fe1310ccaaa45635aaf90b1/docs/toolchain_api.md#cc_args-requires_true) for `requires_true` (`requires_false`) says that the given variable will be checked for truthiness (falsiness). so we were trying to use them in the following setup:
```
cc_nested_args(
name = "if_not_cc_test",
args = ["-static"],
requires_false = "@rules_cc//cc/toolchains/variables:is_cc_test",
)
cc_args(
name = "fully_static_args",
actions = [
"@rules_cc//cc/toolchains/actions:link_actions",
],
nested = [":if_not_cc_test"],
requires_false = "@rules_cc//cc/toolchains/variables:runtime_library_search_directories",
)
cc_feature(
name = "fully_static_link",
args = [":fully_static_args"],
overrides = "@rules_cc//cc/toolchains/features/legacy:fully_static_link",
visibility = ["//visibility:public"],
)
```
We were expecting `requires_false = "@rules_cc//cc/toolchains/variables:runtime_library_search_directories"` to work, as Bazel's `expand_if_true`/`expand_if_false` [evaluate an empty sequence](https://github.com/bazelbuild/bazel/blob/3c0915e46491e7490154dd07cb18ff03d02c05bc/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java#L619) to `false`, however we get an error as the current implementation [restricts the allowed types to `bool`](https://github.com/bazelbuild/rules_cc/blob/01a1eced1d61670e3fe1310ccaaa45635aaf90b1/cc/toolchains/impl/nested_args.bzl#L247).
Patching `nested_args.bzl` to accept `list` as well makes our setup work.
We think either the documentation should be updated to reflect that only variables of type `bool` are accepted, or the implementation should match what `expand_if_true`/`expand_if_false` do.
Contributor guide
Assessment
This issue has not been assessed yet.