FR: propagate aspects using labels as the matching edge key

rickeylev commented 3 weeks ago

Description of the feature request:

Today, aspects traverse through the graph by matching attribute names. This mostly works, but has several drawbacks and short comings. Instead, I propose to use labels to identify matching edges. An attribute can specify a list of "aspect_edge" labels. An aspect then specifies a list of aspect edge type labels it cares about. e.g.

# foo.bzl
foo = rule(attrs = {
  "deps": attr.label_list(aspect_edge_types=["@bazel_tools//aspects:deps_edge"])
}, ...)
foo_aspect = aspect(
  aspect_edge_types = ["@bazel_tools//aspects:deps_edge"], ...
)

# bazel_tools/aspects/BUILD
aspect_edge_type(name="deps")

Motivation

@tjgq summed it up rather nicely in an internal doc, "Avoid Starlark Aspects", back in 2019:

Aspects are stringly typed

An attribute of the same name doesn’t necessarily mean the same for every rule. For example, js_library.deps contains library dependencies, but js_module.deps contains module dependencies. As another (cross-language) example, ts_library.runtime_deps looks superficially similar to java_library.runtime_deps, but doesn’t have the same semantics.

It is difficult to generate an exhaustive list of attribute names used by rules for a given language. In addition, anyone can add new rules or attributes at any time and fail to update the list. This is further compounded by the existence of private attributes that aren’t visible in BUILD files. For example, my best attempt for JavaScript is here, but I wouldn’t trust it to be complete.

Aspect propagation may unintentionally cross language boundaries. For example, say you have a genrule that produces output JavaScript files, which is then put in the srcs of a js_library. A JavaScript aspect will happily traverse the genrule.srcs attribute, even though it may contain targets that aren’t JavaScript at all. This is at best an annoyance (you must write the aspect in such a way that it refuses to operate on “invalid” targets) and at worst a potential bug (the aspect generates bogus output for these targets).

Since that doc, some cases of the above have been noted:

private_deps in swift
implementation deps in C++
I think java has runtime_deps ?
protos: lang_library--deps--> lang_proto_library --deps--> proto_library
Private attributes that are conceptually just extra deps and make sense for an aspect to visit or know about

Which category does this issue belong to?

Rules API

What underlying problem are you trying to solve with this feature?

The issue of having a more stable and reliable interface for aspects. Today, rule authors don't have a good way to cleanly support aspects. There's a variety of reason for this, but one of them is that rules and aspects don't have a clear way to communicate what the correct attributes to visit are.

Which operating system are you running Bazel on?

No response

What is the output of `bazel info release`?

No response

If `bazel info release` returns `development version` or `(@non-git)`, tell us how you built Bazel.

No response

What's the output of `git remote get-url origin; git rev-parse HEAD` ?

No response

Have you found anything relevant by searching the web?

I was unable to find prior issues about this topic

Any other information, logs, or outputs that you want to share?

No response

comius commented 2 weeks ago

I think this is worth more discussion.

Current example seems to be very basic and can be interpreted in 2 ways. Either propagate the aspect over "deps" attribute or propagate the aspect over all dependency type attributes. The latter could be essentially a new feature.

@rickeylev Would you care to expand the examples and match them to use cases?

cc @mai93

rickeylev commented 2 weeks ago

@comius I tagged you on the internal chat thread.

It's more about aspect writers knowing they are visiting the right things, and rule authors being able to tell what the semantic meaning is of an attribute.

To take an example from the (internal) python rules: they have 3 attributes: deps, _debugger, and _coverage. debugger and coverage are private attributes because it was a bit cleaner to do it that way than doing e.g. deps = deps + ["//foo:debugger", "//foo:coverage"] during the loading phase.

If someone is writing an aspect to analyze dependencies (type checker, strictdeps checker, "collect all the jars necessary" aspect, etc), then they would also want to visit attributes that are conceptually like deps, but are in e.g. a private attribute. Or some special-cased version of deps (e.g. java has "exports" and "runtime_deps"). As an aspect author, I can review the attributes and visit them, however, this doesn't help (1) the private attributes situation, or (2) what if a the rule author adds a new public attribute?.

This isn't a concrete example, but I hope that gets the gist (I don't have time atm to post something more thought out)

Posterity: there was also mention of an idea to have a "filter function", e.g. an aspect defines fn(target, attr, value) somehow, and that is used for it to decide what attributes to visit. That is an appealing idea, too; this "attrs have a semantic label" could go hand in hand with that.

bazelbuild / bazel