Enable Aspects to specify their configuration fragment dependencies.
Note: This specification currently does not have any effect, but soon...
In the default mode, when an aspect does not call any of the configuration
fragment methods on its AspectDefinition.Builder, the old behavior will
persist; aspects can only access fragments their associated rule has access
to, and have no guarantee as to what those fragments are.
This mode will become deprecated with a future CL.
If an aspect does call a configuration fragment method, it will have a
configuration fragment policy. In a future CL, this will mean it will be
restricted to accessing only those fragments, but will be understood as
requiring access to them for the purposes of dynamic configuration, even if
the rule it is attached to or created by does not otherwise require them.
Eventually, all aspects will be required to declare their configuration
fragments this way.
Skylark aspects may also declare configuration fragments as of this CL.
Two new parameters are added to the aspect() function, fragments and
host_fragments, mirroring the similar parameters for rules.
If both of these parameters are empty or unspecified, the default mode
is used, as with normal aspects.
Also in this CL:
* Minor javadoc fixes for AspectDefinition.
* Additional tests for AspectDefinition.
--
MOS_MIGRATED_REVID=112271713
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 763ae11..3e7bf76 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
@@ -17,20 +17,25 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Multimap;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
import com.google.devtools.build.lib.packages.NativeAspectClass.NativeAspectFactory;
import com.google.devtools.build.lib.util.BinaryPredicate;
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;
+
/**
* The definition of an aspect (see {@link Aspect} for moreinformation.)
*
@@ -56,17 +61,20 @@
private final ImmutableSet<String> requiredProviderNames;
private final ImmutableMap<String, Attribute> attributes;
private final ImmutableMultimap<String, AspectClass> attributeAspects;
+ @Nullable private final ConfigurationFragmentPolicy configurationFragmentPolicy;
private AspectDefinition(
String name,
ImmutableSet<Class<?>> requiredProviders,
ImmutableMap<String, Attribute> attributes,
- ImmutableMultimap<String, AspectClass> attributeAspects) {
+ ImmutableMultimap<String, AspectClass> attributeAspects,
+ @Nullable ConfigurationFragmentPolicy configurationFragmentPolicy) {
this.name = name;
this.requiredProviders = requiredProviders;
this.requiredProviderNames = toStringSet(requiredProviders);
this.attributes = attributes;
this.attributeAspects = attributeAspects;
+ this.configurationFragmentPolicy = configurationFragmentPolicy;
}
public String getName() {
@@ -83,8 +91,8 @@
}
/**
- * Returns the set of {@link com.google.devtools.build.lib.analysis.TransitiveInfoProvider} instances
- * that must be present on a configured target so that this aspect can be applied to it.
+ * Returns the set of {@link com.google.devtools.build.lib.analysis.TransitiveInfoProvider}
+ * instances that must be present on a configured target so that this aspect can be applied to it.
*
* <p>We cannot refer to that class here due to our dependency structure, so this returns a set
* of unconstrained class objects.
@@ -97,11 +105,12 @@
}
/**
- * Returns the set of {@link com.google.devtools.build.lib.analysis.TransitiveInfoProvider}
- * instances that must be present on a configured target so that this aspect can be applied to it.
+ * Returns the set of class names of
+ * {@link com.google.devtools.build.lib.analysis.TransitiveInfoProvider} instances that must be
+ * present on a configured target so that this aspect can be applied to it.
*
- * <p>We cannot refer to that class here due to our dependency structure, so this returns a set
- * of unconstrained class objects.
+ * <p>This set is a mirror of the set returned by {@link #getRequiredProviders}, but contains the
+ * names of the classes rather than the class objects themselves.
*
* <p>If a configured target does not have a required provider, the aspect is silently not created
* for it.
@@ -118,6 +127,17 @@
}
/**
+ * Returns the set of configuration fragments required by this Aspect, or {@code null} if it has
+ * not set a configuration fragment policy, meaning it should inherit from the attached rule.
+ */
+ @Nullable public ConfigurationFragmentPolicy getConfigurationFragmentPolicy() {
+ // TODO(mstaib): When all existing aspects properly set their configuration fragment policy,
+ // this method and the associated member should no longer be nullable.
+ // "inherit from the attached rule" should go away.
+ return configurationFragmentPolicy;
+ }
+
+ /**
* Returns the attribute -> set of labels that are provided by aspects of attribute.
*/
public static ImmutableMultimap<Attribute, Label> visitAspectsIfRequired(
@@ -192,6 +212,16 @@
private final Map<String, Attribute> attributes = new LinkedHashMap<>();
private final Set<Class<?>> requiredProviders = new LinkedHashSet<>();
private final Multimap<String, AspectClass> attributeAspects = LinkedHashMultimap.create();
+ private final ConfigurationFragmentPolicy.Builder configurationFragmentPolicy =
+ new ConfigurationFragmentPolicy.Builder();
+ // TODO(mstaib): When all existing aspects properly set their configuration fragment policy,
+ // remove this flag and the code that interacts with it.
+ /**
+ * True if the aspect definition has intentionally specified a configuration fragment policy by
+ * calling any of the methods which set up the policy, and thus needs the built AspectDefinition
+ * to retain the policy.
+ */
+ private boolean hasConfigurationFragmentPolicy = false;
public Builder(String name) {
this.name = name;
@@ -229,9 +259,6 @@
* Declares that this aspect depends on the given {@link AspectClass} provided
* by direct dependencies through attribute {@code attribute} on the target associated with this
* aspect.
- *
- * <p>Note that {@code ConfiguredAspectFactory} instances are expected in the second argument,
- * but we cannot reference that interface here.
*/
public final Builder attributeAspect(String attribute, AspectClass aspectClass) {
Preconditions.checkNotNull(attribute);
@@ -261,21 +288,96 @@
* configuration is available, starting with ':')
*/
public Builder add(Attribute attribute) {
- Preconditions.checkState(attribute.isImplicit() || attribute.isLateBound());
- Preconditions.checkState(!attributes.containsKey(attribute.getName()),
+ Preconditions.checkArgument(attribute.isImplicit() || attribute.isLateBound());
+ Preconditions.checkArgument(!attributes.containsKey(attribute.getName()),
"An attribute with the name '%s' already exists.", attribute.getName());
attributes.put(attribute.getName(), attribute);
return this;
}
/**
+ * Declares that the implementation of the associated aspect definition requires the given
+ * fragments to be present in this rule's host and target configurations.
+ *
+ * <p>The value is inherited by subclasses.
+ */
+ public Builder requiresConfigurationFragments(Class<?>... configurationFragments) {
+ hasConfigurationFragmentPolicy = true;
+ configurationFragmentPolicy
+ .requiresConfigurationFragments(ImmutableSet.copyOf(configurationFragments));
+ return this;
+ }
+
+ /**
+ * Declares that the implementation of the associated aspect definition requires the given
+ * fragments to be present in the host configuration.
+ *
+ * <p>The value is inherited by subclasses.
+ */
+ public Builder requiresHostConfigurationFragments(Class<?>... configurationFragments) {
+ hasConfigurationFragmentPolicy = true;
+ configurationFragmentPolicy
+ .requiresHostConfigurationFragments(ImmutableSet.copyOf(configurationFragments));
+ return this;
+ }
+
+ /**
+ * Declares the configuration fragments that are required by this rule for the target
+ * configuration.
+ *
+ * <p>In contrast to {@link #requiresConfigurationFragments(Class...)}, this method takes the
+ * Skylark module names of fragments instead of their classes.
+ */
+ public Builder requiresConfigurationFragmentsBySkylarkModuleName(
+ Collection<String> configurationFragmentNames) {
+ // This method is unconditionally called from Skylark code, so only consider the user to have
+ // specified a configuration policy if the collection actually has anything in it.
+ // TODO(mstaib): Stop caring about this as soon as all aspects have configuration policies.
+ hasConfigurationFragmentPolicy =
+ hasConfigurationFragmentPolicy || !configurationFragmentNames.isEmpty();
+ configurationFragmentPolicy
+ .requiresConfigurationFragmentsBySkylarkModuleName(configurationFragmentNames);
+ return this;
+ }
+
+ /**
+ * Declares the configuration fragments that are required by this rule for the host
+ * configuration.
+ *
+ * <p>In contrast to {@link #requiresHostConfigurationFragments(Class...)}, this method takes
+ * the Skylark module names of fragments instead of their classes.
+ */
+ public Builder requiresHostConfigurationFragmentsBySkylarkModuleName(
+ Collection<String> configurationFragmentNames) {
+ // This method is unconditionally called from Skylark code, so only consider the user to have
+ // specified a configuration policy if the collection actually has anything in it.
+ // TODO(mstaib): Stop caring about this as soon as all aspects have configuration policies.
+ hasConfigurationFragmentPolicy =
+ hasConfigurationFragmentPolicy || !configurationFragmentNames.isEmpty();
+ configurationFragmentPolicy
+ .requiresHostConfigurationFragmentsBySkylarkModuleName(configurationFragmentNames);
+ return this;
+ }
+
+ /**
+ * Sets the policy for the case where the configuration is missing required fragments (see
+ * {@link #requiresConfigurationFragments}).
+ */
+ public Builder setMissingFragmentPolicy(MissingFragmentPolicy missingFragmentPolicy) {
+ hasConfigurationFragmentPolicy = true;
+ configurationFragmentPolicy.setMissingFragmentPolicy(missingFragmentPolicy);
+ return this;
+ }
+
+ /**
* Builds the aspect definition.
*
* <p>The builder object is reusable afterwards.
*/
public AspectDefinition build() {
return new AspectDefinition(name, ImmutableSet.copyOf(requiredProviders),
- ImmutableMap.copyOf(attributes), ImmutableMultimap.copyOf(attributeAspects));
+ ImmutableMap.copyOf(attributes), ImmutableSetMultimap.copyOf(attributeAspects),
+ hasConfigurationFragmentPolicy ? configurationFragmentPolicy.build() : null);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
index 1126894..bfe75ec 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -32,6 +32,7 @@
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.Constants;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
@@ -369,6 +370,24 @@
+ "All aspect attributes must be private, so their names must start with <code>_</code>. "
+ "All aspect attributes must be have default values, and be of type "
+ "<code>label</code> or <code>label_list</code>"
+ ),
+ @Param(
+ name = "fragments",
+ type = SkylarkList.class,
+ generic1 = String.class,
+ defaultValue = "[]",
+ doc =
+ "List of names of configuration fragments that the aspect requires "
+ + "in target configuration."
+ ),
+ @Param(
+ name = "host_fragments",
+ type = SkylarkList.class,
+ generic1 = String.class,
+ defaultValue = "[]",
+ doc =
+ "List of names of configuration fragments that the aspect requires "
+ + "in host configuration."
)
},
useEnvironment = true,
@@ -380,6 +399,8 @@
BaseFunction implementation,
SkylarkList attributeAspects,
Object attrs,
+ SkylarkList fragments,
+ SkylarkList hostFragments,
FuncallExpression ast,
Environment funcallEnv)
throws EvalException {
@@ -409,7 +430,13 @@
}
}
- return new SkylarkAspect(implementation, attrAspects.build(), attributes, funcallEnv);
+ return new SkylarkAspect(
+ implementation,
+ attrAspects.build(),
+ attributes,
+ ImmutableSet.copyOf(fragments.getContents(String.class, "fragments")),
+ ImmutableSet.copyOf(hostFragments.getContents(String.class, "host_fragments")),
+ funcallEnv);
}
};
@@ -675,6 +702,8 @@
private final BaseFunction implementation;
private final ImmutableList<String> attributeAspects;
private final ImmutableList<Pair<String, Descriptor>> attributes;
+ private final ImmutableSet<String> fragments;
+ private final ImmutableSet<String> hostFragments;
private final Environment funcallEnv;
private Exported exported;
@@ -682,10 +711,14 @@
BaseFunction implementation,
ImmutableList<String> attributeAspects,
ImmutableList<Pair<String, Descriptor>> attributes,
+ ImmutableSet<String> fragments,
+ ImmutableSet<String> hostFragments,
Environment funcallEnv) {
this.implementation = implementation;
this.attributeAspects = attributeAspects;
this.attributes = attributes;
+ this.fragments = fragments;
+ this.hostFragments = hostFragments;
this.funcallEnv = funcallEnv;
}
@@ -705,6 +738,20 @@
return attributes;
}
+ /**
+ * Gets the set of configuration fragment names needed in the target configuration.
+ */
+ public ImmutableSet<String> getFragments() {
+ return fragments;
+ }
+
+ /**
+ * Gets the set of configuration fragment names needed in the host configuration.
+ */
+ public ImmutableSet<String> getHostFragments() {
+ return hostFragments;
+ }
+
@Override
public boolean isImmutable() {
return implementation.isImmutable();
@@ -782,6 +829,9 @@
for (Pair<String, Descriptor> attribute : attributes) {
builder.add(attribute.second.getAttributeBuilder().build(attribute.first));
}
+ builder.requiresConfigurationFragmentsBySkylarkModuleName(skylarkAspect.getFragments());
+ builder.requiresHostConfigurationFragmentsBySkylarkModuleName(
+ skylarkAspect.getHostFragments());
this.aspectDefinition = builder.build();
}
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 4b622f3..231c0ce 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -480,6 +480,7 @@
"//src/main/java/com/google/devtools/build/lib:events",
"//src/main/java/com/google/devtools/build/lib:java-rules",
"//src/main/java/com/google/devtools/build/lib:packages",
+ "//src/main/java/com/google/devtools/build/lib:skylarkinterface",
"//src/main/java/com/google/devtools/build/lib:util",
"//src/main/java/com/google/devtools/build/lib:vfs",
"//src/main/java/com/google/devtools/build/lib/actions",
@@ -871,6 +872,7 @@
"//src/main/java/com/google/devtools/build/lib:util",
"//src/main/java/com/google/devtools/build/lib:vfs",
"//src/main/java/com/google/devtools/build/lib/actions",
+ "//src/main/java/com/google/devtools/build/lib/rules/cpp",
"//src/main/java/com/google/devtools/build/skyframe",
"//third_party:guava",
"//third_party:guava-testlib",
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 c413271..32a63c8 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
@@ -13,13 +13,23 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis;
+import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.packages.Attribute.attr;
import static org.junit.Assert.fail;
+import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.AspectParameters;
+import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
+import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel;
import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
+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;
@@ -59,23 +69,153 @@
}
@Test
- public void testSimpleAspect() throws Exception {
- new AspectDefinition.Builder("simple")
- .add(attr("$runtime", BuildType.LABEL).value(Label.parseAbsoluteUnchecked("//run:time")))
- .attributeAspect("deps", TestAspectFactory.class)
+ public void testAspectWithImplicitOrLateboundAttribute_AddsToAttributeMap() throws Exception {
+ Attribute implicit = attr("$runtime", BuildType.LABEL)
+ .value(Label.parseAbsoluteUnchecked("//run:time"))
.build();
+ LateBoundLabel<String> latebound = new LateBoundLabel<String>() {
+ @Override
+ public Label getDefault(Rule rule, String configuration) {
+ return Label.parseAbsoluteUnchecked("//run:away");
+ }
+ };
+ AspectDefinition simple = new AspectDefinition.Builder("simple")
+ .add(implicit)
+ .add(attr(":latebound", BuildType.LABEL).value(latebound))
+ .build();
+ assertThat(simple.getAttributes()).containsEntry("$runtime", implicit);
+ assertThat(simple.getAttributes()).containsKey(":latebound");
+ assertThat(simple.getAttributes().get(":latebound").getLateBoundDefault())
+ .isEqualTo(latebound);
}
@Test
- public void testAspectWithUserVisibleAttribute() throws Exception {
+ public void testAspectWithDuplicateAttribute_FailsToAdd() throws Exception {
try {
- new AspectDefinition.Builder("user_visible_attribute")
- .add(attr("invalid", BuildType.LABEL).value(Label.parseAbsoluteUnchecked("//run:time")))
- .attributeAspect("deps", TestAspectFactory.class)
- .build();
- fail(); // expected IllegalStateException
- } catch (IllegalStateException e) {
+ new AspectDefinition.Builder("clash")
+ .add(attr("$runtime", BuildType.LABEL).value(Label.parseAbsoluteUnchecked("//run:time")))
+ .add(attr("$runtime", BuildType.LABEL).value(Label.parseAbsoluteUnchecked("//oops")));
+ fail(); // expected IllegalArgumentException
+ } catch (IllegalArgumentException e) {
// expected
}
}
+
+ @Test
+ public void testAspectWithUserVisibleAttribute_FailsToAdd() throws Exception {
+ try {
+ new AspectDefinition.Builder("user_visible_attribute")
+ .add(
+ attr("invalid", BuildType.LABEL)
+ .value(Label.parseAbsoluteUnchecked("//run:time"))
+ .allowedFileTypes(FileTypeSet.NO_FILE))
+ .build();
+ fail(); // expected IllegalArgumentException
+ } catch (IllegalArgumentException e) {
+ // expected
+ }
+ }
+
+ @Test
+ public void testAttributeAspect_WrapsAndAddsToMap() throws Exception {
+ AspectDefinition withAspects = new AspectDefinition.Builder("attribute_aspect")
+ .attributeAspect("srcs", TestAspectFactory.class)
+ .attributeAspect("deps", new NativeAspectClass<TestAspectFactory>(TestAspectFactory.class))
+ .build();
+ assertThat(withAspects.getAttributeAspects())
+ .containsEntry("srcs", new NativeAspectClass<TestAspectFactory>(TestAspectFactory.class));
+ assertThat(withAspects.getAttributeAspects())
+ .containsEntry("deps", new NativeAspectClass<TestAspectFactory>(TestAspectFactory.class));
+ }
+
+ @Test
+ public void testRequireProvider_AddsToSetOfRequiredProvidersAndNames() throws Exception {
+ AspectDefinition requiresProviders = new AspectDefinition.Builder("required_providers")
+ .requireProvider(String.class)
+ .requireProvider(Integer.class)
+ .build();
+ assertThat(requiresProviders.getRequiredProviders())
+ .containsExactly(String.class, Integer.class);
+ assertThat(requiresProviders.getRequiredProviderNames())
+ .containsExactly("java.lang.String", "java.lang.Integer");
+ }
+
+ @Test
+ public void testNoConfigurationFragmentPolicySetup_ReturnsNull() throws Exception {
+ AspectDefinition noPolicy = new AspectDefinition.Builder("no_policy")
+ .build();
+ assertThat(noPolicy.getConfigurationFragmentPolicy()).isNull();
+ }
+
+ @Test
+ public void testMissingFragmentPolicy_PropagatedToConfigurationFragmentPolicy() throws Exception {
+ AspectDefinition missingFragments = new AspectDefinition.Builder("missing_fragments")
+ .setMissingFragmentPolicy(MissingFragmentPolicy.IGNORE)
+ .build();
+ assertThat(missingFragments.getConfigurationFragmentPolicy()).isNotNull();
+ assertThat(missingFragments.getConfigurationFragmentPolicy().getMissingFragmentPolicy())
+ .isEqualTo(MissingFragmentPolicy.IGNORE);
+ }
+
+ @Test
+ public void testRequiresConfigurationFragments_PropagatedToConfigurationFragmentPolicy()
+ throws Exception {
+ AspectDefinition requiresFragments = new AspectDefinition.Builder("requires_fragments")
+ .requiresConfigurationFragments(Integer.class, String.class)
+ .build();
+ assertThat(requiresFragments.getConfigurationFragmentPolicy()).isNotNull();
+ assertThat(
+ requiresFragments.getConfigurationFragmentPolicy().getRequiredConfigurationFragments())
+ .containsExactly(Integer.class, String.class);
+ }
+
+ @Test
+ public void testRequiresHostConfigurationFragments_PropagatedToConfigurationFragmentPolicy()
+ throws Exception {
+ AspectDefinition requiresFragments = new AspectDefinition.Builder("requires_fragments")
+ .requiresHostConfigurationFragments(Integer.class, String.class)
+ .build();
+ assertThat(requiresFragments.getConfigurationFragmentPolicy()).isNotNull();
+ assertThat(
+ requiresFragments.getConfigurationFragmentPolicy().getRequiredConfigurationFragments())
+ .containsExactly(Integer.class, String.class);
+ }
+
+ @Test
+ public void testRequiresConfigurationFragmentNames_PropagatedToConfigurationFragmentPolicy()
+ throws Exception {
+ AspectDefinition requiresFragments = new AspectDefinition.Builder("requires_fragments")
+ .requiresConfigurationFragmentsBySkylarkModuleName(ImmutableList.of("test_fragment"))
+ .build();
+ assertThat(requiresFragments.getConfigurationFragmentPolicy()).isNotNull();
+ assertThat(
+ requiresFragments.getConfigurationFragmentPolicy()
+ .isLegalConfigurationFragment(TestFragment.class, ConfigurationTransition.NONE))
+ .isTrue();
+ }
+
+ @Test
+ public void testRequiresHostConfigurationFragmentNames_PropagatedToConfigurationFragmentPolicy()
+ throws Exception {
+ AspectDefinition requiresFragments = new AspectDefinition.Builder("requires_fragments")
+ .requiresHostConfigurationFragmentsBySkylarkModuleName(ImmutableList.of("test_fragment"))
+ .build();
+ assertThat(requiresFragments.getConfigurationFragmentPolicy()).isNotNull();
+ assertThat(
+ requiresFragments.getConfigurationFragmentPolicy()
+ .isLegalConfigurationFragment(TestFragment.class, ConfigurationTransition.HOST))
+ .isTrue();
+ }
+
+ @Test
+ public void testEmptySkylarkConfigurationFragmentPolicySetup_ReturnsNull() throws Exception {
+ AspectDefinition noPolicy = new AspectDefinition.Builder("no_policy")
+ .requiresConfigurationFragmentsBySkylarkModuleName(ImmutableList.<String>of())
+ .requiresHostConfigurationFragmentsBySkylarkModuleName(ImmutableList.<String>of())
+ .build();
+ assertThat(noPolicy.getConfigurationFragmentPolicy()).isNull();
+ }
+
+ @SkylarkModule(name = "test_fragment", doc = "test fragment")
+ private static final class TestFragment {}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java
index 0e68f6b..b629094 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java
@@ -18,6 +18,7 @@
import com.google.common.base.Function;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.BuildView.AnalysisResult;
@@ -28,7 +29,12 @@
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.packages.AspectDefinition;
+import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
+import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
+import com.google.devtools.build.lib.rules.java.Jvm;
import com.google.devtools.build.lib.skyframe.AspectValue;
+import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
import org.junit.Test;
@@ -88,6 +94,45 @@
}
@Test
+ public void testAspectAllowsFragmentsToBeSpecified() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "def _impl(target, ctx):",
+ " print('This aspect does nothing')",
+ " return struct()",
+ "MyAspect = aspect(implementation=_impl, fragments=['jvm'], host_fragments=['cpp'])");
+ scratch.file("test/BUILD", "java_library(name = 'xxx',)");
+
+ AnalysisResult analysisResult =
+ update(
+ ImmutableList.of("//test:xxx"),
+ ImmutableList.of("test/aspect.bzl%MyAspect"),
+ false,
+ LOADING_PHASE_THREADS,
+ true,
+ new EventBus());
+ AspectValue aspectValue = Iterables.getOnlyElement(analysisResult.getAspects());
+ AspectKey aspectKey = aspectValue.getKey();
+ AspectDefinition aspectDefinition = aspectKey.getAspect().getDefinition();
+ assertThat(
+ aspectDefinition.getConfigurationFragmentPolicy()
+ .isLegalConfigurationFragment(Jvm.class, ConfigurationTransition.NONE))
+ .isTrue();
+ assertThat(
+ aspectDefinition.getConfigurationFragmentPolicy()
+ .isLegalConfigurationFragment(Jvm.class, ConfigurationTransition.HOST))
+ .isFalse();
+ assertThat(
+ aspectDefinition.getConfigurationFragmentPolicy()
+ .isLegalConfigurationFragment(CppConfiguration.class, ConfigurationTransition.NONE))
+ .isFalse();
+ assertThat(
+ aspectDefinition.getConfigurationFragmentPolicy()
+ .isLegalConfigurationFragment(CppConfiguration.class, ConfigurationTransition.HOST))
+ .isTrue();
+ }
+
+ @Test
public void testAspectPropagating() throws Exception {
scratch.file(
"test/aspect.bzl",
