Skip to content

Commit 60f757c

Browse files
brentleyjonesfmeum
brentleyjones
and
fmeum
authored
Allow Label instances as keys in select (bazelbuild#14755)
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) Co-authored-by: Fabian Meumertzheim <[email protected]>
1 parent 22bede9 commit 60f757c

File tree

5 files changed

+94
-11
lines changed

5 files changed

+94
-11
lines changed

‎site/docs/skylark/macros.md‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,38 @@ macro), use the function [native.package_name()](lib/native.html#package_name).
143143
Note that `native` can only be used in `.bzl` files, and not in `WORKSPACE` or
144144
`BUILD` files.
145145

146+
## Label resolution in macros
147+
148+
Since macros are evaluated in the [loading phase](concepts.md#evaluation-model),
149+
label strings such as `"//foo:bar"` that occur in a macro are interpreted
150+
relative to the `BUILD` file in which the macro is used rather than relative to
151+
the `.bzl` file in which it is defined. This behavior is generally undesirable
152+
for macros that are meant to be used in other repositories, e.g. because they
153+
are part of a published Starlark ruleset.
154+
155+
To get the same behavior as for Starlark rules, wrap the label strings with the
156+
[`Label`](lib/Label.html#Label) constructor:
157+
158+
```python
159+
# @my_ruleset//rules:defs.bzl
160+
def my_cc_wrapper(name, deps = [], **kwargs):
161+
native.cc_library(
162+
name = name,
163+
deps = deps + select({
164+
# Due to the use of Label, this label is resolved within @my_ruleset,
165+
# regardless of its site of use.
166+
Label("//config:needs_foo"): [
167+
# Due to the use of Label, this label will resolve to the correct target
168+
# even if the canonical name of @dep_of_my_ruleset should be different
169+
# in the main workspace, e.g. due to repo mappings.
170+
Label("@dep_of_my_ruleset//tools:foo"),
171+
],
172+
"//conditions:default": [],
173+
}),
174+
**kwargs,
175+
)
176+
```
177+
146178
## Debugging
147179

148180
* `bazel query --output=build //my/path:all` will show you how the `BUILD` file

‎src/main/java/com/google/devtools/build/lib/packages/SelectorList.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import com.google.common.base.Preconditions;
1717
import com.google.common.collect.ImmutableList;
1818
import com.google.common.collect.Iterables;
19+
import com.google.devtools.build.lib.cmdline.Label;
1920
import com.google.devtools.build.lib.collect.nestedset.Depset;
2021
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
2122
import java.util.Arrays;
@@ -87,9 +88,9 @@ public static Object select(Dict<?, ?> dict, String noMatchError) throws EvalExc
8788
+ " to match");
8889
}
8990
for (Object key : dict.keySet()) {
90-
if (!(key instanceof String)) {
91+
if (!(key instanceof String || key instanceof Label)) {
9192
throw Starlark.errorf(
92-
"select: got %s for dict key, want a label string", Starlark.type(key));
93+
"select: got %s for dict key, want a Label or label string", Starlark.type(key));
9394
}
9495
}
9596
return SelectorList.of(new SelectorValue(dict, noMatchError));

‎src/main/java/com/google/devtools/build/lib/packages/StarlarkLibrary.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,8 +330,10 @@ public Depset depset(
330330
name = "x",
331331
positional = true,
332332
doc =
333-
"A dict that maps configuration conditions to values. Each key is a label string"
334-
+ " that identifies a config_setting instance."),
333+
"A dict that maps configuration conditions to values. Each key is a "
334+
+ "<a href=\"$BE_ROOT/../skylark/lib/Label.html\">Label</a> or a label string"
335+
+ " that identifies a config_setting, constraint_setting, or constraint_value"
336+
+ " instance."),
335337
@Param(
336338
name = "no_match_error",
337339
defaultValue = "''",

‎src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -579,10 +579,11 @@ StarlarkAspectApi aspect(
579579
@StarlarkMethod(
580580
name = "Label",
581581
doc =
582-
"Creates a Label referring to a BUILD target. Use this function only when you want to"
583-
+ " give a default value for the label attributes. The argument must refer to an"
584-
+ " absolute label. The repo part of the label (or its absence) is interpreted in the"
585-
+ " context of the repo where this Label() call appears. Example: <br><pre"
582+
"Creates a Label referring to a BUILD target. Use this function when you want to give a"
583+
+ " default value for the label attributes of a rule or when referring to a target"
584+
+ " via an absolute label from a macro. The argument must refer to an absolute label."
585+
+ " The repo part of the label (or its absence) is interpreted in the context of the"
586+
+ " repo where this Label() call appears. Example: <br><pre"
586587
+ " class=language-python>Label(\"//tools:default\")</pre>",
587588
parameters = {
588589
@Param(name = "label_string", doc = "the label string."),

‎src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java‎

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,8 @@ public void configKeyTypeChecking_Int() throws Exception {
427427
" name = 'int_key',",
428428
" srcs = select({123: ['a.java']})",
429429
")");
430-
assertTargetError("//java/foo:int_key", "select: got int for dict key, want a label string");
430+
assertTargetError(
431+
"//java/foo:int_key", "select: got int for dict key, want a Label or label string");
431432
}
432433

433434
@Test
@@ -439,7 +440,8 @@ public void configKeyTypeChecking_Bool() throws Exception {
439440
" name = 'bool_key',",
440441
" srcs = select({True: ['a.java']})",
441442
")");
442-
assertTargetError("//java/foo:bool_key", "select: got bool for dict key, want a label string");
443+
assertTargetError(
444+
"//java/foo:bool_key", "select: got bool for dict key, want a Label or label string");
443445
}
444446

445447
@Test
@@ -452,7 +454,7 @@ public void configKeyTypeChecking_None() throws Exception {
452454
" srcs = select({None: ['a.java']})",
453455
")");
454456
assertTargetError(
455-
"//java/foo:none_key", "select: got NoneType for dict key, want a label string");
457+
"//java/foo:none_key", "select: got NoneType for dict key, want a Label or label string");
456458
}
457459

458460
@Test
@@ -1526,4 +1528,49 @@ public void publicVisibilityConfigSetting_defaultIsPrivate() throws Exception {
15261528
assertThat(getConfiguredTarget("//a:binary")).isNotNull();
15271529
assertNoEvents();
15281530
}
1531+
1532+
@Test
1533+
public void selectWithLabelKeysInMacro() throws Exception {
1534+
writeConfigRules();
1535+
scratch.file("java/BUILD");
1536+
scratch.file(
1537+
"java/macros.bzl",
1538+
"def my_java_binary(name, deps = [], **kwargs):",
1539+
" native.java_binary(",
1540+
" name = name,",
1541+
" deps = select({",
1542+
" Label('//conditions:a'): [Label('//java/foo:a')],",
1543+
" '//conditions:b': [Label('//java/foo:b')],",
1544+
" }) + select({",
1545+
" '//conditions:a': [Label('//java/foo:a2')],",
1546+
" Label('//conditions:b'): [Label('//java/foo:b2')],",
1547+
" }),",
1548+
" **kwargs,",
1549+
" )");
1550+
scratch.file(
1551+
"java/foo/BUILD",
1552+
"load('//java:macros.bzl', 'my_java_binary')",
1553+
"my_java_binary(",
1554+
" name = 'binary',",
1555+
" srcs = ['binary.java'],",
1556+
")",
1557+
"java_library(",
1558+
" name = 'a',",
1559+
" srcs = ['a.java'])",
1560+
"java_library(",
1561+
" name = 'b',",
1562+
" srcs = ['b.java'])",
1563+
"java_library(",
1564+
" name = 'a2',",
1565+
" srcs = ['a2.java'])",
1566+
"java_library(",
1567+
" name = 'b2',",
1568+
" srcs = ['b2.java'])");
1569+
1570+
checkRule(
1571+
"//java/foo:binary",
1572+
"--foo=b",
1573+
/*expected:*/ ImmutableList.of("bin java/foo/libb.jar", "bin java/foo/libb2.jar"),
1574+
/*not expected:*/ ImmutableList.of("bin java/foo/liba.jar", "bin java/foo/liba2.jar"));
1575+
}
15291576
}

0 commit comments

Comments
 (0)