Skip to content

Commit

Permalink
Make implicit attribute label lookup consistent.
Browse files Browse the repository at this point in the history
Previously, as of 0c8049f, these labels
were treated as relative to the repository of the rule in some places
but not others.

From the other commit's message:
They [implicit attribute defaults] now all point to @bazel_tools anyway,
so there is no need [to special-case them].

--
Change-Id: If337eb2579ae613ba09cab0e0c927691922c0a39
Reviewed-on: https://bazel-review.googlesource.com/#/c/2783/
MOS_MIGRATED_REVID=114313341
  • Loading branch information
bsilver8192 authored and dslomov committed Feb 10, 2016
1 parent de3e9d5 commit ee9e688
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -319,37 +319,22 @@ private void resolveImplicitAttributes(Rule rule, BuildConfiguration configurati
: BuildType.LABEL.cast(attribute.getDefaultValue(rule));

if (label != null) {
label = ruleLabel.resolveRepositoryRelative(label);
builder.put(attribute, LabelAndConfiguration.of(label, configuration));
}
} else if (attribute.getType() == BuildType.LABEL_LIST) {
List<Label> labelList;
if (mappedAttributes.contains(attribute.getName())) {
labelList = new ArrayList<>();
for (Label label : attributeMap.get(attribute.getName(), BuildType.LABEL_LIST)) {
if (attribute.getName().equals("$config_dependencies")) {
// This is a hack necessary due to the confluence of the following circumstances:
// - We need to call ruleLabel.resolveRepositoryRelative() on every label except
// implicit ones so that the implicit labels specified in rule class definitions
// work as expected
// - The way dependencies for selectors is loaded is through the
// $config_dependencies attribute, and thus the labels there need to be a verbatim
// copy of those in the BUILD file (because
// AggregatingAttributeMapper#visitLabels() calls
// Label#resolveRepositoryRelative() on them, and calling it twice would be wrong
// Thus, we are stuck with the situation where the only implicit attribute on which
// Label#resolveRepositoryRelative needs to be called here is $config_dependencies.
//
// This is a bad state of affairs and the proper fix would be not to use labels in the
// default repository to signal configured targets in the main repository in SkyKeys.
label = ruleLabel.resolveRepositoryRelative(label);
}
labelList.add(label);
}
} else {
labelList = BuildType.LABEL_LIST.cast(attribute.getDefaultValue(rule));
}

for (Label label : labelList) {
label = ruleLabel.resolveRepositoryRelative(label);
builder.put(attribute, LabelAndConfiguration.of(label, configuration));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,41 @@ public void testParamFileSuffix() throws Exception {
assertEquals("foo/t.exe.params", fragment.getPathString());
}

@Test
public void testLabelAttributeDefault() throws Exception {
scratch.file(
"my_rule.bzl",
"def _impl(ctx):",
" return",
"my_rule = rule(",
" implementation = _impl,",
" attrs = {",
" 'explicit_dep': attr.label(default = Label('//:dep')),",
" '_implicit_dep': attr.label(default = Label('//:dep')),",
" 'explicit_dep_list': attr.label_list(default = [Label('//:dep')]),",
" '_implicit_dep_list': attr.label_list(default = [Label('//:dep')]),",
" }",
")");

scratch.file(
"BUILD", "filegroup(name='dep')", "load('/my_rule', 'my_rule')", "my_rule(name='r')");

invalidatePackages();
SkylarkRuleContext context = createRuleContext("@//:r");
Label explicitDepLabel =
(Label) evalRuleContextCode(context, "ruleContext.attr.explicit_dep.label");
assertThat(explicitDepLabel).isEqualTo(Label.parseAbsolute("@//:dep"));
Label implicitDepLabel =
(Label) evalRuleContextCode(context, "ruleContext.attr._implicit_dep.label");
assertThat(implicitDepLabel).isEqualTo(Label.parseAbsolute("@//:dep"));
Label explicitDepListLabel =
(Label) evalRuleContextCode(context, "ruleContext.attr.explicit_dep_list[0].label");
assertThat(explicitDepListLabel).isEqualTo(Label.parseAbsolute("@//:dep"));
Label implicitDepListLabel =
(Label) evalRuleContextCode(context, "ruleContext.attr._implicit_dep_list[0].label");
assertThat(implicitDepListLabel).isEqualTo(Label.parseAbsolute("@//:dep"));
}

@Test
public void testRelativeLabelInExternalRepository() throws Exception {
scratch.file("BUILD");
Expand Down

0 comments on commit ee9e688

Please sign in to comment.