Fix aspect attributes targeting PackageGroup.
A NullPointerException occured on such attributes while aspect was visiting target rule, because target rule had no such attribute.
RELNOTES: None.
PiperOrigin-RevId: 315272194
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java b/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java
index 31179bf..eeb7b5c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java
@@ -107,9 +107,12 @@
}
if (prerequisiteTarget instanceof PackageGroup) {
+ Attribute configuredAttribute = RawAttributeMapper.of(rule).getAttributeDefinition(attrName);
+ if (configuredAttribute == null) { // handles aspects
+ configuredAttribute = attribute;
+ }
boolean containsPackageSpecificationProvider =
- RawAttributeMapper.of(rule)
- .getAttributeDefinition(attrName)
+ configuredAttribute
.getRequiredProviders()
.getDescription()
.contains("PackageSpecificationProvider");
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
index 2df81ba..8760a00 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -327,6 +327,17 @@
}
@Test
+ public void aspectDependsOnPackageGroup() throws Exception {
+ setRulesAvailableInTests(
+ TestAspects.BASE_RULE, TestAspects.PACKAGE_GROUP_ATTRIBUTE_ASPECT_RULE);
+ pkg("extra", "package_group(name='extra')");
+ pkg("a", "rule_with_package_group_deps_aspect(name='a', foo=[':b'])", "base(name='b')");
+
+ getConfiguredTarget("//a:a");
+ assertContainsEventWithFrequency("bad aspect", 0);
+ }
+
+ @Test
public void aspectWithComputedAttribute() throws Exception {
setRulesAvailableInTests(TestAspects.BASE_RULE, TestAspects.COMPUTED_ATTRIBUTE_ASPECT_RULE);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index 709d697..b0520c6 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -623,6 +623,7 @@
TestAspects.ALL_ATTRIBUTES_WITH_TOOL_ASPECT,
TestAspects.BAR_PROVIDER_ASPECT,
TestAspects.EXTRA_ATTRIBUTE_ASPECT,
+ TestAspects.PACKAGE_GROUP_ATTRIBUTE_ASPECT,
TestAspects.COMPUTED_ATTRIBUTE_ASPECT,
TestAspects.FOO_PROVIDER_ASPECT,
TestAspects.ASPECT_REQUIRING_PROVIDER_SETS,
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD
index 3033a75..0723ee0 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD
@@ -63,6 +63,7 @@
"//src/main/java/com/google/devtools/build/lib/analysis:extra_action_artifacts_provider",
"//src/main/java/com/google/devtools/build/lib/analysis:file_provider",
"//src/main/java/com/google/devtools/build/lib/analysis:inconsistent_aspect_order_exception",
+ "//src/main/java/com/google/devtools/build/lib/analysis:package_specification_provider",
"//src/main/java/com/google/devtools/build/lib/analysis:resolved_toolchain_context",
"//src/main/java/com/google/devtools/build/lib/analysis:rule_definition_environment",
"//src/main/java/com/google/devtools/build/lib/analysis:server_directories",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
index f324777..7fa346a 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
@@ -30,6 +30,7 @@
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.PackageSpecificationProvider;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.analysis.RuleContext;
@@ -338,6 +339,24 @@
}
}
+ /** An aspect that defines its own implicit attribute, requiring PackageSpecificationProvider. */
+ public static class PackageGroupAttributeAspect extends BaseAspect {
+ @Override
+ public AspectDefinition getDefinition(AspectParameters aspectParameters) {
+ return PACKAGE_GROUP_ATTRIBUTE_ASPECT_DEFINITION;
+ }
+ }
+
+ public static final PackageGroupAttributeAspect PACKAGE_GROUP_ATTRIBUTE_ASPECT =
+ new PackageGroupAttributeAspect();
+ private static final AspectDefinition PACKAGE_GROUP_ATTRIBUTE_ASPECT_DEFINITION =
+ new AspectDefinition.Builder(PACKAGE_GROUP_ATTRIBUTE_ASPECT)
+ .add(
+ attr("$dep", LABEL)
+ .value(Label.parseAbsoluteUnchecked("//extra:extra"))
+ .mandatoryNativeProviders(ImmutableList.of(PackageSpecificationProvider.class)))
+ .build();
+
public static final ComputedAttributeAspect COMPUTED_ATTRIBUTE_ASPECT =
new ComputedAttributeAspect();
private static final AspectDefinition COMPUTED_ATTRIBUTE_ASPECT_DEFINITION =
@@ -674,6 +693,17 @@
attr("foo", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE)
.aspect(EXTRA_ATTRIBUTE_ASPECT));
+ /** A rule that defines an {@link PackageGroupAttributeAspect} on one of its attributes. */
+ public static final MockRule PACKAGE_GROUP_ATTRIBUTE_ASPECT_RULE =
+ () ->
+ MockRule.ancestor(BASE_RULE.getClass())
+ .factory(DummyRuleFactory.class)
+ .define(
+ "rule_with_package_group_deps_aspect",
+ attr("foo", LABEL_LIST)
+ .allowedFileTypes(FileTypeSet.ANY_FILE)
+ .aspect(PACKAGE_GROUP_ATTRIBUTE_ASPECT));
+
/** A rule that defines an {@link ComputedAttributeAspect} on one of its attributes. */
public static final MockRule COMPUTED_ATTRIBUTE_ASPECT_RULE =
() ->
