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

Add support for allow_single_file attribute to label_list and label_keyed_string_dict #5355

Open
ash2k opened this issue Jun 8, 2018 · 8 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: feature request

Comments

@ash2k
Copy link
Contributor

ash2k commented Jun 8, 2018

Description of the problem / feature request:

attr.label() supports allow_single_file attribute which is very convenient. For some reason attr.label_keyed_string_dict() and attr.label_list() do not although all of them support allow_files.

I think it would be useful and consistent to have support for allow_single_file attribute in the later two.

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

In some private codebase I use label_keyed_string_dict and I have to assert that the value resolves to a single file and fail() if not. Would be nice to remove this manual check.

@laurentlb
Copy link
Contributor

What would that mean in a label_list? That every item in the list is a single file? At the end, you still end up with a collection of files. In that case, why would you want to allow ["a.cc", "b.cc", "c.cc"], but not a filegroup that returns the same information?

@ash2k
Copy link
Contributor Author

ash2k commented Jun 12, 2018

I don't have a use case for this flag for label_list. I just mentioned it for completeness/consistency reasons. Maybe someone in their custom rule treats each item in that list differently somehow? e.g. "Each item goes into a file. Files are numbered with integers. 1 file per label in the list". I don't know :)

So, if it does not make sense, just ignore this case.

@laurentlb
Copy link
Contributor

For label_keyed_string_dict, this looks like a reasonable feature request.

@ash2k
Copy link
Contributor Author

ash2k commented Jul 11, 2018

I'd like to extend this feature request to also include the executable parameter. It is supported by label attribute type but not by label_keyed_string_dict and label_list. They all support providers and cfg but executable is missing.

I tried to use FilesToRunProvider in the providers parameter (thinking that maybe that way I can/am supposed to block non-executable labels) but Bazel does not like it and says name 'FilesToRunProvider' is not defined. I think this is either a bug (the symbol should be "exported" and be usable somehow) or this thing is not a provider actually. In that case the documentation should be fixed (but why is it called ...Provider then?). Am I missing something?

My use case: I'd like to be able to bazel run multiple targets and I wrote a rule to do that. It accepts a label_list but I cannot use the executable bit on it. So I have to check that each specified target is executable manually which is not ideal.

p.s. I'm wondering why not make all "label" attributes consistent and have the same set of "filtering" parameters?

@laurentlb laurentlb added P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Starlark and removed category: extensibility > skylark labels Nov 21, 2018
@brandjon
Copy link
Member

p.s. I'm wondering why not make all "label" attributes consistent and have the same set of "filtering" parameters?

I agree we could be more consistent here. But there's a cost to changing the API, and I wouldn't want to make it without a clearer sense of exactly what we're aiming for. Leaving this issue open but not prioritized.

@brandjon brandjon added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Build-Language and removed P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Starlark labels Feb 16, 2021
@jkrems
Copy link

jkrems commented May 21, 2021

FWIW, I also just ran into this with a label_keyed_string_dict where I wanted to map single files to a string each (in my case: HTTP route patterns).

@seh
Copy link

seh commented May 21, 2021

I wrote this function after inspecting the keys (pointing at files) that wound up in my label_keyed_string_dict:

def _file_from_label_keyed_string_dict_key(k):
    # NB: The Targets in a label_keyed_string_dict attribute have the key's
    # source file in a depset, as opposed being represented directly as in a
    # label_list attribute.
    files = k.files.to_list()
    if len(files) != 1:
        fail(msg = "Unexpected number of files in target {}: {}".format(k, len(files)))
    return files[0]

I have no idea whether I can count on this working in the future.

@brandjon brandjon added untriaged team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts and removed team-Build-Language labels Nov 4, 2022
@thesayyn
Copy link
Contributor

I don't see why this isn't P2. Since other attr types have this already, label_keyed_string_dict should have it too.

@comius comius removed the untriaged label Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: feature request
Projects
None yet
Development

No branches or pull requests

8 participants