diff options
13 files changed, 199 insertions, 81 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 937cf74c45..656ca2595f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -81,6 +81,7 @@ import com.google.devtools.build.lib.rules.fileset.FilesetProvider; import com.google.devtools.build.lib.shell.ShellUtils; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.Type; +import com.google.devtools.build.lib.syntax.Type.LabelClass; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.OrderedSetMultimap; import com.google.devtools.build.lib.util.Preconditions; @@ -1083,8 +1084,7 @@ public final class RuleContext extends TargetContext throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging() + " attribute " + attributeName + " is not defined"); } - if (!(attributeDefinition.getType() == BuildType.LABEL - || attributeDefinition.getType() == BuildType.LABEL_LIST)) { + if (attributeDefinition.getType().getLabelClass() != LabelClass.DEPENDENCY) { throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName + " is not a label type attribute"); } @@ -1125,8 +1125,7 @@ public final class RuleContext extends TargetContext throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging() + " attribute " + attributeName + " is not defined"); } - if (!(attributeDefinition.getType() == BuildType.LABEL - || attributeDefinition.getType() == BuildType.LABEL_LIST)) { + if (attributeDefinition.getType().getLabelClass() != LabelClass.DEPENDENCY) { throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName + " is not a label type attribute"); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java index 86d736d60a..33f7d348f6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java @@ -36,6 +36,8 @@ import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.syntax.Type; +import com.google.devtools.build.lib.syntax.Type.LabelClass; +import com.google.devtools.build.lib.syntax.Type.LabelVisitor; import com.google.devtools.build.lib.util.Preconditions; import java.util.Collection; @@ -739,7 +741,7 @@ public class ConstraintSemantics { AttributeMap attributes = ruleContext.attributes(); for (String attr : attributes.getAttributeNames()) { Attribute attrDef = attributes.getAttributeDefinition(attr); - if ((attrDef.getType() != BuildType.LABEL && attrDef.getType() != BuildType.LABEL_LIST) + if (attrDef.getType().getLabelClass() != LabelClass.DEPENDENCY || attrDef.skipConstraintsOverride()) { continue; } @@ -819,21 +821,22 @@ public class ConstraintSemantics { * Adds all label values from the given select to the given set. Automatically handles different * value types (e.g. labels vs. label lists). */ - private static void addSelectValuesToSet(BuildType.Selector<?> select, Set<Label> set) { + private static void addSelectValuesToSet(BuildType.Selector<?> select, final Set<Label> set) { Type<?> type = select.getOriginalType(); - if (type == BuildType.LABEL || type == BuildType.NODEP_LABEL) { - set.addAll(((BuildType.Selector<Label>) select).getEntries().values()); - } else if (type == BuildType.LABEL_LIST || type == BuildType.NODEP_LABEL_LIST) { - for (List<Label> labels : ((BuildType.Selector<List<Label>>) select).getEntries().values()) { - set.addAll(labels); + LabelVisitor visitor = new LabelVisitor() { + @Override + public void visit(Label label) { + set.add(label); } - } else if (type == BuildType.LABEL_DICT_UNARY) { - for (Map<String, Label> mapEntry : - ((BuildType.Selector<Map<String, Label>>) select).getEntries().values()) { - set.addAll(mapEntry.values()); + }; + for (Object value : select.getEntries().values()) { + try { + type.visitLabels(visitor, value); + } catch (InterruptedException ex) { + // Because the LabelVisitor does not throw InterruptedException, it should not be thrown + // by visitLabels here. + throw new AssertionError(ex); } - } else { - throw new IllegalStateException("Expected a label-based type for this select"); } } } diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java index 338d4bfce8..a305e515d2 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java @@ -25,11 +25,12 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy; import com.google.devtools.build.lib.syntax.Type; +import com.google.devtools.build.lib.syntax.Type.LabelClass; +import com.google.devtools.build.lib.syntax.Type.LabelVisitor; import com.google.devtools.build.lib.util.Preconditions; import java.util.Collection; import java.util.LinkedHashMap; import java.util.LinkedHashSet; -import java.util.List; import java.util.Map; import javax.annotation.Nullable; @@ -190,32 +191,36 @@ public final class AspectDefinition { * Collects all attribute labels from the specified aspectDefinition. */ public static void addAllAttributesOfAspect( - Rule from, - Multimap<Attribute, Label> labelBuilder, + final Rule from, + final Multimap<Attribute, Label> labelBuilder, Aspect aspect, DependencyFilter dependencyFilter) { ImmutableMap<String, Attribute> attributes = aspect.getDefinition().getAttributes(); - for (Attribute aspectAttribute : attributes.values()) { + for (final Attribute aspectAttribute : attributes.values()) { if (!dependencyFilter.apply(aspect, aspectAttribute)) { continue; } - if (aspectAttribute.getType() == BuildType.LABEL) { - Label label = maybeGetRepositoryRelativeLabel( - from, BuildType.LABEL.cast(aspectAttribute.getDefaultValue(from))); - if (label != null) { - labelBuilder.put(aspectAttribute, label); - } - } else if (aspectAttribute.getType() == BuildType.LABEL_LIST) { - List<Label> defaultLabels = BuildType.LABEL_LIST.cast( + Type type = aspectAttribute.getType(); + if (type.getLabelClass() != LabelClass.DEPENDENCY) { + continue; + } + try { + type.visitLabels( + new LabelVisitor() { + @Override + public void visit(Label label) { + Label repositoryRelative = maybeGetRepositoryRelativeLabel(from, label); + if (repositoryRelative == null) { + return; + } + labelBuilder.put(aspectAttribute, repositoryRelative); + } + }, aspectAttribute.getDefaultValue(from)); - if (defaultLabels != null) { - for (Label defaultLabel : defaultLabels) { - Label label = maybeGetRepositoryRelativeLabel(from, defaultLabel); - if (label != null) { - labelBuilder.put(aspectAttribute, label); - } - } - } + } catch (InterruptedException ex) { + // Because the LabelVisitor does not throw InterruptedException, it should not be thrown + // by visitLabels here. + throw new AssertionError(ex); } } } diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java index 2288a76210..f5e9b95eb9 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java @@ -36,6 +36,7 @@ import com.google.devtools.build.lib.syntax.Runtime; import com.google.devtools.build.lib.syntax.SkylarkCallbackFunction; 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.LabelClass; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.Preconditions; @@ -507,7 +508,7 @@ public final class Attribute implements Comparable<Attribute> { * Makes the built attribute producing a single artifact. */ public Builder<TYPE> singleArtifact() { - Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST), + Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY, "attribute '%s' must be a label-valued type", name); return setPropertyFlag(PropertyFlag.SINGLE_ARTIFACT, "single_artifact"); } @@ -517,7 +518,7 @@ public final class Attribute implements Comparable<Attribute> { * This flag is introduced to handle plugins, do not use it in other cases. */ public Builder<TYPE> silentRuleClassFilter() { - Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST), + Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY, "must be a label-valued type"); return setPropertyFlag(PropertyFlag.SILENT_RULECLASS_FILTER, "silent_ruleclass_filter"); } @@ -526,7 +527,7 @@ public final class Attribute implements Comparable<Attribute> { * Skip analysis time filetype check. Don't use it if avoidable. */ public Builder<TYPE> skipAnalysisTimeFileTypeCheck() { - Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST), + Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY, "must be a label-valued type"); return setPropertyFlag(PropertyFlag.SKIP_ANALYSIS_TIME_FILETYPE_CHECK, "skip_analysis_time_filetype_check"); @@ -801,7 +802,7 @@ public final class Attribute implements Comparable<Attribute> { * other words, it works for 'deps' attributes, but not 'srcs' attributes. */ public Builder<TYPE> allowedRuleClasses(Predicate<RuleClass> allowedRuleClasses) { - Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST), + Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY, "must be a label-valued type"); propertyFlags.add(PropertyFlag.STRICT_LABEL_CHECKING); allowedRuleClassesForLabels = allowedRuleClasses; @@ -831,7 +832,7 @@ public final class Attribute implements Comparable<Attribute> { * other words, it works for 'deps' attributes, but not 'srcs' attributes. */ public Builder<TYPE> allowedFileTypes(FileTypeSet allowedFileTypes) { - Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST), + Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY, "must be a label-valued type"); propertyFlags.add(PropertyFlag.STRICT_LABEL_CHECKING); allowedFileTypesForLabels = Preconditions.checkNotNull(allowedFileTypes); @@ -886,7 +887,7 @@ public final class Attribute implements Comparable<Attribute> { * other words, it works for 'deps' attributes, but not 'srcs' attributes. */ public Builder<TYPE> allowedRuleClassesWithWarning(Predicate<RuleClass> allowedRuleClasses) { - Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST), + Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY, "must be a label-valued type"); propertyFlags.add(PropertyFlag.STRICT_LABEL_CHECKING); allowedRuleClassesForLabelsWarning = allowedRuleClasses; @@ -914,7 +915,7 @@ public final class Attribute implements Comparable<Attribute> { */ public final Builder<TYPE> mandatoryNativeProvidersList( Iterable<? extends Iterable<Class<? extends TransitiveInfoProvider>>> providersList) { - Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST), + Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY, "must be a label-valued type"); ImmutableList.Builder<ImmutableList<Class<? extends TransitiveInfoProvider>>> listBuilder = ImmutableList.builder(); @@ -941,7 +942,7 @@ public final class Attribute implements Comparable<Attribute> { */ public Builder<TYPE> mandatoryProvidersList( Iterable<? extends Iterable<SkylarkProviderIdentifier>> providersList){ - Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST), + Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY, "must be a label-valued type"); ImmutableList.Builder<ImmutableSet<SkylarkProviderIdentifier>> listBuilder = ImmutableList.builder(); @@ -1086,13 +1087,13 @@ public final class Attribute implements Comparable<Attribute> { // do not modify this.allowedFileTypesForLabels, instead create a copy. FileTypeSet allowedFileTypesForLabels = this.allowedFileTypesForLabels; - if ((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST)) { + if (type.getLabelClass() == LabelClass.DEPENDENCY) { if (isPrivateAttribute(name) && allowedFileTypesForLabels == null) { allowedFileTypesForLabels = FileTypeSet.ANY_FILE; } Preconditions.checkNotNull( allowedFileTypesForLabels, "allowedFileTypesForLabels not set for %s", name); - } else if ((type == BuildType.OUTPUT) || (type == BuildType.OUTPUT_LIST)) { + } else if (type.getLabelClass() == LabelClass.OUTPUT) { // TODO(bazel-team): Set the default to no file type and make explicit calls instead. if (allowedFileTypesForLabels == null) { allowedFileTypesForLabels = FileTypeSet.ANY_FILE; @@ -1769,8 +1770,8 @@ public final class Attribute implements Comparable<Attribute> { Preconditions.checkNotNull(configTransition); Preconditions.checkArgument( (configTransition == ConfigurationTransition.NONE && configurator == null) - || type == BuildType.LABEL || type == BuildType.LABEL_LIST - || type == BuildType.NODEP_LABEL || type == BuildType.NODEP_LABEL_LIST, + || type.getLabelClass() == LabelClass.DEPENDENCY + || type.getLabelClass() == LabelClass.NONDEP_REFERENCE, "Configuration transitions can only be specified for label or label list attributes"); Preconditions.checkArgument( isLateBound(name) == (defaultValue instanceof LateBoundDefault), @@ -1975,8 +1976,8 @@ public final class Attribute implements Comparable<Attribute> { * Returns true if this attribute's value can be influenced by the build configuration. */ public boolean isConfigurable() { - return !(type == BuildType.OUTPUT // Excluded because of Rule#populateExplicitOutputFiles. - || type == BuildType.OUTPUT_LIST + // Output types are excluded because of Rule#populateExplicitOutputFiles. + return !(type.getLabelClass() == LabelClass.OUTPUT || getPropertyFlag(PropertyFlag.NONCONFIGURABLE)); } 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 ae3b1214ed..051b7e033f 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 @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.syntax.SelectorValue; 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.LabelClass; import com.google.devtools.build.lib.syntax.Type.ListType; import java.util.Collections; import java.util.LinkedHashMap; @@ -47,7 +48,7 @@ public final class BuildType { * attributes that it's worth treating them specially (and providing support * for resolution of relative-labels in the <code>convert()</code> method). */ - public static final Type<Label> LABEL = new LabelType(); + public static final Type<Label> LABEL = new LabelType(LabelClass.DEPENDENCY); /** * The type of a dictionary of {@linkplain #LABEL labels}. */ @@ -62,7 +63,7 @@ public final class BuildType { * certain rules want to verify the type of a target referenced by one of their attributes, but * if there was a dependency edge there, it would be a circular dependency. */ - public static final Type<Label> NODEP_LABEL = new LabelType(); + public static final Type<Label> NODEP_LABEL = new LabelType(LabelClass.NONDEP_REFERENCE); /** * The type of a list of {@linkplain #NODEP_LABEL labels} that do not cause * dependencies. @@ -136,8 +137,7 @@ public final class BuildType { * Returns whether the specified type is a label type or not. */ public static boolean isLabelType(Type<?> type) { - return type == LABEL || type == LABEL_LIST || type == LABEL_DICT_UNARY - || type == NODEP_LABEL || type == NODEP_LABEL_LIST || type == FILESET_ENTRY_LIST; + return type.getLabelClass() != LabelClass.NONE; } /** @@ -182,6 +182,11 @@ public final class BuildType { } @Override + public LabelClass getLabelClass() { + return LabelClass.FILESET_ENTRY; + } + + @Override public FilesetEntry getDefaultValue() { return null; } @@ -195,6 +200,12 @@ public final class BuildType { } private static class LabelType extends Type<Label> { + private final LabelClass labelClass; + + LabelType(LabelClass labelClass) { + this.labelClass = labelClass; + } + @Override public Label cast(Object value) { return (Label) value; @@ -216,6 +227,11 @@ public final class BuildType { } @Override + public LabelClass getLabelClass() { + return labelClass; + } + + @Override public Label convert(Object x, Object what, Object context) throws ConversionException { if (x instanceof Label) { @@ -327,6 +343,11 @@ public final class BuildType { } @Override + public LabelClass getLabelClass() { + return LabelClass.OUTPUT; + } + + @Override public String toString() { return "output"; } diff --git a/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java b/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java index a5d41ee631..88fe6d4157 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java +++ b/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.packages; import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.packages.DependencyFilter.AttributeInfoProvider; +import com.google.devtools.build.lib.syntax.Type.LabelClass; import com.google.devtools.build.lib.util.BinaryPredicate; /** @@ -38,8 +39,8 @@ public abstract class DependencyFilter new DependencyFilter() { @Override public boolean apply(AttributeInfoProvider infoProvider, Attribute attribute) { - // isHostConfiguration() is only defined for labels and label lists. - if (attribute.getType() != BuildType.LABEL && attribute.getType() != BuildType.LABEL_LIST) { + // getConfigurationTransition() is only defined for labels which introduce a dependency. + if (attribute.getType().getLabelClass() != LabelClass.DEPENDENCY) { return true; } @@ -62,8 +63,7 @@ public abstract class DependencyFilter new DependencyFilter() { @Override public boolean apply(AttributeInfoProvider infoProvider, Attribute attribute) { - return attribute.getType() != BuildType.NODEP_LABEL - && attribute.getType() != BuildType.NODEP_LABEL_LIST; + return attribute.getType().getLabelClass() != LabelClass.NONDEP_REFERENCE; } }; /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java index abec031476..08113b6f20 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java @@ -46,6 +46,7 @@ import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor; import com.google.devtools.build.lib.syntax.SkylarkType; 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.LabelClass; import com.google.devtools.build.lib.syntax.UserDefinedFunction; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.FileTypeSet; @@ -235,7 +236,7 @@ public final class SkylarkAttr { Object fileTypesObj = arguments.get(ALLOW_SINGLE_FILE_ARG); setAllowedFileTypes(ALLOW_SINGLE_FILE_ARG, fileTypesObj, ast, builder); builder.setPropertyFlag("SINGLE_ARTIFACT"); - } else if (type.equals(BuildType.LABEL) || type.equals(BuildType.LABEL_LIST)) { + } else if (type.getLabelClass() == LabelClass.DEPENDENCY) { builder.allowedFileTypes(FileTypeSet.NO_FILE); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java index 7e7b69cda1..8dfc1579d8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java @@ -64,6 +64,7 @@ import com.google.devtools.build.lib.syntax.SkylarkDict; import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkType; import com.google.devtools.build.lib.syntax.Type; +import com.google.devtools.build.lib.syntax.Type.LabelClass; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; @@ -206,7 +207,7 @@ public final class SkylarkRuleContext { for (Attribute a : attributes) { String attrName = a.getName(); Type<?> type = a.getType(); - if (type != BuildType.OUTPUT && type != BuildType.OUTPUT_LIST) { + if (type.getLabelClass() != LabelClass.OUTPUT) { continue; } ImmutableList.Builder<Artifact> artifactsBuilder = ImmutableList.builder(); @@ -290,7 +291,10 @@ public final class SkylarkRuleContext { for (Attribute a : attributes) { Type<?> type = a.getType(); Object val = attributeValueExtractor.apply(a); - if (type != BuildType.LABEL && type != BuildType.LABEL_LIST) { + // TODO(mstaib): Remove the LABEL_DICT_UNARY special case of this conditional + // LABEL_DICT_UNARY was previously not treated as a dependency-bearing type, and was put into + // Skylark as a Map<String, Label>; this special case preserves that behavior temporarily. + if (type.getLabelClass() != LabelClass.DEPENDENCY || type == BuildType.LABEL_DICT_UNARY) { attrBuilder.put(a.getPublicName(), val == null ? Runtime.NONE // Attribute values should be type safe : SkylarkType.convertToSkylark(val, null)); @@ -336,10 +340,26 @@ public final class SkylarkRuleContext { prereq = Runtime.NONE; } attrBuilder.put(skyname, prereq); - } else { - // Type.LABEL_LIST + } else if (type == BuildType.LABEL_LIST + || (type == BuildType.LABEL && a.hasSplitConfigurationTransition())) { List<?> allPrereq = ruleContext.getPrerequisites(a.getName(), Mode.DONT_CHECK); attrBuilder.put(skyname, SkylarkList.createImmutable(allPrereq)); + } else if (type == BuildType.LABEL_DICT_UNARY) { + Map<Label, TransitiveInfoCollection> prereqsByLabel = new LinkedHashMap<>(); + for (TransitiveInfoCollection target + : ruleContext.getPrerequisites(a.getName(), Mode.DONT_CHECK)) { + prereqsByLabel.put(target.getLabel(), target); + } + ImmutableMap.Builder<String, TransitiveInfoCollection> attrValue = + new ImmutableMap.Builder<>(); + for (Map.Entry<String, Label> entry : ((Map<String, Label>) val).entrySet()) { + attrValue.put(entry.getKey(), prereqsByLabel.get(entry.getValue())); + } + attrBuilder.put(skyname, attrValue.build()); + } else { + throw new IllegalArgumentException( + "Can't transform attribute " + a.getName() + " of type " + type + + " to a Skylark object"); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSuiteRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSuiteRule.java index a08e6e0930..55244b470a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSuiteRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSuiteRule.java @@ -22,6 +22,7 @@ import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder; import com.google.devtools.build.lib.syntax.Type; +import com.google.devtools.build.lib.util.FileTypeSet; /** * Definition of the {@code cc_toolchain_suite} rule. @@ -34,6 +35,7 @@ public final class CcToolchainSuiteRule implements RuleDefinition { .setUndocumented() .add(attr("toolchains", BuildType.LABEL_DICT_UNARY) .mandatory() + .allowedFileTypes(FileTypeSet.NO_FILE) .nonconfigurable("Used during configuration creation")) .add(attr("proto", Type.STRING) .nonconfigurable("Used during configuration creation")) diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeSuiteRule.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeSuiteRule.java index 8b28e91c29..22de4df4a3 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeSuiteRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeSuiteRule.java @@ -35,7 +35,7 @@ public final class JavaRuntimeSuiteRule implements RuleDefinition { /* <!-- #BLAZE_RULE(java_runtime_suite).ATTRIBUTE(runtimes) --> A map from each supported architecture to the corresponding <code>java_runtime</code>. <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ - .add(attr("runtimes", BuildType.LABEL_DICT_UNARY)) + .add(attr("runtimes", BuildType.LABEL_DICT_UNARY).allowedFileTypes(FileTypeSet.NO_FILE)) /* <!-- #BLAZE_RULE(java_runtime_suite).ATTRIBUTE(default) --> The default <code>java_runtime</code>, used if <a href="${link java_runtime_suite.runtimes}"><code>runtimes</code></a> diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Type.java b/src/main/java/com/google/devtools/build/lib/syntax/Type.java index dbbfc79c42..387794b40f 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Type.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Type.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.syntax; import com.google.common.base.Joiner; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -139,6 +140,31 @@ public abstract class Type<T> { */ public abstract void visitLabels(LabelVisitor visitor, Object value) throws InterruptedException; + /** Classifications of labels by their usage. */ + public enum LabelClass { + /** Used for types which are not labels. */ + NONE, + /** Used for types which use labels to declare a dependency. */ + DEPENDENCY, + /** + * Used for types which use labels to reference another target but do not declare a dependency, + * in cases where doing so would cause a dependency cycle. + */ + NONDEP_REFERENCE, + /** Used for types which use labels to declare an output path. */ + OUTPUT, + /** + * Used for types which contain Fileset entries, which contain labels but do not produce + * normal dependencies. + */ + FILESET_ENTRY + } + + /** Returns the class of labels contained by this type, if any. */ + public LabelClass getLabelClass() { + return LabelClass.NONE; + } + /** * Implementation of concatenation for this type (e.g. "val1 + val2"). Returns null to * indicate concatenation isn't supported. @@ -423,6 +449,8 @@ public abstract class Type<T> { private final Map<KeyT, ValueT> empty = ImmutableMap.of(); + private final LabelClass labelClass; + @Override public void visitLabels(LabelVisitor visitor, Object value) throws InterruptedException { for (Entry<KeyT, ValueT> entry : cast(value).entrySet()) { @@ -433,12 +461,24 @@ public abstract class Type<T> { public static <KEY, VALUE> DictType<KEY, VALUE> create( Type<KEY> keyType, Type<VALUE> valueType) { - return new DictType<>(keyType, valueType); + LabelClass keyLabelClass = keyType.getLabelClass(); + LabelClass valueLabelClass = valueType.getLabelClass(); + Preconditions.checkArgument( + keyLabelClass == LabelClass.NONE + || valueLabelClass == LabelClass.NONE + || keyLabelClass == valueLabelClass, + "A DictType's keys and values must be the same class of label if both contain labels, " + + "but the key type " + keyType + " contains " + keyLabelClass + " labels, while " + + "the value type " + valueType + " contains " + valueLabelClass + " labels."); + LabelClass labelClass = (keyLabelClass != LabelClass.NONE) ? keyLabelClass : valueLabelClass; + + return new DictType<>(keyType, valueType, labelClass); } - private DictType(Type<KeyT> keyType, Type<ValueT> valueType) { + private DictType(Type<KeyT> keyType, Type<ValueT> valueType, LabelClass labelClass) { this.keyType = keyType; this.valueType = valueType; + this.labelClass = labelClass; } public Type<KeyT> getKeyType() { @@ -449,6 +489,11 @@ public abstract class Type<T> { return valueType; } + @Override + public LabelClass getLabelClass() { + return labelClass; + } + @SuppressWarnings("unchecked") @Override public Map<KeyT, ValueT> cast(Object value) { @@ -511,6 +556,11 @@ public abstract class Type<T> { } @Override + public LabelClass getLabelClass() { + return elemType.getLabelClass(); + } + + @Override public List<ElemT> getDefaultValue() { return empty; } diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java index 078d6af669..578db1b98f 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java @@ -1383,4 +1383,25 @@ public class SkylarkRuleContextTest extends SkylarkTestCase { "ruleContext.coverage_instrumented(ruleContext.attr.deps[0])"); assertThat((Boolean) result).isTrue(); } + + @Test + public void testStringKeyedLabelDictAttributeInSkylarkRuleContext() throws Exception { + scratch.file("jvm/BUILD", + "java_runtime(name='runtime', srcs=[], java_home='')", + "java_runtime_suite(", + " name = 'suite',", + " runtimes = {'x86': ':runtime'},", + " default = ':runtime',", + ")"); + + invalidatePackages(); + SkylarkRuleContext ruleContext = createRuleContext("//jvm:suite"); + assertNoEvents(); + String keyString = + (String) evalRuleContextCode(ruleContext, "ruleContext.attr.runtimes.keys()[0]"); + assertThat(keyString).isEqualTo("x86"); + Label valueLabel = + (Label) evalRuleContextCode(ruleContext, "ruleContext.attr.runtimes.values()[0]"); + assertThat(valueLabel).isEqualTo(Label.parseAbsolute("//jvm:runtime")); + } } diff --git a/src/test/java/com/google/devtools/build/lib/testutil/BuildRuleWithDefaultsBuilder.java b/src/test/java/com/google/devtools/build/lib/testutil/BuildRuleWithDefaultsBuilder.java index 7db103eba3..e0e6b77407 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/BuildRuleWithDefaultsBuilder.java +++ b/src/test/java/com/google/devtools/build/lib/testutil/BuildRuleWithDefaultsBuilder.java @@ -20,6 +20,8 @@ import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.syntax.Type; +import com.google.devtools.build.lib.syntax.Type.LabelClass; +import com.google.devtools.build.lib.syntax.Type.ListType; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.Preconditions; @@ -57,8 +59,8 @@ public class BuildRuleWithDefaultsBuilder extends BuildRuleBuilder { */ private String getDummyFileLabel(String rulePkg, String filePkg, String extension, Type<?> attrType) { - boolean isInput = (attrType == BuildType.LABEL || attrType == BuildType.LABEL_LIST); - String fileName = (isInput ? "dummy_input" : "dummy_output") + extension; + boolean isOutput = attrType.getLabelClass() == LabelClass.OUTPUT; + String fileName = (isOutput ? "dummy_output" : "dummy_input") + extension; generateFiles.add(filePkg + "/" + fileName); if (rulePkg.equals(filePkg)) { return ":" + fileName; @@ -122,7 +124,7 @@ public class BuildRuleWithDefaultsBuilder extends BuildRuleBuilder { } } if (label != null) { - if (attrType == BuildType.LABEL_LIST || attrType == BuildType.OUTPUT_LIST) { + if (attrType instanceof ListType<?>) { addMultiValueAttributes(attribute.getName(), label); } else { setSingleValueAttribute(attribute.getName(), label); @@ -175,17 +177,10 @@ public class BuildRuleWithDefaultsBuilder extends BuildRuleBuilder { public BuildRuleWithDefaultsBuilder populateAttributes(String rulePkg, boolean heuristics) { for (Attribute attribute : ruleClass.getAttributes()) { if (attribute.isMandatory()) { - if (attribute.getType() == BuildType.LABEL_LIST - || attribute.getType() == BuildType.OUTPUT_LIST) { - if (attribute.isNonEmpty()) { - populateLabelAttribute(rulePkg, attribute); - } else { - // TODO(bazel-team): actually here an empty list would be fine, but BuildRuleBuilder - // doesn't support that, and it makes little sense anyway - populateLabelAttribute(rulePkg, attribute); - } - } else if (attribute.getType() == BuildType.LABEL - || attribute.getType() == BuildType.OUTPUT) { + if (BuildType.isLabelType(attribute.getType())) { + // TODO(bazel-team): actually an empty list would be fine in the case where + // attribute instanceof ListType && !attribute.isNonEmpty(), but BuildRuleBuilder + // doesn't support that, and it makes little sense anyway populateLabelAttribute(rulePkg, attribute); } else { // Non label type attributes |