For native rule classes, (de)serialize only explicit attrs
Native rule classes can provide default values for rules after they're
deserialized, so there isn't a need to serialize those default values.
This doesn't apply yet to rules with Skylark-defined rule classes, due
to the non-serializablity of Skylark rule classes.
--
MOS_MIGRATED_REVID=112066930
diff --git a/src/main/java/com/google/devtools/build/lib/packages/ExternalPackageBuilder.java b/src/main/java/com/google/devtools/build/lib/packages/ExternalPackageBuilder.java
index 8570899..b771763 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ExternalPackageBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ExternalPackageBuilder.java
@@ -20,6 +20,7 @@
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.events.StoredEventHandler;
+import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
import com.google.devtools.build.lib.syntax.FuncallExpression;
import com.google.devtools.build.lib.util.Preconditions;
@@ -40,8 +41,10 @@
InterruptedException {
StoredEventHandler eventHandler = new StoredEventHandler();
+ BuildLangTypedAttributeValuesMap attributeValues = new BuildLangTypedAttributeValuesMap(kwargs);
Rule tempRule =
- RuleFactory.createRule(pkg, ruleClass, kwargs, eventHandler, ast, ast.getLocation(), null);
+ RuleFactory.createRule(
+ pkg, ruleClass, attributeValues, eventHandler, ast, ast.getLocation(), /*env=*/ null);
pkg.addEvents(eventHandler.getEvents());
overwriteRule(pkg, tempRule);
for (Map.Entry<String, Label> entry :
@@ -64,8 +67,11 @@
attributes.put("actual", actual);
}
StoredEventHandler handler = new StoredEventHandler();
+ BuildLangTypedAttributeValuesMap attributeValues =
+ new BuildLangTypedAttributeValuesMap(attributes);
Rule rule =
- RuleFactory.createRule(pkg, bindRuleClass, attributes, handler, null, location, null);
+ RuleFactory.createRule(
+ pkg, bindRuleClass, attributeValues, handler, /*ast=*/ null, location, /*env=*/ null);
overwriteRule(pkg, rule);
rule.setVisibility(ConstantRuleVisibility.PUBLIC);
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index ecf1df7..bc78044 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -1001,17 +1001,16 @@
}
/**
- * Returns a new Rule belonging to this package instance, and uses the given Label.
+ * Creates a new {@link Rule} {@code r} where {@code r.getPackage()} is the {@link Package}
+ * associated with this {@link Builder}.
*
- * <p>Useful for RuleClass instantiation, where the rule name is checked by trying to create a
- * Label. This label can then be used again here.
+ * <p>The created {@link Rule} will have no attribute values, no output files, and therefore
+ * will be in an invalid state.
*/
- Rule newRuleWithLabel(Label label, RuleClass ruleClass, Location location) {
- return newRuleWithLabelAndAttrContainer(label, ruleClass, location,
- new AttributeContainer(ruleClass));
- }
-
- Rule newRuleWithLabelAndAttrContainer(Label label, RuleClass ruleClass, Location location,
+ Rule createRule(
+ Label label,
+ RuleClass ruleClass,
+ Location location,
AttributeContainer attributeContainer) {
return new Rule(pkg, label, ruleClass, location, attributeContainer);
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index ba9111c..1de065e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.packages.GlobCache.BadGlobException;
import com.google.devtools.build.lib.packages.License.DistributionType;
import com.google.devtools.build.lib.packages.Preprocessor.AstAfterPreprocessing;
+import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature;
import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature.Param;
import com.google.devtools.build.lib.syntax.AssignmentStatement;
@@ -923,7 +924,8 @@
Environment env)
throws RuleFactory.InvalidRuleException, Package.NameConflictException, InterruptedException {
RuleClass ruleClass = getBuiltInRuleClass(ruleClassName, ruleFactory);
- RuleFactory.createAndAddRule(context, ruleClass, kwargs, ast, env);
+ BuildLangTypedAttributeValuesMap attributeValues = new BuildLangTypedAttributeValuesMap(kwargs);
+ RuleFactory.createAndAddRule(context, ruleClass, attributeValues, ast, env);
}
private static RuleClass getBuiltInRuleClass(String ruleClassName, RuleFactory ruleFactory) {
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 1b07ae0..8e00db4 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
@@ -36,6 +36,7 @@
import com.google.devtools.build.lib.packages.BuildType.SelectorList;
import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
+import com.google.devtools.build.lib.packages.RuleFactory.AttributeValuesMap;
import com.google.devtools.build.lib.syntax.Argument;
import com.google.devtools.build.lib.syntax.BaseFunction;
import com.google.devtools.build.lib.syntax.Environment;
@@ -45,6 +46,7 @@
import com.google.devtools.build.lib.syntax.Runtime;
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.syntax.Type.ConversionException;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.util.StringUtil;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -1273,104 +1275,189 @@
}
/**
- * Helper function for {@link RuleFactory#createAndAddRule}.
+ * Creates a new {@link Rule} {@code r} where {@code r.getPackage()} is the {@link Package}
+ * associated with {@code pkgBuilder}.
+ *
+ * <p>The created {@link Rule} will be populated with attribute values from {@code
+ * attributeValues} or the default attribute values associated with this {@link RuleClass} and
+ * {@code pkgBuilder}.
+ *
+ * <p>The created {@link Rule} will also be populated with output files. These output files
+ * will have been collected from the explicitly provided values of type {@link BuildType#OUTPUT}
+ * and {@link BuildType#OUTPUT_LIST} as well as from the implicit outputs determined by this
+ * {@link RuleClass} and the values in {@code attributeValues}.
+ *
+ * <p>This performs several validity checks. Invalid output file labels result in a thrown {@link
+ * LabelSyntaxException}. All other errors are reported on {@code eventHandler}.
*/
- Rule createRuleWithLabel(Package.Builder pkgBuilder, Label ruleLabel,
- Map<String, Object> attributeValues, EventHandler eventHandler, FuncallExpression ast,
- Location location) throws LabelSyntaxException, InterruptedException {
- Rule rule = pkgBuilder.newRuleWithLabel(ruleLabel, this, location);
- createRuleCommon(rule, pkgBuilder, attributeValues, eventHandler, ast);
- return rule;
- }
-
- private void createRuleCommon(Rule rule, Package.Builder pkgBuilder,
- Map<String, Object> attributeValues, EventHandler eventHandler, FuncallExpression ast)
+ Rule createRule(
+ Package.Builder pkgBuilder,
+ Label ruleLabel,
+ AttributeValuesMap attributeValues,
+ EventHandler eventHandler,
+ @Nullable FuncallExpression ast,
+ Location location,
+ AttributeContainer attributeContainer)
throws LabelSyntaxException, InterruptedException {
- populateRuleAttributeValues(
- rule, pkgBuilder, attributeValues, eventHandler, ast);
+ Rule rule = pkgBuilder.createRule(ruleLabel, this, location, attributeContainer);
+ populateRuleAttributeValues(rule, pkgBuilder, attributeValues, eventHandler);
rule.populateOutputFiles(eventHandler, pkgBuilder);
- rule.checkForNullLabels();
- rule.checkValidityPredicate(eventHandler);
- }
-
- /**
- * Populates the attributes table of new rule "rule" from the
- * "attributeValues" mapping from attribute names to values in the build
- * language. Errors are reported on "reporter". "ast" is used to associate
- * location information with each rule attribute.
- */
- private void populateRuleAttributeValues(Rule rule,
- Package.Builder pkgBuilder,
- Map<String, Object> attributeValues,
- EventHandler eventHandler,
- FuncallExpression ast)
- throws InterruptedException {
- BitSet definedAttrs = new BitSet(); // set of attr indices
-
- for (Map.Entry<String, Object> entry : attributeValues.entrySet()) {
- String attributeName = entry.getKey();
- Object attributeValue = entry.getValue();
- if (attributeValue == Runtime.NONE) { // Ignore all None values.
- continue;
- }
- Integer attrIndex = setRuleAttributeValue(rule, eventHandler, attributeName, attributeValue);
- if (attrIndex != null) {
- definedAttrs.set(attrIndex);
- checkAttrValNonEmpty(rule, eventHandler, attributeValue, attrIndex);
- }
- }
-
- // Save the location of each non-default attribute definition:
if (ast != null) {
- for (Argument.Passed arg : ast.getArguments()) {
- if (arg.isKeyword()) {
- String name = arg.getName();
- Integer attrIndex = getAttributeIndex(name);
- if (attrIndex != null) {
- rule.setAttributeLocation(attrIndex, arg.getValue().getLocation());
- }
- }
- }
+ populateAttributeLocations(rule, ast);
}
-
- List<Attribute> attrsWithComputedDefaults = new ArrayList<>();
-
- // Set defaults; ensure that every mandatory attribute has a value. Use
- // the default if none is specified.
- int numAttributes = getAttributeCount();
- for (int attrIndex = 0; attrIndex < numAttributes; ++attrIndex) {
- if (!definedAttrs.get(attrIndex)) {
- Attribute attr = getAttribute(attrIndex);
- if (attr.isMandatory()) {
- rule.reportError(rule.getLabel() + ": missing value for mandatory "
- + "attribute '" + attr.getName() + "' in '"
- + name + "' rule", eventHandler);
- }
-
- if (attr.hasComputedDefault()) {
- attrsWithComputedDefaults.add(attr);
- } else {
- Object defaultValue = getAttributeNoncomputedDefaultValue(attr, pkgBuilder);
- checkAttrValNonEmpty(rule, eventHandler, defaultValue, attrIndex);
- rule.setAttributeValue(attr, defaultValue, /*explicit=*/false);
- checkAllowedValues(rule, attr, eventHandler);
- }
- }
- }
-
- // Evaluate and set any computed defaults now that all non-computed
- // TODO(bazel-team): remove this special casing. Thanks to configurable attributes refactoring,
- // computed defaults don't get bound to their final values at this point, so we no longer
- // have to wait until regular attributes have been initialized.
- for (Attribute attr : attrsWithComputedDefaults) {
- rule.setAttributeValue(attr, attr.getDefaultValue(rule), /*explicit=*/false);
- }
-
- // Now that all attributes are bound to values, collect and store configurable attribute keys.
- populateConfigDependenciesAttribute(rule);
checkForDuplicateLabels(rule, eventHandler);
checkThirdPartyRuleHasLicense(rule, pkgBuilder, eventHandler);
checkForValidSizeAndTimeoutValues(rule, eventHandler);
+ rule.checkForNullLabels();
+ rule.checkValidityPredicate(eventHandler);
+ return rule;
+ }
+
+ /**
+ * Populates the attributes table of the new {@link Rule} with the values in the {@code
+ * attributeValues} map and with default values provided by this {@link RuleClass} and the {@code
+ * pkgBuilder}.
+ *
+ * <p>Errors are reported on {@code eventHandler}.
+ */
+ private void populateRuleAttributeValues(
+ Rule rule,
+ Package.Builder pkgBuilder,
+ AttributeValuesMap attributeValues,
+ EventHandler eventHandler) {
+ BitSet definedAttrIndices =
+ populateDefinedRuleAttributeValues(rule, attributeValues, eventHandler);
+ populateDefaultRuleAttributeValues(rule, pkgBuilder, definedAttrIndices, eventHandler);
+ // Now that all attributes are bound to values, collect and store configurable attribute keys.
+ populateConfigDependenciesAttribute(rule);
+ }
+
+ /**
+ * Populates the attributes table of the new {@link Rule} with the values in the {@code
+ * attributeValues} map.
+ *
+ * <p>Handles the special cases of the attribute named {@code "name"} and attributes with value
+ * {@link Runtime#NONE}.
+ *
+ * <p>Returns a bitset {@code b} where {@code b.get(i)} is {@code true} if this method set a
+ * value for the attribute with index {@code i} in this {@link RuleClass}. Errors are reported
+ * on {@code eventHandler}.
+ */
+ private BitSet populateDefinedRuleAttributeValues(
+ Rule rule, AttributeValuesMap attributeValues, EventHandler eventHandler) {
+ BitSet definedAttrIndices = new BitSet();
+ for (String attributeName : attributeValues.getAttributeNames()) {
+ // The attribute named "name" was handled in a special way already.
+ if (attributeName.equals("name")) {
+ continue;
+ }
+
+ Object attributeValue = attributeValues.getAttributeValue(attributeName);
+ // Ignore all None values.
+ if (attributeValue == Runtime.NONE) {
+ continue;
+ }
+
+ // Check that the attribute's name belongs to a valid attribute for this rule class.
+ Integer attrIndex = getAttributeIndex(attributeName);
+ if (attrIndex == null) {
+ rule.reportError(
+ String.format(
+ "%s: no such attribute '%s' in '%s' rule", rule.getLabel(), attributeName, name),
+ eventHandler);
+ continue;
+ }
+ Attribute attr = getAttribute(attrIndex);
+
+ // Convert the build-lang value to a native value, if necessary.
+ Object nativeAttributeValue;
+ if (attributeValues.valuesAreBuildLanguageTyped()) {
+ try {
+ nativeAttributeValue = convertFromBuildLangType(rule, attr, attributeValue);
+ } catch (ConversionException e) {
+ rule.reportError(String.format("%s: %s", rule.getLabel(), e.getMessage()), eventHandler);
+ continue;
+ }
+ } else {
+ nativeAttributeValue = attributeValue;
+ }
+
+ boolean explicit = attributeValues.isAttributeExplicitlySpecified(attributeName);
+ setRuleAttributeValue(rule, eventHandler, attr, nativeAttributeValue, explicit);
+ definedAttrIndices.set(attrIndex);
+ checkAttrValNonEmpty(rule, eventHandler, attributeValue, attr);
+ }
+ return definedAttrIndices;
+ }
+
+ /** Populates attribute locations for attributes defined in {@code ast}. */
+ private void populateAttributeLocations(Rule rule, FuncallExpression ast) {
+ for (Argument.Passed arg : ast.getArguments()) {
+ if (arg.isKeyword()) {
+ String name = arg.getName();
+ Integer attrIndex = getAttributeIndex(name);
+ if (attrIndex != null) {
+ rule.setAttributeLocation(attrIndex, arg.getValue().getLocation());
+ }
+ }
+ }
+ }
+
+ /**
+ * Populates the attributes table of the new {@link Rule} with default values provided by this
+ * {@link RuleClass} and the {@code pkgBuilder}. This will only provide values for attributes
+ * that haven't already been populated, using {@code definedAttrIndices} to determine whether an
+ * attribute was populated.
+ *
+ * <p>Errors are reported on {@code eventHandler}.
+ */
+ private void populateDefaultRuleAttributeValues(
+ Rule rule, Package.Builder pkgBuilder, BitSet definedAttrIndices, EventHandler eventHandler) {
+ // Set defaults; ensure that every mandatory attribute has a value. Use the default if none
+ // is specified.
+ List<Attribute> attrsWithComputedDefaults = new ArrayList<>();
+ int numAttributes = getAttributeCount();
+ for (int attrIndex = 0; attrIndex < numAttributes; ++attrIndex) {
+ if (definedAttrIndices.get(attrIndex)) {
+ continue;
+ }
+ Attribute attr = getAttribute(attrIndex);
+ if (attr.isMandatory()) {
+ rule.reportError(
+ String.format(
+ "%s: missing value for mandatory attribute '%s' in '%s' rule",
+ rule.getLabel(),
+ attr.getName(),
+ name),
+ eventHandler);
+ }
+
+ if (attr.hasComputedDefault()) {
+ // Note that it is necessary to set all non-computed default values before calling
+ // Attribute#getDefaultValue for computed default attributes. Computed default attributes
+ // may have a condition predicate (i.e. the predicate returned by Attribute#getCondition)
+ // that depends on non-computed default attribute values, and that condition predicate is
+ // evaluated by the call to Attribute#getDefaultValue.
+ attrsWithComputedDefaults.add(attr);
+ } else {
+ Object defaultValue = getAttributeNoncomputedDefaultValue(attr, pkgBuilder);
+ checkAttrValNonEmpty(rule, eventHandler, defaultValue, attr);
+ rule.setAttributeValue(attr, defaultValue, /*explicit=*/ false);
+ checkAllowedValues(rule, attr, eventHandler);
+ }
+ }
+
+ // Set computed default attribute values now that all other (i.e. non-computed) default values
+ // have been set.
+ for (Attribute attr : attrsWithComputedDefaults) {
+ // If Attribute#hasComputedDefault is true, Attribute#getDefaultValue returns the computed
+ // default function object. Note that we don't evaluate the computed default function here
+ // because it may depend on other attribute values that are configurable (i.e. they came
+ // from select({..}) expressions in the build language, and they require configuration data
+ // from the analysis phase to be resolved). We're setting the attribute value to a
+ // reference to the computed default function.
+ rule.setAttributeValue(attr, attr.getDefaultValue(rule), /*explicit=*/ false);
+ }
}
/**
@@ -1398,9 +1485,7 @@
}
private void checkAttrValNonEmpty(
- Rule rule, EventHandler eventHandler, Object attributeValue, Integer attrIndex) {
-
- Attribute attr = getAttribute(attrIndex);
+ Rule rule, EventHandler eventHandler, Object attributeValue, Attribute attr) {
if (!attr.isNonEmpty()) {
return;
}
@@ -1525,65 +1610,64 @@
}
/**
- * Sets the value of attribute "attrName" in rule "rule", by converting the
- * build-language value "attrVal" to the appropriate type for the attribute.
- * Returns the attribute index iff successful, null otherwise.
+ * Sets the value of attribute {@code attr} in {@code rule} to the native value {@code
+ * nativeAttrVal}, and sets the value's explicitness to {@code explicit}.
*
- * <p>In case of failure, error messages are reported on "handler", and "rule"
- * is marked as containing errors.
+ * <p>Handles the special case of the "visibility" attribute by also setting the rule's
+ * visibility with {@link Rule#setVisibility}.
+ *
+ * <p>Checks that {@code nativeAttrVal} is an allowed value via {@link #checkAllowedValues}.
*/
- @SuppressWarnings("unchecked")
- private Integer setRuleAttributeValue(Rule rule,
- EventHandler eventHandler,
- String attrName,
- Object attrVal) {
- if (attrName.equals("name")) {
- return null; // "name" is handled specially
- }
-
- Integer attrIndex = getAttributeIndex(attrName);
- if (attrIndex == null) {
- rule.reportError(rule.getLabel() + ": no such attribute '" + attrName
- + "' in '" + name + "' rule", eventHandler);
- return null;
- }
-
- Attribute attr = getAttribute(attrIndex);
- Object converted;
- try {
- String what = "attribute '" + attrName + "' in '" + name + "' rule";
- converted = BuildType.selectableConvert(attr.getType(), attrVal, what, rule.getLabel());
-
- if ((converted instanceof SelectorList<?>) && !attr.isConfigurable()) {
- rule.reportError(rule.getLabel() + ": attribute \"" + attr.getName()
- + "\" is not configurable", eventHandler);
- return null;
- }
-
- if ((converted instanceof List<?>) && !(converted instanceof GlobList<?>)) {
- if (attr.isOrderIndependent()) {
- converted = Ordering.natural().sortedCopy((List<? extends Comparable<?>>) converted);
- }
- converted = ImmutableList.copyOf((List<?>) converted);
- }
- } catch (Type.ConversionException e) {
- rule.reportError(rule.getLabel() + ": " + e.getMessage(), eventHandler);
- return null;
- }
-
- if (attrName.equals("visibility")) {
- List<Label> attrList = (List<Label>) converted;
+ private static void setRuleAttributeValue(
+ Rule rule,
+ EventHandler eventHandler,
+ Attribute attr,
+ Object nativeAttrVal,
+ boolean explicit) {
+ if (attr.getName().equals("visibility")) {
+ @SuppressWarnings("unchecked")
+ List<Label> attrList = (List<Label>) nativeAttrVal;
if (!attrList.isEmpty()
&& ConstantRuleVisibility.LEGACY_PUBLIC_LABEL.equals(attrList.get(0))) {
- rule.reportError(rule.getLabel() + ": //visibility:legacy_public only allowed in package "
- + "declaration", eventHandler);
+ rule.reportError(
+ rule.getLabel() + ": //visibility:legacy_public only allowed in package declaration",
+ eventHandler);
}
rule.setVisibility(PackageFactory.getVisibility(rule.getLabel(), attrList));
}
-
- rule.setAttributeValue(attr, converted, /*explicit=*/true);
+ rule.setAttributeValue(attr, nativeAttrVal, explicit);
checkAllowedValues(rule, attr, eventHandler);
- return attrIndex;
+ }
+
+ /**
+ * Converts the build-language-typed {@code buildLangValue} to a native value via {@link
+ * BuildType#selectableConvert}. Canonicalizes the value's order if it is a {@link List} type
+ * (but not a {@link GlobList}) and {@code attr.isOrderIndependent()} returns {@code true}.
+ *
+ * <p>Throws {@link ConversionException} if the conversion fails, or if {@code buildLangValue}
+ * is a selector expression but {@code attr.isConfigurable()} is {@code false}.
+ */
+ private static Object convertFromBuildLangType(Rule rule, Attribute attr, Object buildLangValue)
+ throws ConversionException {
+ String what = String.format("attribute '%s' in '%s' rule", attr.getName(), rule.getRuleClass());
+ Object converted =
+ BuildType.selectableConvert(attr.getType(), buildLangValue, what, rule.getLabel());
+
+ if ((converted instanceof SelectorList<?>) && !attr.isConfigurable()) {
+ throw new ConversionException(
+ String.format("attribute \"%s\" is not configurable", attr.getName()));
+ }
+
+ if ((converted instanceof List<?>) && !(converted instanceof GlobList<?>)) {
+ if (attr.isOrderIndependent()) {
+ @SuppressWarnings("unchecked")
+ List<? extends Comparable<?>> list = (List<? extends Comparable<?>>) converted;
+ converted = Ordering.natural().sortedCopy(list);
+ }
+ converted = ImmutableList.copyOf((List<?>) converted);
+ }
+
+ return converted;
}
/**
@@ -1595,7 +1679,8 @@
* <p>If the rule is configurable, all of its potential values are evaluated, and errors for each
* of the invalid values are reported.
*/
- void checkAllowedValues(Rule rule, Attribute attribute, EventHandler eventHandler) {
+ private static void checkAllowedValues(
+ Rule rule, Attribute attribute, EventHandler eventHandler) {
if (attribute.checkAllowedValues()) {
PredicateWithMessage<Object> allowedValues = attribute.getAllowedValues();
Iterable<?> values =
@@ -1603,9 +1688,13 @@
attribute.getName(), attribute.getType());
for (Object value : values) {
if (!allowedValues.apply(value)) {
- rule.reportError(String.format(rule.getLabel() + ": invalid value in '%s' attribute: %s",
- attribute.getName(),
- allowedValues.getErrorReason(value)), eventHandler);
+ rule.reportError(
+ String.format(
+ "%s: invalid value in '%s' attribute: %s",
+ rule.getLabel(),
+ attribute.getName(),
+ allowedValues.getErrorReason(value)),
+ eventHandler);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
index 7982a62..4837b04 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
@@ -34,12 +34,12 @@
import javax.annotation.Nullable;
/**
- * Given a rule class and a set of attributes, returns a Rule instance. Also
- * performs a number of checks and associates the rule and the owning package
+ * Given a {@link RuleClass} and a set of attribute values, returns a {@link Rule} instance. Also
+ * performs a number of checks and associates the {@link Rule} and the owning {@link Package}
* with each other.
*
- * <p>Note: the code that actually populates the RuleClass map has been moved
- * to {@link RuleClassProvider}.
+ * <p>Note: the code that actually populates the RuleClass map has been moved to {@link
+ * RuleClassProvider}.
*/
public class RuleFactory {
@@ -73,20 +73,20 @@
* Creates and returns a rule instance.
*
* <p>It is the caller's responsibility to add the rule to the package (the caller may choose not
- * to do so if, for example, the rule has errors).</p>
+ * to do so if, for example, the rule has errors).
*/
static Rule createRule(
Package.Builder pkgBuilder,
RuleClass ruleClass,
- Map<String, Object> attributeValues,
+ BuildLangTypedAttributeValuesMap attributeValues,
EventHandler eventHandler,
- FuncallExpression ast,
+ @Nullable FuncallExpression ast,
Location location,
@Nullable Environment env)
throws InvalidRuleException, InterruptedException {
Preconditions.checkNotNull(ruleClass);
String ruleClassName = ruleClass.getName();
- Object nameObject = attributeValues.get("name");
+ Object nameObject = attributeValues.getAttributeValue("name");
if (nameObject == null) {
throw new InvalidRuleException(ruleClassName + " rule has no 'name' attribute");
} else if (!(nameObject instanceof String)) {
@@ -113,38 +113,48 @@
AttributesAndLocation generator =
generatorAttributesForMacros(attributeValues, env, location, label);
try {
- return ruleClass.createRuleWithLabel(
- pkgBuilder, label, generator.attributes, eventHandler, ast, generator.location);
+ return ruleClass.createRule(
+ pkgBuilder,
+ label,
+ generator.attributes,
+ eventHandler,
+ ast,
+ generator.location,
+ new AttributeContainer(ruleClass));
} catch (LabelSyntaxException e) {
throw new RuleFactory.InvalidRuleException(ruleClass + " " + e.getMessage());
}
}
/**
- * Creates a rule instance, adds it to the package and returns it.
+ * Creates a {@link Rule} instance, adds it to the {@link Package.Builder} and returns it.
*
- * @param pkgBuilder the under-construction package to which the rule belongs
- * @param ruleClass the class of the rule; this must not be null
- * @param attributeValues a map of attribute names to attribute values. Each
- * attribute must be defined for this class of rule, and have a value
- * of the appropriate type. There must be a map entry for each
- * non-optional attribute of this class of rule.
+ * @param pkgBuilder the under-construction {@link Package.Builder} to which the rule belongs
+ * @param ruleClass the {@link RuleClass} of the rule
+ * @param attributeValues a {@link BuildLangTypedAttributeValuesMap} mapping attribute names to
+ * attribute values of build-language type. Each attribute must be defined for this class of
+ * rule, and have a build-language-typed value which can be converted to the appropriate
+ * native type of the attribute (i.e. via {@link BuildType#selectableConvert}). There must
+ * be a map entry for each non-optional attribute of this class of rule.
* @param eventHandler a eventHandler on which errors and warnings are reported during
- * rule creation
+ * rule creation
* @param ast the abstract syntax tree of the rule expression (optional)
* @param location the location at which this rule was declared
+ * @param env the lexical environment of the function call which declared this rule (optional)
* @throws InvalidRuleException if the rule could not be constructed for any
- * reason (e.g. no <code>name</code> attribute is defined)
- * @throws InvalidRuleException, NameConflictException
+ * reason (e.g. no {@code name} attribute is defined)
+ * @throws NameConflictException if the rule's name or output files conflict with others in this
+ * package
+ * @throws InterruptedException if interrupted
*/
static Rule createAndAddRule(
Package.Builder pkgBuilder,
RuleClass ruleClass,
- Map<String, Object> attributeValues,
+ BuildLangTypedAttributeValuesMap attributeValues,
EventHandler eventHandler,
- FuncallExpression ast,
+ @Nullable FuncallExpression ast,
Location location,
- Environment env)
+ @Nullable Environment env)
throws InvalidRuleException, NameConflictException, InterruptedException {
Rule rule = createRule(
pkgBuilder, ruleClass, attributeValues, eventHandler, ast, location, env);
@@ -152,12 +162,31 @@
return rule;
}
+ /**
+ * Creates a {@link Rule} instance, adds it to the {@link Package.Builder} and returns it.
+ *
+ * @param context the package-building context in which this rule was declared
+ * @param ruleClass the {@link RuleClass} of the rule
+ * @param attributeValues a {@link BuildLangTypedAttributeValuesMap} mapping attribute names to
+ * attribute values of build-language type. Each attribute must be defined for this class
+ * of rule, and have a build-language-typed value which can be converted to the appropriate
+ * native type of the attribute (i.e. via {@link BuildType#selectableConvert}). There must
+ * be a map entry for each non-optional attribute of this class of rule.
+ * @param ast the abstract syntax tree of the rule expression (mandatory because this looks up a
+ * {@link Location} from the {@code ast})
+ * @param env the lexical environment of the function call which declared this rule (optional)
+ * @throws InvalidRuleException if the rule could not be constructed for any reason (e.g. no
+ * {@code name} attribute is defined)
+ * @throws NameConflictException if the rule's name or output files conflict with others in this
+ * package
+ * @throws InterruptedException if interrupted
+ */
public static Rule createAndAddRule(
PackageContext context,
RuleClass ruleClass,
- Map<String, Object> attributeValues,
+ BuildLangTypedAttributeValuesMap attributeValues,
FuncallExpression ast,
- Environment env)
+ @Nullable Environment env)
throws InvalidRuleException, NameConflictException, InterruptedException {
return createAndAddRule(
context.pkgBuilder,
@@ -170,8 +199,8 @@
}
/**
- * InvalidRuleException is thrown by createRule() if the Rule could not be
- * constructed. It contains an error message.
+ * InvalidRuleException is thrown by {@link Rule} creation methods if the {@link Rule} could
+ * not be constructed. It contains an error message.
*/
public static class InvalidRuleException extends Exception {
private InvalidRuleException(String message) {
@@ -179,32 +208,89 @@
}
}
- /** Pair of attributes and location */
+ /** A pair of attributes and location. */
private static final class AttributesAndLocation {
- final Map<String, Object> attributes;
+ final BuildLangTypedAttributeValuesMap attributes;
final Location location;
- AttributesAndLocation(Map<String, Object> attributes, Location location) {
+ AttributesAndLocation(BuildLangTypedAttributeValuesMap attributes, Location location) {
this.attributes = attributes;
this.location = location;
}
}
/**
+ * A wrapper around an map of named attribute values that specifies whether the map's values
+ * are of "build-language" or of "native" types.
+ */
+ public interface AttributeValuesMap {
+ /**
+ * Returns {@code true} if all the map's values are "build-language typed", i.e., resulting
+ * from the evaluation of an expression in the build language. Returns {@code false} if all
+ * the map's values are "natively typed", i.e. of a type returned by {@link
+ * BuildType#selectableConvert}.
+ */
+ boolean valuesAreBuildLanguageTyped();
+
+ Iterable<String> getAttributeNames();
+
+ Object getAttributeValue(String attributeName);
+
+ boolean isAttributeExplicitlySpecified(String attributeName);
+ }
+
+ /** A {@link AttributeValuesMap} of explicit "build-language" values. */
+ public static final class BuildLangTypedAttributeValuesMap implements AttributeValuesMap {
+
+ private final Map<String, Object> attributeValues;
+
+ public BuildLangTypedAttributeValuesMap(Map<String, Object> attributeValues) {
+ this.attributeValues = attributeValues;
+ }
+
+ private boolean containsAttributeNamed(String attributeName) {
+ return attributeValues.containsKey(attributeName);
+ }
+
+ @Override
+ public boolean valuesAreBuildLanguageTyped() {
+ return true;
+ }
+
+ @Override
+ public Iterable<String> getAttributeNames() {
+ return attributeValues.keySet();
+ }
+
+ @Override
+ public Object getAttributeValue(String attributeName) {
+ return attributeValues.get(attributeName);
+ }
+
+ @Override
+ public boolean isAttributeExplicitlySpecified(String attributeName) {
+ return true;
+ }
+ }
+
+ /**
* If the rule was created by a macro, this method sets the appropriate values for the
* attributes generator_{name, function, location} and returns all attributes.
*
* <p>Otherwise, it returns the given attributes without any changes.
*/
private static AttributesAndLocation generatorAttributesForMacros(
- Map<String, Object> args, @Nullable Environment env, Location location, Label label) {
+ BuildLangTypedAttributeValuesMap args,
+ @Nullable Environment env,
+ Location location,
+ Label label) {
// Returns the original arguments if a) there is only the rule itself on the stack
// trace (=> no macro) or b) the attributes have already been set by Python pre-processing.
if (env == null) {
return new AttributesAndLocation(args, location);
}
- boolean hasName = args.containsKey("generator_name");
- boolean hasFunc = args.containsKey("generator_function");
+ boolean hasName = args.containsAttributeNamed("generator_name");
+ boolean hasFunc = args.containsAttributeNamed("generator_function");
// TODO(bazel-team): resolve cases in our code where hasName && !hasFunc, or hasFunc && !hasName
if (hasName || hasFunc) {
return new AttributesAndLocation(args, location);
@@ -217,23 +303,25 @@
FuncallExpression generator = topCall.first;
BaseFunction function = topCall.second;
String name = generator.getNameArg();
+
ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
-
- builder.putAll(args);
- builder.put("generator_name", (name == null) ? args.get("name") : name);
+ for (String attributeName : args.getAttributeNames()) {
+ builder.put(attributeName, args.getAttributeValue(attributeName));
+ }
+ builder.put("generator_name", (name == null) ? args.getAttributeValue("name") : name);
builder.put("generator_function", function.getName());
if (generator.getLocation() != null) {
location = generator.getLocation();
}
-
String relativePath = maybeGetRelativeLocation(location, label);
if (relativePath != null) {
builder.put("generator_location", relativePath);
}
try {
- return new AttributesAndLocation(builder.build(), location);
+ return new AttributesAndLocation(
+ new BuildLangTypedAttributeValuesMap(builder.build()), location);
} catch (IllegalArgumentException ex) {
// We just fall back to the default case and swallow any messages.
return new AttributesAndLocation(args, location);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleSerializer.java b/src/main/java/com/google/devtools/build/lib/packages/RuleSerializer.java
index 658515c..f7470e9 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleSerializer.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleSerializer.java
@@ -36,48 +36,34 @@
builder.setName(rule.getLabel().getName());
builder.setRuleClass(rule.getRuleClass());
builder.setPublicByDefault(rule.getRuleClassObject().isPublicByDefault());
+
RawAttributeMapper rawAttributeMapper = RawAttributeMapper.of(rule);
+ boolean isSkylark = rule.getRuleClassObject().isSkylark();
+
for (Attribute attr : rule.getAttributes()) {
Object rawAttributeValue = rawAttributeMapper.getRawAttributeValue(rule, attr);
+ boolean isExplicit = rule.isAttributeValueExplicitlySpecified(attr);
+
+ if (!isSkylark && !isExplicit) {
+ // If the rule class is native (i.e. not Skylark-defined), then we can skip serialization
+ // of implicit attribute values. The native rule class can provide the same default value
+ // for the attribute after deserialization.
+ continue;
+ }
Object valueToSerialize;
- if (rawAttributeValue instanceof ComputedDefault) {
- if (rule.getRuleClassObject().isSkylark()) {
- // If the rule class is Skylark-defined (i.e. rule.getRuleClassObject().isSkylark() is
- // true), and the attribute has a ComputedDefault value, we must serialize it. The
- // Skylark-defined ComputedDefault function won't be available after deserialization due
- // to Skylark's non-serializability. Fortunately (from the perspective of rule
- // serialization), Skylark doesn't support defining rule classes with ComputedDefault
- // attributes, and so the only ComputedDefault attributes we need to worry about for
- // Skylark-defined rule classes are those declared in those rule classes' natively
- // defined base rule classes.
- //
- // See the comment for SKYLARK_RULE_CLASS_COMPUTED_DEFAULT_ATTRIBUTES for the locations
- // of these expected attributes.
- //
- // The RawAttributeMapper#get method, inherited from AbstractAttributeMapper, evaluates
- // the ComputedDefault function, so we use that, after verifying the attribute's name is
- // expected.
- Preconditions.checkState(
- SKYLARK_RULE_CLASS_COMPUTED_DEFAULT_ATTRIBUTES.contains(attr.getName()),
- "Unexpected ComputedDefault value for %s in %s",
- attr,
- rule);
- valueToSerialize = rawAttributeMapper.get(attr.getName(), attr.getType());
- } else {
- // If the rule class is native (i.e. not Skylark-defined), we can skip serialization of
- // attributes with ComputedDefault values. The native rule class can provide the same
- // ComputedDefault value for the attribute after deserialization.
- //
- // TODO(mschaller): While the native rule class *could* provide it, it doesn't yet. Make
- // it so! For now, we fall back to flattening the set of all possible values, computed
- // using AggregatingAttributeMapper.
- Iterable<Object> possibleValues =
- AggregatingAttributeMapper.of(rule).getPossibleAttributeValues(rule, attr);
- valueToSerialize =
- AggregatingAttributeMapper.flattenAttributeValues(attr.getType(), possibleValues);
- }
+ if (isExplicit) {
+ valueToSerialize = rawAttributeValue;
+ } else if (rawAttributeValue instanceof ComputedDefault) {
+ // If the rule class is Skylark-defined (i.e. rule.getRuleClassObject().isSkylark() is
+ // true), and the attribute has a ComputedDefault value, then we must serialize what it
+ // evaluates to. The Skylark-defined ComputedDefault function won't be available after
+ // deserialization due to Skylark's non-serializability.
+ valueToSerialize = evaluateSkylarkComputedDefault(rule, rawAttributeMapper, attr);
} else {
+ // If the rule class is Skylark-defined and the attribute value is implicit, then we
+ // must serialize it. The Skylark-defined rule class won't be available after
+ // deserialization due to Skylark's non-serializability.
valueToSerialize = rawAttributeValue;
}
@@ -85,11 +71,38 @@
AttributeSerializer.getAttributeProto(
attr,
valueToSerialize,
- rule.isAttributeValueExplicitlySpecified(attr),
+ isExplicit,
/*includeGlobs=*/ true,
/*encodeBooleanAndTriStateAsIntegerAndString=*/ false));
}
return builder;
}
+
+ /**
+ * Evaluates a {@link ComputedDefault} attribute value for a {@link Rule} with a
+ * Skylark-defined {@link RuleClass}.
+ *
+ * <p>Fortunately (from the perspective of rule serialization), Skylark doesn't support defining
+ * rule classes with {@link ComputedDefault} attributes, and so the only {@link
+ * ComputedDefault} attributes we need to worry about for Skylark-defined rule classes are
+ * declared in those rule classes' natively-defined base rule classes.
+ *
+ * <p>See the comment for {@link #SKYLARK_RULE_CLASS_COMPUTED_DEFAULT_ATTRIBUTES} for the
+ * locations of these expected attributes. None of them have dependencies on other attributes
+ * which are configurable, so they can be evaluated here without loss of fidelity.
+ *
+ * <p>The {@link RawAttributeMapper#get} method, inherited from {@link
+ * AbstractAttributeMapper}, evaluates the {@link ComputedDefault} function, so we use that,
+ * after verifying the attribute's name is expected.
+ */
+ private static Object evaluateSkylarkComputedDefault(
+ Rule rule, RawAttributeMapper rawAttributeMapper, Attribute attr) {
+ Preconditions.checkState(
+ SKYLARK_RULE_CLASS_COMPUTED_DEFAULT_ATTRIBUTES.contains(attr.getName()),
+ "Unexpected ComputedDefault value for %s in %s",
+ attr,
+ rule);
+ return rawAttributeMapper.get(attr.getName(), attr.getType());
+ }
}
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 0815f36..cc5898e 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
@@ -64,6 +64,7 @@
import com.google.devtools.build.lib.packages.RuleClass.Builder;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
import com.google.devtools.build.lib.packages.RuleFactory;
+import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
import com.google.devtools.build.lib.packages.SkylarkAspectClass;
import com.google.devtools.build.lib.packages.TargetUtils;
@@ -466,8 +467,9 @@
"Invalid rule class hasn't been exported by a Skylark file");
}
PackageContext pkgContext = (PackageContext) env.lookup(PackageFactory.PKG_CONTEXT);
- return RuleFactory.createAndAddRule(
- pkgContext, ruleClass, (Map<String, Object>) args[0], ast, env);
+ BuildLangTypedAttributeValuesMap attributeValues =
+ new BuildLangTypedAttributeValuesMap((Map<String, Object>) args[0]);
+ return RuleFactory.createAndAddRule(pkgContext, ruleClass, attributeValues, ast, env);
} catch (InvalidRuleException | NameConflictException | NoSuchVariableException e) {
throw new EvalException(ast.getLocation(), e.getMessage());
}
