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

Label.relative behavior change #14152

Closed
keith opened this issue Oct 21, 2021 · 2 comments
Closed

Label.relative behavior change #14152

keith opened this issue Oct 21, 2021 · 2 comments

Comments

@keith
Copy link
Member

keith commented Oct 21, 2021

With this code:

def _rule(repository_ctx):
    fail(Label("@foo//bazel").relative(str(repository_ctx.attr.some_label)))

rule = repository_rule(
    attrs = {
        "some_label": attr.label(
            mandatory = True,
            allow_single_file = [".txt"],
        ),
    },
    implementation = _rule,
)

And calling this with @foo//bazel/third_party:foo.txt

And bazel 4.2, you get: @foo//bazel/third_party:foo.txt
But with bazel 5.x (01a8182) you get: //bazel/third_party:foo.txt

Based on the examples here, I do not believe this new behavior was intentional: https://docs.bazel.build/versions/main/skylark/lib/Label.html#relative

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

With this project relativerepro.zip

Run USE_BAZEL_VERSION=4.2.0 bazelisk build ... and USE_BAZEL_VERSION=last_green bazelisk build ... and note the difference in output.

@Wyverald
Copy link
Member

Hi Keith, thanks for filing this! This is actually an intentional change to fix an existing bug surrounding repo mapping (see 463e8c8). It's not about Label.relative per se, but the Label(...) constructor function.

In Bazel 5.0, str(Label("@foo//:bar")) will be //:bar if @foo is the main repo. If you do str(Label("@not_foo//:bar")), it should still be @not_foo//:bar.

Previously, str(Label("@foo//:bar")) was @foo//:bar simply because the code was not correctly passing in the appropriate repo mapping, which meant that we didn't know that @foo was actually the main repo, so we just kept it verbatim. This would obviously be wrong if user-specified repo mappings were actually in play: for example, if @foo mapped to @actual_foo, this label wouldn't be correctly resolved.

I sent a PR to fix the usage pattern in Envoy at envoyproxy/envoy#18711.

If I missed something, please feel free to reopen this.

@keith
Copy link
Member Author

keith commented Oct 22, 2021

thanks for the context! and the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants