Implement propagation along dependencies for Skylark aspects.
--
MOS_MIGRATED_REVID=106694515
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 3d47a22..f60eb63 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -68,7 +68,7 @@
import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.skyframe.ActionLookupValue;
import com.google.devtools.build.lib.skyframe.AspectValue;
-import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
+import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.CoverageReportValue;
import com.google.devtools.build.lib.skyframe.SkyframeAnalysisResult;
@@ -451,7 +451,7 @@
}
});
- List<AspectKey> aspectKeys = new ArrayList<>();
+ List<AspectValueKey> aspectKeys = new ArrayList<>();
for (String aspect : aspects) {
// Syntax: label%aspect
@@ -478,8 +478,8 @@
}
} else {
@SuppressWarnings("unchecked")
- final Class<? extends ConfiguredAspectFactory> aspectFactoryClass =
- (Class<? extends ConfiguredAspectFactory>)
+ final Class<? extends ConfiguredNativeAspectFactory> aspectFactoryClass =
+ (Class<? extends ConfiguredNativeAspectFactory>)
ruleClassProvider.getAspectFactoryMap().get(aspect);
if (aspectFactoryClass != null) {
for (ConfiguredTargetKey targetSpec : targetSpecs) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspectFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspectFactory.java
index 7c24768..6671640 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspectFactory.java
@@ -13,16 +13,21 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis;
-import com.google.devtools.build.lib.packages.AspectFactory;
+import com.google.devtools.build.lib.packages.AspectParameters;
/**
- * Instantiation of {@link AspectFactory} with the actual types.
- *
- * <p>This is needed because {@link AspectFactory} is needed in the {@code packages} package to
- * do loading phase things properly and to be able to specify them on attributes, but the actual
- * classes are in the {@code view} package, which is not available there.
+ * Creates the Skyframe node of an aspect.
*/
-public interface ConfiguredAspectFactory
- extends AspectFactory<ConfiguredTarget, RuleContext, Aspect> {
-
+public interface ConfiguredAspectFactory {
+ /**
+ * Creates the aspect based on the configured target of the associated rule.
+ *
+ * @param base the configured target of the associated rule
+ * @param context the context of the associated configured target plus all the attributes the
+ * aspect itself has defined
+ * @param parameters information from attributes of the rule that have requested this
+ * aspect
+ */
+ Aspect create(ConfiguredTarget base, RuleContext context, AspectParameters parameters)
+ throws InterruptedException;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredNativeAspectFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredNativeAspectFactory.java
new file mode 100644
index 0000000..e315141
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredNativeAspectFactory.java
@@ -0,0 +1,28 @@
+// Copyright 2015 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.analysis;
+
+import com.google.devtools.build.lib.packages.NativeAspectClass;
+
+/**
+ * A union of {@link ConfiguredAspectFactory} and
+ * {@link com.google.devtools.build.lib.packages.NativeAspectClass.NativeAspectFactory}
+ * All native aspect classes should implement this interface.
+ *
+ */
+public interface ConfiguredNativeAspectFactory
+ extends ConfiguredAspectFactory, NativeAspectClass.NativeAspectFactory {
+
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index 36d497f..9d0cd4c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -34,8 +34,8 @@
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.graph.Digraph;
import com.google.devtools.build.lib.graph.Node;
-import com.google.devtools.build.lib.packages.AspectFactory;
import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.NativeAspectClass.NativeAspectFactory;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
@@ -90,7 +90,7 @@
private final Map<String, RuleClass> ruleClassMap = new HashMap<>();
private final Map<String, Class<? extends RuleDefinition>> ruleDefinitionMap =
new HashMap<>();
- private final Map<String, Class<? extends AspectFactory<?, ?, ?>>> aspectFactoryMap =
+ private final Map<String, Class<? extends NativeAspectFactory>> aspectFactoryMap =
new HashMap<>();
private final Map<Class<? extends RuleDefinition>, RuleClass> ruleMap = new HashMap<>();
private final Map<Class<? extends RuleDefinition>, RuleDefinition> ruleDefinitionInstanceCache =
@@ -140,7 +140,7 @@
}
public Builder addAspectFactory(
- String name, Class<? extends AspectFactory<?, ?, ?>> configuredAspectFactoryClass) {
+ String name, Class<? extends ConfiguredNativeAspectFactory> configuredAspectFactoryClass) {
aspectFactoryMap.put(name, configuredAspectFactoryClass);
return this;
@@ -291,7 +291,7 @@
/**
* Maps aspect name to the aspect factory meta class.
*/
- private final ImmutableMap<String, Class<? extends AspectFactory<?, ?, ?>>> aspectFactoryMap;
+ private final ImmutableMap<String, Class<? extends NativeAspectFactory>> aspectFactoryMap;
/**
* The configuration options that affect the behavior of the rules.
@@ -319,7 +319,7 @@
String runfilesPrefix,
ImmutableMap<String, RuleClass> ruleClassMap,
ImmutableMap<String, Class<? extends RuleDefinition>> ruleDefinitionMap,
- ImmutableMap<String, Class<? extends AspectFactory<?, ?, ?>>> aspectFactoryMap,
+ ImmutableMap<String, Class<? extends NativeAspectFactory>> aspectFactoryMap,
String defaultWorkspaceFile,
ImmutableList<BuildInfoFactory> buildInfoFactories,
ImmutableList<Class<? extends FragmentOptions>> configurationOptions,
@@ -361,7 +361,7 @@
}
@Override
- public Map<String, Class<? extends AspectFactory<?, ?, ?>>> getAspectFactoryMap() {
+ public Map<String, Class<? extends NativeAspectFactory>> getAspectFactoryMap() {
return aspectFactoryMap;
}
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 8d77bee..a18902b 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
@@ -27,7 +27,6 @@
import com.google.devtools.build.lib.collect.ImmutableSortedKeyListMultimap;
import com.google.devtools.build.lib.packages.AspectClass;
import com.google.devtools.build.lib.packages.AspectDefinition;
-import com.google.devtools.build.lib.packages.AspectFactory;
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault;
@@ -560,11 +559,11 @@
RuleClass ruleClass = ((Rule) target).getRuleClassObject();
ImmutableSet.Builder<AspectWithParameters> result = ImmutableSet.builder();
for (AspectWithParameters candidateClass : aspectCandidates) {
- ConfiguredAspectFactory candidate =
- (ConfiguredAspectFactory) AspectFactory.Util.create(candidateClass.getAspectClass());
+ AspectClass aspectClass = candidateClass.getAspectClass();
if (Sets.difference(
- candidate.getDefinition().getRequiredProviders(),
- ruleClass.getAdvertisedProviders()).isEmpty()) {
+ aspectClass.getDefinition().getRequiredProviders(),
+ ruleClass.getAdvertisedProviders())
+ .isEmpty()) {
result.add(candidateClass);
}
}
@@ -581,8 +580,8 @@
Set<AspectWithParameters> aspectCandidates = new LinkedHashSet<>();
for (Map.Entry<AspectClass, AspectParameters> aspectWithParameters :
attribute.getAspectsWithParameters(originalRule).entrySet()) {
- aspectCandidates.add(
- new AspectWithParameters(aspectWithParameters.getKey(), aspectWithParameters.getValue()));
+ AspectClass key = aspectWithParameters.getKey();
+ aspectCandidates.add(new AspectWithParameters(key, aspectWithParameters.getValue()));
}
if (aspectDefinition != null) {
for (AspectClass aspect : aspectDefinition.getAttributeAspects().get(attribute.getName())) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
index d861459..14ef155 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
@@ -130,14 +130,12 @@
providers.addAll(base.providers.keySet());
// Merge output group providers.
- List<OutputGroupProvider> outputGroupProviders =
- getAllProviders(base, aspects, OutputGroupProvider.class);
- OutputGroupProvider mergedOutputGroupProvider = OutputGroupProvider.merge(outputGroupProviders);
+ OutputGroupProvider mergedOutputGroupProvider =
+ OutputGroupProvider.merge(getAllProviders(base, aspects, OutputGroupProvider.class));
// Merge Skylark providers.
- List<SkylarkProviders> skylarkProviders =
- getAllProviders(base, aspects, SkylarkProviders.class);
- SkylarkProviders mergedSkylarkProviders = SkylarkProviders.merge(skylarkProviders);
+ SkylarkProviders mergedSkylarkProviders =
+ SkylarkProviders.merge(getAllProviders(base, aspects, SkylarkProviders.class));
// Validate that all other providers are only provided once.
for (Aspect aspect : aspects) {
@@ -174,7 +172,7 @@
builder.put(OutputGroupProvider.class, mergedOutputGroupProvider);
}
if (mergedSkylarkProviders != null) {
- builder.put(SkylarkProviders.class, skylarkProviders);
+ builder.put(SkylarkProviders.class, mergedSkylarkProviders);
}
this.providers = builder.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java b/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java
index c01100b..c01e10d 100644
--- a/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java
@@ -25,7 +25,7 @@
import com.google.devtools.build.lib.analysis.AnalysisUtils;
import com.google.devtools.build.lib.analysis.Aspect;
import com.google.devtools.build.lib.analysis.Aspect.Builder;
-import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
+import com.google.devtools.build.lib.analysis.ConfiguredNativeAspectFactory;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.RuleContext;
@@ -70,7 +70,7 @@
/**
* Generates ide-build information for Android Studio.
*/
-public class AndroidStudioInfoAspect implements ConfiguredAspectFactory {
+public class AndroidStudioInfoAspect implements ConfiguredNativeAspectFactory {
public static final String NAME = "AndroidStudioInfoAspect";
// Output groups.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java b/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java
index 9c82922..834379c 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java
@@ -17,7 +17,13 @@
/**
* A class of aspects.
*
- * <p>This interface serves as a factory for {@link AspectFactory}.
+ * <p>This interface serves as a factory for {@code AspectFactory}.
+ * {@code AspectFactory} type argument is a placeholder for
+ * a {@link com.google.devtools.build.lib.analysis.ConfiguredAspectFactory}, which is
+ * an analysis-phase class. All loading-phase code uses {@code AspectClass<?>},
+ * whereas analysis-phase code uses {@code AspectClass<ConfiguredAspectFactory>}.
+ * The latter is what all real implementations of this interface should implement.
+ *
*/
public interface AspectClass {
@@ -26,8 +32,5 @@
*/
String getName();
- /**
- * Instantiates an {@link AspectFactory} for this aspect class.
- */
- AspectFactory<?, ?, ?> newInstance();
+ AspectDefinition getDefinition();
}
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 a9c75d7..7c5fa83 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
@@ -22,6 +22,7 @@
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.NativeAspectClass.NativeAspectFactory;
import com.google.devtools.build.lib.util.BinaryPredicate;
import java.util.LinkedHashMap;
@@ -145,14 +146,13 @@
LinkedHashMultimap<Attribute, Label> result = LinkedHashMultimap.create();
for (AspectClass candidateClass : attribute.getAspects()) {
- AspectFactory<?, ?, ?> candidate = AspectFactory.Util.create(candidateClass);
// Check if target satisfies condition for this aspect (has to provide all required
// TransitiveInfoProviders)
if (!advertisedProviders.containsAll(
- candidate.getDefinition().getRequiredProviderNames())) {
+ candidateClass.getDefinition().getRequiredProviderNames())) {
continue;
}
- addAllAttributesOfAspect((Rule) from, result, candidate.getDefinition(), Rule.ALL_DEPS);
+ addAllAttributesOfAspect((Rule) from, result, candidateClass.getDefinition(), Rule.ALL_DEPS);
}
return ImmutableMultimap.copyOf(result);
}
@@ -214,21 +214,38 @@
* by direct dependencies through attribute {@code attribute} on the target associated with this
* aspect.
*
- * <p>Note that {@code AspectFactory} instances are expected in the second argument, but we
- * cannot reference that interface here.
+ * <p>Note that {@code ConfiguredAspectFactory} instances are expected in the second argument,
+ * but we cannot reference that interface here.
*/
@SafeVarargs
public final Builder attributeAspect(
- String attribute, Class<? extends AspectFactory<?, ?, ?>>... aspectFactories) {
+ String attribute, Class<? extends NativeAspectFactory>... aspectFactories) {
Preconditions.checkNotNull(attribute);
- for (Class<? extends AspectFactory<?, ?, ?>> aspectFactory : aspectFactories) {
- this.attributeAspects.put(
- attribute, new NativeAspectClass(Preconditions.checkNotNull(aspectFactory)));
+ for (Class<? extends NativeAspectFactory> aspectFactory : aspectFactories) {
+ this
+ .attributeAspect(
+ attribute, new NativeAspectClass<>(Preconditions.checkNotNull(aspectFactory)));
}
return this;
}
/**
+ * 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);
+
+ this.attributeAspects.put(attribute, Preconditions.checkNotNull(aspectClass));
+
+ return this;
+ }
+
+ /**
* Adds an attribute to the aspect.
*
* <p>Since aspects do not appear in BUILD files, the attribute must be either implicit
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java b/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java
deleted file mode 100644
index bbc1fea..0000000
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java
+++ /dev/null
@@ -1,54 +0,0 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.packages;
-
-/**
- * Creates the Skyframe node of an aspect.
- *
- * <p>Also has a reference to the definition of the aspect.
- */
-public interface AspectFactory<TConfiguredTarget, TRuleContext, TAspect> {
- /**
- * Creates the aspect based on the configured target of the associated rule.
- *
- * @param base the configured target of the associated rule
- * @param context the context of the associated configured target plus all the attributes the
- * aspect itself has defined
- * @param parameters information from attributes of the rule that have requested this
- * aspect
- */
- TAspect create(TConfiguredTarget base, TRuleContext context, AspectParameters parameters)
- throws InterruptedException;
-
- /**
- * Returns the definition of the aspect.
- */
- AspectDefinition getDefinition();
-
- /**
- * Dummy wrapper class for utility methods because interfaces cannot even have static ones.
- */
- public static final class Util {
- private Util() {
- // Should never be instantiated
- }
-
- public static AspectFactory<?, ?, ?> create(AspectClass aspectClass) {
- // TODO(bazel-team): This should be cached somehow, because this method is invoked quite often
-
- return aspectClass.newInstance();
- }
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
index 63a1a3c..b501579 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
@@ -25,6 +25,7 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.NativeAspectClass.NativeAspectFactory;
import com.google.devtools.build.lib.syntax.ClassObject;
import com.google.devtools.build.lib.syntax.ClassObject.SkylarkClassObject;
import com.google.devtools.build.lib.syntax.EvalException;
@@ -701,7 +702,7 @@
* Asserts that a particular aspect probably needs to be computed for all direct dependencies
* through this attribute.
*/
- public Builder<TYPE> aspect(Class<? extends AspectFactory<?, ?, ?>> aspect) {
+ public <T extends NativeAspectFactory> Builder<TYPE> aspect(Class<T> aspect) {
Function<Rule, AspectParameters> noParameters = new Function<Rule, AspectParameters>() {
@Override
public AspectParameters apply(Rule input) {
@@ -717,9 +718,9 @@
*
* @param evaluator function that extracts aspect parameters from rule.
*/
- public Builder<TYPE> aspect(Class<? extends AspectFactory<?, ?, ?>> aspect,
- Function<Rule, AspectParameters> evaluator) {
- this.aspects.add(new RuleAspect(new NativeAspectClass(aspect), evaluator));
+ public <T extends NativeAspectFactory> Builder<TYPE> aspect(
+ Class<T> aspect, Function<Rule, AspectParameters> evaluator) {
+ this.aspects.add(new RuleAspect(new NativeAspectClass<T>(aspect), evaluator));
return this;
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/NativeAspectClass.java b/src/main/java/com/google/devtools/build/lib/packages/NativeAspectClass.java
index 66360c1..7477ea0 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NativeAspectClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NativeAspectClass.java
@@ -18,12 +18,13 @@
* A class of aspects that are implemented natively in Bazel.
*
* <p>This class just wraps a {@link java.lang.Class} implementing the
- * aspect factory. All wrappers of the same class are
+ * aspect factory. All wrappers of the same class are equal.
*/
-public final class NativeAspectClass implements AspectClass {
- private final Class<? extends AspectFactory<?, ?, ?>> nativeClass;
+public final class NativeAspectClass<T extends NativeAspectClass.NativeAspectFactory>
+ implements AspectClass {
+ private final Class<? extends T> nativeClass;
- public NativeAspectClass(Class<? extends AspectFactory<?, ?, ?>> nativeClass) {
+ public NativeAspectClass(Class<? extends T> nativeClass) {
this.nativeClass = nativeClass;
}
@@ -33,7 +34,11 @@
}
@Override
- public AspectFactory<?, ?, ?> newInstance() {
+ public AspectDefinition getDefinition() {
+ return newInstance().getDefinition();
+ }
+
+ public T newInstance() {
try {
return nativeClass.newInstance();
} catch (Exception e) {
@@ -53,4 +58,14 @@
}
return nativeClass.equals(((NativeAspectClass) obj).nativeClass);
}
+
+ /**
+ * Every native aspect should implement this interface.
+ */
+ public interface NativeAspectFactory {
+ /**
+ * Returns the definition of the aspect.
+ */
+ AspectDefinition getDefinition();
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
index df1cb5e..9825e2d 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
@@ -692,9 +692,8 @@
LinkedHashMultimap<Attribute, Label> labels = LinkedHashMultimap.create();
for (Attribute attribute : this.getAttributes()) {
for (AspectClass candidateClass : attribute.getAspects()) {
- AspectFactory<?, ?, ?> candidate = AspectFactory.Util.create(candidateClass);
- AspectDefinition.addAllAttributesOfAspect(Rule.this, labels,
- candidate.getDefinition(), predicate);
+ AspectDefinition.addAllAttributesOfAspect(
+ Rule.this, labels, candidateClass.getDefinition(), predicate);
}
}
return labels.values();
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
index e50645e..db69c24 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
@@ -15,6 +15,7 @@
package com.google.devtools.build.lib.packages;
import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.packages.NativeAspectClass.NativeAspectFactory;
import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.syntax.Environment.Extension;
import com.google.devtools.build.lib.syntax.Mutability;
@@ -45,11 +46,6 @@
Map<String, RuleClass> getRuleClassMap();
/**
- * Returns a map from aspect names to aspect factory objects.
- */
- Map<String, Class<? extends AspectFactory<?, ?, ?>>> getAspectFactoryMap();
-
- /**
* Returns a new Skylark Environment instance for rule creation.
* Implementations need to be thread safe.
* Be sure to close() the mutability before you return the results of said evaluation.
@@ -65,6 +61,11 @@
@Nullable Map<PathFragment, Extension> importMap);
/**
+ * Returns a map from aspect names to aspect factory objects.
+ */
+ Map<String, Class<? extends NativeAspectFactory>> getAspectFactoryMap();
+
+ /**
* Returns the default content of the WORKSPACE file.
*
* <p>Used to provide external dependencies for built-in rules. Rules defined here can be
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/ConservativeAspectResolver.java b/src/main/java/com/google/devtools/build/lib/query2/output/ConservativeAspectResolver.java
index c9f2321..7404126 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/ConservativeAspectResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/ConservativeAspectResolver.java
@@ -20,7 +20,6 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.AspectClass;
import com.google.devtools.build.lib.packages.AspectDefinition;
-import com.google.devtools.build.lib.packages.AspectFactory;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
@@ -44,10 +43,7 @@
for (Attribute attribute : ((Rule) target).getAttributes()) {
for (AspectClass aspectFactory : attribute.getAspects()) {
AspectDefinition.addAllAttributesOfAspect(
- (Rule) target,
- result,
- AspectFactory.Util.create(aspectFactory).getDefinition(),
- Rule.ALL_DEPS);
+ (Rule) target, result, aspectFactory.getDefinition(), Rule.ALL_DEPS);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/PreciseAspectResolver.java b/src/main/java/com/google/devtools/build/lib/query2/output/PreciseAspectResolver.java
index 4a22d49..011cd9c 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/PreciseAspectResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/PreciseAspectResolver.java
@@ -22,7 +22,6 @@
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.AspectClass;
import com.google.devtools.build.lib.packages.AspectDefinition;
-import com.google.devtools.build.lib.packages.AspectFactory;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.NoSuchThingException;
@@ -97,10 +96,7 @@
@Override
public boolean apply(@Nullable Rule rule, @Nullable Attribute attribute) {
for (AspectClass aspectClass : attribute.getAspects()) {
- if (!AspectFactory.Util.create(aspectClass)
- .getDefinition()
- .getAttributes()
- .isEmpty()) {
+ if (!aspectClass.getDefinition().getAttributes().isEmpty()) {
return true;
}
}
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 ddfe3ff..b27fb45 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
@@ -28,6 +28,7 @@
import static com.google.devtools.build.lib.syntax.Type.STRING_LIST;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
@@ -40,7 +41,11 @@
import com.google.devtools.build.lib.analysis.config.RunUnder;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.packages.AspectClass;
+import com.google.devtools.build.lib.packages.AspectDefinition;
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;
@@ -83,6 +88,7 @@
import java.util.HashMap;
import java.util.Map;
+import java.util.Objects;
import java.util.concurrent.ExecutionException;
/**
@@ -336,12 +342,26 @@
returnType = SkylarkAspect.class,
documented = false, // TODO(dslomov): Experimental, document later.
mandatoryPositionals = {@Param(name = "implementation", type = BaseFunction.class)},
+ optionalPositionals = {
+ @Param(
+ name = "attr_aspects",
+ type = SkylarkList.class,
+ generic1 = String.class,
+ defaultValue = "[]"
+ )
+ },
useEnvironment = true
)
private static final BuiltinFunction aspect =
new BuiltinFunction("aspect") {
- public SkylarkAspect invoke(BaseFunction implementation, Environment funcallEnv) {
- return new SkylarkAspect(implementation, funcallEnv);
+ public SkylarkAspect invoke(
+ BaseFunction implementation, SkylarkList attributeAspects, Environment funcallEnv)
+ throws ConversionException {
+ ImmutableList.Builder<String> builder = ImmutableList.<String>builder();
+ for (Object attributeAspect : attributeAspects) {
+ builder.add(STRING.convert(attributeAspect, ""));
+ }
+ return new SkylarkAspect(implementation, builder.build(), funcallEnv);
}
};
@@ -399,14 +419,20 @@
}
}
- public static void exportRuleFunctions(Environment env, PathFragment skylarkFile) {
+ public static void exportRuleFunctionsAndAspects(Environment env, PackageIdentifier skylarkFile) {
for (String name : env.getGlobals().getDirectVariableNames()) {
try {
Object value = env.lookup(name);
if (value instanceof RuleFunction) {
RuleFunction function = (RuleFunction) value;
if (function.skylarkFile == null) {
- function.export(skylarkFile, name);
+ function.export(skylarkFile.getPackageFragment(), name);
+ }
+ }
+ if (value instanceof SkylarkAspect) {
+ SkylarkAspect skylarkAspect = (SkylarkAspect) value;
+ if (!skylarkAspect.isExported()) {
+ skylarkAspect.export(skylarkFile, name);
}
}
} catch (NoSuchVariableException e) {
@@ -539,10 +565,16 @@
*/
public static class SkylarkAspect implements SkylarkValue {
private final BaseFunction implementation;
+ private final ImmutableList<String> attributeAspects;
private final Environment funcallEnv;
+ private Exported exported;
- public SkylarkAspect(BaseFunction implementation, Environment funcallEnv) {
+ public SkylarkAspect(
+ BaseFunction implementation,
+ ImmutableList<String> attributeAspects,
+ Environment funcallEnv) {
this.implementation = implementation;
+ this.attributeAspects = attributeAspects;
this.funcallEnv = funcallEnv;
}
@@ -550,6 +582,10 @@
return implementation;
}
+ public ImmutableList<String> getAttributeAspects() {
+ return attributeAspects;
+ }
+
public Environment getFuncallEnv() {
return funcallEnv;
}
@@ -564,5 +600,100 @@
Printer.append(buffer, "Aspect:");
implementation.write(buffer, quotationMark);
}
+
+ public String getName() {
+ return exported != null ? exported.toString() : "<skylark aspect>";
+ }
+
+ void export(PackageIdentifier extensionFile, String name) {
+ this.exported = new Exported(extensionFile, name);
+ }
+
+ public boolean isExported() {
+ return exported != null;
+ }
+
+ private PackageIdentifier getExtensionFile() {
+ Preconditions.checkArgument(isExported());
+ return exported.extensionFile;
+ }
+
+ private String getExportedName() {
+ Preconditions.checkArgument(isExported());
+ return exported.name;
+ }
+
+ @Immutable
+ private static class Exported {
+ private final PackageIdentifier extensionFile;
+ private final String name;
+
+ public Exported(PackageIdentifier extensionFile, String name) {
+ this.extensionFile = extensionFile;
+ this.name = name;
+ }
+
+ public String toString() {
+ return extensionFile.toString() + "%" + name;
+ }
+ }
+ }
+
+ /**
+ * Implementation of an aspect class defined in Skylark.
+ */
+ @Immutable
+ public static final class SkylarkAspectClass implements AspectClass {
+ private final AspectDefinition aspectDefinition;
+ private final PackageIdentifier extensionFile;
+ private final String exportedName;
+
+ public SkylarkAspectClass(SkylarkAspect skylarkAspect) {
+ Preconditions.checkArgument(skylarkAspect.isExported(), "Skylark aspects must be exported");
+ AspectDefinition.Builder builder = new AspectDefinition.Builder(skylarkAspect.getName());
+ for (String attributeAspect : skylarkAspect.getAttributeAspects()) {
+ builder.attributeAspect(attributeAspect, this);
+ }
+ this.aspectDefinition = builder.build();
+
+ this.extensionFile = skylarkAspect.getExtensionFile();
+ this.exportedName = skylarkAspect.getExportedName();
+ }
+
+ @Override
+ public String getName() {
+ return aspectDefinition.getName();
+ }
+
+ @Override
+ public AspectDefinition getDefinition() {
+ return aspectDefinition;
+ }
+
+ public PackageIdentifier getExtensionFile() {
+ return extensionFile;
+ }
+
+ public String getExportedName() {
+ return exportedName;
+ }
+
+ public int hashCode() {
+ return Objects.hash(extensionFile, exportedName);
+ }
+
+ public boolean equals(Object other) {
+ if (this == other) {
+ return true;
+ }
+ if (!(other instanceof SkylarkAspectClass)) {
+ return false;
+ }
+
+ SkylarkAspectClass that = (SkylarkAspectClass) other;
+ return Objects.equals(this.extensionFile, that.extensionFile)
+ && Objects.equals(this.exportedName, that.exportedName);
+ }
+
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java
index ca2a648..e3e0856 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java
@@ -15,7 +15,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.Aspect;
-import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
+import com.google.devtools.build.lib.analysis.ConfiguredNativeAspectFactory;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.RuleContext;
@@ -38,7 +38,7 @@
* <p>One would think that using the compile time classpath would be enough, but alas, those are
* ijars,
*/
-public class AndroidNeverlinkAspect implements ConfiguredAspectFactory {
+public class AndroidNeverlinkAspect implements ConfiguredNativeAspectFactory {
public static final String NAME = "AndroidNeverlinkAspect";
private static final ImmutableList<String> ATTRIBUTES =
ImmutableList.of(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AbstractJ2ObjcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AbstractJ2ObjcProtoAspect.java
index 8eff147..7d7ff05 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AbstractJ2ObjcProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AbstractJ2ObjcProtoAspect.java
@@ -19,7 +19,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.Aspect;
-import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
+import com.google.devtools.build.lib.analysis.ConfiguredNativeAspectFactory;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.RuleContext;
@@ -49,7 +49,7 @@
* by this class and provided by proto_library will be exported all the way to objc_binary for ObjC
* compilation and linking into the final application bundle.
*/
-public abstract class AbstractJ2ObjcProtoAspect implements ConfiguredAspectFactory {
+public abstract class AbstractJ2ObjcProtoAspect implements ConfiguredNativeAspectFactory {
public static final String NAME = "J2ObjcProtoAspect";
public AspectDefinition getDefinition() {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
index 84427d4..9fc55cc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
@@ -25,7 +25,7 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ParameterFile;
import com.google.devtools.build.lib.analysis.Aspect;
-import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
+import com.google.devtools.build.lib.analysis.ConfiguredNativeAspectFactory;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.RuleContext;
@@ -54,7 +54,7 @@
/**
* J2ObjC transpilation aspect for Java rules.
*/
-public class J2ObjcAspect implements ConfiguredAspectFactory {
+public class J2ObjcAspect implements ConfiguredNativeAspectFactory {
public static final String NAME = "J2ObjcAspect";
/**
* Adds the attribute aspect args to the given AspectDefinition.Builder.
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 57dc717..13764f6 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
@@ -25,21 +25,21 @@
import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.events.StoredEventHandler;
-import com.google.devtools.build.lib.packages.AspectFactory;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
+import com.google.devtools.build.lib.packages.NativeAspectClass;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspect;
+import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspectClass;
import com.google.devtools.build.lib.skyframe.ASTFileLookupValue.ASTLookupInputException;
import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
-import com.google.devtools.build.lib.skyframe.AspectValue.NativeAspectKey;
-import com.google.devtools.build.lib.skyframe.AspectValue.SkylarkAspectKey;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.DependencyEvaluationException;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor.BuildViewProvider;
import com.google.devtools.build.lib.syntax.Type.ConversionException;
@@ -55,30 +55,38 @@
/**
* The Skyframe function that generates aspects.
*/
-public final class AspectFunction<T extends AspectKey> implements SkyFunction {
+public final class AspectFunction implements SkyFunction {
private final BuildViewProvider buildViewProvider;
private final RuleClassProvider ruleClassProvider;
- private final AspectFactoryCreator<T> aspectFactoryCreator;
- public static AspectFunction<NativeAspectKey> createNativeAspectFunction(
- BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
- return new AspectFunction<>(
- buildViewProvider, ruleClassProvider, new NativeAspectFactoryCreator());
- }
-
- public static AspectFunction<SkylarkAspectKey> createSkylarkAspectFunction(
- BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
- return new AspectFunction<>(
- buildViewProvider, ruleClassProvider, new SkylarkAspectFactoryCreator());
- }
-
- private AspectFunction(
- BuildViewProvider buildViewProvider,
- RuleClassProvider ruleClassProvider,
- AspectFactoryCreator<T> aspectFactoryCreator) {
+ public AspectFunction(BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
this.buildViewProvider = buildViewProvider;
this.ruleClassProvider = ruleClassProvider;
- this.aspectFactoryCreator = aspectFactoryCreator;
+ }
+
+ /**
+ * Load Skylark aspect from an extension file. Is to be called from a SkyFunction.
+ *
+ * @return {@code null} if dependencies cannot be satisfied.
+ */
+ @Nullable
+ public static SkylarkAspect loadSkylarkAspect(
+ Environment env, PackageIdentifier extensionFile, String skylarkValueName)
+ throws ASTLookupInputException, ConversionException {
+ SkyKey importFileKey;
+ importFileKey = SkylarkImportLookupValue.key(extensionFile);
+ SkylarkImportLookupValue skylarkImportLookupValue =
+ (SkylarkImportLookupValue) env.getValue(importFileKey);
+ if (skylarkImportLookupValue == null) {
+ return null;
+ }
+
+ Object skylarkValue = skylarkImportLookupValue.getEnvironmentExtension().get(skylarkValueName);
+ if (!(skylarkValue instanceof SkylarkAspect)) {
+ throw new ConversionException(
+ skylarkValueName + " from " + extensionFile.toString() + " is not an aspect");
+ }
+ return (SkylarkAspect) skylarkValue;
}
@Nullable
@@ -88,7 +96,28 @@
SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
NestedSetBuilder<Package> transitivePackages = NestedSetBuilder.stableOrder();
AspectKey key = (AspectKey) skyKey.argument();
- ConfiguredAspectFactory aspectFactory = aspectFactoryCreator.createAspectFactory(skyKey, env);
+ ConfiguredAspectFactory aspectFactory;
+ if (key.getAspect() instanceof NativeAspectClass<?>) {
+ aspectFactory =
+ (ConfiguredAspectFactory) ((NativeAspectClass<?>) key.getAspect()).newInstance();
+ } else if (key.getAspect() instanceof SkylarkAspectClass) {
+ SkylarkAspectClass skylarkAspectClass = (SkylarkAspectClass) key.getAspect();
+ SkylarkAspect skylarkAspect;
+ try {
+ skylarkAspect =
+ loadSkylarkAspect(
+ env, skylarkAspectClass.getExtensionFile(), skylarkAspectClass.getExportedName());
+ } catch (ASTLookupInputException | ConversionException e) {
+ throw new AspectFunctionException(skyKey, e);
+ }
+ if (skylarkAspect == null) {
+ return null;
+ }
+
+ aspectFactory = new SkylarkAspectFactory(skylarkAspect.getName(), skylarkAspect);
+ } else {
+ throw new IllegalStateException();
+ }
PackageValue packageValue =
(PackageValue) env.getValue(PackageValue.key(key.getLabel().getPackageIdentifier()));
@@ -148,9 +177,15 @@
}
ListMultimap<Attribute, ConfiguredTarget> depValueMap =
- ConfiguredTargetFunction.computeDependencies(env, resolver, ctgValue,
- aspectFactory.getDefinition(), key.getParameters(), configConditions,
- ruleClassProvider, view.getHostConfiguration(ctgValue.getConfiguration()),
+ ConfiguredTargetFunction.computeDependencies(
+ env,
+ resolver,
+ ctgValue,
+ key.getAspect().getDefinition(),
+ key.getParameters(),
+ configConditions,
+ ruleClassProvider,
+ view.getHostConfiguration(ctgValue.getConfiguration()),
transitivePackages);
return createAspect(
@@ -247,57 +282,4 @@
}
}
- /**
- * Factory for {@link ConfiguredAspectFactory} given a particular kind of {@link AspectKey}.
- */
- private interface AspectFactoryCreator<T extends AspectKey> {
- ConfiguredAspectFactory createAspectFactory(SkyKey skyKey, Environment env)
- throws AspectFunctionException;
- }
-
- /**
- * Factory for native aspects.
- */
- private static class NativeAspectFactoryCreator implements AspectFactoryCreator<NativeAspectKey> {
-
- @Override
- public ConfiguredAspectFactory createAspectFactory(SkyKey skyKey, Environment env) {
- NativeAspectKey key = (NativeAspectKey) skyKey.argument();
- return (ConfiguredAspectFactory) AspectFactory.Util.create(key.getAspect());
- }
- }
-
- /**
- * Factory for Skylark aspects.
- */
- private static class SkylarkAspectFactoryCreator
- implements AspectFactoryCreator<SkylarkAspectKey> {
-
- @Override
- public ConfiguredAspectFactory createAspectFactory(SkyKey skyKey, Environment env)
- throws AspectFunctionException {
- SkylarkAspectKey skylarkAspectKey = (SkylarkAspectKey) skyKey.argument();
- SkyKey importFileKey;
- try {
- importFileKey = SkylarkImportLookupValue.key(skylarkAspectKey.getExtensionFile());
- } catch (ASTLookupInputException e) {
- throw new AspectFunctionException(skyKey, e);
- }
- SkylarkImportLookupValue skylarkImportLookupValue =
- (SkylarkImportLookupValue) env.getValue(importFileKey);
- if (skylarkImportLookupValue == null) {
- return null;
- }
- Object skylarkValue = skylarkImportLookupValue
- .getEnvironmentExtension()
- .get(skylarkAspectKey.getSkylarkValueName());
- if (!(skylarkValue instanceof SkylarkAspect)) {
- throw new AspectFunctionException(
- new ConversionException(skylarkAspectKey.getSkylarkValueName() + " from "
- + skylarkAspectKey.getExtensionFile().toString() + " is not an aspect"));
- }
- SkylarkAspect skylarkAspect = (SkylarkAspect) skylarkValue;
- return new SkylarkAspectFactory(skylarkAspectKey.getSkylarkValueName(), skylarkAspect);
- }
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index 336056f..318b668 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -15,7 +15,6 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.base.Objects;
-import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.analysis.Aspect;
import com.google.devtools.build.lib.analysis.AspectWithParameters;
@@ -38,65 +37,59 @@
public final class AspectValue extends ActionLookupValue {
/**
+ * A base class for keys that have AspectValue as a Sky value.
+ */
+ public abstract static class AspectValueKey extends ActionLookupKey {
+
+ public abstract String getDescription();
+ }
+
+ /**
* A base class for a key representing an aspect applied to a particular target.
*/
- public abstract static class AspectKey extends ActionLookupKey {
- protected final Label label;
- protected final BuildConfiguration configuration;
+ public static final class AspectKey extends AspectValueKey {
+ private final Label label;
+ private final BuildConfiguration configuration;
+ private final AspectWithParameters aspect;
+ private final String aspectName;
- protected AspectKey(Label label, BuildConfiguration configuration) {
+ protected AspectKey(
+ Label label,
+ BuildConfiguration configuration,
+ AspectClass aspectClass,
+ AspectParameters parameters) {
this.label = label;
this.configuration = configuration;
+ this.aspectName = aspectClass.getName();
+ this.aspect = new AspectWithParameters(aspectClass, parameters);
}
@Override
+ SkyFunctionName getType() {
+ return SkyFunctions.ASPECT;
+ }
+
+
+ @Override
public Label getLabel() {
return label;
}
- public abstract AspectParameters getParameters();
-
- public abstract String getDescription();
-
- public BuildConfiguration getConfiguration() {
- return configuration;
- }
- }
-
- /**
- * The key of an action that is generated by a native aspect.
- */
- public static final class NativeAspectKey extends AspectKey {
- private final AspectWithParameters aspect;
-
- private NativeAspectKey(
- Label label,
- BuildConfiguration configuration,
- AspectClass aspectClass ,
- AspectParameters parameters) {
- super(label, configuration);
- Preconditions.checkNotNull(parameters);
- this.aspect = new AspectWithParameters(aspectClass, parameters);
- }
-
public AspectClass getAspect() {
return aspect.getAspectClass();
}
- @Override
@Nullable
public AspectParameters getParameters() {
return aspect.getParameters();
}
- @Override
public String getDescription() {
return String.format("%s of %s", aspect.getAspectClass().getName(), getLabel());
}
- @Override
- SkyFunctionName getType() {
- return SkyFunctions.NATIVE_ASPECT;
+ public BuildConfiguration getConfiguration() {
+ return configuration;
}
@Override
@@ -110,11 +103,11 @@
return true;
}
- if (!(other instanceof NativeAspectKey)) {
+ if (!(other instanceof AspectKey)) {
return false;
}
- NativeAspectKey that = (NativeAspectKey) other;
+ AspectKey that = (AspectKey) other;
return Objects.equal(label, that.label)
&& Objects.equal(configuration, that.configuration)
&& Objects.equal(aspect, that.aspect);
@@ -129,20 +122,30 @@
}
/**
- * The key of an action that is generated by a skylark aspect.
+ * The key for a skylark aspect.
*/
- public static class SkylarkAspectKey extends AspectKey {
- private final PackageIdentifier extensionFile;
- private final String skylarkFunctionName;
+ public static class SkylarkAspectLoadingKey extends AspectValueKey {
- private SkylarkAspectKey(
+ private final Label targetLabel;
+ private final BuildConfiguration targetConfiguration;
+ private final PackageIdentifier extensionFile;
+ private final String skylarkValueName;
+
+ private SkylarkAspectLoadingKey(
Label targetLabel,
BuildConfiguration targetConfiguration,
PackageIdentifier extensionFile,
String skylarkFunctionName) {
- super(targetLabel, targetConfiguration);
+ this.targetLabel = targetLabel;
+ this.targetConfiguration = targetConfiguration;
+
this.extensionFile = extensionFile;
- this.skylarkFunctionName = skylarkFunctionName;
+ this.skylarkValueName = skylarkFunctionName;
+ }
+
+ @Override
+ SkyFunctionName getType() {
+ return SkyFunctions.LOAD_SKYLARK_ASPECT;
}
public PackageIdentifier getExtensionFile() {
@@ -150,24 +153,20 @@
}
public String getSkylarkValueName() {
- return skylarkFunctionName;
+ return skylarkValueName;
}
- @Override
- public AspectParameters getParameters() {
- return AspectParameters.EMPTY;
+ public Label getTargetLabel() {
+ return targetLabel;
}
- @Override
+ public BuildConfiguration getTargetConfiguration() {
+ return targetConfiguration;
+ }
+
public String getDescription() {
- // Skylark aspects are referred to on command line with <file>%<value name>
- return String.format(
- "%s%%%s of %s", extensionFile.toString(), skylarkFunctionName, getLabel());
- }
-
- @Override
- SkyFunctionName getType() {
- return SkyFunctions.SKYLARK_ASPECT;
+ // Skylark aspects are referred to on command line with <file>%<value ame>
+ return String.format("%s%%%s of %s", extensionFile.toString(), skylarkValueName, targetLabel);
}
}
@@ -215,24 +214,25 @@
AspectClass aspectFactory,
AspectParameters additionalConfiguration) {
return new SkyKey(
- SkyFunctions.NATIVE_ASPECT,
- new NativeAspectKey(label, configuration, aspectFactory, additionalConfiguration));
+ SkyFunctions.ASPECT,
+ new AspectKey(label, configuration, aspectFactory, additionalConfiguration));
}
- public static SkyKey key(AspectKey aspectKey) {
+ public static SkyKey key(AspectValueKey aspectKey) {
return new SkyKey(aspectKey.getType(), aspectKey);
}
- public static NativeAspectKey createAspectKey(
+ public static AspectKey createAspectKey(
Label label, BuildConfiguration configuration, AspectClass aspectFactory) {
- return new NativeAspectKey(label, configuration, aspectFactory, AspectParameters.EMPTY);
+ return new AspectKey(label, configuration, aspectFactory, AspectParameters.EMPTY);
}
- public static SkylarkAspectKey createSkylarkAspectKey(
+ public static SkylarkAspectLoadingKey createSkylarkAspectKey(
Label targetLabel,
BuildConfiguration targetConfiguration,
- PackageIdentifier bzlFile,
- String skylarkFunctionName) {
- return new SkylarkAspectKey(targetLabel, targetConfiguration, bzlFile, skylarkFunctionName);
+ PackageIdentifier skylarkFile,
+ String skylarkExportName) {
+ return new SkylarkAspectLoadingKey(
+ targetLabel, targetConfiguration, skylarkFile, skylarkExportName);
}
}
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 237feea..4493229 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
@@ -47,7 +47,6 @@
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.AspectClass;
import com.google.devtools.build.lib.packages.AspectDefinition;
-import com.google.devtools.build.lib.packages.AspectFactory;
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
@@ -510,7 +509,8 @@
}
ConfiguredTarget depConfiguredTarget = configuredTargetMap.get(depKey);
for (AspectWithParameters depAspect : dep.getAspects()) {
- if (!aspectMatchesConfiguredTarget(depConfiguredTarget, depAspect.getAspectClass())) {
+ AspectClass depAspectClass = depAspect.getAspectClass();
+ if (!aspectMatchesConfiguredTarget(depConfiguredTarget, depAspectClass)) {
continue;
}
@@ -522,11 +522,12 @@
// The configured target should have been created in resolveConfiguredTargetDependencies()
throw new IllegalStateException(e);
} catch (NoSuchThingException e) {
- AspectFactory<?, ?, ?> depAspectFactory =
- AspectFactory.Util.create(depAspect.getAspectClass());
throw new AspectCreationException(
- String.format("Evaluation of aspect %s on %s failed: %s",
- depAspectFactory.getDefinition().getName(), dep.getLabel(), e.toString()));
+ String.format(
+ "Evaluation of aspect %s on %s failed: %s",
+ depAspectClass.getDefinition().getName(),
+ dep.getLabel(),
+ e.toString()));
}
if (aspectValue == null) {
@@ -550,7 +551,7 @@
private static boolean aspectMatchesConfiguredTarget(
ConfiguredTarget dep, AspectClass aspectClass) {
- AspectDefinition aspectDefinition = AspectFactory.Util.create(aspectClass).getDefinition();
+ AspectDefinition aspectDefinition = aspectClass.getDefinition();
for (Class<?> provider : aspectDefinition.getRequiredProviders()) {
if (dep.getProvider(provider.asSubclass(TransitiveInfoProvider.class)) == null) {
return false;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
index ee3d395..b041211 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
@@ -65,8 +65,9 @@
SkyFunctionName.create("CONFIGURED_TARGET");
public static final SkyFunctionName POST_CONFIGURED_TARGET =
SkyFunctionName.create("POST_CONFIGURED_TARGET");
- public static final SkyFunctionName NATIVE_ASPECT = SkyFunctionName.create("NATIVE_ASPECT");
- public static final SkyFunctionName SKYLARK_ASPECT = SkyFunctionName.create("SKYLARK_ASPECT");
+ public static final SkyFunctionName ASPECT = SkyFunctionName.create("ASPECT");
+ public static final SkyFunctionName LOAD_SKYLARK_ASPECT =
+ SkyFunctionName.create("LOAD_SKYLARK_ASPECT");
public static final SkyFunctionName TARGET_COMPLETION =
SkyFunctionName.create("TARGET_COMPLETION");
public static final SkyFunctionName ASPECT_COMPLETION =
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index f7a85e6..e1dc3a0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -63,7 +63,7 @@
import com.google.devtools.build.lib.pkgcache.LoadingPhaseRunner;
import com.google.devtools.build.lib.skyframe.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.skyframe.AspectFunction.AspectCreationException;
-import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
+import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey;
import com.google.devtools.build.lib.skyframe.BuildInfoCollectionValue.BuildInfoKeyAndConfig;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ConfiguredValueCreationException;
import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ConflictException;
@@ -238,7 +238,7 @@
public SkyframeAnalysisResult configureTargets(
EventHandler eventHandler,
List<ConfiguredTargetKey> values,
- List<AspectKey> aspectKeys,
+ List<AspectValueKey> aspectKeys,
EventBus eventBus,
boolean keepGoing)
throws InterruptedException, ViewCreationFailedException {
@@ -253,7 +253,7 @@
Collection<AspectValue> goodAspects = Lists.newArrayListWithCapacity(values.size());
NestedSetBuilder<Package> packages = NestedSetBuilder.stableOrder();
- for (AspectKey aspectKey : aspectKeys) {
+ for (AspectValueKey aspectKey : aspectKeys) {
AspectValue value = (AspectValue) result.get(AspectValue.key(aspectKey));
if (value == null) {
// Skip aspects that couldn't be applied to targets.
@@ -320,8 +320,8 @@
"Analysis of target '"
+ ConfiguredTargetValue.extractLabel(topLevel)
+ "' failed; build aborted";
- } else if (topLevel.argument() instanceof AspectKey) {
- AspectKey aspectKey = (AspectKey) topLevel.argument();
+ } else if (topLevel.argument() instanceof AspectValueKey) {
+ AspectValueKey aspectKey = (AspectValueKey) topLevel.argument();
errorMsg = "Analysis of aspect '" + aspectKey.getDescription() + "' failed; build aborted";
} else {
assert false;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 1b5dfd8..f13505b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -92,7 +92,7 @@
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.pkgcache.TransitivePackageLoader;
import com.google.devtools.build.lib.profiler.AutoProfiler;
-import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
+import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey;
import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.FileDirtinessChecker;
import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ActionCompletedReceiver;
import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ProgressSupplier;
@@ -351,12 +351,8 @@
map.put(SkyFunctions.TRANSITIVE_TRAVERSAL, new TransitiveTraversalFunction());
map.put(SkyFunctions.CONFIGURED_TARGET,
new ConfiguredTargetFunction(new BuildViewProvider(), ruleClassProvider));
- map.put(
- SkyFunctions.NATIVE_ASPECT,
- AspectFunction.createNativeAspectFunction(new BuildViewProvider(), ruleClassProvider));
- map.put(
- SkyFunctions.SKYLARK_ASPECT,
- AspectFunction.createSkylarkAspectFunction(new BuildViewProvider(), ruleClassProvider));
+ map.put(SkyFunctions.ASPECT, new AspectFunction(new BuildViewProvider(), ruleClassProvider));
+ map.put(SkyFunctions.LOAD_SKYLARK_ASPECT, new ToplevelSkylarkAspectFunction());
map.put(SkyFunctions.POST_CONFIGURED_TARGET,
new PostConfiguredTargetFunction(new BuildViewProvider(), ruleClassProvider));
map.put(SkyFunctions.BUILD_CONFIGURATION,
@@ -1320,13 +1316,16 @@
}
/** Configures a given set of configured targets. */
- public EvaluationResult<ActionLookupValue> configureTargets(EventHandler eventHandler,
- List<ConfiguredTargetKey> values, List<AspectKey> aspectKeys, boolean keepGoing)
- throws InterruptedException {
+ public EvaluationResult<ActionLookupValue> configureTargets(
+ EventHandler eventHandler,
+ List<ConfiguredTargetKey> values,
+ List<AspectValueKey> aspectKeys,
+ boolean keepGoing)
+ throws InterruptedException {
checkActive();
List<SkyKey> keys = new ArrayList<>(ConfiguredTargetValue.keys(values));
- for (AspectKey aspectKey : aspectKeys) {
+ for (AspectValueKey aspectKey : aspectKeys) {
keys.add(AspectValue.key(aspectKey));
}
// Make sure to not run too many analysis threads. This can cause memory thrashing.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
index 916803a..c8775c9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
@@ -21,7 +21,6 @@
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.SkylarkProviderValidationUtil;
import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspect;
import com.google.devtools.build.lib.rules.SkylarkRuleContext;
@@ -37,11 +36,11 @@
public class SkylarkAspectFactory implements ConfiguredAspectFactory {
private final String name;
- private final SkylarkAspect aspectFunction;
+ private final SkylarkAspect skylarkAspect;
- public SkylarkAspectFactory(String name, SkylarkAspect aspectFunction) {
+ public SkylarkAspectFactory(String name, SkylarkAspect skylarkAspect) {
this.name = name;
- this.aspectFunction = aspectFunction;
+ this.skylarkAspect = skylarkAspect;
}
@Override
@@ -58,14 +57,14 @@
Environment env =
Environment.builder(mutability)
.setSkylark()
- .setGlobals(aspectFunction.getFuncallEnv().getGlobals())
+ .setGlobals(skylarkAspect.getFuncallEnv().getGlobals())
.setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler())
.build(); // NB: loading phase functions are not available: this is analysis already,
// so we do *not* setLoadingPhase().
Object aspectSkylarkObject;
try {
aspectSkylarkObject =
- aspectFunction
+ skylarkAspect
.getImplementation()
.call(
ImmutableList.<Object>of(base, skylarkRuleContext),
@@ -105,12 +104,7 @@
.registerPhantomFuncall(
String.format("%s(...)", name),
base.getTarget().getAssociatedRule().getLocation(),
- aspectFunction.getImplementation());
+ skylarkAspect.getImplementation());
}
}
-
- @Override
- public AspectDefinition getDefinition() {
- return new AspectDefinition.Builder(name).build();
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
index c0c72f0..bba850b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
@@ -177,7 +177,7 @@
// Skylark UserDefinedFunction-s in that file will share this function definition Environment,
// which will be frozen by the time it is returned by createExtension.
- Extension extension = createExtension(ast, file, importMap, env);
+ Extension extension = createExtension(ast, pkgId, importMap, env);
return new SkylarkImportLookupValue(
extension, new SkylarkFileDependency(label, fileDependencies.build()));
@@ -229,26 +229,27 @@
*/
private Extension createExtension(
BuildFileAST ast,
- PathFragment file,
+ PackageIdentifier packageIdentifier,
Map<PathFragment, Extension> importMap,
Environment env)
- throws InterruptedException, SkylarkImportLookupFunctionException {
+ throws InterruptedException, SkylarkImportLookupFunctionException {
StoredEventHandler eventHandler = new StoredEventHandler();
// TODO(bazel-team): this method overestimates the changes which can affect the
// Skylark RuleClass. For example changes to comments or unused functions can modify the hash.
// A more accurate - however much more complicated - way would be to calculate a hash based on
// the transitive closure of the accessible AST nodes.
- try (Mutability mutability = Mutability.create("importing %s", file)) {
+ try (Mutability mutability = Mutability.create("importing %s", packageIdentifier)) {
com.google.devtools.build.lib.syntax.Environment extensionEnv =
ruleClassProvider.createSkylarkRuleClassEnvironment(
mutability, eventHandler, ast.getContentHashCode(), importMap)
.setupOverride("native", packageFactory.getNativeModule());
ast.exec(extensionEnv, eventHandler);
- SkylarkRuleClassFunctions.exportRuleFunctions(extensionEnv, file);
+ SkylarkRuleClassFunctions.exportRuleFunctionsAndAspects(extensionEnv, packageIdentifier);
Event.replayEventsOn(env.getListener(), eventHandler.getEvents());
if (eventHandler.hasErrors()) {
- throw new SkylarkImportLookupFunctionException(SkylarkImportFailedException.errors(file));
+ throw new SkylarkImportLookupFunctionException(
+ SkylarkImportFailedException.errors(packageIdentifier.getPackageFragment()));
}
return new Extension(extensionEnv);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
new file mode 100644
index 0000000..fabb1bb
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
@@ -0,0 +1,85 @@
+// Copyright 2015 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.packages.AspectParameters;
+import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspect;
+import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspectClass;
+import com.google.devtools.build.lib.skyframe.ASTFileLookupValue.ASTLookupInputException;
+import com.google.devtools.build.lib.skyframe.AspectValue.SkylarkAspectLoadingKey;
+import com.google.devtools.build.lib.syntax.Type.ConversionException;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionException;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+
+import javax.annotation.Nullable;
+
+/**
+ * SkyFunction to load aspects from Skylark extensions and calculate their values.
+ *
+ * Used for loading top-level aspects. At top level, in
+ * {@link com.google.devtools.build.lib.analysis.BuildView}, we cannot invoke two SkyFunctions
+ * one aftre another, so BuildView call this function to do the work.
+ */
+public class ToplevelSkylarkAspectFunction implements SkyFunction {
+
+ @Nullable
+ @Override
+ public SkyValue compute(SkyKey skyKey, Environment env)
+ throws LoadSkylarkAspectFunctionException, InterruptedException {
+ SkylarkAspectLoadingKey aspectLoadingKey = (SkylarkAspectLoadingKey) skyKey.argument();
+ String skylarkValueName = aspectLoadingKey.getSkylarkValueName();
+ PackageIdentifier extensionFile = aspectLoadingKey.getExtensionFile();
+ SkylarkAspect skylarkAspect = null;
+ try {
+ skylarkAspect = AspectFunction.loadSkylarkAspect(env, extensionFile, skylarkValueName);
+ } catch (ASTLookupInputException | ConversionException e) {
+ throw new LoadSkylarkAspectFunctionException(e, skyKey);
+ }
+ if (skylarkAspect == null) {
+ return null;
+ }
+ SkyKey aspectKey =
+ AspectValue.key(
+ aspectLoadingKey.getTargetLabel(),
+ aspectLoadingKey.getTargetConfiguration(),
+ new SkylarkAspectClass(skylarkAspect),
+ AspectParameters.EMPTY);
+
+ return env.getValue(aspectKey);
+ }
+
+ @Nullable
+ @Override
+ public String extractTag(SkyKey skyKey) {
+ return null;
+ }
+
+ /**
+ * Exceptions thrown from ToplevelSkylarkAspectFunction.
+ */
+ public class LoadSkylarkAspectFunctionException extends SkyFunctionException {
+
+ public LoadSkylarkAspectFunctionException(Exception cause, SkyKey childKey) {
+ super(cause, childKey);
+ }
+
+ public LoadSkylarkAspectFunctionException(Exception cause) {
+ super(cause, Transience.PERSISTENT);
+ }
+ }
+}
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 09dd5e2..b81f2f2 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
@@ -34,7 +34,7 @@
* A dummy aspect factory. Is there to demonstrate how to define aspects and so that we can test
* {@code attributeAspect}.
*/
- public static final class TestAspectFactory implements ConfiguredAspectFactory {
+ public static final class TestAspectFactory implements ConfiguredNativeAspectFactory {
private final AspectDefinition definition;
/**
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 67889a8..1a7a24c 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
@@ -27,7 +27,6 @@
import com.google.devtools.build.lib.analysis.util.TestAspects.AspectRequiringRule;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.AspectDefinition;
-import com.google.devtools.build.lib.packages.AspectFactory;
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.NativeAspectClass;
@@ -111,9 +110,7 @@
private ListMultimap<Attribute, Dependency> dependentNodeMap(
String targetName, Class<? extends ConfiguredAspectFactory> aspect) throws Exception {
AspectDefinition aspectDefinition =
- aspect == null
- ? null
- : AspectFactory.Util.create(new NativeAspectClass(aspect)).getDefinition();
+ aspect == null ? null : new NativeAspectClass(aspect).getDefinition();
Target target = packageManager.getTarget(reporter, Label.parseAbsolute(targetName));
return dependencyResolver.dependentNodeMap(
new TargetAndConfiguration(target, getTargetConfiguration()),
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 433cb22..1b2ffbb 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
@@ -26,7 +26,7 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.Aspect;
-import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
+import com.google.devtools.build.lib.analysis.ConfiguredNativeAspectFactory;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
@@ -133,7 +133,7 @@
/**
* A base class for mock aspects to reduce boilerplate.
*/
- public abstract static class BaseAspect implements ConfiguredAspectFactory {
+ public abstract static class BaseAspect implements ConfiguredNativeAspectFactory {
@Override
public Aspect create(ConfiguredTarget base, RuleContext ruleContext,
AspectParameters parameters) {
@@ -222,7 +222,7 @@
/**
* An aspect that raises an error.
*/
- public static class ErrorAspect implements ConfiguredAspectFactory {
+ public static class ErrorAspect implements ConfiguredNativeAspectFactory {
@Override
public Aspect create(ConfiguredTarget base, RuleContext ruleContext,
AspectParameters parameters) {
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 aa5ab30..7633d4f 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
@@ -21,8 +21,12 @@
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.analysis.BuildView.AnalysisResult;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.SkylarkProviders;
import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.skyframe.AspectValue;
+import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
import javax.annotation.Nullable;
@@ -42,7 +46,7 @@
AnalysisResult analysisResult =
update(
ImmutableList.of("//test:xxx"),
- ImmutableList.<String>of("test/aspect.bzl%MyAspect"),
+ ImmutableList.of("test/aspect.bzl%MyAspect"),
false,
LOADING_PHASE_THREADS,
true,
@@ -58,6 +62,82 @@
}
}))
.containsExactly("//test:xxx");
+ assertThat(
+ transform(
+ analysisResult.getAspects(),
+ new Function<AspectValue, String>() {
+ @Nullable
+ @Override
+ public String apply(AspectValue aspectValue) {
+ return String.format(
+ "%s(%s)",
+ aspectValue.getAspect().getName(),
+ aspectValue.getLabel().toString());
+ }
+ }))
+ .containsExactly("test/aspect.bzl%MyAspect(//test:xxx)");
+ }
+
+ public void testAspectPropagating() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "def _impl(target, ctx):",
+ " s = set([target.label])",
+ " for i in ctx.attr.deps:",
+ " s += i.target_labels",
+ " return struct(target_labels = s)",
+ "",
+ "MyAspect = aspect(",
+ " implementation=_impl,",
+ " attr_aspects=['deps'],",
+ ")");
+ scratch.file(
+ "test/BUILD",
+ "java_library(",
+ " name = 'yyy',",
+ ")",
+ "java_library(",
+ " name = 'xxx',",
+ " srcs = ['A.java'],",
+ " deps = [':yyy'],",
+ ")");
+
+ AnalysisResult analysisResult =
+ update(
+ ImmutableList.of("//test:xxx"),
+ ImmutableList.of("test/aspect.bzl%MyAspect"),
+ false,
+ LOADING_PHASE_THREADS,
+ true,
+ new EventBus());
+ assertThat(
+ transform(
+ analysisResult.getTargetsToBuild(),
+ new Function<ConfiguredTarget, String>() {
+ @Nullable
+ @Override
+ public String apply(ConfiguredTarget configuredTarget) {
+ return configuredTarget.getLabel().toString();
+ }
+ }))
+ .containsExactly("//test:xxx");
+ AspectValue aspectValue = analysisResult.getAspects().iterator().next();
+ SkylarkProviders skylarkProviders = aspectValue.getAspect().getProvider(SkylarkProviders.class);
+ assertThat(skylarkProviders).isNotNull();
+ Object names = skylarkProviders.getValue("target_labels");
+ assertThat(names).isInstanceOf(SkylarkNestedSet.class);
+ assertThat(
+ transform(
+ (SkylarkNestedSet) names,
+ new Function<Object, String>() {
+ @Nullable
+ @Override
+ public String apply(Object o) {
+ assertThat(o).isInstanceOf(Label.class);
+ return o.toString();
+ }
+ }))
+ .containsExactly("//test:xxx", "//test:yyy");
}
public void testAspectFailingExecution() throws Exception {
@@ -85,7 +165,7 @@
"ERROR /workspace/test/BUILD:1:1: in java_library rule //test:xxx: \n"
+ "Traceback (most recent call last):\n"
+ "\tFile \"/workspace/test/BUILD\", line 1\n"
- + "\t\tMyAspect(...)\n"
+ + "\t\ttest/aspect.bzl%MyAspect(...)\n"
+ "\tFile \"/workspace/test/aspect.bzl\", line 2, in _impl\n"
+ "\t\t1 / 0\n"
+ "integer division by zero");
