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

workspace name in local aspect hides rule providers #11734

Closed
aherrmann opened this issue Jul 9, 2020 · 7 comments
Closed

workspace name in local aspect hides rule providers #11734

aherrmann opened this issue Jul 9, 2020 · 7 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug untriaged

Comments

@aherrmann
Copy link
Contributor

Description of the problem:

Invoking an aspect that is defined in the local workspace but called by prefixing the workspace name (@workspace) will hide providers returned by rule the implementation from the aspect implementation. I.e. this breaks the following sentence from the aspect documentation

The set of providers for an aspect application A(X) is the union of providers that come from the implementation of a rule for target X and from the implementation of aspect A.

This seems to be another instance of #11128

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

Create the following project

WORKSPACE

workspace(name = "repro")

repro.bzl

RuleProvider = provider()
AspectProvider = provider()

def _my_rule_impl(ctx):
    return [DefaultInfo(), RuleProvider()]

my_rule = rule(_my_rule_impl)

def _my_aspect_impl(target, ctx):
    print(target, "RuleProvider in target", RuleProvider in target)
    return [AspectProvider()]

my_aspect = aspect(_my_aspect_impl)

def _my_aspect_rule_impl(ctx):
    return [DefaultInfo(), RuleProvider()]

my_aspect_rule = rule(
    _my_aspect_rule_impl,
    attrs = {
        "deps": attr.label_list(aspects = [my_aspect]),
    },
)

BUILD.bazel

load("//:repro.bzl", "my_rule", "my_aspect_rule")
my_rule(name = "my_rule")
my_aspect_rule(name = "my_aspect_rule", deps = [":my_rule"])

First invoke the aspect as follows and observe the expected output:

$ bazel build //:my_rule --aspects //:repro.bzl%my_aspect
DEBUG: /home/aj/tweag.io/da/bazel-projects/repro/aspect_providers/repro.bzl:10:10: <target //:my_rule, keys:[RuleProvider, OutputGroupInfo]> RuleProvider in target True

Then invoke the aspect prefixed by the workspace name and observe the unexpected output:

$ bazel build //:my_rule --aspects @repro//:repro.bzl%my_aspect
DEBUG: /home/aj/.cache/bazel/_bazel_aj/bb353fdb0caf79c073ce577a80add763/external/repro/repro.bzl:10:10: <target //:my_rule, keys:[RuleProvider, OutputGroupInfo]> RuleProvider in target False

Note how the boolean value is False even though the provider RuleProvider is still listed on the textual target description.

Side note, the expected output is produced in both cases of:

$ bazel build //:my_aspect_rule
$ bazel build @repro//:my_aspect_rule

What operating system are you running Bazel on?

Ubuntu 19.10

What's the output of bazel info release?

release 3.3.1

(Also observed on 2.1.0)

Have you found anything relevant by searching the web?

#11128 bundles issues of this type.

@brandjon
Copy link
Member

This is very likely a repo-renaming issue, where we're failing to recognize that @repro//:repro.bzl is the same .bzl file as //:repro.bzl, resulting in loading it twice with incompatible notions of the RuleProvider symbol.

One of my wishlist items for this year is to reexamine the semantics of repo renaming. The outcome would inform how we handle avoiding this whole class of bugs.

@hlopko
Copy link
Member

hlopko commented Jun 9, 2021

We just encountered the same in Rules Rust (bazelbuild/rules_rust#749).

@Wyverald Wyverald added P2 We'll consider working on this in future. (Assignee optional) and removed P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) labels Sep 22, 2021
@Wyverald Wyverald self-assigned this Sep 22, 2021
@hlopko
Copy link
Member

hlopko commented Nov 14, 2021

Just for data to help prioritizing, we hit another instance of this bug in bazelbuild/rules_rust#1010.

@Wyverald
Copy link
Member

Wyverald commented Dec 8, 2021

I did some digging in another issue (#10590 (comment)) and I believe I understand this issue now. It should only appear when the aspect is named on the command line, since that doesn't go through repo mapping. The eventual fix would be a "delayed label" type thing where we can make command-line labels go through repo mapping.

@Wyverald
Copy link
Member

Wyverald commented Dec 8, 2021

For the record, fixing this issue is on our radar for the mid term, as we'd need it before Bzlmod is officially launched.

@hlopko
Copy link
Member

hlopko commented Jun 14, 2022

Could I kindly ask for status update? It seems bzlmod is moving forward, was this issue maybe already fixed?

@Wyverald
Copy link
Member

Dupe of #15989 (already fixed)

@Wyverald Wyverald closed this as not planned Won't fix, can't repro, duplicate, stale Nov 18, 2022
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-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug untriaged
Projects
None yet
Development

No branches or pull requests

5 participants