Expose parameterized aspects to Skylark.
There are no syntactic changes within Skylark; the only difference is that aspects may have non-implicit attributes, so long as they have type 'string' and use the 'values' restriction. Such aspects may only be requested by rules which have attributes with types and names matching the non-implicit, non-defaulted attributes of the aspect.
This is not yet a complete match for native AspectParameters functionality since implicit attributes cannot yet be affected by attribute values, but that will be added later.
Implicit aspects are still required to have default values. Non-implicit aspect attributes are considered "required" unless they have a default value. An error will occur if they are applied to a rule that does not "cover" all required attributes by having attributes of matching name and type. While non-implicit aspect attributes with a default are not required, they will still take on the value of a rule attribute with the same name and type if it is present.
Aspects with non-implicit, non-defaulted ("required") attributes cannot be requested on the command line, only by a rule.
RELNOTES: Expose parameterized aspects to Skylark.
--
MOS_MIGRATED_REVID=121711715
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
index dfd7e5c..93f3f4c 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
@@ -55,7 +55,7 @@
SkylarkAspectClass skylarkAspectClass,
AspectDefinition definition,
AspectParameters parameters) {
- return new Aspect(skylarkAspectClass, definition, parameters);
+ return new Aspect(skylarkAspectClass, definition, parameters);
}
/**
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 c9b8a3b..4e3f08a 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
@@ -23,6 +23,7 @@
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.syntax.Type;
import com.google.devtools.build.lib.util.Preconditions;
import java.util.Collection;
@@ -289,7 +290,9 @@
* configuration is available, starting with ':')
*/
public Builder add(Attribute attribute) {
- Preconditions.checkArgument(attribute.isImplicit() || attribute.isLateBound());
+ Preconditions.checkArgument(attribute.isImplicit() || attribute.isLateBound()
+ || (attribute.getType() == Type.STRING && attribute.checkAllowedValues()),
+ "Invalid attribute '%s' (%s)", attribute.getName(), attribute.getType());
Preconditions.checkArgument(!attributes.containsKey(attribute.getName()),
"An attribute with the name '%s' already exists.", attribute.getName());
attributes.put(attribute.getName(), attribute);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectParameters.java b/src/main/java/com/google/devtools/build/lib/packages/AspectParameters.java
index 0cf175e..6862f10 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectParameters.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectParameters.java
@@ -58,7 +58,7 @@
}
/**
- * Returns collection of values for specified key, or null if key is missing.
+ * Returns collection of values for specified key, or an empty collection if key is missing.
*/
public ImmutableCollection<String> getAttribute(String key) {
return attributes.get(key);
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 501b46c..2b8a738 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
@@ -19,6 +19,7 @@
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
@@ -61,6 +62,9 @@
public static final Predicate<RuleClass> NO_RULE = Predicates.alwaysFalse();
+ /**
+ * Wraps the information necessary to construct an Aspect.
+ */
private abstract static class RuleAspect<C extends AspectClass> {
protected final C aspectClass;
protected final Function<Rule, AspectParameters> parametersExtractor;
@@ -70,6 +74,14 @@
this.parametersExtractor = parametersExtractor;
}
+ public String getName() {
+ return this.aspectClass.getName();
+ }
+
+ public ImmutableSet<String> getRequiredParameters() {
+ return ImmutableSet.<String>of();
+ }
+
public abstract Aspect getAspect(Rule rule);
}
@@ -86,19 +98,40 @@
}
private static class SkylarkRuleAspect extends RuleAspect<SkylarkAspectClass> {
- private final AspectDefinition definition;
+ private final SkylarkAspect aspect;
- public SkylarkRuleAspect(SkylarkAspectClass aspectClass, AspectDefinition definition) {
- super(aspectClass, NO_PARAMETERS);
- this.definition = definition;
+ public SkylarkRuleAspect(SkylarkAspect aspect) {
+ super(aspect.getAspectClass(), aspect.getDefaultParametersExtractor());
+ this.aspect = aspect;
+ }
+
+ @Override
+ public ImmutableSet<String> getRequiredParameters() {
+ return aspect.getParamAttributes();
}
@Override
public Aspect getAspect(Rule rule) {
- return Aspect.forSkylark(
- aspectClass,
- definition,
- parametersExtractor.apply(rule));
+ AspectParameters parameters = parametersExtractor.apply(rule);
+ return Aspect.forSkylark(aspectClass, aspect.getDefinition(parameters), parameters);
+ }
+ }
+
+ /**
+ * A RuleAspect that just wraps a pre-existing Aspect that doesn't vary with the Rule.
+ * For instance, this may come from a DeserializedSkylarkAspect.
+ */
+ private static class PredefinedRuleAspect extends RuleAspect<AspectClass> {
+ private Aspect aspect;
+
+ public PredefinedRuleAspect(Aspect aspect) {
+ super(aspect.getAspectClass(), null);
+ this.aspect = aspect;
+ }
+
+ @Override
+ public Aspect getAspect(Rule rule) {
+ return aspect;
}
}
@@ -351,14 +384,13 @@
}
}
- private static final Function<Rule, AspectParameters> NO_PARAMETERS =
- new Function<Rule, AspectParameters>() {
- @Override
- public AspectParameters apply(Rule input) {
- return AspectParameters.EMPTY;
- }
- };
-
+ public ImmutableMap<String, ImmutableSet<String>> getRequiredAspectParameters() {
+ ImmutableMap.Builder<String, ImmutableSet<String>> paramBuilder = ImmutableMap.builder();
+ for (RuleAspect<?> aspect : aspects) {
+ paramBuilder.put(aspect.getName(), aspect.getRequiredParameters());
+ }
+ return paramBuilder.build();
+ }
/**
* Creates a new attribute builder.
@@ -871,8 +903,13 @@
return this.aspect(aspect, noParameters);
}
- public Builder<TYPE> aspect(SkylarkAspectClass aspectClass, AspectDefinition definition) {
- this.aspects.add(new SkylarkRuleAspect(aspectClass, definition));
+ public Builder<TYPE> aspect(SkylarkAspect skylarkAspect) {
+ this.aspects.add(new SkylarkRuleAspect(skylarkAspect));
+ return this;
+ }
+
+ public Builder<TYPE> aspect(final Aspect aspect) {
+ this.aspects.add(new PredefinedRuleAspect(aspect));
return this;
}
@@ -1637,8 +1674,9 @@
/**
* Returns a replica builder of this Attribute.
*/
- public Attribute.Builder<?> cloneBuilder() {
- Builder<?> builder = new Builder<>(name, this.type);
+ public <TYPE> Attribute.Builder<TYPE> cloneBuilder(Type<TYPE> tp) {
+ Preconditions.checkArgument(tp == this.type);
+ Builder<TYPE> builder = new Builder<TYPE>(name, tp);
builder.allowedFileTypesForLabels = allowedFileTypesForLabels;
builder.allowedRuleClassesForLabels = allowedRuleClassesForLabels;
builder.allowedRuleClassesForLabelsWarning = allowedRuleClassesForLabelsWarning;
@@ -1655,4 +1693,8 @@
return builder;
}
+
+ public Attribute.Builder<?> cloneBuilder() {
+ return cloneBuilder(this.type);
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 7fd42dc..b4aac3b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -1343,6 +1343,7 @@
throws LabelSyntaxException, InterruptedException {
Rule rule = pkgBuilder.createRule(ruleLabel, this, location, attributeContainer);
populateRuleAttributeValues(rule, pkgBuilder, attributeValues, eventHandler);
+ checkAspectAllowedValues(rule, eventHandler);
rule.populateOutputFiles(eventHandler, pkgBuilder);
if (ast != null) {
populateAttributeLocations(rule, ast);
@@ -1741,6 +1742,30 @@
}
}
+ private static void checkAspectAllowedValues(
+ Rule rule, EventHandler eventHandler) {
+ for (Attribute attrOfRule : rule.getAttributes()) {
+ for (Aspect aspect : attrOfRule.getAspects(rule)) {
+ for (Attribute attrOfAspect : aspect.getDefinition().getAttributes().values()) {
+ // By this point the AspectDefinition has been created and values assigned.
+ if (attrOfAspect.checkAllowedValues()) {
+ PredicateWithMessage<Object> allowedValues = attrOfAspect.getAllowedValues();
+ Object value = attrOfAspect.getDefaultValue(rule);
+ if (!allowedValues.apply(value)) {
+ rule.reportError(
+ String.format(
+ "%s: invalid value in '%s' attribute: %s",
+ rule.getLabel(),
+ attrOfAspect.getName(),
+ allowedValues.getErrorReason(value)),
+ eventHandler);
+ }
+ }
+ }
+ }
+ }
+ }
+
@Override
public String toString() {
return name;
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkAspect.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkAspect.java
new file mode 100644
index 0000000..d679273
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkAspect.java
@@ -0,0 +1,172 @@
+// Copyright 2016 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;
+
+import com.google.common.base.Function;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
+import com.google.devtools.build.lib.syntax.BaseFunction;
+import com.google.devtools.build.lib.syntax.Environment;
+import com.google.devtools.build.lib.syntax.Printer;
+import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.util.Pair;
+import com.google.devtools.build.lib.util.Preconditions;
+
+import javax.annotation.Nullable;
+
+/**
+ * A Skylark value that is a result of an 'aspect(..)' function call.
+ */
+@SkylarkModule(name = "aspect", doc = "", documented = false)
+public class SkylarkAspect implements SkylarkValue {
+ private final BaseFunction implementation;
+ private final ImmutableList<String> attributeAspects;
+ private final ImmutableList<Attribute> attributes;
+ private final ImmutableSet<String> paramAttributes;
+ private final ImmutableSet<String> fragments;
+ private final ImmutableSet<String> hostFragments;
+ private final Environment funcallEnv;
+ private SkylarkAspectClass aspectClass;
+
+ public SkylarkAspect(
+ BaseFunction implementation,
+ ImmutableList<String> attributeAspects,
+ ImmutableList<Attribute> attributes,
+ ImmutableSet<String> paramAttributes,
+ ImmutableSet<String> fragments,
+ ImmutableSet<String> hostFragments,
+ Environment funcallEnv) {
+ this.implementation = implementation;
+ this.attributeAspects = attributeAspects;
+ this.attributes = attributes;
+ this.paramAttributes = paramAttributes;
+ this.fragments = fragments;
+ this.hostFragments = hostFragments;
+ this.funcallEnv = funcallEnv;
+ ImmutableList.Builder<Pair<String, Attribute>> builder = ImmutableList.builder();
+ }
+
+ public BaseFunction getImplementation() {
+ return implementation;
+ }
+
+ public ImmutableList<String> getAttributeAspects() {
+ return attributeAspects;
+ }
+
+ public Environment getFuncallEnv() {
+ return funcallEnv;
+ }
+
+ public ImmutableList<Attribute> getAttributes() {
+ return attributes;
+ }
+
+ @Override
+ public boolean isImmutable() {
+ return implementation.isImmutable();
+ }
+
+ @Override
+ public void write(Appendable buffer, char quotationMark) {
+ Printer.append(buffer, "Aspect:");
+ implementation.write(buffer, quotationMark);
+ }
+
+ public String getName() {
+ return getAspectClass().getName();
+ }
+
+ public SkylarkAspectClass getAspectClass() {
+ Preconditions.checkState(isExported());
+ return aspectClass;
+ }
+
+ public ImmutableSet<String> getParamAttributes() {
+ return paramAttributes;
+ }
+
+ public void export(Label extensionLabel, String name) {
+ Preconditions.checkArgument(!isExported());
+ this.aspectClass = new SkylarkAspectClass(extensionLabel, name);
+ }
+
+ public AspectDefinition getDefinition(AspectParameters aspectParams) {
+ AspectDefinition.Builder builder = new AspectDefinition.Builder(getName());
+ for (String attributeAspect : attributeAspects) {
+ builder.attributeAspect(attributeAspect, aspectClass);
+ }
+ for (Attribute attribute : attributes) {
+ Attribute attr = attribute; // Might be reassigned.
+ if (!aspectParams.getAttribute(attr.getName()).isEmpty()) {
+ String value = aspectParams.getOnlyValueOfAttribute(attr.getName());
+ Preconditions.checkState(!Attribute.isImplicit(attr.getName()));
+ Preconditions.checkState(attr.getType() == Type.STRING);
+ Preconditions.checkArgument(aspectParams.getAttribute(attr.getName()).size() == 1,
+ String.format("Aspect %s parameter %s has %d values (must have exactly 1).",
+ getName(),
+ attr.getName(),
+ aspectParams.getAttribute(attr.getName()).size()));
+ attr = attr.cloneBuilder(Type.STRING).value(value).build(attr.getName());
+ }
+ builder.add(attr);
+ }
+ builder.requiresConfigurationFragmentsBySkylarkModuleName(fragments);
+ builder.requiresHostConfigurationFragmentsBySkylarkModuleName(hostFragments);
+ return builder.build();
+ }
+
+ public boolean isExported() {
+ return aspectClass != null;
+ }
+
+ public Function<Rule, AspectParameters> getDefaultParametersExtractor() {
+ return new Function<Rule, AspectParameters>() {
+ @Nullable
+ @Override
+ public AspectParameters apply(Rule rule) {
+ AttributeMap ruleAttrs = RawAttributeMapper.of(rule);
+ AspectParameters.Builder builder = new AspectParameters.Builder();
+ for (Attribute aspectAttr : attributes) {
+ if (!Attribute.isImplicit(aspectAttr.getName())) {
+ String param = aspectAttr.getName();
+ Attribute ruleAttr = ruleAttrs.getAttributeDefinition(param);
+ if (paramAttributes.contains(aspectAttr.getName())) {
+ // These are preconditions because if they are false, RuleFunction.call() should
+ // already have generated an error.
+ Preconditions.checkArgument(ruleAttr != null,
+ String.format("Cannot apply aspect %s to %s that does not define attribute '%s'.",
+ getName(),
+ rule.getTargetKind(),
+ param));
+ Preconditions.checkArgument(ruleAttr.getType() == Type.STRING,
+ String.format("Cannot apply aspect %s to %s with non-string attribute '%s'.",
+ getName(),
+ rule.getTargetKind(),
+ param));
+ }
+ if (ruleAttr != null && ruleAttr.getType() == aspectAttr.getType()) {
+ builder.addAttribute(param, (String) ruleAttrs.get(param, ruleAttr.getType()));
+ }
+ }
+ }
+ return builder.build();
+ }
+ };
+ }
+}
