bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
22.99k stars 4.03k forks source link

Can not manipulate select statement in macros #8419

Open Globegitter opened 5 years ago

Globegitter commented 5 years ago

Description of the feature request:

The rules_k8s have a rule k8s_objects which takes a list of labels (k8s_object targets) and creates multiple targets with these labels, with custom endings, that it knows have been created from the underlying k8s_object. So there is code like [x + ".create" for x in objects] If objects is now a select statement that fails with something like: select is not iterable. IT would be nice if I could still iterate over a select statement similar to a dict, so I could manipulate the labels in the macro.

Feature requests: what underlying problem are you trying to solve with this feature?

Now I have to use a normal dict as a workaround which the macro then converts into a select statement. I think this ends up being confusing for the end-user as that is not usual

What's the output of bazel info release?

0.25.2

Have you found anything relevant by searching the web?

https://github.com/bazelbuild/bazel/issues/8171 but seems not directly related

Any other information, logs, or outputs that you want to share?

For the exact use of this see PR: https://github.com/bazelbuild/rules_k8s/pull/342

gregestren commented 5 years ago

I'm glad you found the workaround of just using a direct dict. I acknowledge its limited appeal, although I think you could also make the argument that it's a cleaner approach, since macro arguments are not rule arguments and select() is really only intended for rule arguments. That said, this is already getting conceptually more complicated than most users are prepared for, especially when it all looks the same in a BUILD file.

This general request has been discussed a lot, particularly with the Starlark designers. In principle everyone supports it. But no one's yet figured out a clean design. I think it'd have to be its own dedicated mini-project to make this happen (i.e. go through a design review vs. just spinning off a PR when someone has a few spare cycles).

The core problem is that there are fundamental limits to how deeply macros can explore select(). Today select() is a 100% opaque object, which is not giving macros enough exposure. But we can't swing all the way in the other direction and make them 100% transparent. Macros simply don't have the power to manipulate select() as flexibly as rules can (unless we did something crazy like late-evaluate macros in Bazel's analysis phase).

So the question is how far to turn that knob. A simple iteration like you describe seems reasonable since it doesn't imply macros have to know anything about which select() path will actually get chosen. But it's easy to imagine all kind of things macros would like to do that might blur these distinctions, and at worst try to make macros exceed their actual power. So... should we

And how can we prevent select() from being a "special" Starlark symbol any more than it already is? The more it represents an exception from standard Pythonic / Starlarkish expressions the messier the overall language is.

I have no doubt reasonable answers can come out of this. But it needs to be designed. So FYI about why I think we're not going to get a solution to this tomorrow.

Paging @brandjon who I've discussed this with on and off for some time.

gregestren commented 5 years ago

Also @serynth - we've talked about having a better list of worthwhile but not-currently-prioritized projects. The label I just created for this issue is a simple attempt to nudge that idea forward (although I'm sure we'll want to do more).

brandjon commented 5 years ago

I know I had a whole host of opinions last time select() in macros came up, and I've forgotten them all now. Off-hand, I'm inclined to favor an approach of "expose the structure of select" (opening it up to compatibility requirements, but how often would we break that?) and provide utilities in skylib to do abstract operations over them, like map and union.

gregestren commented 5 years ago

To be clear, I'm happy to support anyone who wants to try designing an answer to this (design review, code review, code help, etc.) Starting from @brandjon 's suggestions would be a good start.

cpsauer commented 3 years ago

Just ran into this, too, trying to make cross-platform cc macros that can call into Java on Android. In my case, there's enough Android specialization already that I'll just weave a workaround into that, but very interested in a solution if one comes about. (subscribing and +1 :)

c-parsons commented 3 years ago

Can we just make the underlying dictionary items passed to select() readable and immutable, like dict.items ?

x = select({
    "foo" : "bar",
    "baz" : "qux",
    "//conditions:default" : "default"
})
for k, v in x.items():
   print(k, v)

We don't expect selects to be excessively large, so I don't have efficiency concerns, and this allows macros to reconstruct selects as they see fit.

WDYT?

c-parsons commented 3 years ago

Scratch my previous message. Let me clarify a more rigorous proposal:

Some background:

As a mechanism to workaround this issue, I'm proposing that macro writers and users use the following convention:

Thus, instead of:

static_deps = ["//foo:bar"] + select({
        "//axisone:condition": ["//baz:qux"],
        "//conditions:default": [],
    }) + select({
        "//axistwo:conditiontwo": ["//my:othertarget"],
        "//conditions:default": [],
    })

You might create:

static_deps = ["//foo:bar"] + [{
        "//axisone:condition": ["//baz:qux"],
        "//conditions:default": [],
    }] + [{
        "//axistwo:conditiontwo": ["//my:othertarget"],
        "//conditions:default": [],
    }]

Creating an actual selector list from this type is fairly trivial:

def createselect(orig):
    result = []
        for v in orig:
            if type(v) == "dict":
                result += select(v)
            else:
                result += [v]
    return result

Upstream design proposal:

Expose a native method on selector list, items(), which returns an immutable data structure akin to the above proposal.

Thus:

    s = ["//foo:bar"] + select({
        "//axisone:condition": ["//baz:qux"],
        "//conditions:default": [],
    }) + select({
        "//axistwo:conditiontwo": ["//my:othertarget"],
        "//conditions:default": [],
    })

    readable_items = ["//foo:bar"] + [{
        "//axisone:condition": ["//baz:qux"],
        "//conditions:default": [],
    }] + [{
        "//axistwo:conditiontwo": ["//my:othertarget"],
        "//conditions:default": [],
    }]

    print(s.items() == readable_items) # true

I'll mail a PR which takes this approach, and ties it to an experimental flag.

gregestren commented 2 years ago

Hi anyone still reading this. Please see the latest conversation at https://github.com/bazelbuild/bazel/issues/14157. I'm trying to push forward a solution to this, but need the help of anyone who would like to see this happen.

gregestren commented 1 year ago

We just reviewed this issue. My last comment remains relevant.

sgowroji commented 1 year ago

Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.

fmeum commented 1 year ago

@sgowroji Still relevant