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

Config transition turns label-type attributes into lists #15892

Closed
adam-azarchs opened this issue Jul 15, 2022 · 3 comments
Closed

Config transition turns label-type attributes into lists #15892

adam-azarchs opened this issue Jul 15, 2022 · 3 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability Issues for Configurability team type: documentation (cleanup)

Comments

@adam-azarchs
Copy link
Contributor

Description of the bug:

Setting cfg with a transition in attr.Label makes ctx.attr.<attribute> return a label list instead of a single label. See example below.

It's quite possible that this is intended behavior. Certainly for a one-to-many transition it would need to have this effect. In that case this is really a documentation problem, because it certainly isn't obvious that changing the configuration should change the type of the attribute value (and in fact the docs for the cfg parameter make no mention of transitions, though it does link to the docs on configurations, which do, but still don't mention anything about the type change).

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

WORKSPACE:

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "bazel_skylib",
    sha256 = "f7be3474d42aae265405a592bb7da8e171919d74c16f082a5457840f06054728",
    urls = [
        "https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.2.1/bazel-skylib-1.2.1.tar.gz",
        "https://github.com/bazelbuild/bazel-skylib/releases/download/1.2.1/bazel-skylib-1.2.1.tar.gz",
    ],
)

BUILD.bazel:

load("@bazel_skylib//rules:common_settings.bzl", "string_setting")
load(":my_rule.bzl", "my_rule", "my_transition_rule")

string_setting(
    name = "kind",
    build_setting_default = "baz",
    values = [
        "bar",
        "baz",
    ],
)

filegroup(
    name = "empty",
)

my_rule(
    name = "without_transition",
    src = ":empty",
)

my_transition_rule(
    name = "with_transition",
    src = ":empty",
)

my_rule.bzl:

def _foo_transition_impl(settings, attr):
    _ignore = settings  # buildifier: disable=unused-variable
    return {
        "//:kind": "bar",
    }

_foo_transition = transition(
    implementation = _foo_transition_impl,
    inputs = [],
    outputs = ["//:kind"],
)

def _rule_impl(ctx):
    print(type(ctx.attr.src))

my_rule = rule(
    attrs = {
        "src": attr.label(
            cfg = "target",
        ),
    },
    implementation = _rule_impl,
)

my_transition_rule = rule(
    attrs = {
        "src": attr.label(
            cfg = _foo_transition,
        ),
        "_allowlist_function_transition": attr.label(
            default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
        ),
    },
    implementation = _rule_impl,
)
$ bazel build :without_transition :with_transition
DEBUG: my_rule.bzl:14:10: Target
DEBUG: my_rule.bzl:14:10: list

Which operating system are you running Bazel on?

linux

What is the output of bazel info release?

release 5.2.0

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 master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

Open issue tracking documentation (and lack thereof) for transitions:
#8377

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

No response

@pcjanzen
Copy link
Contributor

The behavior you're seeing is documented at https://bazel.build/rules/config#accessing-attributes-with-transitions

When attaching a transition to an outgoing edge (regardless of whether the transition is a 1:1 or 1:2+ transition), ctx.attr is forced to be a list if it isn't already. The order of elements in this list is unspecified.

@adam-azarchs
Copy link
Contributor Author

So it does! In my defense, that document is a bit of a wall of text. It might be a good idea to link to that later section from https://bazel.build/rules/config#outgoing-edge-transitions, to aid others who might stop reading after the section which appears to tell them what they need to know.

@gregestren gregestren added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Jul 18, 2022
copybara-service bot pushed a commit that referenced this issue Jul 26, 2022
Move the link to `#accessing-attributes-with-transitions`
from the section about how to define 1:2 transitions into the section
on how to attach an outgoing transition to an attribute, since that's
the section people are most likely to be reading when they need to
see that link.

This got me, leading to me opening #15892, because I wasn't implementing a 1:2+ transition and hadn't kept reading to the end of the document.

Closes #15895.

PiperOrigin-RevId: 463243990
Change-Id: I77584e2a0e5fd44695d5e7ce5ba23127d6fc278e
@adam-azarchs
Copy link
Contributor Author

This was closed by #15895

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability Issues for Configurability team type: documentation (cleanup)
Projects
None yet
Development

No branches or pull requests

4 participants