search

facebook / buck2

Build system, successor to Buck
https://buck2.build/
Apache License 2.0
3.53k stars 215 forks source link

`attrs.query()` doesn't support non-literal target expressions #516

Open zjturner opened 10 months ago

zjturner commented 10 months ago

If I write this on the command line:

buck2 query set(root//foo/...)

it will list all targets defined in a BUCK file within the directory foo or in any directory beneath it.

However, when using an attrs.query() argument to a rule, it seems this is not possible. buck2 errors and tells me that it expected a target literal, not a pattern. This is easiest to see when using the deps_query argument a standard prelude rule, but it happens with any attrs.query() argument. It don't really understand Rust well, but it seems like it could be because of this (and also the comment suggests it's maybe intentional?).

Can anyone shed some light on this?

wendy728 commented 9 months ago

The implementation is intentional - the query attr is actually backed by a dep attr, and a dep attr also expects a single target literal. I think it may not be terrible to implement but @cjhopman can comment on the difficulty or if we want to support it at all. What's your use case?

zjturner commented 9 months ago

I was trying to implement a rule like alias() but which supported target expressions and query functions, not just literals.

It isn’t blocking me from anything, it’s just me experimenting with everything and trying to figure out as much as I can about everything 🙂

that said, it’s pretty surprising to a new user for buck2 query to behave differently than attrs.query in this way. Hopefully they can be consistent if for no other reason than following the principle of least surprise. But id be interested to learn if there’s any factors that make that ill-advised

ndmitchell commented 9 months ago

I think if we had been implementing Buck2 from scratch we might not have implemented attrs.query at all - its something Buck1 has, so we mirrored. But I'm not really sure of it's expressive power, if it's a good idea or not, or really anything much about it. @cjhopman is the expert.

wendy728 commented 9 months ago

Chatted with @cjhopman, this is quite non trivial to implement and we don't have plans to support it. cc @cjhopman for any technical details if you have further questions @zjturner

zjturner commented 9 months ago

That's too bad, because it would be a great way to have, for example, "target alias groups" which is really useful.