diff options
author | Mark Schaller <mschaller@google.com> | 2015-12-30 21:42:51 +0000 |
---|---|---|
committer | Philipp Wollermann <philwo@google.com> | 2016-01-04 12:58:45 +0000 |
commit | 118aac76db9af7230ba32c2f56108382c819b7e3 (patch) | |
tree | 765bce0b8f220a7b05e3beecfa9d5ac8b079f3fc /src/main/java | |
parent | 9201fda7dcc82d40ee47fadee31d6eb6923605db (diff) |
Permit proto serialization of configured attribute values
This commit adds proto messages that represent configurable values,
and modifies attribute value serialization code to handle those
values, which are called SelectorLists.
--
MOS_MIGRATED_REVID=111149272
Diffstat (limited to 'src/main/java')
11 files changed, 806 insertions, 245 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java index 714086d19a..2c1837a836 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java @@ -93,6 +93,7 @@ public abstract class AbstractAttributeMapper implements AttributeMap { * type. This happens whether or not it's a computed default. */ @VisibleForTesting // Should be protected + @Nullable public <T> Attribute.ComputedDefault getComputedDefault(String attributeName, Type<T> type) { int index = getIndexWithTypeCheck(attributeName, type); Object value = attributes.getAttributeValue(index); diff --git a/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java index 7164bf6395..ea2a57e5d8 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java @@ -13,11 +13,34 @@ // limitations under the License. package com.google.devtools.build.lib.packages; +import static com.google.devtools.build.lib.packages.BuildType.DISTRIBUTIONS; +import static com.google.devtools.build.lib.packages.BuildType.FILESET_ENTRY_LIST; +import static com.google.devtools.build.lib.packages.BuildType.LABEL; +import static com.google.devtools.build.lib.packages.BuildType.LABEL_DICT_UNARY; +import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; +import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST_DICT; +import static com.google.devtools.build.lib.packages.BuildType.LICENSE; +import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL; +import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL_LIST; +import static com.google.devtools.build.lib.packages.BuildType.OUTPUT; +import static com.google.devtools.build.lib.packages.BuildType.OUTPUT_LIST; +import static com.google.devtools.build.lib.packages.BuildType.TRISTATE; +import static com.google.devtools.build.lib.syntax.Type.BOOLEAN; +import static com.google.devtools.build.lib.syntax.Type.INTEGER; +import static com.google.devtools.build.lib.syntax.Type.INTEGER_LIST; +import static com.google.devtools.build.lib.syntax.Type.STRING; +import static com.google.devtools.build.lib.syntax.Type.STRING_DICT; +import static com.google.devtools.build.lib.syntax.Type.STRING_DICT_UNARY; +import static com.google.devtools.build.lib.syntax.Type.STRING_LIST; +import static com.google.devtools.build.lib.syntax.Type.STRING_LIST_DICT; + import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableList.Builder; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.CollectionUtils; import com.google.devtools.build.lib.packages.BuildType.Selector; @@ -26,11 +49,13 @@ import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.Preconditions; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import javax.annotation.Nullable; @@ -41,6 +66,10 @@ import javax.annotation.Nullable; */ public class AggregatingAttributeMapper extends AbstractAttributeMapper { + @SuppressWarnings("unchecked") + private static final ImmutableSet<Type<?>> scalarTypes = + ImmutableSet.of(INTEGER, STRING, LABEL, NODEP_LABEL, OUTPUT, BOOLEAN, TRISTATE, LICENSE); + /** * Store for all of this rule's attributes that are non-configurable. These are * unconditionally available to computed defaults no matter what dependencies @@ -172,6 +201,106 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper { } /** + * Returns a list of the possible values of the specified attribute in the specified rule. + * + * <p>If the attribute's value is a simple value, then this returns a singleton list of that + * value. + * + * <p>If the attribute's value is an expression containing one or many {@code select(...)} + * expressions, then this returns a list of all values that expression may evaluate to. + * + * <p>If the attribute does not have an explicit value for this rule, and the rule provides a + * computed default, the computed default function is evaluated given the rule's other attribute + * values as inputs and the output is returned in a singleton list. + * + * <p>If the attribute does not have an explicit value for this rule, and the rule provides a + * computed default, and the computed default function depends on other attributes whose values + * contain {@code select(...)} expressions, then the computed default function is evaluated for + * every possible combination of input values, and the list of outputs is returned. + */ + public Iterable<Object> getPossibleAttributeValues(Rule rule, Attribute attr) { + // Values may be null, so use normal collections rather than immutable collections. + // This special case for the visibility attribute is needed because its value is replaced + // with an empty list during package loading if it is public or private in order not to visit + // the package called 'visibility'. + if (attr.getName().equals("visibility")) { + List<Object> result = new ArrayList<>(1); + result.add(rule.getVisibility().getDeclaredLabels()); + return result; + } + return Lists.<Object>newArrayList(visitAttribute(attr.getName(), attr.getType())); + } + + /** + * Coerces the list {@param possibleValues} of values of type {@param attrType} to a single + * value of that type, in the following way: + * + * <p>If the list contains a single value, return that value. + * + * <p>If the list contains zero or multiple values and the type is a scalar type, return {@code + * null}. + * + * <p>If the list contains zero or multiple values and the type is a collection or map type, + * merge the collections/maps in the list and return the merged collection/map. + */ + @Nullable + @SuppressWarnings("unchecked") + public static Object flattenAttributeValues(Type<?> attrType, Iterable<Object> possibleValues) { + // If there is only one possible value, return it. + if (Iterables.size(possibleValues) == 1) { + return Iterables.getOnlyElement(possibleValues); + } + + // Otherwise, there are multiple possible values. To conform to the message shape expected by + // query output's clients, we must transform the list of possible values. This transformation + // will be lossy, but this is the best we can do. + + // If the attribute's type is not a collection type, return null. Query output's clients do + // not support list values for scalar attributes. + if (scalarTypes.contains(attrType)) { + return null; + } + + // If the attribute's type is a collection type, merge the list of collections into a single + // collection. This is a sensible solution for query output's clients, which are happy to get + // the union of possible values. + if (attrType == STRING_LIST + || attrType == LABEL_LIST + || attrType == NODEP_LABEL_LIST + || attrType == OUTPUT_LIST + || attrType == DISTRIBUTIONS + || attrType == INTEGER_LIST + || attrType == FILESET_ENTRY_LIST) { + Builder<Object> builder = ImmutableList.builder(); + for (Object possibleValue : possibleValues) { + Collection<Object> collection = (Collection<Object>) possibleValue; + for (Object o : collection) { + builder.add(o); + } + } + return builder.build(); + } + + // Same for maps as for collections. + if (attrType == STRING_DICT + || attrType == STRING_DICT_UNARY + || attrType == STRING_LIST_DICT + || attrType == LABEL_DICT_UNARY + || attrType == LABEL_LIST_DICT) { + Map<Object, Object> mergedDict = new HashMap<>(); + for (Object possibleValue : possibleValues) { + Map<Object, Object> stringDict = (Map<Object, Object>) possibleValue; + for (Entry<Object, Object> entry : stringDict.entrySet()) { + mergedDict.put(entry.getKey(), entry.getValue()); + } + } + return mergedDict; + } + + throw new AssertionError("Unknown type: " + attrType); + } + + /** * Returns a list of all possible values an attribute can take for this rule. * * <p>Note that when an attribute uses multiple selects, it can potentially take on many diff --git a/src/main/java/com/google/devtools/build/lib/packages/AttributeSerializer.java b/src/main/java/com/google/devtools/build/lib/packages/AttributeSerializer.java index f380041402..915244990a 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AttributeSerializer.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AttributeSerializer.java @@ -34,259 +34,258 @@ import static com.google.devtools.build.lib.syntax.Type.STRING_DICT_UNARY; import static com.google.devtools.build.lib.syntax.Type.STRING_LIST; import static com.google.devtools.build.lib.syntax.Type.STRING_LIST_DICT; -import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.BuildType.Selector; +import com.google.devtools.build.lib.packages.BuildType.SelectorList; import com.google.devtools.build.lib.query2.proto.proto2api.Build; +import com.google.devtools.build.lib.query2.proto.proto2api.Build.Attribute.Discriminator; +import com.google.devtools.build.lib.query2.proto.proto2api.Build.Attribute.SelectorEntry; +import com.google.devtools.build.lib.query2.proto.proto2api.Build.Attribute.SelectorEntry.Builder; +import com.google.devtools.build.lib.query2.proto.proto2api.Build.Attribute.Tristate; +import com.google.devtools.build.lib.query2.proto.proto2api.Build.LabelDictUnaryEntry; +import com.google.devtools.build.lib.query2.proto.proto2api.Build.LabelListDictEntry; +import com.google.devtools.build.lib.query2.proto.proto2api.Build.StringDictEntry; +import com.google.devtools.build.lib.query2.proto.proto2api.Build.StringDictUnaryEntry; +import com.google.devtools.build.lib.query2.proto.proto2api.Build.StringListDictEntry; import com.google.devtools.build.lib.syntax.GlobCriteria; import com.google.devtools.build.lib.syntax.GlobList; +import com.google.devtools.build.lib.syntax.Type; -import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Map.Entry; + +import javax.annotation.Nullable; /** Common utilities for serializing {@link Attribute}s as protocol buffers. */ public class AttributeSerializer { + private static final ImmutableSet<Type<?>> depTypes = + ImmutableSet.<Type<?>>of( + STRING, LABEL, OUTPUT, STRING_LIST, LABEL_LIST, OUTPUT_LIST, DISTRIBUTIONS); + + private static final ImmutableSet<Type<?>> noDepTypes = + ImmutableSet.<Type<?>>of(NODEP_LABEL_LIST, NODEP_LABEL); + private AttributeSerializer() {} /** - * Returns the possible values of the specified attribute in the specified rule. For - * non-configured attributes, this is a single value. For configurable attributes, this - * may be multiple values. + * Convert attribute value to proto representation. + * + * <p>If {@param value} is null, only the {@code name}, {@code explicitlySpecified}, {@code + * nodep} (if applicable), and {@code type} fields will be included in the proto message. + * + * <p>If {@param includeGlobs} is true then {@link GlobCriteria} will be included in the proto + * message if present. + * + * <p>If {@param encodeBooleanAndTriStateAsIntegerAndString} is true then boolean and tristate + * values are also encoded as integers and strings. */ - public static Iterable<Object> getAttributeValues(Rule rule, Attribute attr) { - // Values may be null, so use normal collections rather than immutable collections. - if (attr.getName().equals("visibility")) { - List<Object> result = new ArrayList<>(1); - result.add(rule.getVisibility().getDeclaredLabels()); - return result; + public static Build.Attribute getAttributeProto( + Attribute attr, + @Nullable Object value, + boolean explicitlySpecified, + boolean includeGlobs, + boolean encodeBooleanAndTriStateAsIntegerAndString) { + Type<?> type = attr.getType(); + Build.Attribute.Builder attrPb = Build.Attribute.newBuilder(); + attrPb.setName(attr.getName()); + attrPb.setExplicitlySpecified(explicitlySpecified); + maybeSetNoDep(type, attrPb); + + if (value instanceof SelectorList<?>) { + attrPb.setType(Discriminator.SELECTOR_LIST); + writeSelectorListToBuilder(attrPb, type, (SelectorList<?>) value, includeGlobs); } else { - return Lists.<Object>newArrayList( - AggregatingAttributeMapper.of(rule).visitAttribute(attr.getName(), attr.getType())); + attrPb.setType(ProtoUtils.getDiscriminatorFromType(type)); + if (value != null) { + AttributeBuilderAdapter adapter = + new AttributeBuilderAdapter(attrPb, encodeBooleanAndTriStateAsIntegerAndString); + writeAttributeValueToBuilder(adapter, type, value, includeGlobs); + } } + + return attrPb.build(); } - /** - * Convert Attribute to proto representation. If {@code includeGlobs} is true then include - * globs expressions when present, omit otherwise. - */ - @SuppressWarnings("unchecked") - public static Build.Attribute getAttributeProto( - Attribute attr, Iterable<Object> values, boolean explicitlySpecified, boolean includeGlobs) { - // Get the attribute type. We need to convert and add appropriately - com.google.devtools.build.lib.syntax.Type<?> type = attr.getType(); + private static void maybeSetNoDep(Type<?> type, Build.Attribute.Builder attrPb) { + if (depTypes.contains(type)) { + attrPb.setNodep(false); + } else if (noDepTypes.contains(type)) { + attrPb.setNodep(true); + } + } - Build.Attribute.Builder attrPb = Build.Attribute.newBuilder(); + private static void writeSelectorListToBuilder( + Build.Attribute.Builder attrPb, + Type<?> type, + SelectorList<?> selectorList, + boolean includeGlobs) { + Build.Attribute.SelectorList.Builder selectorListBuilder = + Build.Attribute.SelectorList.newBuilder(); + selectorListBuilder.setType(ProtoUtils.getDiscriminatorFromType(type)); + for (Selector<?> selector : selectorList.getSelectors()) { + Build.Attribute.Selector.Builder selectorBuilder = Build.Attribute.Selector.newBuilder(); + // Note that the order of entries returned by selector.getEntries is stable. The map's + // entries' order is preserved from the sorting performed by the SelectorValue constructor. + for (Entry<Label, ?> entry : selector.getEntries().entrySet()) { + Builder selectorEntryBuilder = SelectorEntry.newBuilder(); + selectorEntryBuilder.setLabel(entry.getKey().toString()); + writeAttributeValueToBuilder( + new SelectorEntryBuilderAdapter(selectorEntryBuilder), + type, + entry.getValue(), + includeGlobs); + selectorBuilder.addEntries(selectorEntryBuilder); + } - // Set the type, name and source - attrPb.setName(attr.getName()); - attrPb.setType(ProtoUtils.getDiscriminatorFromType(type)); - attrPb.setExplicitlySpecified(explicitlySpecified); + selectorListBuilder.addElements(selectorBuilder); + } + attrPb.setSelectorList(selectorListBuilder); + } - // Convenience binding for single-value attributes. Because those attributes can only - // have a single value, when we encounter configurable versions of them we need to - // react somehow to having multiple possible values to report. We currently just - // refrain from setting *any* value in that scenario. This variable is set to null - // to indicate that. - // - // For example, for "linkstatic = select({':foo': 0, ':bar': 1})", "values" will contain [0, 1]. - // Since linkstatic is a single-value string element, its proto field (string_value) can't - // store both values. Since no use case today actually needs this, we just skip it. - // - // TODO(bazel-team): support this properly. This will require syntactic change to build.proto - // (or reinterpretation of its current fields). - Object singleAttributeValue = Iterables.size(values) == 1 - ? Iterables.getOnlyElement(values) - : null; - - /* - * Set the appropriate type and value. Since string and string list store - * values for multiple types, use the toString() method on the objects - * instead of casting them. Note that Boolean and TriState attributes have - * both an integer and string representation. - */ + /** + * Set the appropriate type and value. Since string and string list store values for multiple + * types, use the toString() method on the objects instead of casting them. + */ + @SuppressWarnings("unchecked") + private static void writeAttributeValueToBuilder( + AttributeValueBuilderAdapter builder, Type<?> type, Object value, boolean includeGlobs) { if (type == INTEGER) { - if (singleAttributeValue != null) { - attrPb.setIntValue((Integer) singleAttributeValue); - } + builder.setIntValue((Integer) value); } else if (type == STRING || type == LABEL || type == NODEP_LABEL || type == OUTPUT) { - if (singleAttributeValue != null) { - attrPb.setStringValue(singleAttributeValue.toString()); - } - attrPb.setNodep(type == NODEP_LABEL); + builder.setStringValue(value.toString()); } else if (type == STRING_LIST || type == LABEL_LIST || type == NODEP_LABEL_LIST || type == OUTPUT_LIST || type == DISTRIBUTIONS) { - for (Object value : values) { - for (Object entry : (Collection<?>) value) { - attrPb.addStringListValue(entry.toString()); - } + for (Object entry : (Collection<?>) value) { + builder.addStringListValue(entry.toString()); } - attrPb.setNodep(type == NODEP_LABEL_LIST); } else if (type == INTEGER_LIST) { - for (Object value : values) { - for (Integer entry : (Collection<Integer>) value) { - attrPb.addIntListValue(entry); - } + for (Integer entry : (Collection<Integer>) value) { + builder.addIntListValue(entry); } } else if (type == BOOLEAN) { - if (singleAttributeValue != null) { - if ((Boolean) singleAttributeValue) { - attrPb.setStringValue("true"); - attrPb.setBooleanValue(true); - } else { - attrPb.setStringValue("false"); - attrPb.setBooleanValue(false); - } - // This maintains partial backward compatibility for external users of the - // protobuf that were expecting an integer field and not a true boolean. - attrPb.setIntValue((Boolean) singleAttributeValue ? 1 : 0); - } + builder.setBooleanValue((Boolean) value); } else if (type == TRISTATE) { - if (singleAttributeValue != null) { - switch ((TriState) singleAttributeValue) { - case AUTO: - attrPb.setIntValue(-1); - attrPb.setStringValue("auto"); - attrPb.setTristateValue(Build.Attribute.Tristate.AUTO); - break; - case NO: - attrPb.setIntValue(0); - attrPb.setStringValue("no"); - attrPb.setTristateValue(Build.Attribute.Tristate.NO); - break; - case YES: - attrPb.setIntValue(1); - attrPb.setStringValue("yes"); - attrPb.setTristateValue(Build.Attribute.Tristate.YES); - break; - default: - throw new AssertionError("Expected AUTO/NO/YES to cover all possible cases"); - } - } + builder.setTristateValue(triStateToProto((TriState) value)); } else if (type == LICENSE) { - if (singleAttributeValue != null) { - License license = (License) singleAttributeValue; - Build.License.Builder licensePb = Build.License.newBuilder(); - for (License.LicenseType licenseType : license.getLicenseTypes()) { - licensePb.addLicenseType(licenseType.toString()); - } - for (Label exception : license.getExceptions()) { - licensePb.addException(exception.toString()); - } - attrPb.setLicense(licensePb); + License license = (License) value; + Build.License.Builder licensePb = Build.License.newBuilder(); + for (License.LicenseType licenseType : license.getLicenseTypes()) { + licensePb.addLicenseType(licenseType.toString()); + } + for (Label exception : license.getExceptions()) { + licensePb.addException(exception.toString()); } + builder.setLicense(licensePb); } else if (type == STRING_DICT) { - // TODO(bazel-team): support better de-duping here and in other dictionaries. - for (Object value : values) { - Map<String, String> dict = (Map<String, String>) value; - for (Map.Entry<String, String> keyValueList : dict.entrySet()) { - Build.StringDictEntry entry = Build.StringDictEntry.newBuilder() - .setKey(keyValueList.getKey()) - .setValue(keyValueList.getValue()) - .build(); - attrPb.addStringDictValue(entry); - } + Map<String, String> dict = (Map<String, String>) value; + for (Map.Entry<String, String> keyValueList : dict.entrySet()) { + StringDictEntry.Builder entry = + StringDictEntry.newBuilder() + .setKey(keyValueList.getKey()) + .setValue(keyValueList.getValue()); + builder.addStringDictValue(entry); } } else if (type == STRING_DICT_UNARY) { - for (Object value : values) { - Map<String, String> dict = (Map<String, String>) value; - for (Map.Entry<String, String> dictEntry : dict.entrySet()) { - Build.StringDictUnaryEntry entry = Build.StringDictUnaryEntry.newBuilder() - .setKey(dictEntry.getKey()) - .setValue(dictEntry.getValue()) - .build(); - attrPb.addStringDictUnaryValue(entry); - } + Map<String, String> dict = (Map<String, String>) value; + for (Map.Entry<String, String> dictEntry : dict.entrySet()) { + StringDictUnaryEntry.Builder entry = + StringDictUnaryEntry.newBuilder() + .setKey(dictEntry.getKey()) + .setValue(dictEntry.getValue()); + builder.addStringDictUnaryValue(entry); } } else if (type == STRING_LIST_DICT) { - for (Object value : values) { - Map<String, List<String>> dict = (Map<String, List<String>>) value; - for (Map.Entry<String, List<String>> dictEntry : dict.entrySet()) { - Build.StringListDictEntry.Builder entry = Build.StringListDictEntry.newBuilder() - .setKey(dictEntry.getKey()); - for (Object dictEntryValue : dictEntry.getValue()) { - entry.addValue(dictEntryValue.toString()); - } - attrPb.addStringListDictValue(entry); + Map<String, List<String>> dict = (Map<String, List<String>>) value; + for (Map.Entry<String, List<String>> dictEntry : dict.entrySet()) { + StringListDictEntry.Builder entry = + StringListDictEntry.newBuilder().setKey(dictEntry.getKey()); + for (Object dictEntryValue : dictEntry.getValue()) { + entry.addValue(dictEntryValue.toString()); } + builder.addStringListDictValue(entry); } } else if (type == LABEL_DICT_UNARY) { - for (Object value : values) { - Map<String, Label> dict = (Map<String, Label>) value; - for (Map.Entry<String, Label> dictEntry : dict.entrySet()) { - Build.LabelDictUnaryEntry entry = Build.LabelDictUnaryEntry.newBuilder() - .setKey(dictEntry.getKey()) - .setValue(dictEntry.getValue().toString()) - .build(); - attrPb.addLabelDictUnaryValue(entry); - } + Map<String, Label> dict = (Map<String, Label>) value; + for (Map.Entry<String, Label> dictEntry : dict.entrySet()) { + LabelDictUnaryEntry.Builder entry = + LabelDictUnaryEntry.newBuilder() + .setKey(dictEntry.getKey()) + .setValue(dictEntry.getValue().toString()); + builder.addLabelDictUnaryValue(entry); } } else if (type == LABEL_LIST_DICT) { - for (Object value : values) { - Map<String, List<Label>> dict = (Map<String, List<Label>>) value; - for (Map.Entry<String, List<Label>> dictEntry : dict.entrySet()) { - Build.LabelListDictEntry.Builder entry = Build.LabelListDictEntry.newBuilder() - .setKey(dictEntry.getKey()); - for (Object dictEntryValue : dictEntry.getValue()) { - entry.addValue(dictEntryValue.toString()); - } - attrPb.addLabelListDictValue(entry); + Map<String, List<Label>> dict = (Map<String, List<Label>>) value; + for (Map.Entry<String, List<Label>> dictEntry : dict.entrySet()) { + LabelListDictEntry.Builder entry = + LabelListDictEntry.newBuilder().setKey(dictEntry.getKey()); + for (Object dictEntryValue : dictEntry.getValue()) { + entry.addValue(dictEntryValue.toString()); } + builder.addLabelListDictValue(entry); } } else if (type == FILESET_ENTRY_LIST) { - for (Object value : values) { - List<FilesetEntry> filesetEntries = (List<FilesetEntry>) value; - for (FilesetEntry filesetEntry : filesetEntries) { - Build.FilesetEntry.Builder filesetEntryPb = Build.FilesetEntry.newBuilder() - .setSource(filesetEntry.getSrcLabel().toString()) - .setDestinationDirectory(filesetEntry.getDestDir().getPathString()) - .setSymlinkBehavior(symlinkBehaviorToPb(filesetEntry.getSymlinkBehavior())) - .setStripPrefix(filesetEntry.getStripPrefix()) - .setFilesPresent(filesetEntry.getFiles() != null); - - if (filesetEntry.getFiles() != null) { - for (Label file : filesetEntry.getFiles()) { - filesetEntryPb.addFile(file.toString()); - } - } + List<FilesetEntry> filesetEntries = (List<FilesetEntry>) value; + for (FilesetEntry filesetEntry : filesetEntries) { + Build.FilesetEntry.Builder filesetEntryPb = + Build.FilesetEntry.newBuilder() + .setSource(filesetEntry.getSrcLabel().toString()) + .setDestinationDirectory(filesetEntry.getDestDir().getPathString()) + .setSymlinkBehavior(symlinkBehaviorToPb(filesetEntry.getSymlinkBehavior())) + .setStripPrefix(filesetEntry.getStripPrefix()) + .setFilesPresent(filesetEntry.getFiles() != null); - if (filesetEntry.getExcludes() != null) { - for (String exclude : filesetEntry.getExcludes()) { - filesetEntryPb.addExclude(exclude); - } + if (filesetEntry.getFiles() != null) { + for (Label file : filesetEntry.getFiles()) { + filesetEntryPb.addFile(file.toString()); } + } - attrPb.addFilesetListValue(filesetEntryPb); + if (filesetEntry.getExcludes() != null) { + for (String exclude : filesetEntry.getExcludes()) { + filesetEntryPb.addExclude(exclude); + } } + + builder.addFilesetListValue(filesetEntryPb); } } else { throw new AssertionError("Unknown type: " + type); } - if (includeGlobs) { - for (Object value : values) { - if (value instanceof GlobList<?>) { - GlobList<?> globList = (GlobList<?>) value; + if (includeGlobs && value instanceof GlobList<?>) { + GlobList<?> globList = (GlobList<?>) value; - for (GlobCriteria criteria : globList.getCriteria()) { - Build.GlobCriteria.Builder criteriaPb = Build.GlobCriteria.newBuilder() - .setGlob(criteria.isGlob()); - for (String include : criteria.getIncludePatterns()) { - criteriaPb.addInclude(include); - } - for (String exclude : criteria.getExcludePatterns()) { - criteriaPb.addExclude(exclude); - } - - attrPb.addGlobCriteria(criteriaPb); - } + for (GlobCriteria criteria : globList.getCriteria()) { + Build.GlobCriteria.Builder criteriaPb = + Build.GlobCriteria.newBuilder().setGlob(criteria.isGlob()); + for (String include : criteria.getIncludePatterns()) { + criteriaPb.addInclude(include); + } + for (String exclude : criteria.getExcludePatterns()) { + criteriaPb.addExclude(exclude); } + + builder.addGlobCriteria(criteriaPb); } } + } - return attrPb.build(); + private static Tristate triStateToProto(TriState value) { + switch (value) { + case AUTO: + return Tristate.AUTO; + case NO: + return Tristate.NO; + case YES: + return Tristate.YES; + default: + throw new AssertionError("Expected AUTO/NO/YES to cover all possible cases"); + } } // This is needed because I do not want to use the SymlinkBehavior from the @@ -304,5 +303,252 @@ public class AttributeSerializer { } } + /** + * An adapter used by {@link #writeAttributeValueToBuilder} in order to reuse the same code for + * writing to both {@link Build.Attribute.Builder} and {@link SelectorEntry.Builder} objects. + */ + private interface AttributeValueBuilderAdapter { + + void addStringListValue(String s); + + void addFilesetListValue(Build.FilesetEntry.Builder builder); + + void addGlobCriteria(Build.GlobCriteria.Builder builder); + + void addLabelDictUnaryValue(LabelDictUnaryEntry.Builder builder); + + void addLabelListDictValue(LabelListDictEntry.Builder builder); + + void addIntListValue(int i); + + void addStringDictUnaryValue(StringDictUnaryEntry.Builder builder); + + void addStringDictValue(StringDictEntry.Builder builder); + + void addStringListDictValue(StringListDictEntry.Builder builder); + + void setBooleanValue(boolean b); + + void setIntValue(int i); + + void setLicense(Build.License.Builder builder); + + void setStringValue(String s); + + void setTristateValue(Tristate tristate); + } + + /** + * An {@link AttributeValueBuilderAdapter} which writes to a {@link Build.Attribute.Builder}. + * + * <p>If {@param encodeBooleanAndTriStateAsIntegerAndString} is {@code true}, then {@link + * Boolean} and {@link TriState} attribute values also write to the integer and string fields. + * This offers backwards compatibility to clients that expect attribute values of those types. + */ + private static class AttributeBuilderAdapter implements AttributeValueBuilderAdapter { + private final boolean encodeBooleanAndTriStateAsIntegerAndString; + private final Build.Attribute.Builder attributeBuilder; + + private AttributeBuilderAdapter( + Build.Attribute.Builder attributeBuilder, + boolean encodeBooleanAndTriStateAsIntegerAndString) { + this.attributeBuilder = Preconditions.checkNotNull(attributeBuilder); + this.encodeBooleanAndTriStateAsIntegerAndString = encodeBooleanAndTriStateAsIntegerAndString; + } + + public void addStringListValue(String s) { + attributeBuilder.addStringListValue(s); + } + + @Override + public void addFilesetListValue(Build.FilesetEntry.Builder builder) { + attributeBuilder.addFilesetListValue(builder); + } + + @Override + public void addGlobCriteria(Build.GlobCriteria.Builder builder) { + attributeBuilder.addGlobCriteria(builder); + } + + @Override + public void addLabelDictUnaryValue(LabelDictUnaryEntry.Builder builder) { + attributeBuilder.addLabelDictUnaryValue(builder); + } + + @Override + public void addLabelListDictValue(LabelListDictEntry.Builder builder) { + attributeBuilder.addLabelListDictValue(builder); + } + + @Override + public void addIntListValue(int i) { + attributeBuilder.addIntListValue(i); + } + + @Override + public void addStringDictUnaryValue(StringDictUnaryEntry.Builder builder) { + attributeBuilder.addStringDictUnaryValue(builder); + } + + @Override + public void addStringDictValue(StringDictEntry.Builder builder) { + attributeBuilder.addStringDictValue(builder); + } + + @Override + public void addStringListDictValue(StringListDictEntry.Builder builder) { + attributeBuilder.addStringListDictValue(builder); + } + + @Override + public void setBooleanValue(boolean b) { + if (b) { + attributeBuilder.setBooleanValue(true); + if (encodeBooleanAndTriStateAsIntegerAndString) { + attributeBuilder.setStringValue("true"); + attributeBuilder.setIntValue(1); + } + } else { + attributeBuilder.setBooleanValue(false); + if (encodeBooleanAndTriStateAsIntegerAndString) { + attributeBuilder.setStringValue("false"); + attributeBuilder.setIntValue(0); + } + } + } + + @Override + public void setIntValue(int i) { + attributeBuilder.setIntValue(i); + } + + @Override + public void setLicense(Build.License.Builder builder) { + attributeBuilder.setLicense(builder); + } + + @Override + public void setStringValue(String s) { + attributeBuilder.setStringValue(s); + } + + @Override + public void setTristateValue(Tristate tristate) { + switch (tristate) { + case AUTO: + attributeBuilder.setTristateValue(Tristate.AUTO); + if (encodeBooleanAndTriStateAsIntegerAndString) { + attributeBuilder.setIntValue(-1); + attributeBuilder.setStringValue("auto"); + } + break; + case NO: + attributeBuilder.setTristateValue(Tristate.NO); + if (encodeBooleanAndTriStateAsIntegerAndString) { + attributeBuilder.setIntValue(0); + attributeBuilder.setStringValue("no"); + } + break; + case YES: + attributeBuilder.setTristateValue(Tristate.YES); + if (encodeBooleanAndTriStateAsIntegerAndString) { + attributeBuilder.setIntValue(1); + attributeBuilder.setStringValue("yes"); + } + break; + default: + throw new AssertionError("Expected AUTO/NO/YES to cover all possible cases"); + } + } + } + + /** + * An {@link AttributeValueBuilderAdapter} which writes to a {@link SelectorEntry.Builder}. + * + * <p>Note that there is no {@code encodeBooleanAndTriStateAsIntegerAndString} parameter needed + * here. This is because the clients that expect those alternate encodings of boolean and + * tristate attribute values do not support {@link SelectorList} values. When providing output to + * those clients, we compute the set of possible attribute values (expanding {@link SelectorList} + * values, evaluating computed defaults, and flattening collections of collections; see {@link + * com.google.devtools.build.lib.packages.AggregatingAttributeMapper#getPossibleAttributeValues} + * and {@link + * com.google.devtools.build.lib.packages.AggregatingAttributeMapper#flattenAttributeValues}). + */ + private static class SelectorEntryBuilderAdapter implements AttributeValueBuilderAdapter { + private final SelectorEntry.Builder selectorEntryBuilder; + + private SelectorEntryBuilderAdapter(Builder selectorEntryBuilder) { + this.selectorEntryBuilder = Preconditions.checkNotNull(selectorEntryBuilder); + } + + public void addStringListValue(String s) { + selectorEntryBuilder.addStringListValue(s); + } + + @Override + public void addFilesetListValue(Build.FilesetEntry.Builder builder) { + selectorEntryBuilder.addFilesetListValue(builder); + } + + @Override + public void addGlobCriteria(Build.GlobCriteria.Builder builder) { + selectorEntryBuilder.addGlobCriteria(builder); + } + + @Override + public void addLabelDictUnaryValue(LabelDictUnaryEntry.Builder builder) { + selectorEntryBuilder.addLabelDictUnaryValue(builder); + } + + @Override + public void addLabelListDictValue(LabelListDictEntry.Builder builder) { + selectorEntryBuilder.addLabelListDictValue(builder); + } + + @Override + public void addIntListValue(int i) { + selectorEntryBuilder.addIntListValue(i); + } + + @Override + public void addStringDictUnaryValue(StringDictUnaryEntry.Builder builder) { + selectorEntryBuilder.addStringDictUnaryValue(builder); + } + + @Override + public void addStringDictValue(StringDictEntry.Builder builder) { + selectorEntryBuilder.addStringDictValue(builder); + } + + @Override + public void addStringListDictValue(StringListDictEntry.Builder builder) { + selectorEntryBuilder.addStringListDictValue(builder); + } + + @Override + public void setBooleanValue(boolean b) { + selectorEntryBuilder.setBooleanValue(b); + } + + @Override + public void setIntValue(int i) { + selectorEntryBuilder.setIntValue(i); + } + + @Override + public void setLicense(Build.License.Builder builder) { + selectorEntryBuilder.setLicense(builder); + } + + @Override + public void setStringValue(String s) { + selectorEntryBuilder.setStringValue(s); + } + + @Override + public void setTristateValue(Tristate tristate) { + selectorEntryBuilder.setTristateValue(tristate); + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java index 831b279666..736f09eb40 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java +++ b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java @@ -28,7 +28,6 @@ import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.syntax.Type.ConversionException; import com.google.devtools.build.lib.syntax.Type.DictType; import com.google.devtools.build.lib.syntax.Type.ListType; -import com.google.devtools.build.lib.util.Preconditions; import java.util.Collection; import java.util.Collections; @@ -399,6 +398,11 @@ public final class BuildType { this.elements = builder.build(); } + SelectorList(List<Selector<T>> elements, Type<T> originalType) { + this.elements = ImmutableList.copyOf(elements); + this.originalType = originalType; + } + /** * Returns a syntactically order-preserved list of all values and selectors for this attribute. */ @@ -444,18 +448,16 @@ public final class BuildType { Label.parseAbsoluteUnchecked(DEFAULT_CONDITION_KEY); private final Type<T> originalType; - private final Map<Label, T> map; + private final ImmutableMap<Label, T> map; private final boolean hasDefaultCondition; @VisibleForTesting - Selector(Object x, String what, @Nullable Label context, Type<T> originalType) + Selector(ImmutableMap<?, ?> x, String what, @Nullable Label context, Type<T> originalType) throws ConversionException { - Preconditions.checkState(x instanceof Map<?, ?>); - this.originalType = originalType; Map<Label, T> result = Maps.newLinkedHashMap(); boolean foundDefaultCondition = false; - for (Entry<?, ?> entry : ((Map<?, ?>) x).entrySet()) { + for (Entry<?, ?> entry : x.entrySet()) { Label key = LABEL.convert(entry.getKey(), what, context); if (key.equals(DEFAULT_CONDITION_LABEL)) { foundDefaultCondition = true; @@ -468,8 +470,11 @@ public final class BuildType { /** * Returns the selector's (configurability pattern --gt; matching values) map. + * + * <p>Entries in this map retain the order of the entries in the map provided to the {@link + * #Selector} constructor. */ - public Map<Label, T> getEntries() { + public ImmutableMap<Label, T> getEntries() { return map; } diff --git a/src/main/java/com/google/devtools/build/lib/packages/ProtoUtils.java b/src/main/java/com/google/devtools/build/lib/packages/ProtoUtils.java index 619b0cb94e..f1e6d4fe58 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/ProtoUtils.java +++ b/src/main/java/com/google/devtools/build/lib/packages/ProtoUtils.java @@ -42,6 +42,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; +import com.google.devtools.build.lib.query2.proto.proto2api.Build; import com.google.devtools.build.lib.query2.proto.proto2api.Build.Attribute.Discriminator; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.Preconditions; @@ -91,30 +92,49 @@ public class ProtoUtils { static final ImmutableSetMultimap<Discriminator, Type<?>> INVERSE_TYPE_MAP = TYPE_MAP.asMultimap().inverse(); - /** - * Returns the appropriate Attribute.Discriminator value from an internal attribute type. - */ - public static Discriminator getDiscriminatorFromType( - Type<?> type) { + /** Returns the {@link Discriminator} value corresponding to the provided {@link Type}. */ + public static Discriminator getDiscriminatorFromType(Type<?> type) { Preconditions.checkArgument(TYPE_MAP.containsKey(type), type); return TYPE_MAP.get(type); } + /** Returns the {@link Type} associated with an {@link Build.Attribute}. */ + static Type<?> getTypeFromAttributePb( + Build.Attribute attrPb, String ruleClassName, String attrName) { + Optional<Boolean> nodepHint = + attrPb.hasNodep() ? Optional.of(attrPb.getNodep()) : Optional.<Boolean>absent(); + Discriminator attrPbDiscriminator = attrPb.getType(); + boolean isSelectorList = attrPbDiscriminator.equals(Discriminator.SELECTOR_LIST); + return getTypeFromDiscriminator( + isSelectorList ? attrPb.getSelectorList().getType() : attrPbDiscriminator, + nodepHint, + ruleClassName, + attrName); + } + /** - * Returns the appropriate set of internal attribute types from an Attribute.Discriminator value. + * Returns the set of {@link Type}s associated with a {@link Discriminator} value. + * + * <p>The set will contain more than one {@link Type} when {@param discriminator} is either + * {@link Discriminator#STRING} or {@link Discriminator#STRING_LIST}, because each of them + * corresponds with two {@link Type} values. A nodeps hint is needed to determine which {@link + * Type} applies. */ - public static ImmutableSet<Type<?>> getTypesFromDiscriminator(Discriminator discriminator) { + private static ImmutableSet<Type<?>> getTypesFromDiscriminator(Discriminator discriminator) { Preconditions.checkArgument(INVERSE_TYPE_MAP.containsKey(discriminator), discriminator); return INVERSE_TYPE_MAP.get(discriminator); } /** - * Returns the appropriate internal attribute type from an Attribute.Discriminator value, given - * an optional nodeps hint. + * Returns the {@link Type} associated with a {@link Discriminator} value, given an optional + * nodeps hint. */ - public static Type<?> getTypeFromDiscriminator(Discriminator discriminator, - Optional<Boolean> nodeps, String ruleClassName, String attrName) { - Preconditions.checkArgument(INVERSE_TYPE_MAP.containsKey(discriminator)); + private static Type<?> getTypeFromDiscriminator( + Discriminator discriminator, + Optional<Boolean> nodeps, + String ruleClassName, + String attrName) { + Preconditions.checkArgument(INVERSE_TYPE_MAP.containsKey(discriminator), discriminator); ImmutableSet<Type<?>> possibleTypes = ProtoUtils.getTypesFromDiscriminator(discriminator); Type<?> preciseType; if (possibleTypes.size() == 1) { diff --git a/src/main/java/com/google/devtools/build/lib/packages/RawAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/RawAttributeMapper.java index 164b9d37f6..cbe270238a 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RawAttributeMapper.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RawAttributeMapper.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.packages; 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.packages.Attribute.ComputedDefault; import com.google.devtools.build.lib.packages.BuildType.Selector; import com.google.devtools.build.lib.packages.BuildType.SelectorList; import com.google.devtools.build.lib.syntax.Type; @@ -27,19 +28,23 @@ import java.util.List; import javax.annotation.Nullable; /** - * {@link AttributeMap} implementation that returns raw attribute information as contained - * within a {@link Rule}. In particular, configurable attributes of the form - * { config1: "value1", config2: "value2" } are passed through without being resolved to a - * final value. + * {@link AttributeMap} implementation that returns raw attribute information as contained within + * a {@link Rule} via {@link #getRawAttributeValue}. In particular, configurable attributes + * of the form { config1: "value1", config2: "value2" } are passed through without being resolved + * to a final value when obtained via that method. */ public class RawAttributeMapper extends AbstractAttributeMapper { - RawAttributeMapper(Package pkg, RuleClass ruleClass, Label ruleLabel, - AttributeContainer attributes) { + + RawAttributeMapper( + Package pkg, RuleClass ruleClass, Label ruleLabel, AttributeContainer attributes) { super(pkg, ruleClass, ruleLabel, attributes); } public static RawAttributeMapper of(Rule rule) { - return new RawAttributeMapper(rule.getPackage(), rule.getRuleClassObject(), rule.getLabel(), + return new RawAttributeMapper( + rule.getPackage(), + rule.getRuleClassObject(), + rule.getLabel(), rule.getAttributeContainer()); } @@ -92,4 +97,61 @@ public class RawAttributeMapper extends AbstractAttributeMapper { } return builder.build(); } + + /** + * See {@link #getRawAttributeValue(Rule, Attribute)}. + * + * <p>{@param attrName} must be the name of an {@link Attribute} defined by the {@param rule}'s + * {@link RuleClass}. + */ + @Nullable + public Object getRawAttributeValue(Rule rule, String attrName) { + Attribute attr = + Preconditions.checkNotNull(getAttributeDefinition(attrName), "%s %s", rule, attrName); + return getRawAttributeValue(rule, attr); + } + + /** + * Returns the object associated with the {@param rule}'s {@param attr}. + * + * <p>Handles the special case of the "visibility" attribute by returning {@param rule}'s {@link + * RuleVisibility}'s declared labels. + * + * <p>The returned object will be a {@link SelectorList} if the attribute value contains a + * {@code select(...)} expression. + * + * <p>The returned object will be a {@link ComputedDefault} if the rule doesn't explicitly + * declare an attribute value and the rule's class provides a computed default for it. + * + * <p>Otherwise, the returned object will be the type declared by the {@param attr}, or {@code + * null}. + */ + @Nullable + public Object getRawAttributeValue(Rule rule, Attribute attr) { + // This special case for the visibility attribute is needed because its value is replaced + // with an empty list during package loading if it is public or private in order not to visit + // the package called 'visibility'. + if (attr.getName().equals("visibility")) { + return rule.getVisibility().getDeclaredLabels(); + } + + // If the attribute value contains one or more select(...) expressions, then return + // the SelectorList object representing those expressions. + SelectorList<?> selectorList = getSelectorList(attr.getName(), attr.getType()); + if (selectorList != null) { + return selectorList; + } + + // If the attribute value is not explicitly declared, and the rule class provides a computed + // default value for it, then we should return the ComputedDefault object. + // + // We check for the existence of a ComputedDefault value because AbstractAttributeMapper#get + // returns either an explicitly declared attribute value or the result of evaluating the + // computed default function, but does not specify which one it is. + ComputedDefault computedDefault = getComputedDefault(attr.getName(), attr.getType()); + if (computedDefault != null) { + return computedDefault; + } + return get(attr.getName(), attr.getType()); + } } 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 5d49b4b420..658515cb49 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 @@ -13,23 +13,81 @@ // limitations under the License. package com.google.devtools.build.lib.packages; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.packages.Attribute.ComputedDefault; import com.google.devtools.build.lib.query2.proto.proto2api.Build; +import com.google.devtools.build.lib.util.Preconditions; /** Serialize a {@link Rule} as its protobuf representation. */ public class RuleSerializer { + // Skylark doesn't support defining rule classes with ComputedDefault attributes. Therefore, the + // only ComputedDefault attributes we expect to see for Skylark-defined rule classes are + // those declared in those rule classes' natively defined base rule classes, which are: + // + // 1. The "timeout" attribute in SkylarkRuleClassFunctions.testBaseRule + // 2. The "deprecation" attribute in BaseRuleClasses.commonCoreAndSkylarkAttributes + // 3. The "testonly" attribute in BaseRuleClasses.commonCoreAndSkylarkAttributes + private static final ImmutableSet<String> SKYLARK_RULE_CLASS_COMPUTED_DEFAULT_ATTRIBUTES = + ImmutableSet.of("timeout", "deprecation", "testonly"); + public static Build.Rule.Builder serializeRule(Rule rule) { Build.Rule.Builder builder = Build.Rule.newBuilder(); builder.setName(rule.getLabel().getName()); builder.setRuleClass(rule.getRuleClass()); builder.setPublicByDefault(rule.getRuleClassObject().isPublicByDefault()); - for (Attribute attribute : rule.getAttributes()) { + RawAttributeMapper rawAttributeMapper = RawAttributeMapper.of(rule); + for (Attribute attr : rule.getAttributes()) { + Object rawAttributeValue = rawAttributeMapper.getRawAttributeValue(rule, attr); + + 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); + } + } else { + valueToSerialize = rawAttributeValue; + } + builder.addAttribute( AttributeSerializer.getAttributeProto( - attribute, - AttributeSerializer.getAttributeValues(rule, attribute), - rule.isAttributeValueExplicitlySpecified(attribute), - /*includeGlobs=*/ true)); + attr, + valueToSerialize, + rule.isAttributeValueExplicitlySpecified(attr), + /*includeGlobs=*/ true, + /*encodeBooleanAndTriStateAsIntegerAndString=*/ false)); } return builder; } diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java index a6351d4df9..c6e79e1f14 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java +++ b/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java @@ -24,8 +24,8 @@ import com.google.devtools.build.lib.collect.CompactHashSet; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.graph.Digraph; import com.google.devtools.build.lib.graph.Node; +import com.google.devtools.build.lib.packages.AggregatingAttributeMapper; import com.google.devtools.build.lib.packages.Attribute; -import com.google.devtools.build.lib.packages.AttributeSerializer; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.query2.engine.OutputFormatterCallback; @@ -334,12 +334,13 @@ public abstract class OutputFormatter implements Serializable { out.printf(" name = \"%s\",%n", rule.getName()); for (Attribute attr : rule.getAttributes()) { - Pair<Iterable<Object>, AttributeValueSource> values = getAttributeValues(rule, attr); + Pair<Iterable<Object>, AttributeValueSource> values = + getPossibleAttributeValuesAndSources(rule, attr); if (Iterables.size(values.first) != 1) { - continue; // TODO(bazel-team): handle configurable attributes. + continue; // TODO(bazel-team): handle configurable attributes. } if (values.second != AttributeValueSource.RULE) { - continue; // Don't print default values. + continue; // Don't print default values. } Object value = Iterables.getOnlyElement(values.first); out.printf(" %s = ", attr.getPublicName()); @@ -538,17 +539,18 @@ public abstract class OutputFormatter implements Serializable { } /** - * Returns the possible values of the specified attribute in the specified rule. For - * non-configured attributes, this is a single value. For configurable attributes, this - * may be multiple values. + * Returns the possible values of the specified attribute in the specified rule. For simple + * attributes, this is a single value. For configurable and computed attributes, this may be a + * list of values. See {@link AggregatingAttributeMapper#getPossibleAttributeValues} for how the + * value(s) is/are made. * * @return a pair, where the first value is the set of possible values and the * second is an enum that tells where the values come from (declared on the * rule, declared as a package level default or a * global default) */ - protected static Pair<Iterable<Object>, AttributeValueSource> getAttributeValues( - Rule rule, Attribute attr) { + protected static Pair<Iterable<Object>, AttributeValueSource> + getPossibleAttributeValuesAndSources(Rule rule, Attribute attr) { AttributeValueSource source; if (attr.getName().equals("visibility")) { @@ -564,7 +566,9 @@ public abstract class OutputFormatter implements Serializable { ? AttributeValueSource.RULE : AttributeValueSource.DEFAULT; } - return Pair.of(AttributeSerializer.getAttributeValues(rule, attr), source); + Iterable<Object> possibleAttributeValues = + AggregatingAttributeMapper.of(rule).getPossibleAttributeValues(rule, attr); + return Pair.of(possibleAttributeValues, source); } /** diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java index 0fc1ffc582..e1f8ea78a5 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java +++ b/src/main/java/com/google/devtools/build/lib/query2/output/ProtoOutputFormatter.java @@ -19,13 +19,15 @@ import static com.google.devtools.build.lib.query2.proto.proto2api.Build.Target. import static com.google.devtools.build.lib.query2.proto.proto2api.Build.Target.Discriminator.RULE; import static com.google.devtools.build.lib.query2.proto.proto2api.Build.Target.Discriminator.SOURCE_FILE; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.graph.Digraph; +import com.google.devtools.build.lib.packages.AggregatingAttributeMapper; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.AttributeSerializer; +import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.EnvironmentGroup; import com.google.devtools.build.lib.packages.InputFile; import com.google.devtools.build.lib.packages.OutputFile; @@ -140,12 +142,19 @@ public class ProtoOutputFormatter extends AbstractUnorderedFormatter { || !includeAttribute(attr)) { continue; } - rulePb.addAttribute( + Iterable<Object> possibleAttributeValues = + AggregatingAttributeMapper.of(rule).getPossibleAttributeValues(rule, attr); + Object flattenedAttributeValue = + AggregatingAttributeMapper.flattenAttributeValues( + attr.getType(), possibleAttributeValues); + Build.Attribute serializedAttribute = AttributeSerializer.getAttributeProto( attr, - AttributeSerializer.getAttributeValues(rule, attr), + flattenedAttributeValue, rule.isAttributeValueExplicitlySpecified(attr), - /*includeGlobs=*/ false)); + /*includeGlobs=*/ false, + /*encodeBooleanAndTriStateAsIntegerAndString=*/ true); + rulePb.addAttribute(serializedAttribute); } postProcess(rule, rulePb); @@ -166,12 +175,17 @@ public class ProtoOutputFormatter extends AbstractUnorderedFormatter { aspectResolver.computeAspectDependencies(target); // Add information about additional attributes from aspects. for (Entry<Attribute, Collection<Label>> entry : aspectsDependencies.asMap().entrySet()) { - rulePb.addAttribute( + Attribute attribute = entry.getKey(); + Collection<Label> labels = entry.getValue(); + Object attributeValue = getAspectAttributeValue(attribute, labels); + Build.Attribute serializedAttribute = AttributeSerializer.getAttributeProto( - entry.getKey(), - Lists.<Object>newArrayList(entry.getValue()), + attribute, + attributeValue, /*explicitlySpecified=*/ false, - /*includeGlobs=*/ false)); + /*includeGlobs=*/ false, + /*encodeBooleanAndTriStateAsIntegerAndString=*/ true); + rulePb.addAttribute(serializedAttribute); } // Add all deps from aspects as rule inputs of current target. for (Label label : aspectsDependencies.values()) { @@ -291,6 +305,22 @@ public class ProtoOutputFormatter extends AbstractUnorderedFormatter { return targetPb.build(); } + private static Object getAspectAttributeValue(Attribute attribute, Collection<Label> labels) { + com.google.devtools.build.lib.syntax.Type<?> attributeType = attribute.getType(); + if (attributeType.equals(BuildType.LABEL)) { + Preconditions.checkState(labels.size() == 1, "attribute=%s, labels=%s", attribute, labels); + return Iterables.getOnlyElement(labels); + } else { + Preconditions.checkState( + attributeType.equals(BuildType.LABEL_LIST), + "attribute=%s, type=%s, labels=%s", + attribute, + attributeType, + labels); + return labels; + } + } + /** Further customize the proto output */ protected void postProcess(Rule rule, Build.Rule.Builder rulePb) { } diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java index 2af71b7414..14fdfa604d 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java +++ b/src/main/java/com/google/devtools/build/lib/query2/output/XmlOutputFormatter.java @@ -137,7 +137,8 @@ class XmlOutputFormatter extends AbstractUnorderedFormatter { elem = doc.createElement("rule"); elem.setAttribute("class", rule.getRuleClass()); for (Attribute attr : rule.getAttributes()) { - Pair<Iterable<Object>, AttributeValueSource> values = getAttributeValues(rule, attr); + Pair<Iterable<Object>, AttributeValueSource> values = + getPossibleAttributeValuesAndSources(rule, attr); if (values.second == AttributeValueSource.RULE || options.xmlShowDefaultValues) { Element attrElem = createValueElement(doc, attr.getType(), values.first); attrElem.setAttribute("name", attr.getName()); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SelectorValue.java b/src/main/java/com/google/devtools/build/lib/syntax/SelectorValue.java index 8d253b32e9..d4c9366bfa 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SelectorValue.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SelectorValue.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.syntax; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import java.util.Map; @@ -34,16 +35,20 @@ public final class SelectorValue { // TODO(bazel-team): Selectors are currently split between .packages and .syntax . They should // really all be in .packages, but then we'd need to figure out a way how to extend binary // operators, which is a non-trivial problem. - private final Map<?, ?> dictionary; + private final ImmutableMap<?, ?> dictionary; private final Class<?> type; public SelectorValue(Map<?, ?> dictionary) { // Put the dict through a sorting to avoid depending on insertion order. - this.dictionary = new TreeMap<>(dictionary); + this.dictionary = ImmutableMap.copyOf(new TreeMap<>(dictionary)); this.type = dictionary.isEmpty() ? null : Iterables.get(dictionary.values(), 0).getClass(); } - public Map<?, ?> getDictionary() { + /** + * Returns an {@link ImmutableMap} containing the entries in the map provided to {@link + * #SelectorValue} in sorted order. + */ + public ImmutableMap<?, ?> getDictionary() { return dictionary; } |