Skip to content

Allow Label instances as keys in select#14447

Closed
fmeum wants to merge 1 commit intobazelbuild:masterfrom
fmeum:14259-allow-label-in-select
Closed

Allow Label instances as keys in select#14447
fmeum wants to merge 1 commit intobazelbuild:masterfrom
fmeum:14259-allow-label-in-select

Conversation

@fmeum
Copy link
Copy Markdown
Collaborator

@fmeum fmeum commented Dec 19, 2021

When a macro specifies a label string as a key in a select, this label
is resolved relative to the site of use rather than the .bzl file the
macro is defined in. The resolution will lead to incorrect results if
the repository that uses the macro has a different repo mapping, e.g.
because it is created by another Bazel module.

This can be solved by allowing macros to specify label instances created
with the Label constructor instead of label strings everywhere, which
previously was not possible in select.

This commit also updates the docs for Label, select and macros.

Fixes #14259.

@fmeum
Copy link
Copy Markdown
Collaborator Author

fmeum commented Dec 19, 2021

@Wyverald @meteorcloudy

@fmeum fmeum force-pushed the 14259-allow-label-in-select branch from 3bc311e to d009ef0 Compare December 19, 2021 15:15
When a macro specifies a label string as a key in a select, this label
is resolved relative to the site of use rather than the .bzl file the
macro is defined in. The resolution will lead to incorrect results if
the repository that uses the macro has a different repo mapping, e.g.
because it is created by another Bazel module.

This can be solved by allowing macros to specify label instances created
with the `Label` constructor instead of label strings everywhere, which
previously was not possible in select.

This commit also updates the docs for Label, select and macros.

Fixes bazelbuild#14259.
@fmeum fmeum force-pushed the 14259-allow-label-in-select branch from d009ef0 to d299a87 Compare December 19, 2021 15:25
@Wyverald
Copy link
Copy Markdown
Member

What a nice PR! Thank you, Fabian! I'll import this shortly.

@bazel-io bazel-io closed this in 69f8b17 Dec 20, 2021
doc =
"A dict that maps configuration conditions to values. Each key is a label string"
+ " that identifies a config_setting instance."),
"A dict that maps configuration conditions to values. Each key is a "
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this is a nice PR!

My one quibble is documentation that says "A may be X or Y" without differentiating details is fundamentally ambiguous.

You explain the differentiation particularly well in macros.md. I wonder if a cross-reference there would help this.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. I created #14458 as a follow-up, could you take a look?

@meteorcloudy
Copy link
Copy Markdown
Member

Nice, thanks for your contribution! @fmeum

@fmeum
Copy link
Copy Markdown
Collaborator Author

fmeum commented Feb 3, 2022

@Wyverald Could this and #14458 be included in 5.1? #14458 fixes a mistake in the docs this PR introduced.

@brentleyjones
Copy link
Copy Markdown
Contributor

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Feb 3, 2022
@meteorcloudy
Copy link
Copy Markdown
Member

@bazel-io fork 5.1

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Feb 4, 2022
brentleyjones pushed a commit to brentleyjones/bazel that referenced this pull request Feb 8, 2022
When a macro specifies a label string as a key in a select, this label
is resolved relative to the site of use rather than the .bzl file the
macro is defined in. The resolution will lead to incorrect results if
the repository that uses the macro has a different repo mapping, e.g.
because it is created by another Bazel module.

This can be solved by allowing macros to specify label instances created
with the `Label` constructor instead of label strings everywhere, which
previously was not possible in select.

This commit also updates the docs for Label, select and macros.

Fixes bazelbuild#14259.

Closes bazelbuild#14447.

PiperOrigin-RevId: 417386977
(cherry picked from commit 69f8b17)
Wyverald pushed a commit that referenced this pull request Feb 9, 2022
When a macro specifies a label string as a key in a select, this label
is resolved relative to the site of use rather than the .bzl file the
macro is defined in. The resolution will lead to incorrect results if
the repository that uses the macro has a different repo mapping, e.g.
because it is created by another Bazel module.

This can be solved by allowing macros to specify label instances created
with the `Label` constructor instead of label strings everywhere, which
previously was not possible in select.

This commit also updates the docs for Label, select and macros.

Fixes #14259.

Closes #14447.

PiperOrigin-RevId: 417386977
(cherry picked from commit 69f8b17)

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
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

Successfully merging this pull request may close these issues.

Bzlmod: a project cannot provide a macro that uses select statement

6 participants