bazelbuild / bazel-skylib

Common useful functions and rules for Bazel
https://bazel.build/
Apache License 2.0
389 stars 178 forks source link

Feature Request: Negative/Not conditionals for `selects` #272

Open UebelAndre opened 4 years ago

UebelAndre commented 4 years ago

I don't have a clear proposal right now but wanted to put the request up to see if anyone else has thoughts or is interested.

I would like to use negative conditionals in selects functions. I'm not sure what that might look like. My initial thought was that there would be some string parsing done on the strings passed into the selects macros.

Example:

selects.with_or({
    (
        # This is a normal condition
        ":some_normal_condition",

        # This is one proposal that uses the `!` character to indicate
        # this condition is a negative condition
        "!:some_negative_condition",

        # This is similar to the previous which uses `!` but instead there
        # is a wrapper function that will modify the string to have any
        # required token for future parsing of the `selects.*` macros.
        selects.not(":some_other_other_negative_condition"),
    ): [
        "some_src.cc",
    ],
    "//conditions:default": [],
})

To my knowledge the only way to do negative conditionals is the following which does seem to complicate potential solutions.

select({
    ":some_condition": [],
    "//conditions:default": [
        "src_included_when_some_condition_is_false.cc",
    ],
})

Perhaps in the coming days come up with a nicer proposal but I also know the owners of this project are incredibly talented people and may also already have brilliant insights on various implementations or potential issues.

Looking forward to responses πŸ˜„

UebelAndre commented 4 years ago

@c-parsons @laurentlb @jin @aiuto @juliexxia Hey everyone,

Just looking at the CODEOWNERS file and thought I'd reach out to you guys directly. Is this something worth considering?

c-parsons commented 4 years ago

Paging @juliexxia and @gregestren :)

gregestren commented 4 years ago

Hi @UebelAndre - someone was working on exactly this theme recently and we've exchanged a few emails. I don't know their GitHub handle but I forwarded them to this link. Stay tuned...

To be clear, where does //conditions:default fall short for you?

UebelAndre commented 4 years ago

Hey @gregestren! Thank you so much for your reply πŸ˜„

The thing I'm currently working on is mapping arbitrary Rust cfg declarations into Bazel select statements in cargo-raze. The situation I'm running into is, when there's a cfg like the following:

cfg(any(target_family = "windows", not(target_os = "ios"), not(target_arch = "powerpc"))

What I'd like to do is map that to the following:

selects.with_or({
    (
        "@io_bazel_rules_rust//rust/platform:windows",
        not("@io_bazel_rules_rust//rust/platform:ios"),
        not("@io_bazel_rules_rust//rust/platform:powerpc"),
    ): [
        "my_src.rs",
    ],
    "//condition:default": [],
})

Excusing for a second that that might not be the greatest example (though I've come across some legitimate cases of this which I'll try to find later), I do not have a way to express not. In fact, I also don't know of a great way to express and either... suggestions are welcome but that's my use case πŸ˜…

gregestren commented 4 years ago

Would windows already imply not ios or powerpc? How close would

select({
    "@io_bazel_rules_rust//rust/platform:ios": [],  # empty
    "@io_bazel_rules_rust//rust/platform:powerpc": [], # empty
    "//conditions:default": ["my_src.rs"]
   })

come?

UebelAndre commented 4 years ago

That does seem like it would solve for that specific example.

But it's not so much how to write a specific configuration, it's trying to transcribe an arbitrary Rust configurations into a Bazel select statement such that there are no duplicated dependencies in any rendered statement.

UebelAndre commented 4 years ago

Here's an additional example:

cfg(all(target_arch = "wasm32", not(any(target_os = "emscripten", target_os = "wasi"))))
gregestren commented 4 years ago

Got it. Let's wait for feedback on the proposal I mentioned in https://github.com/bazelbuild/bazel-skylib/issues/272#issuecomment-689657496.

UebelAndre commented 4 years ago

Sounds good! Any idea when you might be able to share that proposal or do you know when a pull request might be posted? πŸ˜ƒ

gregestren commented 4 years ago

It's up to the author. But they basically implemented:

config_setting(name = "mylib_v2", values = {...})
selects.config_setting_group(name = "mylib_v1", match_none = [":mylib_v2"])

It would probably take some poking around to see how that interacts with, e.g., config_setting_group.match_all and if it's realistically possible to chain everything together well.

gregestren commented 4 years ago

There's another paradigm getting some attention recently about moving more complicated logic into Starlark expressions.

That idea is that arbitrary boolean logic gets, well, complicated, and the more specialized APIs we need to support it the more complex our API surface area gets. And we already have an API for generic computation: Starlark.

Examples:

I'm not sure how that might fit here but it's an interesting idea to have one way to do all boolean logic and just invoke it where needed.

UebelAndre commented 3 years ago

Any updates here? πŸ™

gregestren commented 3 years ago

I haven't heard any, but I'll ping the author again.

gregestren commented 3 years ago

I have pinged them and there is some code, not yet shared, which looks pretty good.

I just asked for a PR at https://github.com/bazelbuild/bazel-skylib.

alandonovan commented 3 years ago

Greg, I completely agree with your general comment, and I've wondered in the past why we don't use Starlark expressions to express conditions for selects, but I think there are genuine obstacles here.

select values, like all rule attribute values, must be "plain old data", so that they can be inspected using blaze query, and serialized by various caching systems. Consequently, conditions must be referenced by name, not as (say) a function closure. The name refers to a config_setting, which is an instance of a rule that does only one thing: return a ConfigMatchingProvider that contains a boolean, matches. I think any other rule can in principle return a ConfigMatchingProvider with a matches flag computed however it wants, so you could define a macro that creates instances of this new rule using an arbitrary Starlark expression. However, you would still have to declare a named rule instance and refer to its name. Also, you can't put arbitrary Starlark expressions in a BUILD file, so you would need to declare the function in a .bzl file. All of this is very unsatisfactory and roundabout, and raises the question of how important is it really that rule attributes must be transparent, serializable data.

It reminds me of the situation with genrules: we all agree genrule has problems, and that Starlark expressions and formatting and templates are much richer that an ad-hoc shell/make language, but you can't easily make a one-off genrule based on Starlark expressions from within a BUILD file because there's no way to write an expression outside of a function.

UebelAndre commented 3 years ago

Hey, just pinging this thread. Any more updates?

tetromino commented 3 years ago

I've re-pinged the person who I think @gregestren was talking to

tetromino commented 3 years ago

As for the proposal to use Starlark expressions in select - it's an excellent idea, but it would be somewhat difficult to do, and it's unlikely to happen in the near future given current priorities. So for now, I think we'd be welcome to negative conditionals in selects.

UebelAndre commented 3 years ago

Negative conditions would be solve for nearly all of my use cases so would be thrilled to see that implemented and merged.

Starlark conditionals i can live without for now πŸ˜ƒ

mikael-s-persson commented 8 months ago

I know this is an old issue but if others fall upon this as I did (looking for logical "not" for settings / select), here is a workaround that seems to work in BUILD files (example for doing "not(msvc)":

# This is a "true" setting.
bool_setting(
    name = "always_true_setting",
    build_setting_default = True,
    visibility = ["//visibility:private"],
)
# This is a "false" setting.
bool_setting(
    name = "always_false_setting",
    build_setting_default = False,
    visibility = ["//visibility:private"],
)

# Make an alias for true or false setting that is equal to "not("@bazel_tools//tools/cpp:msvc")" (if "not" existed).
alias(
    name = "tools_cpp_not_msvc_setting",
    actual = select({
        "@bazel_tools//tools/cpp:msvc": ":always_false_setting",
        "//conditions:default": ":always_true_setting",
    }),
)
config_setting(
    name = "tools_cpp_not_msvc",
    flag_values = {":tools_cpp_not_msvc_setting": "True"},
)

Obviously, an actual function like "selects.not" would be obviously useful instead of this per-setting workaround.

gregestren commented 8 months ago

@mikael-s-persson and others - I know this is an old issue but we're still thinking about this. Because this continues to pop up regularly.

I'm currently an advocate of just exposing ConfigMatchingProvider to user-written Starlark rules. That would let anyone write their own custom config_setting triggered on arbitrary Starlark. It should make the skylib utilities obsolete, or at least upgrade them to a cleaner implementation.

As noted in https://github.com/bazelbuild/bazel-skylib/issues/272#issuecomment-718245039 this still involves overhead vs. something super-cool like select({lambda myflag: myflag == "abc": ... }). But I'm also not sure how to further simplify the UI.

Also see https://docs.google.com/document/d/1p02Y9joQSgdXtiTA2mJ_UXHBiBDPPTqrO-oJcupLAu8/edit#heading=h.icwoewbtra8.