Do not execute aspect implementation if target advertizes but fails to provide a provider.
Previously we always executed the function, but didn't add the aspect to
the deps.
--
PiperOrigin-RevId: 148887089
MOS_MIGRATED_REVID=148887089
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
index 8339c8a..f1dc77d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
@@ -98,6 +98,10 @@
return new ConfiguredAspect(real.descriptor, real.getProviders());
}
+ public static ConfiguredAspect forNonapplicableTarget(AspectDescriptor descriptor) {
+ return new ConfiguredAspect(descriptor, TransitiveInfoProviderMap.of());
+ }
+
/**
* Builder for {@link ConfiguredAspect}.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/rules/Alias.java b/src/main/java/com/google/devtools/build/lib/rules/Alias.java
index 5b91f47..3a4570c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/Alias.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/Alias.java
@@ -63,8 +63,7 @@
.allowedFileTypes(FileTypeSet.ANY_FILE)
.allowedRuleClasses(ANY_RULE)
.mandatory())
- // Stopgap until b/35213665 is fixed: should be canHaveAnyProvider()
- .advertiseProvider(AliasProvider.class)
+ .canHaveAnyProvider()
.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
index 20fe22d..b3f5cc5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
@@ -50,7 +50,6 @@
import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.TriState;
-import com.google.devtools.build.lib.rules.AliasProvider;
import com.google.devtools.build.lib.rules.java.JavaCommon;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
import com.google.devtools.build.lib.rules.java.JavaCompilationInfoProvider;
@@ -130,9 +129,7 @@
// For proto_lang_toolchain rules, where we just want to get at their runtime deps.
ImmutableSet.<Class<?>>of(ProtoLangToolchainProvider.class),
// For android_sdk rules, where we just want to get at aidl runtime deps.
- ImmutableSet.<Class<?>>of(AndroidSdkProvider.class),
- // Let this aspect "see through" alias targets until b/35213665 is fixed
- ImmutableSet.<Class<?>>of(AliasProvider.class)))
+ ImmutableSet.<Class<?>>of(AndroidSdkProvider.class)))
// Parse labels since we don't have RuleDefinitionEnvironment.getLabel like in a rule
.add(attr(ASPECT_DESUGAR_PREREQ, LABEL).cfg(HOST).exec()
.value(Label.parseAbsoluteUnchecked(toolsRepository + "//tools/android:desugar_java8")))
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index 71f6912..8437ab9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -427,17 +427,21 @@
return null;
}
- ConfiguredAspect configuredAspect =
- view.getConfiguredTargetFactory().createAspect(
- analysisEnvironment,
- associatedTarget,
- aspectPath,
- aspectFactory,
- aspect,
- directDeps,
- configConditions,
- aspectConfiguration,
- view.getHostConfiguration(aspectConfiguration));
+ ConfiguredAspect configuredAspect;
+ if (ConfiguredTargetFunction.aspectMatchesConfiguredTarget(associatedTarget, aspect)) {
+ configuredAspect = view.getConfiguredTargetFactory().createAspect(
+ analysisEnvironment,
+ associatedTarget,
+ aspectPath,
+ aspectFactory,
+ aspect,
+ directDeps,
+ configConditions,
+ aspectConfiguration,
+ view.getHostConfiguration(aspectConfiguration));
+ } else {
+ configuredAspect = ConfiguredAspect.forNonapplicableTarget(aspect.getDescriptor());
+ }
events.replayOn(env.getListener());
if (events.hasErrors()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index c19e3c0..b0eaf3c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -91,7 +91,6 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
-import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.Semaphore;
@@ -943,7 +942,7 @@
return aspectKey;
}
- private static boolean aspectMatchesConfiguredTarget(final ConfiguredTarget dep, Aspect aspect) {
+ static boolean aspectMatchesConfiguredTarget(final ConfiguredTarget dep, Aspect aspect) {
return aspect.getDefinition().getRequiredProviders().isSatisfiedBy(
new Predicate<Class<?>>() {
@Override
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 438c0d4..48563a1 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
@@ -129,6 +129,74 @@
}
@Test
+ public void aspectIsNotCreatedIfAdvertisedProviderIsNotPresentWithAlias() throws Exception {
+ setRulesAvailableInTests(new TestAspects.BaseRule(), new TestAspects.LiarRule(),
+ new TestAspects.AspectRequiringProviderRule());
+
+ pkg("a",
+ "aspect_requiring_provider(name='a', foo=[':b'])",
+ "alias(name = 'b_alias', actual = ':b')",
+ "liar(name='b', foo=[])");
+
+ ConfiguredTarget a = getConfiguredTarget("//a:a");
+ assertThat(a.getProvider(RuleInfo.class).getData()).containsExactly("rule //a:a");
+ }
+
+ @Test
+ public void aspectIsNotPropagatedThroughLiars() throws Exception {
+ setRulesAvailableInTests(new TestAspects.BaseRule(),
+ new TestAspects.LiarRule(),
+ new TestAspects.HonestRule(),
+ new TestAspects.AspectRequiringProviderRule());
+
+ pkg("a",
+ "aspect_requiring_provider(name='a', foo=[':b_alias'])",
+ "alias(name = 'b_alias', actual = ':b')",
+ "liar(name='b', foo=[':c'])",
+ "honest(name = 'c', foo = [])"
+ );
+
+ ConfiguredTarget a = getConfiguredTarget("//a:a");
+ assertThat(a.getProvider(RuleInfo.class).getData()).containsExactly("rule //a:a");
+ }
+
+ @Test
+ public void aspectPropagatedThroughAliasRule() throws Exception {
+ setRulesAvailableInTests(new TestAspects.BaseRule(), new TestAspects.HonestRule(),
+ new TestAspects.AspectRequiringProviderRule());
+
+ pkg("a",
+ "aspect_requiring_provider(name='a', foo=[':b_alias'])",
+ "alias(name = 'b_alias', actual = ':b')",
+ "honest(name='b', foo=[])");
+
+ ConfiguredTarget a = getConfiguredTarget("//a:a");
+ assertThat(a.getProvider(RuleInfo.class).getData()).containsExactly(
+ "rule //a:a", "aspect //a:b");
+ }
+
+ @Test
+ public void aspectPropagatedThroughAliasRuleAndHonestRules() throws Exception {
+ setRulesAvailableInTests(new TestAspects.BaseRule(), new TestAspects.HonestRule(),
+ new TestAspects.AspectRequiringProviderRule());
+
+ pkg("a",
+ "aspect_requiring_provider(name='a', foo=[':b'])",
+ "alias(name = 'b_alias', actual = ':b')",
+ "honest(name='b', foo=[':c'])",
+ "honest(name='c', foo=[])"
+ );
+
+ ConfiguredTarget a = getConfiguredTarget("//a:a");
+ assertThat(a.getProvider(RuleInfo.class).getData()).containsExactly(
+ "rule //a:a", "aspect //a:b", "aspect //a:c");
+ }
+
+
+
+
+
+ @Test
public void aspectCreationWorksThroughBind() throws Exception {
setRulesAvailableInTests(new TestAspects.BaseRule(), new TestAspects.HonestRule(),
new TestAspects.AspectRequiringProviderRule());
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 2f5595f..e8bad99 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
@@ -384,6 +384,7 @@
private static final AspectDefinition ASPECT_REQUIRING_PROVIDER_DEFINITION =
new AspectDefinition.Builder(ASPECT_REQUIRING_PROVIDER)
.requireProviders(RequiredProvider.class)
+ .propagateAlongAttribute("foo")
.build();
private static final AspectDefinition ASPECT_REQUIRING_PROVIDER_SETS_DEFINITION =
new AspectDefinition.Builder(ASPECT_REQUIRING_PROVIDER_SETS)
