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

Allow private rule attributes ("_something") to be set from wrapper macros declared with the rule. #18563

Open
aiuto opened this issue Jun 2, 2023 · 5 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: feature request

Comments

@aiuto
Copy link
Contributor

aiuto commented Jun 2, 2023

It's a common idiom to use a wrapper macro (call it "foo") around a rule (call it "_foo") to do things like compute defaults. Sometimes these defaults are for values which we want to compute at load time (e.g. glob() results). We can pass this from the wrapper to the implementation with an attribute named "private_something", but that is not idiomatic. We could enhance readability by allowing _x attribute names for things passed solely from the wrapper to the implementation.

@Wyverald Wyverald added the team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts label Jun 2, 2023
@Wyverald
Copy link
Member

Wyverald commented Jun 2, 2023

Any suggestions as to how you'd like this ability to be limited to wrapper macros only? We definitely shouldn't just open up usage of private attributes to any caller.

@aiuto
Copy link
Contributor Author

aiuto commented Jun 2, 2023

Not yet. The important part of the FR is "macros declared with the rule".
So it might be that they are from the same .bzl file, but that makes it harder to hide implementations.
Or, perhaps an extension of starlark visibility.
Or, we make this use case an explicit construct, so that a rule has 3 parts. The BUILD file to build time interface, the analysis time interface (this is the current rule() functionality) and the analysis time implementation. That could help with stardoc problems too, where we want to pull attribute declarations from the underlying rule, but document the name of the wrapper method.

@pcjanzen
Copy link
Contributor

pcjanzen commented Jun 2, 2023

"Same .bzl file" isn't really a good heuristic: Another common idiom is to put the implementation into a different file, so that the rule and the macro can have the same name, and the "correct" name appears in bazel query and aspects.

@aiuto
Copy link
Contributor Author

aiuto commented Jun 3, 2023

Yes. That is exactly why I said it makes it harder to hide implementations. What I really want use to think about is the third idea, where we realize that many rules have a load time component which is integral to the rule. How we will represent that.

@comius comius added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Aug 22, 2023
@comius
Copy link
Contributor

comius commented Aug 22, 2023

P2 because I'm working on implementation of wrapper macros.

I dislike mixing of private/public attributes, because they affect subrule/extend_rule proposals, and I prefer a strict split between them.

There's another way. Wrapper macro can both eliminate public attribute from the interface and also set it (for the rule). That is def wrapper(attr1, attr3): return {"attr2": value}. So there's a way to get the desired functionality.

I'd argue that this way is better, because the top-level rule then knows into which subrule to pass the now public attribute.

Good use case for this are launcher in Java and Python rules. I think further insights will be possible when rules start using wrapper macros.

There's also potential for wrapper macros to remove computed defaults, which I'll look into.

@aiuto
Are there any other concrete use cases for this, that would give more insights?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: feature request
Projects
None yet
Development

No branches or pull requests

7 participants