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: allow attrs accept selects. #1698

Closed
ahyangyi opened this issue Aug 29, 2016 · 8 comments
Closed

Feature request: allow attrs accept selects. #1698

ahyangyi opened this issue Aug 29, 2016 · 8 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request
Milestone

Comments

@ahyangyi
Copy link

ahyangyi commented Aug 29, 2016

I tried to put a select of lists of strings to a rule's attr expecting a string_list or a label_list, but that didn't work. This seems an important feature missing, since without selects we have absolutely no flexibility to write BUILDs that work under different configurations.

Thanks.

@aehlig aehlig added type: feature request P3 We're not considering working on this, but happy to review a PR. (No assignee) category: extensibility > skylark labels Aug 29, 2016
@ahyangyi
Copy link
Author

ahyangyi commented Aug 30, 2016

When I think about it, shouldn't rules and aspects also have access to user configuration inputs, like the values of --cpu, --compiler and -c?

The built-in rules indeed behave differently when these configuration inputs change, so I can't see this being a technical impossibility. Supporting this would make rules closer to their built-in counterparts.

@ahyangyi
Copy link
Author

ahyangyi commented Sep 6, 2016

FYI:

Currently one can work around the problem where attrs cannot accept selects by adding a level of indirection: attrs can use labels and labels can point to filegroups or genrules that uses selects.

Similarly, rules and aspects can access various config_settings if you create some dummy files and a filegroup that uses select to conditionally depend on different dummy files. You can use an implicit dependency to depend on such a dummy filegroup and parse its filename to figure out whatever you need in config_settings.

These are tedious, but before the official Bazel team fixes this issue, you can use these tricks to work around the limits.

@ulfjack
Copy link
Contributor

ulfjack commented Dec 1, 2016

Do you have an example that doesn't work and Bazel output?

@ahyangyi
Copy link
Author

ahyangyi commented Dec 2, 2016

@ulfjack Thanks. That's a good question. I revisited this feature request and tested and can confirm that said rules can indeed accept selects.

My original intention, however, was to set the default value of an attr to a select. For example:

def _cat_impl(ctx):
    srcs = ctx.files.srcs
    output = ctx.outputs.cat

    ctx.action(
        command = "cat %s > %s" % (" ".join([x.path for x in srcs]), output.path),
        inputs = srcs,
        outputs = [output],
        )

cat = rule(
    implementation = _cat_impl,
    attrs = {
        "srcs": attr.label_list(allow_files=True, default=select({"//conditions:default": []})),
        },
    outputs = {
        "cat": "%{name}.cat",
        },
)

This fails:

ERROR: /home/<redacted-but-thats-me>/test/cat/cat.bzl:14:17: Traceback (most recent call last):
        File "/home/<redacted-but-thats-me>/test/cat/cat.bzl", line 11
                rule(implementation = _cat_impl, attrs ...": []}))}, ..."})
        File "/home/<redacted-but-thats-me>/test/cat/cat.bzl", line 14, in rule
                attr.label_list(allow_files = True, default = sele...": []}))
expected sequence of Labels or sequence of Labels-returning function for 'default' while calling label_list but got select of list instead: selector({"//conditions:default": []}).
ERROR: error loading package 'cat': Extension file 'cat/cat.bzl' has errors.

But I wish that could be accepted.

@ulfjack
Copy link
Contributor

ulfjack commented Dec 2, 2016

@laurentlb

@laurentlb
Copy link
Contributor

Thanks for the feature request, that makes sense.

The workaround for now is to wrap the rule with a macro, and use the select in the macro.

@dslomov dslomov added this to the 0.6 milestone Dec 8, 2016
@excitoon
Copy link
Contributor

excitoon commented Mar 5, 2018

This is the same issue as #287.

@gregestren
Copy link
Contributor

Duplicate of #287

@gregestren gregestren marked this as a duplicate of #287 Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request
Projects
None yet
Development

No branches or pull requests

7 participants