bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.24k stars 4.08k forks source link

required_aspect_providers doesn't work if aspects are not in the right order in the rule #5995

Open endobson opened 6 years ago

endobson commented 6 years ago

Description of the problem / feature request:

I expect there to be no diference between the following, yet one works and the other doesn't. https://gist.github.com/endobson/0c7335a469b5b23d478a480f5f58fc75

rule_order1 = rule(
  implementation = _rule_impl,
  attrs = {
    "deps": attr.label_list(aspects=[aspect1, aspect2])
  },
)

rule_order2 = rule(
  implementation = _rule_impl,
  attrs = {
    "deps": attr.label_list(aspects=[aspect2, aspect1])
  },
)

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

bazel build //:target_order1 => passes. bazel build //:target_order2 => fails.

What operating system are you running Bazel on?

MacOS

What's the output of bazel info release?

release 0.16.1- (@non-git)

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

From the release's sources.

laurentlb commented 6 years ago

Since aspect2 needs the value from aspect1, it seems normal that aspect2 must be applied after aspect1. Did I miss something?

endobson commented 6 years ago

I didn't have any expectation that the order of the supplied aspects is relevant to how they are applied. In particular I assumed that they are applied in parallel unless required_aspect_providers made a particular order better.

From the docs: https://docs.bazel.build/versions/master/skylark/lib/attr.html#label_list Aspects that should be applied to the dependency or dependencies specified by this attribute.

https://docs.bazel.build/versions/master/skylark/lib/globals.html#aspect Allow the aspect to inspect other aspects. If the aspect propagates along a dependency, and the underlying rule sends a different aspect along that dependency, and that aspect provides one of the providers listed here, this aspect will see the providers provided by that aspect.

In particular reading the documentation on aspect it seems that there is no mention of order and all the preconditions are met even if it comes earlier in the list of aspects on the rule.

Maybe this just needs a doc fix, to indicate that order is important.

Also I think that required_aspect_providers is a big misnomer as nothing is required about it and if it isn't met it fails very silently, but maybe that should be a different issue.

endobson commented 6 years ago

Filed #6080 for the required issue.

laurentlb commented 6 years ago

@c-parsons Can you take a look, and triage both this and #6080?

dslomov commented 6 years ago

This is WAI: the order of aspects in rule.aspects parameter is significant (this is somewhat of a corner case). We should document it.

trainman419 commented 6 years ago

I'm trying to apply multiple aspects and use required_aspect_providers to pass data between then, and I'm running into a similar issue trying to apply the aspects from the command line:

$ bazel build --aspects aspect.bzl%aspect1,aspect.bzl%aspect2 //:data
Starting local Bazel server and connecting to it...
ERROR: /home/austin/zoox/0c7335a469b5b23d478a480f5f58fc75/BUILD:2:1: in //:aspect.bzl%aspect2 aspect on filegroup rule //:data: 
Traceback (most recent call last):
    File "/home/austin/zoox/0c7335a469b5b23d478a480f5f58fc75/BUILD", line 2
        //:aspect.bzl%aspect2(...)
    File "/home/austin/zoox/0c7335a469b5b23d478a480f5f58fc75/aspect.bzl", line 8, in _aspect2_impl
        provider2(value = target[provider1].value)
    File "/home/austin/zoox/0c7335a469b5b23d478a480f5f58fc75/aspect.bzl", line 8, in provider2
        target[provider1]
<target //:data> (rule 'filegroup') doesn't contain declared provider 'provider1'
ERROR: Analysis of aspect '//:aspect.bzl%aspect2 of //:data' failed; build aborted: Analysis of target '//:data' failed; build aborted
github-actions[bot] commented 2 weeks ago

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.