diff options
Diffstat (limited to 'src')
11 files changed, 275 insertions, 394 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java index 235dc375f4..deee99aefc 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java @@ -104,6 +104,12 @@ public abstract class AbstractConfiguredTarget @Override public Object getValue(String name) { switch (name) { + case FILES_FIELD: + case DEFAULT_RUNFILES_FIELD: + case DATA_RUNFILES_FIELD: + case FilesToRunProvider.SKYLARK_NAME: + // Standard fields should be proxied to their default provider object + return getDefaultProvider().getValue(name); case LABEL_FIELD: return getLabel(); default: @@ -199,18 +205,7 @@ public abstract class AbstractConfiguredTarget if (OutputGroupProvider.SKYLARK_NAME.equals(providerKey)) { return get(OutputGroupProvider.SKYLARK_CONSTRUCTOR); } - switch (providerKey) { - case FILES_FIELD: - case DEFAULT_RUNFILES_FIELD: - case DATA_RUNFILES_FIELD: - case FilesToRunProvider.SKYLARK_NAME: - // Standard fields should be proxied to their default provider object - return getDefaultProvider().getValue(providerKey); - case OutputGroupProvider.SKYLARK_NAME: - return get(OutputGroupProvider.SKYLARK_CONSTRUCTOR); - default: - return rawGetSkylarkProvider(providerKey); - } + return rawGetSkylarkProvider(providerKey); } /** Implement in subclasses to get a skylark provider for a given {@code providerKey}. */ 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 de978e0940..771189afa6 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 @@ -70,17 +70,18 @@ import com.google.devtools.build.lib.packages.NativeProvider; import com.google.devtools.build.lib.packages.OutputFile; import com.google.devtools.build.lib.packages.PackageSpecification; import com.google.devtools.build.lib.packages.RawAttributeMapper; -import com.google.devtools.build.lib.packages.RequiredProviders; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.packages.RuleErrorConsumer; +import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.rules.AliasProvider; import com.google.devtools.build.lib.rules.MakeVariableProvider; import com.google.devtools.build.lib.rules.fileset.FilesetProvider; import com.google.devtools.build.lib.shell.ShellUtils; +import com.google.devtools.build.lib.syntax.ClassObject; 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; @@ -1974,101 +1975,154 @@ public final class RuleContext extends TargetContext } } - /** - * Because some rules still have to use allowedRuleClasses to do rule dependency validation. A - * dependency is valid if it is from a rule in allowedRuledClasses, OR if all of the providers - * in requiredProviders are provided by the target. - */ - private void validateRuleDependency(ConfiguredTarget prerequisite, Attribute attribute) { - - Set<String> unfulfilledRequirements = new LinkedHashSet<>(); - if (checkRuleDependencyClass(prerequisite, attribute, unfulfilledRequirements)) { - return; + private String getMissingMandatoryProviders(ConfiguredTarget prerequisite, Attribute attribute){ + ImmutableList<ImmutableSet<SkylarkProviderIdentifier>> mandatoryProvidersList = + attribute.getMandatoryProvidersList(); + if (mandatoryProvidersList.isEmpty()) { + return null; } - - if (checkRuleDependencyClassWarnings(prerequisite, attribute)) { - return; + List<List<String>> missingProvidersList = new ArrayList<>(); + for (ImmutableSet<SkylarkProviderIdentifier> providers : mandatoryProvidersList) { + List<String> missing = new ArrayList<>(); + for (SkylarkProviderIdentifier provider : providers) { + // A rule may require a built-in provider that is always implicitly provided, e.g. "files" + ImmutableSet<String> availableKeys = + ImmutableSet.copyOf(((ClassObject) prerequisite).getKeys()); + if ((prerequisite.get(provider) == null) + && !(provider.isLegacy() && availableKeys.contains(provider.getLegacyId()))) { + missing.add(provider.toString()); + } + } + if (missing.isEmpty()) { + return null; + } else { + missingProvidersList.add(missing); + } } - - if (checkRuleDependencyMandatoryProviders(prerequisite, attribute, unfulfilledRequirements)) { - return; + StringBuilder missingProviders = new StringBuilder(); + Joiner joinProvider = Joiner.on("', '"); + for (List<String> providers : missingProvidersList) { + if (missingProviders.length() > 0) { + missingProviders.append(" or "); + } + missingProviders.append((providers.size() > 1) ? "[" : "") + .append("'"); + joinProvider.appendTo(missingProviders, providers); + missingProviders.append("'") + .append((providers.size() > 1) ? "]" : ""); } + return missingProviders.toString(); + } - // not allowed rule class and some mandatory providers missing => reject. - if (!unfulfilledRequirements.isEmpty()) { - attributeError( - attribute.getName(), StringUtil.joinEnglishList(unfulfilledRequirements, "and")); + private String getMissingMandatoryNativeProviders( + ConfiguredTarget prerequisite, Attribute attribute) { + List<ImmutableList<Class<? extends TransitiveInfoProvider>>> mandatoryProvidersList = + attribute.getMandatoryNativeProvidersList(); + if (mandatoryProvidersList.isEmpty()) { + return null; + } + List<List<String>> missingProvidersList = new ArrayList<>(); + for (ImmutableList<Class<? extends TransitiveInfoProvider>> providers + : mandatoryProvidersList) { + List<String> missing = new ArrayList<>(); + for (Class<? extends TransitiveInfoProvider> provider : providers) { + if (prerequisite.getProvider(provider) == null) { + missing.add(provider.getSimpleName()); + } + } + if (missing.isEmpty()) { + return null; + } else { + missingProvidersList.add(missing); + } + } + StringBuilder missingProviders = new StringBuilder(); + Joiner joinProvider = Joiner.on(", "); + for (List<String> providers : missingProvidersList) { + if (missingProviders.length() > 0) { + missingProviders.append(" or "); + } + missingProviders.append((providers.size() > 1) ? "[" : ""); + joinProvider.appendTo(missingProviders, providers); + missingProviders.append((providers.size() > 1) ? "]" : ""); } + return missingProviders.toString(); } - /** Check if prerequisite should be allowed based on its rule class. */ - private boolean checkRuleDependencyClass( - ConfiguredTarget prerequisite, Attribute attribute, Set<String> unfulfilledRequirements) { + /** + * Because some rules still have to use allowedRuleClasses to do rule dependency validation. + * A dependency is valid if it is from a rule in allowedRuleClasses, OR if all of the providers + * in mandatoryNativeProviders AND mandatoryProvidersList are provided by the target. + */ + private void validateRuleDependency(ConfiguredTarget prerequisite, Attribute attribute) { + Target prerequisiteTarget = prerequisite.getTarget(); + RuleClass ruleClass = ((Rule) prerequisiteTarget).getRuleClassObject(); + String notAllowedRuleClassesMessage = null; + if (attribute.getAllowedRuleClassesPredicate() != Predicates.<RuleClass>alwaysTrue()) { - if (attribute - .getAllowedRuleClassesPredicate() - .apply(((Rule) prerequisite.getTarget()).getRuleClassObject())) { + if (attribute.getAllowedRuleClassesPredicate().apply(ruleClass)) { // prerequisite has an allowed rule class => accept. - return true; + return; } // remember that the rule class that was not allowed; // but maybe prerequisite provides required providers? do not reject yet. - unfulfilledRequirements.add( + notAllowedRuleClassesMessage = badPrerequisiteMessage( - prerequisite.getTarget().getTargetKind(), + prerequisiteTarget.getTargetKind(), prerequisite, "expected " + attribute.getAllowedRuleClassesPredicate(), - false)); + false); } - return false; - } - /** - * Check if prerequisite should be allowed with warning based on its rule class. - * - * <p>If yes, also issues said warning. - */ - private boolean checkRuleDependencyClassWarnings( - ConfiguredTarget prerequisite, Attribute attribute) { - if (attribute - .getAllowedRuleClassesWarningPredicate() - .apply(((Rule) prerequisite.getTarget()).getRuleClassObject())) { + if (attribute.getAllowedRuleClassesWarningPredicate().apply(ruleClass)) { Predicate<RuleClass> allowedRuleClasses = attribute.getAllowedRuleClassesPredicate(); - reportBadPrerequisite( - attribute, - prerequisite.getTarget().getTargetKind(), - prerequisite, + reportBadPrerequisite(attribute, prerequisiteTarget.getTargetKind(), prerequisite, allowedRuleClasses == Predicates.<RuleClass>alwaysTrue() ? null : "expected " + allowedRuleClasses, true); // prerequisite has a rule class allowed with a warning => accept, emitting a warning. - return true; + return; } - return false; - } - /** Check if prerequisite should be allowed based on required providers on the attribute. */ - private boolean checkRuleDependencyMandatoryProviders( - ConfiguredTarget prerequisite, Attribute attribute, Set<String> unfulfilledRequirements) { - RequiredProviders requiredProviders = attribute.getRequiredProviders(); + // If we get here, we have no allowed rule class. + // If mandatory providers are specified, try them. + Set<String> unfulfilledRequirements = new LinkedHashSet<>(); + if (!attribute.getMandatoryNativeProvidersList().isEmpty() + || !attribute.getMandatoryProvidersList().isEmpty()) { + boolean hadAllMandatoryProviders = true; + + String missingNativeProviders = getMissingMandatoryNativeProviders(prerequisite, attribute); + if (missingNativeProviders != null) { + unfulfilledRequirements.add( + "'" + prerequisite.getLabel() + "' does not have mandatory providers: " + + missingNativeProviders); + hadAllMandatoryProviders = false; + } - if (requiredProviders.acceptsAny()) { - // If no required providers specified, we do not know if we should accept. - return false; - } + String missingProviders = getMissingMandatoryProviders(prerequisite, attribute); + if (missingProviders != null) { + unfulfilledRequirements.add( + "'" + prerequisite.getLabel() + "' does not have mandatory providers: " + + missingProviders); + hadAllMandatoryProviders = false; + } - if (prerequisite.satisfies(requiredProviders)) { - return true; + if (hadAllMandatoryProviders) { + // all mandatory providers are present => accept. + return; + } } - unfulfilledRequirements.add( - String.format( - "'%s' does not have mandatory providers: %s", - prerequisite.getLabel(), - prerequisite.missingProviders(requiredProviders).getDescription())); + // not allowed rule class and some mandatory providers missing => reject. + if (notAllowedRuleClassesMessage != null) { + unfulfilledRequirements.add(notAllowedRuleClassesMessage); + } - return false; + if (!unfulfilledRequirements.isEmpty()) { + attributeError( + attribute.getName(), StringUtil.joinEnglishList(unfulfilledRequirements, "and")); + } } private void validateDirectPrerequisite(Attribute attribute, ConfiguredTarget prerequisite) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TransitiveInfoCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/TransitiveInfoCollection.java index b3b4894e2f..b174a47699 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/TransitiveInfoCollection.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/TransitiveInfoCollection.java @@ -16,7 +16,6 @@ package com.google.devtools.build.lib.analysis; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.packages.RequiredProviders; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; import com.google.devtools.build.lib.syntax.SkylarkIndexable; @@ -80,25 +79,4 @@ public interface TransitiveInfoCollection extends SkylarkIndexable, SkylarkProvi * <b>null</b>.</p> */ @Nullable BuildConfiguration getConfiguration(); - - /** - * Checks whether this {@link TransitiveInfoCollection} satisfies given {@link RequiredProviders}. - */ - default boolean satisfies(RequiredProviders providers) { - return providers.isSatisfiedBy( - aClass -> getProvider(aClass.asSubclass(TransitiveInfoProvider.class)) != null, - id -> this.get(id) != null); - } - - /** - * Returns providers that this {@link TransitiveInfoCollection} misses from a given {@link - * RequiredProviders}. - * - * <p>If none are missing, returns {@link RequiredProviders} that accept any set of providers. - */ - default RequiredProviders missingProviders(RequiredProviders providers) { - return providers.getMissing( - aClass -> getProvider(aClass.asSubclass(TransitiveInfoProvider.class)) != null, - id -> this.get(id) != null); - } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java index f1d7dff981..bb2456c08e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java @@ -14,14 +14,16 @@ package com.google.devtools.build.lib.bazel.rules; +import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.PackageSpecificationProvider; import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper; import com.google.devtools.build.lib.packages.PackageGroup; import com.google.devtools.build.lib.packages.RawAttributeMapper; -import com.google.devtools.build.lib.packages.RequiredProviders; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.rules.AliasProvider; @@ -75,10 +77,15 @@ public class BazelPrerequisiteValidator } if (prerequisiteTarget instanceof PackageGroup) { - RequiredProviders requiredProviders = - RawAttributeMapper.of(rule).getAttributeDefinition(attrName).getRequiredProviders(); + ImmutableList<ImmutableList<Class<? extends TransitiveInfoProvider>>> + mandatoryNativeProviders = + RawAttributeMapper.of(rule) + .getAttributeDefinition(attrName) + .getMandatoryNativeProvidersList(); boolean containsPackageSpecificationProvider = - requiredProviders.getDescription().contains("PackageSpecificationProvider"); + mandatoryNativeProviders + .stream() + .anyMatch(list -> list.contains(PackageSpecificationProvider.class)); // TODO(plf): Add the PackageSpecificationProvider to the 'visibility' attribute. if (!attrName.equals("visibility") && !containsPackageSpecificationProvider) { context.reportError( 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 bd3605627f..bcf63410d6 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 @@ -457,8 +457,10 @@ public final class Attribute implements Comparable<Attribute> { private Predicate<AttributeMap> condition; private Set<PropertyFlag> propertyFlags = EnumSet.noneOf(PropertyFlag.class); private PredicateWithMessage<Object> allowedValues = null; - private RequiredProviders.Builder requiredProvidersBuilder = - RequiredProviders.acceptAnyBuilder(); + private ImmutableList<ImmutableSet<SkylarkProviderIdentifier>> mandatoryProvidersList = + ImmutableList.<ImmutableSet<SkylarkProviderIdentifier>>of(); + private ImmutableList<ImmutableList<Class<? extends TransitiveInfoProvider>>> + mandatoryNativeProvidersList = ImmutableList.of(); private HashMap<String, RuleAspect<?>> aspects = new LinkedHashMap<>(); /** @@ -954,11 +956,11 @@ public final class Attribute implements Comparable<Attribute> { * #allowedRuleClasses}. * * <p>If the attribute contains Labels of any other rule type (other than this or those set in - * allowedRuleClasses()) and they fulfill {@link #getRequiredProviders()}}, the build continues - * without error. Else the build fails during analysis. + * allowedRuleClasses()) and they fulfill {@link #getMandatoryNativeProvidersList()}}, the build + * continues without error. Else the build fails during analysis. * - * <p>If neither this nor {@link #allowedRuleClassesForLabels} is set, only rules that fulfill - * {@link #getRequiredProviders()} build without error. + * <p>If neither this nor {@link #allowedRuleClassesForLabels} is set, only rules that + * fulfill {@link #getMandatoryNativeProvidersList()} build without error. * * <p>This only works on a per-target basis, not on a per-file basis; with other words, it works * for 'deps' attributes, but not 'srcs' attributes. @@ -976,10 +978,12 @@ public final class Attribute implements Comparable<Attribute> { Iterable<? extends Iterable<Class<? extends TransitiveInfoProvider>>> providersList) { Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY, "must be a label-valued type"); - + ImmutableList.Builder<ImmutableList<Class<? extends TransitiveInfoProvider>>> listBuilder + = ImmutableList.builder(); for (Iterable<Class<? extends TransitiveInfoProvider>> providers : providersList) { - this.requiredProvidersBuilder.addNativeSet(ImmutableSet.copyOf(providers)); + listBuilder.add(ImmutableList.<Class<? extends TransitiveInfoProvider>>copyOf(providers)); } + this.mandatoryNativeProvidersList = listBuilder.build(); return this; } @@ -1001,9 +1005,12 @@ public final class Attribute implements Comparable<Attribute> { Iterable<? extends Iterable<SkylarkProviderIdentifier>> providersList){ Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY, "must be a label-valued type"); + ImmutableList.Builder<ImmutableSet<SkylarkProviderIdentifier>> listBuilder + = ImmutableList.builder(); for (Iterable<SkylarkProviderIdentifier> providers : providersList) { - this.requiredProvidersBuilder.addSkylarkSet(ImmutableSet.copyOf(providers)); + listBuilder.add(ImmutableSet.copyOf(providers)); } + this.mandatoryProvidersList = listBuilder.build(); return this; } @@ -1174,7 +1181,8 @@ public final class Attribute implements Comparable<Attribute> { validityPredicate, condition, allowedValues, - requiredProvidersBuilder.build(), + mandatoryProvidersList, + mandatoryNativeProvidersList, ImmutableList.copyOf(aspects.values())); } } @@ -1787,7 +1795,10 @@ public final class Attribute implements Comparable<Attribute> { private final PredicateWithMessage<Object> allowedValues; - private final RequiredProviders requiredProviders; + private final ImmutableList<ImmutableSet<SkylarkProviderIdentifier>> mandatoryProvidersList; + + private final ImmutableList<ImmutableList<Class<? extends TransitiveInfoProvider>>> + mandatoryNativeProvidersList; private final ImmutableList<RuleAspect<?>> aspects; @@ -1817,7 +1828,9 @@ public final class Attribute implements Comparable<Attribute> { ValidityPredicate validityPredicate, Predicate<AttributeMap> condition, PredicateWithMessage<Object> allowedValues, - RequiredProviders requiredProviders, + ImmutableList<ImmutableSet<SkylarkProviderIdentifier>> mandatoryProvidersList, + ImmutableList<ImmutableList<Class<? extends TransitiveInfoProvider>>> + mandatoryNativeProvidersList, ImmutableList<RuleAspect<?>> aspects) { Preconditions.checkNotNull(configTransition); Preconditions.checkArgument( @@ -1851,7 +1864,8 @@ public final class Attribute implements Comparable<Attribute> { this.validityPredicate = validityPredicate; this.condition = condition; this.allowedValues = allowedValues; - this.requiredProviders = requiredProviders; + this.mandatoryProvidersList = mandatoryProvidersList; + this.mandatoryNativeProvidersList = mandatoryNativeProvidersList; this.aspects = aspects; } @@ -2050,8 +2064,17 @@ public final class Attribute implements Comparable<Attribute> { return allowedRuleClassesForLabelsWarning; } - public RequiredProviders getRequiredProviders() { - return requiredProviders; + /** + * Returns the list of sets of mandatory Skylark providers. + */ + public ImmutableList<ImmutableSet<SkylarkProviderIdentifier>> getMandatoryProvidersList() { + return mandatoryProvidersList; + } + + /** Returns the list of lists of mandatory native providers. */ + public ImmutableList<ImmutableList<Class<? extends TransitiveInfoProvider>>> + getMandatoryNativeProvidersList() { + return mandatoryNativeProvidersList; } public FileTypeSet getAllowedFileTypesPredicate() { @@ -2210,7 +2233,8 @@ public final class Attribute implements Comparable<Attribute> { builder.allowedFileTypesForLabels = allowedFileTypesForLabels; builder.allowedRuleClassesForLabels = allowedRuleClassesForLabels; builder.allowedRuleClassesForLabelsWarning = allowedRuleClassesForLabelsWarning; - builder.requiredProvidersBuilder = requiredProviders.copyAsBuilder(); + builder.mandatoryNativeProvidersList = mandatoryNativeProvidersList; + builder.mandatoryProvidersList = mandatoryProvidersList; builder.validityPredicate = validityPredicate; builder.condition = condition; builder.configTransition = configTransition; diff --git a/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java b/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java index 5b494b7d07..23dc9b887d 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java @@ -13,14 +13,13 @@ // limitations under the License. package com.google.devtools.build.lib.packages; -import com.google.common.base.Joiner; +import com.google.common.base.Predicate; +import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Preconditions; -import java.util.function.Function; -import java.util.function.Predicate; -import javax.annotation.Nullable; /** * Represents a constraint on a set of providers required by a dependency (of a rule @@ -53,10 +52,6 @@ public final class RequiredProviders { */ private final ImmutableList<ImmutableSet<SkylarkProviderIdentifier>> skylarkProviders; - public String getDescription() { - return constraint.getDescription(this); - } - /** * Represents one of the constraints as desctibed in {@link RequiredProviders} */ @@ -64,10 +59,8 @@ public final class RequiredProviders { /** Accept any dependency */ ANY { @Override - public boolean satisfies( - AdvertisedProviderSet advertisedProviderSet, - RequiredProviders requiredProviders, - Builder missing) { + public boolean satisfies(AdvertisedProviderSet advertisedProviderSet, + RequiredProviders requiredProviders) { return true; } @@ -75,28 +68,15 @@ public final class RequiredProviders { public boolean satisfies( Predicate<Class<?>> hasNativeProvider, Predicate<SkylarkProviderIdentifier> hasSkylarkProvider, - RequiredProviders requiredProviders, - Builder missingProviders) { + RequiredProviders requiredProviders) { return true; } - - @Override - Builder copyAsBuilder(RequiredProviders providers) { - return acceptAnyBuilder(); - } - - @Override - public String getDescription(RequiredProviders providers) { - return "no providers required"; - } }, /** Accept no dependency */ NONE { @Override - public boolean satisfies( - AdvertisedProviderSet advertisedProviderSet, - RequiredProviders requiredProviders, - Builder missing) { + public boolean satisfies(AdvertisedProviderSet advertisedProviderSet, + RequiredProviders requiredProviders) { return false; } @@ -104,124 +84,56 @@ public final class RequiredProviders { public boolean satisfies( Predicate<Class<?>> hasNativeProvider, Predicate<SkylarkProviderIdentifier> hasSkylarkProvider, - RequiredProviders requiredProviders, - Builder missingProviders) { + RequiredProviders requiredProviders) { return false; } - - @Override - Builder copyAsBuilder(RequiredProviders providers) { - return acceptNoneBuilder(); - } - - @Override - public String getDescription(RequiredProviders providers) { - return "no providers accepted"; - } }, - /** Accept a dependency that has all providers from one of the sets. */ RESTRICTED { @Override - public boolean satisfies( - final AdvertisedProviderSet advertisedProviderSet, - RequiredProviders requiredProviders, - Builder missing) { - if (advertisedProviderSet.canHaveAnyProvider()) { - return true; - } - return satisfies( - advertisedProviderSet.getNativeProviders()::contains, - advertisedProviderSet.getSkylarkProviders()::contains, - requiredProviders, - missing); - } - - @Override - public boolean satisfies( - Predicate<Class<?>> hasNativeProvider, + public boolean satisfies(Predicate<Class<?>> hasNativeProvider, Predicate<SkylarkProviderIdentifier> hasSkylarkProvider, - RequiredProviders requiredProviders, - Builder missingProviders) { + RequiredProviders requiredProviders) { for (ImmutableSet<Class<?>> nativeProviderSet : requiredProviders.nativeProviders) { - if (nativeProviderSet.stream().allMatch(hasNativeProvider)) { + if (Iterables.all(nativeProviderSet, hasNativeProvider)) { return true; } - - // Collect missing providers - if (missingProviders != null) { - missingProviders.addNativeSet( - nativeProviderSet - .stream() - .filter(hasNativeProvider.negate()) - .collect(ImmutableSet.toImmutableSet())); - } } for (ImmutableSet<SkylarkProviderIdentifier> skylarkProviderSet : requiredProviders.skylarkProviders) { - if (skylarkProviderSet.stream().allMatch(hasSkylarkProvider)) { + if (Iterables.all(skylarkProviderSet, hasSkylarkProvider)) { return true; } - // Collect missing providers - if (missingProviders != null) { - missingProviders.addSkylarkSet( - skylarkProviderSet - .stream() - .filter(hasSkylarkProvider.negate()) - .collect(ImmutableSet.toImmutableSet())); - } } return false; } - - @Override - Builder copyAsBuilder(RequiredProviders providers) { - Builder result = acceptAnyBuilder(); - for (ImmutableSet<Class<?>> nativeProviderSet : providers.nativeProviders) { - result.addNativeSet(nativeProviderSet); - } - for (ImmutableSet<SkylarkProviderIdentifier> skylarkProviderSet : - providers.skylarkProviders) { - result.addSkylarkSet(skylarkProviderSet); - } - return result; - } - - @Override - public String getDescription(RequiredProviders providers) { - StringBuilder result = new StringBuilder(); - describe(result, providers.nativeProviders, Class::getSimpleName); - describe(result, providers.skylarkProviders, id -> "'" + id.toString() + "'"); - return result.toString(); - } }; /** Checks if {@code advertisedProviderSet} satisfies these {@code RequiredProviders} */ - public abstract boolean satisfies( - AdvertisedProviderSet advertisedProviderSet, - RequiredProviders requiredProviders, - Builder missing); + public boolean satisfies(final AdvertisedProviderSet advertisedProviderSet, + RequiredProviders requiredProviders) { + if (advertisedProviderSet.canHaveAnyProvider()) { + return true; + } + return satisfies( + aClass -> advertisedProviderSet.getNativeProviders().contains(aClass), + Predicates.in(advertisedProviderSet.getSkylarkProviders()), + requiredProviders); + } /** * Checks if a set of providers encoded by predicates {@code hasNativeProviders} and {@code * hasSkylarkProvider} satisfies these {@code RequiredProviders} */ - abstract boolean satisfies( - Predicate<Class<?>> hasNativeProvider, + abstract boolean satisfies(Predicate<Class<?>> hasNativeProvider, Predicate<SkylarkProviderIdentifier> hasSkylarkProvider, - RequiredProviders requiredProviders, - @Nullable Builder missingProviders); - - abstract Builder copyAsBuilder(RequiredProviders providers); - - /** Returns a string describing the providers that can be presented to the user. */ - abstract String getDescription(RequiredProviders providers); + RequiredProviders requiredProviders); } /** Checks if {@code advertisedProviderSet} satisfies this {@code RequiredProviders} instance. */ public boolean isSatisfiedBy(AdvertisedProviderSet advertisedProviderSet) { - return constraint.satisfies(advertisedProviderSet, this, null); + return constraint.satisfies(advertisedProviderSet, this); } /** @@ -231,40 +143,7 @@ public final class RequiredProviders { public boolean isSatisfiedBy( Predicate<Class<?>> hasNativeProvider, Predicate<SkylarkProviderIdentifier> hasSkylarkProvider) { - return constraint.satisfies(hasNativeProvider, hasSkylarkProvider, this, null); - } - - /** - * Returns providers that are missing. If none are missing, returns {@code RequiredProviders} that - * accept anything. - */ - public RequiredProviders getMissing( - Predicate<Class<?>> hasNativeProvider, - Predicate<SkylarkProviderIdentifier> hasSkylarkProvider) { - Builder builder = acceptAnyBuilder(); - if (constraint.satisfies(hasNativeProvider, hasSkylarkProvider, this, builder)) { - // Ignore all collected missing providers. - return acceptAnyBuilder().build(); - } - return builder.build(); - } - - /** - * Returns providers that are missing. If none are missing, returns {@code RequiredProviders} that - * accept anything. - */ - public RequiredProviders getMissing(AdvertisedProviderSet set) { - Builder builder = acceptAnyBuilder(); - if (constraint.satisfies(set, this, builder)) { - // Ignore all collected missing providers. - return acceptAnyBuilder().build(); - } - return builder.build(); - } - - /** Returns true if this {@code RequiredProviders} instance accept any set of providers. */ - public boolean acceptsAny() { - return constraint.equals(Constraint.ANY); + return constraint.satisfies(hasNativeProvider, hasSkylarkProvider, this); } @@ -282,22 +161,6 @@ public final class RequiredProviders { this.skylarkProviders = skylarkProviders; } - /** Helper method to describe lists of sets of things. */ - private static <T> void describe( - StringBuilder result, - ImmutableList<ImmutableSet<T>> listOfSets, - Function<T, String> describeOne) { - Joiner joiner = Joiner.on(", "); - for (ImmutableSet<T> ids : listOfSets) { - if (result.length() > 0) { - result.append(" or "); - } - result.append((ids.size() > 1) ? "[" : ""); - joiner.appendTo(result, ids.stream().map(describeOne).iterator()); - result.append((ids.size() > 1) ? "]" : ""); - } - } - /** * A builder for {@link RequiredProviders} that accepts any dependency * unless restriction provider sets are added. @@ -314,11 +177,6 @@ public final class RequiredProviders { return new Builder(true); } - /** Returns a Builder initialized to the same value as this {@code RequiredProvider} */ - public Builder copyAsBuilder() { - return constraint.copyAsBuilder(this); - } - /** A builder for {@link RequiredProviders} */ public static class Builder { private final ImmutableList.Builder<ImmutableSet<Class<?>>> nativeProviders; 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 1951610c21..d3a211034e 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 @@ -276,11 +276,7 @@ public final class SkylarkAttr implements SkylarkValue { SkylarkType.checkType(obj, SkylarkList.class, PROVIDERS_ARG); ImmutableList<ImmutableSet<SkylarkProviderIdentifier>> providersList = buildProviderPredicate( (SkylarkList<?>) obj, PROVIDERS_ARG, ast.getLocation()); - - // If there is at least one empty set, there is no restriction. - if (providersList.stream().noneMatch(ImmutableSet::isEmpty)) { - builder.mandatoryProvidersList(providersList); - } + builder.mandatoryProvidersList(providersList); } if (containsNonNoneKey(arguments, CONFIGURATION_ARG)) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java index 1b783b9926..aa960a87d4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; +import com.google.common.base.Predicate; import com.google.common.base.Supplier; import com.google.common.base.Verify; import com.google.common.base.VerifyException; @@ -42,6 +43,7 @@ import com.google.devtools.build.lib.analysis.MergedConfiguredTarget; import com.google.devtools.build.lib.analysis.MergedConfiguredTarget.DuplicateException; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; import com.google.devtools.build.lib.analysis.ToolchainContext; +import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; @@ -65,6 +67,7 @@ import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.RawAttributeMapper; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClassProvider; +import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.skyframe.AspectFunction.AspectCreationException; @@ -985,7 +988,20 @@ public final class ConfiguredTargetFunction implements SkyFunction { if (!aspect.getDefinition().applyToFiles() && !(dep.getTarget() instanceof Rule)) { return false; } - return dep.satisfies(aspect.getDefinition().getRequiredProviders()); + return aspect.getDefinition().getRequiredProviders().isSatisfiedBy( + new Predicate<Class<?>>() { + @Override + public boolean apply(Class<?> provider) { + return dep.getProvider(provider.asSubclass(TransitiveInfoProvider.class)) != null; + } + }, + new Predicate<SkylarkProviderIdentifier>() { + @Override + public boolean apply(SkylarkProviderIdentifier skylarkProviderIdentifier) { + return dep.get(skylarkProviderIdentifier) != null; + } + } + ); } /** diff --git a/src/test/java/com/google/devtools/build/lib/packages/RequiredProvidersTest.java b/src/test/java/com/google/devtools/build/lib/packages/RequiredProvidersTest.java index ca8a9960d7..e86cc41f16 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RequiredProvidersTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RequiredProvidersTest.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.packages; import static com.google.common.truth.Truth.assertThat; +import com.google.common.base.Predicates; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; @@ -30,9 +31,6 @@ import org.junit.runners.JUnit4; */ @RunWith(JUnit4.class) public class RequiredProvidersTest { - - private static final String NO_PROVIDERS_REQUIRED = "no providers required"; - private static final class P1 {} private static final class P2 {} private static final class P3 {} @@ -61,11 +59,10 @@ public class RequiredProvidersTest { RequiredProviders requiredProviders) { boolean result = requiredProviders.isSatisfiedBy(providers); - assertThat( - requiredProviders.isSatisfiedBy( - providers.getNativeProviders()::contains, - providers.getSkylarkProviders()::contains)) - .isEqualTo(result); + assertThat(requiredProviders.isSatisfiedBy( + Predicates.in(providers.getNativeProviders()), + Predicates.in(providers.getSkylarkProviders()) + )).isEqualTo(result); return result; } @@ -111,34 +108,32 @@ public class RequiredProvidersTest { .addNative(P1.class) .addNative(P2.class) .build(); - assertThat( - validateNative( - providerSet, - NO_PROVIDERS_REQUIRED, - ImmutableSet.of(P1.class, P2.class))) + assertThat(validateNative(providerSet, ImmutableSet.<Class<?>>of(P1.class, P2.class))) .isTrue(); } @Test public void nativeProvidersBranchMatch() { assertThat( - validateNative( - AdvertisedProviderSet.builder().addNative(P1.class).build(), - NO_PROVIDERS_REQUIRED, - ImmutableSet.<Class<?>>of(P1.class), - ImmutableSet.<Class<?>>of(P2.class))) - .isTrue(); + validateNative( + AdvertisedProviderSet.builder() + .addNative(P1.class) + .build(), + ImmutableSet.<Class<?>>of(P1.class), + ImmutableSet.<Class<?>>of(P2.class) + )).isTrue(); } @Test public void nativeProvidersNoMatch() { assertThat( - validateNative( - AdvertisedProviderSet.builder().addNative(P3.class).build(), - "P1 or P2", - ImmutableSet.<Class<?>>of(P1.class), - ImmutableSet.<Class<?>>of(P2.class))) - .isFalse(); + validateNative( + AdvertisedProviderSet.builder() + .addNative(P3.class) + .build(), + ImmutableSet.<Class<?>>of(P1.class), + ImmutableSet.<Class<?>>of(P2.class) + )).isFalse(); } @Test @@ -149,7 +144,6 @@ public class RequiredProvidersTest { .addSkylark(ID_SKYLARK) .build(); assertThat(validateSkylark(providerSet, - NO_PROVIDERS_REQUIRED, ImmutableSet.of( ID_LEGACY, ID_SKYLARK, ID_NATIVE))) .isTrue(); @@ -158,64 +152,44 @@ public class RequiredProvidersTest { @Test public void skylarkProvidersBranchMatch() { assertThat( - validateSkylark( - AdvertisedProviderSet.builder().addSkylark(ID_LEGACY).build(), - NO_PROVIDERS_REQUIRED, - ImmutableSet.of(ID_LEGACY), - ImmutableSet.of(ID_NATIVE))) - .isTrue(); + validateSkylark( + AdvertisedProviderSet.builder() + .addSkylark(ID_LEGACY) + .build(), + ImmutableSet.of(ID_LEGACY), + ImmutableSet.of(ID_NATIVE) + )).isTrue(); } @Test public void skylarkProvidersNoMatch() { assertThat( - validateSkylark( - AdvertisedProviderSet.builder().addSkylark(ID_SKYLARK).build(), - "'p_legacy' or 'p_native'", - ImmutableSet.of(ID_LEGACY), - ImmutableSet.of(ID_NATIVE))) - .isFalse(); - } - - @Test - public void checkDescriptions() { - assertThat(RequiredProviders.acceptAnyBuilder().build().getDescription()) - .isEqualTo("no providers required"); - assertThat(RequiredProviders.acceptNoneBuilder().build().getDescription()) - .isEqualTo("no providers accepted"); - assertThat( - RequiredProviders.acceptAnyBuilder() - .addSkylarkSet(ImmutableSet.of(ID_LEGACY, ID_SKYLARK)) - .addSkylarkSet(ImmutableSet.of(ID_SKYLARK)) - .addNativeSet(ImmutableSet.of(P1.class, P2.class)) - .build() - .getDescription()) - .isEqualTo("[P1, P2] or ['p_legacy', 'p_skylark'] or 'p_skylark'"); + validateSkylark( + AdvertisedProviderSet.builder() + .addSkylark(ID_SKYLARK) + .build(), + ImmutableSet.of(ID_LEGACY), + ImmutableSet.of(ID_NATIVE) + )).isFalse(); } @SafeVarargs - private static boolean validateNative( - AdvertisedProviderSet providerSet, String missing, ImmutableSet<Class<?>>... sets) { + private static boolean validateNative(AdvertisedProviderSet providerSet, + ImmutableSet<Class<?>>... sets) { Builder anyBuilder = RequiredProviders.acceptAnyBuilder(); Builder noneBuilder = RequiredProviders.acceptNoneBuilder(); for (ImmutableSet<Class<?>> set : sets) { anyBuilder.addNativeSet(set); noneBuilder.addNativeSet(set); } - RequiredProviders rpStartingFromAny = anyBuilder.build(); - boolean result = satisfies(providerSet, rpStartingFromAny); - assertThat(rpStartingFromAny.getMissing(providerSet).getDescription()).isEqualTo(missing); - - RequiredProviders rpStaringFromNone = noneBuilder.build(); - assertThat(satisfies(providerSet, rpStaringFromNone)).isEqualTo(result); - assertThat(rpStaringFromNone.getMissing(providerSet).getDescription()).isEqualTo(missing); + boolean result = satisfies(providerSet, anyBuilder.build()); + assertThat(satisfies(providerSet, noneBuilder.build())).isEqualTo(result); return result; } @SafeVarargs private static boolean validateSkylark( AdvertisedProviderSet providerSet, - String missing, ImmutableSet<SkylarkProviderIdentifier>... sets) { Builder anyBuilder = RequiredProviders.acceptAnyBuilder(); Builder noneBuilder = RequiredProviders.acceptNoneBuilder(); @@ -223,14 +197,8 @@ public class RequiredProvidersTest { anyBuilder.addSkylarkSet(set); noneBuilder.addSkylarkSet(set); } - - RequiredProviders rpStartingFromAny = anyBuilder.build(); - boolean result = satisfies(providerSet, rpStartingFromAny); - assertThat(rpStartingFromAny.getMissing(providerSet).getDescription()).isEqualTo(missing); - - RequiredProviders rpStaringFromNone = noneBuilder.build(); - assertThat(satisfies(providerSet, rpStaringFromNone)).isEqualTo(result); - assertThat(rpStaringFromNone.getMissing(providerSet).getDescription()).isEqualTo(missing); + boolean result = satisfies(providerSet, anyBuilder.build()); + assertThat(satisfies(providerSet, noneBuilder.build())).isEqualTo(result); return result; } } diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java index 71e084da10..b346a60513 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java @@ -711,7 +711,7 @@ public class SkylarkIntegrationTest extends BuildViewTestCase { "main_rule = rule(implementation = rule_impl, attrs = {", " 'deps': attr.label_list(providers = [", " 'files', 'data_runfiles', 'default_runfiles',", - " 'files_to_run', 'output_groups',", + " 'files_to_run', 'label', 'output_groups',", " ])", "})"); scratch.file( diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java index c212605bc2..5c32321734 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java @@ -249,18 +249,8 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase { buildAttribute("a1", "b = provider()", "attr.label_list(allow_files = True, providers = ['a', b])"); - assertThat(attr.getRequiredProviders().isSatisfiedBy(set(legacy("a"), declared("b")))).isTrue(); - assertThat(attr.getRequiredProviders().isSatisfiedBy(set(legacy("a")))).isFalse(); - } - - @Test - public void testAttrWithProvidersOneEmpty() throws Exception { - Attribute attr = - buildAttribute( - "a1", - "b = provider()", - "attr.label_list(allow_files = True, providers = [['a', b],[]])"); - assertThat(attr.getRequiredProviders().acceptsAny()).isTrue(); + assertThat(attr.getMandatoryProvidersList()) + .containsExactly(ImmutableSet.of(legacy("a"), declared("b"))); } @Test @@ -269,17 +259,9 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase { buildAttribute("a1", "b = provider()", "attr.label_list(allow_files = True, providers = [['a', b], ['c']])"); - assertThat(attr.getRequiredProviders().isSatisfiedBy(set(legacy("a"), declared("b")))).isTrue(); - assertThat(attr.getRequiredProviders().isSatisfiedBy(set(legacy("c")))).isTrue(); - assertThat(attr.getRequiredProviders().isSatisfiedBy(set(legacy("a")))).isFalse(); - } - - private static AdvertisedProviderSet set(SkylarkProviderIdentifier... ids) { - AdvertisedProviderSet.Builder builder = AdvertisedProviderSet.builder(); - for (SkylarkProviderIdentifier id : ids) { - builder.addSkylark(id); - } - return builder.build(); + assertThat(attr.getMandatoryProvidersList()).containsExactly( + ImmutableSet.of(legacy("a"), declared("b")), + ImmutableSet.of(legacy("c"))); } private void checkAttributeError(String expectedMessage, String... lines) throws Exception { @@ -443,9 +425,12 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase { private static final RuleClass.ConfiguredTargetFactory<Object, Object> DUMMY_CONFIGURED_TARGET_FACTORY = - ruleContext -> { - throw new IllegalStateException(); - }; + new RuleClass.ConfiguredTargetFactory<Object, Object>() { + @Override + public Object create(Object ruleContext) throws InterruptedException { + throw new IllegalStateException(); + } + }; private RuleClass ruleClass(String name) { return new RuleClass.Builder(name, RuleClassType.NORMAL, false) |