Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Negative/Not conditionals for selects #272

Open
UebelAndre opened this issue Sep 7, 2020 · 21 comments
Open

Feature Request: Negative/Not conditionals for selects #272

UebelAndre opened this issue Sep 7, 2020 · 21 comments
Labels
P3 We're not considering to work on this, but happy to review a PR. (No assignee) type: feature request

Comments

@UebelAndre
Copy link
Contributor

UebelAndre commented Sep 7, 2020

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 UebelAndre changed the title Feature Request: Negative conditionals for selects Feature Request: Negative/Not conditionals for selects Sep 7, 2020
@UebelAndre
Copy link
Contributor Author

@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
Copy link
Collaborator

Paging @juliexxia and @gregestren :)

@gregestren
Copy link
Contributor

gregestren commented Sep 9, 2020

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

Here's an additional example:

cfg(all(target_arch = "wasm32", not(any(target_os = "emscripten", target_os = "wasi"))))

@gregestren
Copy link
Contributor

Got it. Let's wait for feedback on the proposal I mentioned in #272 (comment).

@UebelAndre
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

Any updates here? 🙏

@gregestren
Copy link
Contributor

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

@gregestren
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

Hey, just pinging this thread. Any more updates?

@tetromino tetromino added type: feature request P3 We're not considering to work on this, but happy to review a PR. (No assignee) labels Apr 15, 2021
@tetromino
Copy link
Collaborator

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

@tetromino
Copy link
Collaborator

tetromino commented Apr 15, 2021

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
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor

gregestren commented Jan 4, 2024

@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 #272 (comment) 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering to work on this, but happy to review a PR. (No assignee) type: feature request
Projects
None yet
Development

No branches or pull requests

6 participants