diff options
6 files changed, 277 insertions, 17 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java index 10703ba8eb..895b6be4a0 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java @@ -548,7 +548,7 @@ public abstract class DependencyResolver { aspectCandidates.addAll(attribute.getAspects(originalRule)); if (aspect != null) { for (AspectClass aspectClass : - aspect.getDefinition().getAttributeAspects().get(attribute.getName())) { + aspect.getDefinition().getAttributeAspects(attribute)) { if (aspectClass.equals(aspect.getAspectClass())) { aspectCandidates.add(aspect); } else if (aspectClass instanceof NativeAspectClass) { diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java index 20c2a6ab28..85843b879a 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java @@ -14,6 +14,8 @@ package com.google.devtools.build.lib.packages; +import com.google.common.collect.ImmutableCollection; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; @@ -25,14 +27,12 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.Preconditions; - import java.util.Collection; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; - import javax.annotation.Nullable; /** @@ -59,14 +59,18 @@ public final class AspectDefinition { private final ImmutableSet<Class<?>> requiredProviders; private final ImmutableSet<String> requiredProviderNames; private final ImmutableMap<String, Attribute> attributes; - private final ImmutableMultimap<String, AspectClass> attributeAspects; + private final PropagationFunction attributeAspects; @Nullable private final ConfigurationFragmentPolicy configurationFragmentPolicy; + private interface PropagationFunction { + ImmutableCollection<AspectClass> propagate(Attribute attribute); + } + private AspectDefinition( String name, ImmutableSet<Class<?>> requiredProviders, ImmutableMap<String, Attribute> attributes, - ImmutableMultimap<String, AspectClass> attributeAspects, + PropagationFunction attributeAspects, @Nullable ConfigurationFragmentPolicy configurationFragmentPolicy) { this.name = name; this.requiredProviders = requiredProviders; @@ -119,10 +123,10 @@ public final class AspectDefinition { } /** - * Returns the attribute -> set of required aspects map. + * Returns the set of required aspects for a given atribute. */ - public ImmutableMultimap<String, AspectClass> getAttributeAspects() { - return attributeAspects; + public ImmutableCollection<AspectClass> getAttributeAspects(Attribute attribute) { + return attributeAspects.propagate(attribute); } /** @@ -226,6 +230,7 @@ public final class AspectDefinition { private final Map<String, Attribute> attributes = new LinkedHashMap<>(); private final Set<Class<?>> requiredProviders = new LinkedHashSet<>(); private final Multimap<String, AspectClass> attributeAspects = LinkedHashMultimap.create(); + private ImmutableCollection<AspectClass> allAttributesAspects = null; private final ConfigurationFragmentPolicy.Builder configurationFragmentPolicy = new ConfigurationFragmentPolicy.Builder(); @@ -265,12 +270,23 @@ public final class AspectDefinition { */ public final Builder attributeAspect(String attribute, AspectClass aspectClass) { Preconditions.checkNotNull(attribute); + Preconditions.checkState(this.allAttributesAspects == null, + "Specify either aspects for all attributes, or for specific attributes, not both"); this.attributeAspects.put(attribute, Preconditions.checkNotNull(aspectClass)); return this; } + public final Builder allAttributesAspect(AspectClass... aspectClasses) { + Preconditions.checkState(this.attributeAspects.isEmpty(), + "Specify either aspects for all attributes, or for specific attributes, not both"); + Preconditions.checkState(this.allAttributesAspects == null, + "Aspects for all attributes must only be specified once"); + this.allAttributesAspects = ImmutableList.copyOf(aspectClasses); + return this; + } + /** * Adds an attribute to the aspect. * @@ -361,6 +377,35 @@ public final class AspectDefinition { return this; } + @Immutable + private static final class AllAttributesPropagationFunction implements PropagationFunction { + private final ImmutableCollection<AspectClass> aspects; + + private AllAttributesPropagationFunction(ImmutableCollection<AspectClass> aspects) { + this.aspects = aspects; + } + + @Override + public ImmutableCollection<AspectClass> propagate(Attribute attribute) { + return aspects; + } + } + + @Immutable + private static final class PerAttributePropagationFunction implements PropagationFunction { + ImmutableSetMultimap<String, AspectClass> aspects; + + public PerAttributePropagationFunction( + ImmutableSetMultimap<String, AspectClass> aspects) { + this.aspects = aspects; + } + + @Override + public ImmutableCollection<AspectClass> propagate(Attribute attribute) { + return aspects.get(attribute.getName()); + } + } + /** * Builds the aspect definition. * @@ -368,7 +413,10 @@ public final class AspectDefinition { */ public AspectDefinition build() { return new AspectDefinition(name, ImmutableSet.copyOf(requiredProviders), - ImmutableMap.copyOf(attributes), ImmutableSetMultimap.copyOf(attributeAspects), + ImmutableMap.copyOf(attributes), + allAttributesAspects != null + ? new AllAttributesPropagationFunction(allAttributesAspects) + : new PerAttributePropagationFunction(ImmutableSetMultimap.copyOf(attributeAspects)), configurationFragmentPolicy.build()); } } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java index 4bf2c55102..667af95af3 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java @@ -31,7 +31,6 @@ import com.google.devtools.build.lib.packages.NativeAspectClass; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.util.FileTypeSet; - import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -121,8 +120,30 @@ public class AspectDefinitionTest { .attributeAspect("srcs", TEST_ASPECT_CLASS) .attributeAspect("deps", TEST_ASPECT_CLASS) .build(); - assertThat(withAspects.getAttributeAspects()).containsEntry("srcs", TEST_ASPECT_CLASS); - assertThat(withAspects.getAttributeAspects()).containsEntry("deps", TEST_ASPECT_CLASS); + + assertThat(withAspects.getAttributeAspects(createLabelListAttribute("srcs"))) + .containsExactly(TEST_ASPECT_CLASS); + assertThat(withAspects.getAttributeAspects(createLabelListAttribute("deps"))) + .containsExactly(TEST_ASPECT_CLASS); + } + + @Test + public void testAttributeAspect_AllAttributes() throws Exception { + AspectDefinition withAspects = new AspectDefinition.Builder("attribute_aspect") + .allAttributesAspect(TEST_ASPECT_CLASS) + .build(); + + assertThat(withAspects.getAttributeAspects(createLabelListAttribute("srcs"))) + .containsExactly(TEST_ASPECT_CLASS); + assertThat(withAspects.getAttributeAspects(createLabelListAttribute("deps"))) + .containsExactly(TEST_ASPECT_CLASS); + } + + + private static Attribute createLabelListAttribute(String name) { + return Attribute.attr(name, BuildType.LABEL_LIST) + .allowedFileTypes(FileTypeSet.ANY_FILE) + .build(); } @Test 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 9274b759b0..a2e9d23191 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 @@ -425,6 +425,80 @@ public class AspectTest extends AnalysisTestCase { a.getProvider(ExtraActionArtifactsProvider.class) .getTransitiveExtraActionArtifacts(); assertThat(getOnlyElement(extraActionArtifacts).getLabel()).isEqualTo(Label.create("@//a", "b")); + } + + @Test + public void aspectPropagatesToAllAttributes() throws Exception { + setRulesAvailableInTests(new TestAspects.BaseRule(), + new TestAspects.SimpleRule(), + new TestAspects.AllAttributesAspectRule()); + pkg("a", + "simple(name='a', foo=[':b'], foo1=':c', txt='some text')", + "simple(name='b', foo=[], txt='some text')", + "simple(name='c', foo=[], txt='more text')", + "all_attributes_aspect(name='x', foo=[':a'])"); + + ConfiguredTarget a = getConfiguredTarget("//a:x"); + assertThat(a.getProvider(RuleInfo.class).getData()) + .containsExactly("aspect //a:a", "aspect //a:b", "aspect //a:c", "rule //a:x"); + } + + @Test + public void aspectPropagatesToAllAttributesImplicit() throws Exception { + setRulesAvailableInTests(new TestAspects.BaseRule(), + new TestAspects.SimpleRule(), + new TestAspects.ImplicitDepRule(), + new TestAspects.AllAttributesAspectRule()); + + scratch.file( + "extra/BUILD", + "simple(name ='extra')" + ); + pkg("a", + "simple(name='a', foo=[':b'], foo1=':c', txt='some text')", + "simple(name='b', foo=[], txt='some text')", + "implicit_dep(name='c')", + "all_attributes_aspect(name='x', foo=[':a'])"); + update(); + ConfiguredTarget a = getConfiguredTarget("//a:x"); + assertThat(a.getProvider(RuleInfo.class).getData()) + .containsExactly( + "aspect //a:a", + "aspect //a:b", + "aspect //a:c", + "aspect //extra:extra", + "rule //a:x"); } + + + @Test + public void aspectPropagatesToAllAttributesLateBound() throws Exception { + setRulesAvailableInTests(new TestAspects.BaseRule(), + new TestAspects.SimpleRule(), + new TestAspects.LateBoundDepRule(), + new TestAspects.AllAttributesAspectRule()); + + scratch.file( + "extra/BUILD", + "simple(name ='extra')" + ); + pkg("a", + "simple(name='a', foo=[':b'], foo1=':c', txt='some text')", + "simple(name='b', foo=[], txt='some text')", + "late_bound_dep(name='c')", + "all_attributes_aspect(name='x', foo=[':a'])"); + useConfiguration("--plugin=//extra:extra"); + update(); + + ConfiguredTarget a = getConfiguredTarget("//a:x"); + assertThat(a.getProvider(RuleInfo.class).getData()) + .containsExactly( + "aspect //a:a", + "aspect //a:b", + "aspect //a:c", + "aspect //extra:extra", + "rule //a:x"); + } + } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java index be4988ecaf..d498709148 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java @@ -33,7 +33,6 @@ import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.NoSuchThingException; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.util.OrderedSetMultimap; - import java.util.List; import java.util.Set; import javax.annotation.Nullable; @@ -162,6 +161,19 @@ public class DependencyResolverTest extends AnalysisTestCase { } @Test + public void hasAllAttributesAspect() throws Exception { + setRulesAvailableInTests(new TestAspects.BaseRule(), new TestAspects.SimpleRule()); + pkg("a", + "simple(name='a', foo=[':b'])", + "simple(name='b', foo=[])"); + OrderedSetMultimap<Attribute, Dependency> map = + dependentNodeMap("//a:a", TestAspects.ALL_ATTRIBUTES_ASPECT); + assertDep( + map, "foo", "//a:b", + new AspectDescriptor(TestAspects.ALL_ATTRIBUTES_ASPECT)); + } + + @Test public void hasAspectDependencies() throws Exception { setRulesAvailableInTests(new TestAspects.BaseRule()); pkg("a", "base(name='a')"); 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 351ae384ef..263a795dbf 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 @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.collect.nestedset.NestedSet; @@ -47,12 +48,14 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.AspectDefinition; import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel; +import com.google.devtools.build.lib.packages.Attribute.LateBoundLabelList; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.NativeAspectClass; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder; import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; +import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.FileTypeSet; import java.util.List; @@ -116,12 +119,19 @@ public class TestAspects { private static NestedSet<String> collectAspectData(String me, RuleContext ruleContext) { NestedSetBuilder<String> result = new NestedSetBuilder<>(Order.STABLE_ORDER); result.add(me); - if (ruleContext.attributes().has("foo", LABEL_LIST)) { - for (AspectInfo dep : ruleContext.getPrerequisites("foo", Mode.TARGET, AspectInfo.class)) { - result.addTransitive(dep.getData()); + + Iterable<String> attributeNames = ruleContext.attributes().getAttributeNames(); + for (String attributeName : attributeNames) { + Type<?> attributeType = ruleContext.attributes().getAttributeType(attributeName); + if (!LABEL.equals(attributeType) && !LABEL_LIST.equals(attributeType)) { + continue; + } + Iterable<AspectInfo> prerequisites = ruleContext + .getPrerequisites(attributeName, Mode.DONT_CHECK, AspectInfo.class); + for (AspectInfo prerequisite : prerequisites) { + result.addTransitive(prerequisite.getData()); } } - return result.build(); } @@ -212,6 +222,24 @@ public class TestAspects { .build(); /** + * An aspect that propagates along all attributes. + */ + public static class AllAttributesAspect extends BaseAspect { + + @Override + public AspectDefinition getDefinition(AspectParameters aspectParameters) { + return ALL_ATTRIBUTES_ASPECT_DEFINITION; + } + } + public static final NativeAspectClass ALL_ATTRIBUTES_ASPECT = new AllAttributesAspect(); + private static final AspectDefinition ALL_ATTRIBUTES_ASPECT_DEFINITION = + new AspectDefinition.Builder("all_attributes_aspect") + .allAttributesAspect(ALL_ATTRIBUTES_ASPECT) + .build(); + + + + /** * An aspect that requires aspects on the attributes of rules it attaches to. */ public static class AttributeAspect extends BaseAspect { @@ -525,6 +553,29 @@ public class TestAspects { } /** + * A rule that defines an {@link AllAttributesAspect} on one of its attributes. + */ + public static class AllAttributesAspectRule implements RuleDefinition { + + @Override + public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) { + return builder + .add(attr("foo", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE) + .aspect(ALL_ATTRIBUTES_ASPECT)) + .build(); + } + + @Override + public Metadata getMetadata() { + return RuleDefinition.Metadata.builder() + .name("all_attributes_aspect") + .factoryClass(DummyRuleFactory.class) + .ancestors(BaseRule.class) + .build(); + } + } + + /** * A rule that defines a {@link WarningAspect} on one of its attributes. */ public static class WarningAspectRule implements RuleDefinition { @@ -578,6 +629,8 @@ public class TestAspects { public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) { return builder .add(attr("foo", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE)) + .add(attr("foo1", LABEL).allowedFileTypes(FileTypeSet.ANY_FILE)) + .add(attr("txt", STRING)) .build(); } @@ -634,4 +687,56 @@ public class TestAspects { .build(); } } + + /** + * Rule with an implcit dependency. + */ + public static class ImplicitDepRule implements RuleDefinition { + @Override + public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) { + return builder + .add(attr("$dep", LABEL).value(Label.parseAbsoluteUnchecked("//extra:extra"))) + .build(); + } + + @Override + public Metadata getMetadata() { + return RuleDefinition.Metadata.builder() + .name("implicit_dep") + .factoryClass(DummyRuleFactory.class) + .ancestors(BaseRule.class) + .build(); + } + } + + /** + * Rule with a late-bound dependency. + */ + public static class LateBoundDepRule implements RuleDefinition { + private static final LateBoundLabelList<BuildConfiguration> PLUGINS_LABEL_LIST = + new LateBoundLabelList<BuildConfiguration>() { + @Override + public List<Label> resolve(Rule rule, AttributeMap attributes, + BuildConfiguration configuration) { + return configuration.getPlugins(); + } + }; + + @Override + public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) { + return builder + .add(attr(":plugins", LABEL_LIST).value(PLUGINS_LABEL_LIST)) + .build(); + } + + @Override + public Metadata getMetadata() { + return RuleDefinition.Metadata.builder() + .name("late_bound_dep") + .factoryClass(DummyRuleFactory.class) + .ancestors(BaseRule.class) + .build(); + } + } + } |