Expanded support for Aspect public attributes

[lisaonduty]

Description of the problem / feature request:

We would want Aspect public attribute to be able to support label type and also to be used without pre-defined values.

Today the following are supported:
" Public aspect attributes are of type string and are called parameters. Parameters must have a values attribute specified on them. In this case we have a parameter called extension that is allowed to have ‘*’, ‘h’, or ‘cc’ as a value."

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

In our system we are running a lint program for our c-source files.
This is still done from our old build system (not Bazel).
As an option to the linting program it’s possible to point out a file that contains different linting rules (.lnt) (which suppress warnings etc).
In our solution different users (who runs the linting program) points out different rule files (.lnt).

Now we are adding the lint support to our Bazel build system.
We have done that by implement an Aspect.
It all works well except for the ability to point out specific lint rule files (*.lnt)
It would be desirable to let the file be specified in an Aspect attribute which the user could set in the BUILD file. Since we don't know which files that users want to point out.
The problem is that it’s only string attribute that is public for Aspects and for them the values must be specified.

We also plan to use Aspects for other use cases where we also see the same needs.

What operating system are you running Bazel on?

Linux

What's the output of bazel info release?

0.25.0

Have you found anything relevant by searching the web?

No and no attention on the Bazel forum:
https://groups.google.com/forum/#!topic/bazel-discuss/IvfgpZ1nYQM

[laurentlb]

cc @c-parsons

[ghost]

For the benefit of outside parties who may want to implement this, would a Bazel developer please comment on why this restriction exists? This wasn't explained in original commit 74558fc. Thanks!

[shs96c]

This is something that I would find really helpful too. Context: working with a protobuf-like IDL, and I want to pass a generator to each aspect.

[shs96c]

If the aspect implementation had access to the rule that caused the aspect to be called in the first place, then I could access the information without shadowing attributes from the rule in the aspect.

[dgoldstein0]

I'm pretty sure I read the rationale in a design doc, but I don't recall the link. The gist of it though was that they wanted a strong guarantee that there wouldn't be a huge number of possible values for the aspect attributes.

If the aspect implementation had access to the rule that caused the aspect to be called in the first place, then I could access the information without shadowing attributes from the rule in the aspect.

this thought has also occurred to me, but a lot of the value for aspects is that if two rules call the same aspect on the same shared dependency, the aspect node generated and tracked by bazel can be the same one. Attributes throw a small curveball in here in that attributes cause these generated nodes to be specialized with the value of the attribute. But fundamentally they aren't tied to any concept of rule that calls the aspect, though naively that seems to make sense.

Anyhow, I totally agree that reducing restrictions on aspect attributes would be useful. For my use cases, there typically is going to be a single-digit number of different values such an attribute would take on, but we just don't know them ahead of time; they are determined by the users of our rules, and shouldn't be hardcoded in the rules themselves.

So far the only real workarounds I've thought up:

• work within the public attr.string() with limited values option
• define a new aspect for each value of the attribute you want to use, but reuse the same implementation.

Both have their limitations.

[aiuto]

I recently came across a similar need.

I have an aspect that gathers the transitive closure of all providers of a specific type for a target, building a large depset along the way. It is typically used from a rule that then looks at the built up provider on the deps and writes a JSON rep out as a result so that a tool can use it.

Someone wants to apply the same aspect to an arbitrary target, with no wrapping rule. They would invoke the aspect from the bazel command line. So they want the write of the JSON to be in the aspect itself. We can do that, but it could be incredibly expensive to create a write action for the full transifitve closure of all deps of a large target, so we only want to create the JSON and do the write if we have reached the top. It would be super useful if the aspect could know the target which was being built on the command line and only do the write action if we had reached it.

FWIW, I am currently working around some of this with the technique @dgoldstein0 mentioned. I have a single implementation that only writes if passed a top level target rule type. The normal version does no data writing, but there is also a "gather_foo_and_write_for_cc_binary() version that passes "cc_binary" to the implement, which then write the data only if the type is cc_binary. I can only get away with this because cc_binary is usually the topmost rule of a target.

[dgoldstein0]

hm, @aiuto it sounds like what might work better for you is if the CLI interface had a way to apply a rule rather than an aspect to an existing target. So your rule could write the json file, and do the aspect invocation for you. The value here being that you wouldn't have to declare targets with your rule for each target you might want to analyze this way.

e.g. instead of

bazel build //MyExample:example --aspects print.bzl%print_aspect

perhaps something like

bazel build //MyExample:example //a:b //c:d --rules print.bzl%print_rule[deps]

would construct the target print_rule(deps=["//MyExample:example", "//a:b", "//c:d"]) and build it.

If this sounds like a better solution to your problem, perhaps file a ticket for this? it seems orthogonal from wanting information passed into an aspect that really comes from the top of the dependency graph, but needs to be known much deeper down.

[ulfjack]

@aiuto, I think that might be solvable with two aspects. One is only applied to the top-level targets, generates actions, and doesn't propagate down the dependency graph, and the other is propagated down the dependency graph to collect transitive information but doesn't generate actions.

The other alternative is to make the actions really light-weight. There's really no reason why an action should take more than a few hundred bytes of memory (yes, I realize better than most how heavyweight Bazel actions are right now).

[ulfjack]

(The key idea is not to generate the Json upfront, but to have a closure that generates it just-in-time when the action is run.)

[purkhusid]

I came across this need when trying to use rules_scala for our Scala codebase. The scala_proto_library rules in rules_scala is aspect based which is very nice for performance reasons but not as nice when it comes to flexibility. The exact use case for me is:

I have a .proto file that is declared like this:

syntax = "proto2";

import "test/proto/scalapb.proto";

option (scalapb.options) = {
  import: "some.thing.TheObject.theValue"
};

message TestMessage {
    required string message = 1;
}

The extra option I'm declaring makes it so that the scala protobuf compiler adds an extra import to the generated source.
For me to be able to compile the generated sources I'll need to add a dependency on the label that contains some.thing.TheObject.theValue.

This does not seem to be possible with aspects. If the aspect had access to the calling rule I could pass this dependency down to the compilation action which resides in the aspect.

Is there any way to work around this limitation?
The aspect and rule in rules_scala can be found here:
Rule:
https://github.com/bazelbuild/rules_scala/blob/f0c8d0759c3eeec7e7e94cd61e507b9b771b7641/scala_proto/scala_proto.bzl#L54
Aspect:
https://github.com/bazelbuild/rules_scala/blob/f0c8d0759c3eeec7e7e94cd61e507b9b771b7641/scala_proto/private/scalapb_aspect.bzl#L228

[sgowroji]

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.

[thesayyn]

@sgowroji This is a crucial thing to have in aspects. Can we keep it open? I have been searching for this issue for an hour, and just realized it's closed.

[comius]

cc @mai93

Passing label parameters to aspects is intentionally not supported and will unlikely be supported in the future.

The reason is that label valued parameter have too big potential to significantly regress builds.

For each aspect Bazel creates a mirrored dependency graph (DAG). Aspects are evaluated in a way that doesn’t duplicate any work.

When aspect parameters are used, the graph needs to be mirrored for each argument value. That’s why there’s only support for enum like types.

Introducing a label type and accidentally using it on n labels, can make your build n times slower. And this can be a problem for fairly small n-s.

Note also that it’s users responsibility to take care of encoding aspect’s parameters into output paths. (Bazel reports conflicts if they happen)

Alternative solutions are to create multiple aspects or to convert enums to labels. Those are less likely to be accidentally used with large

[aaron-michaux]

I'm stuck being unable to create an aspect, when I can drive the aspect through a rule. THE EXECUTION TIME IS THE SAME. However, the developer experience for the later is worse, because they cannot apply the aspect to a set of queried targets (like bazel --aspects ... //my/targets/...), but instead have to edit the BUILD file to supply the targets.

Because of this limitation, we have to use a convoluted setup (calling out to a bash script which runs cquery, which collates the information we'd get through the aspect). THIS IS SLOW AND BRITTLE, and the inevitable consequence of paternalistic language design.

Bazel developers are best situated to assess the performance of the code that they develop. They are the ones with the code in hand. Literally.

Please consider putting the developer in charge.