diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
7 files changed, 207 insertions, 94 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java index 32469b5764..f024fb225e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.analysis.config.InvalidConfigurationExcepti import com.google.devtools.build.lib.analysis.config.PatchTransition; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.packages.AdvertisedProviderSet; import com.google.devtools.build.lib.packages.Aspect; import com.google.devtools.build.lib.packages.AspectClass; import com.google.devtools.build.lib.packages.AspectDescriptor; @@ -542,13 +543,14 @@ public abstract class DependencyResolver { ImmutableSet.Builder<AspectDescriptor> result = ImmutableSet.builder(); for (Aspect aspectCandidate : aspectCandidates) { - boolean requireAspect = ruleClass.canHaveAnyProvider(); + AdvertisedProviderSet advertisedProviders = ruleClass.getAdvertisedProviders(); + boolean requireAspect = advertisedProviders.canHaveAnyProvider(); if (!requireAspect) { ImmutableList<ImmutableSet<Class<?>>> providersList = aspectCandidate.getDefinition().getRequiredProviders(); for (ImmutableSet<Class<?>> providers : providersList) { - if (Sets.difference(providers, ruleClass.getAdvertisedProviders()).isEmpty()) { + if (Sets.difference(providers, advertisedProviders.getNativeProviders()).isEmpty()) { requireAspect = true; break; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java index a8350680bf..ef1428d2ac 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java @@ -235,7 +235,7 @@ public class BazelRuleClassProvider { RuleContext.Builder context, ConfiguredTarget prerequisite) { Rule rule = context.getRule(); - if (rule.getRuleClassObject().canHaveAnyProvider()) { + if (rule.getRuleClassObject().getAdvertisedProviders().canHaveAnyProvider()) { // testonly-ness will be checked directly between the depender and the target of the alias; // getTarget() called by the depender will not return the alias rule, but its actual target return; diff --git a/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java b/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java new file mode 100644 index 0000000000..78a963ea8f --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java @@ -0,0 +1,162 @@ +// Copyright 2016 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.packages; + +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.util.Preconditions; +import java.util.ArrayList; +import java.util.Objects; + +/** + * Captures the the set of providers rules and aspects can advertise. + * It is either of: + * <ul> + * <li>a set of native and skylark providers</li> + * <li>"can have any provider" set that alias rules have.</li> + * </ul> + * + * <p> + * Native providers should in theory only contain subclasses of + * {@link com.google.devtools.build.lib.analysis.TransitiveInfoProvider}, but + * our current dependency structure does not allow a reference to that class here. + * </p> + */ +// todo(dslomov,vladmos): support declared providers +@Immutable +public final class AdvertisedProviderSet { + private final boolean canHaveAnyProvider; + private final ImmutableSet<Class<?>> nativeProviders; + // todo(dslomov,vladmos): support declared providers + private final ImmutableSet<String> skylarkProviders; + + private AdvertisedProviderSet(boolean canHaveAnyProvider, + ImmutableSet<Class<?>> nativeProviders, + ImmutableSet<String> skylarkProviders) { + this.canHaveAnyProvider = canHaveAnyProvider; + this.nativeProviders = nativeProviders; + this.skylarkProviders = skylarkProviders; + } + + public static final AdvertisedProviderSet ANY = + new AdvertisedProviderSet(true, + ImmutableSet.<Class<?>>of(), + ImmutableSet.<String>of()); + public static final AdvertisedProviderSet EMPTY = + new AdvertisedProviderSet(false, + ImmutableSet.<Class<?>>of(), + ImmutableSet.<String>of()); + + public static AdvertisedProviderSet create( + ImmutableSet<Class<?>> nativeProviders, + ImmutableSet<String> skylarkProviders) { + if (nativeProviders.isEmpty() && skylarkProviders.isEmpty()) { + return EMPTY; + } + return new AdvertisedProviderSet(false, nativeProviders, skylarkProviders); + } + + @Override + public int hashCode() { + return Objects.hash(canHaveAnyProvider, nativeProviders, skylarkProviders); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + + if (!(obj instanceof AdvertisedProviderSet)) { + return false; + } + + AdvertisedProviderSet that = (AdvertisedProviderSet) obj; + return Objects.equals(this.canHaveAnyProvider, that.canHaveAnyProvider) + && Objects.equals(this.nativeProviders, that.nativeProviders) + && Objects.equals(this.skylarkProviders, that.skylarkProviders); + } + + /** Checks whether the rule can have any provider. + * + * Used for alias rules. + */ + public boolean canHaveAnyProvider() { + return canHaveAnyProvider; + } + + /** + * Get all advertised native providers. + */ + public ImmutableSet<Class<?>> getNativeProviders() { + return nativeProviders; + } + + /** + * Get all advertised Skylark providers. + */ + public ImmutableSet<String> getSkylarkProviders() { + return skylarkProviders; + } + + public static Builder builder() { + return new Builder(); + } + + /** Builder for {@link AdvertisedProviderSet} */ + public static class Builder { + private boolean canHaveAnyProvider; + private final ArrayList<Class<?>> nativeProviders; + private final ArrayList<String> skylarkProviders; + private Builder() { + nativeProviders = new ArrayList<>(); + skylarkProviders = new ArrayList<>(); + } + + /** + * Advertise all providers inherited from a parent rule. + */ + public Builder addParent(AdvertisedProviderSet parentSet) { + Preconditions.checkState(!canHaveAnyProvider, "Alias rules inherit from no other rules"); + Preconditions.checkState(!parentSet.canHaveAnyProvider(), + "Cannot inherit from alias rules"); + nativeProviders.addAll(parentSet.getNativeProviders()); + skylarkProviders.addAll(parentSet.getSkylarkProviders()); + return this; + } + + public Builder addNative(Class<?> nativeProvider) { + this.nativeProviders.add(nativeProvider); + return this; + } + + public void canHaveAnyProvider() { + Preconditions.checkState(nativeProviders.isEmpty() && skylarkProviders.isEmpty()); + this.canHaveAnyProvider = true; + } + + public AdvertisedProviderSet build() { + if (canHaveAnyProvider) { + Preconditions.checkState(nativeProviders.isEmpty() && skylarkProviders.isEmpty()); + return ANY; + } + return AdvertisedProviderSet.create( + ImmutableSet.copyOf(nativeProviders), ImmutableSet.copyOf(skylarkProviders)); + } + + public void addSkylark(String providerName) { + skylarkProviders.add(providerName); + } + } +} 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 24d28ae5b4..18cd2ebba6 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 @@ -160,27 +160,28 @@ public final class AspectDefinition { return ImmutableMultimap.of(); } RuleClass ruleClass = ((Rule) to).getRuleClassObject(); - ImmutableSet<Class<?>> providers = ruleClass.getAdvertisedProviders(); - return visitAspectsIfRequired((Rule) from, attribute, ruleClass.canHaveAnyProvider(), - toStringSet(providers), dependencyFilter); + AdvertisedProviderSet providers = ruleClass.getAdvertisedProviders(); + return visitAspectsIfRequired((Rule) from, attribute, + providers, dependencyFilter); } /** * Returns the attribute -> set of labels that are provided by aspects of attribute. */ public static ImmutableMultimap<Attribute, Label> visitAspectsIfRequired( - Rule from, Attribute attribute, boolean canHaveAnyProvider, Set<String> advertisedProviders, + Rule from, Attribute attribute, + AdvertisedProviderSet advertisedProviders, DependencyFilter dependencyFilter) { SetMultimap<Attribute, Label> result = LinkedHashMultimap.create(); for (Aspect candidateClass : attribute.getAspects(from)) { // Check if target satisfies condition for this aspect (has to provide all required // TransitiveInfoProviders) - if (!canHaveAnyProvider) { - ImmutableList<ImmutableSet<String>> providerNamesList = - candidateClass.getDefinition().getRequiredProviderNames(); + if (!advertisedProviders.canHaveAnyProvider()) { + ImmutableList<ImmutableSet<Class<?>>> providerNamesList = + candidateClass.getDefinition().getRequiredProviders(); - for (ImmutableSet<String> providerNames : providerNamesList) { - if (advertisedProviders.containsAll(providerNames)) { + for (ImmutableSet<Class<?>> providerNames : providerNamesList) { + if (advertisedProviders.getNativeProviders().containsAll(providerNames)) { addAllAttributesOfAspect(from, result, candidateClass, dependencyFilter); break; } diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java index bdddfec970..b6b314bbcf 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java @@ -55,7 +55,6 @@ import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.BitSet; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -478,8 +477,7 @@ public final class RuleClass { private PredicateWithMessage<Rule> validityPredicate = PredicatesWithMessage.<Rule>alwaysTrue(); private Predicate<String> preferredDependencyPredicate = Predicates.alwaysFalse(); - private List<Class<?>> advertisedProviders = new ArrayList<>(); - private boolean canHaveAnyProvider = false; + private AdvertisedProviderSet.Builder advertisedProviders = AdvertisedProviderSet.builder(); private BaseFunction configuredTargetFunction = null; private Function<? super Rule, Map<String, Label>> externalBindingsFunction = NO_EXTERNAL_BINDINGS; @@ -531,7 +529,7 @@ public final class RuleClass { attributes.put(attrName, attribute); } - advertisedProviders.addAll(parent.getAdvertisedProviders()); + advertisedProviders.addParent(parent.getAdvertisedProviders()); } // TODO(bazel-team): move this testonly attribute setting to somewhere else // preferably to some base RuleClass implementation. @@ -591,8 +589,7 @@ public final class RuleClass { configuredTargetFactory, validityPredicate, preferredDependencyPredicate, - ImmutableSet.copyOf(advertisedProviders), - canHaveAnyProvider, + advertisedProviders.build(), configuredTargetFunction, externalBindingsFunction, ruleDefinitionEnvironment, @@ -758,8 +755,9 @@ public final class RuleClass { * not be evaluated for the rule. */ public Builder advertiseProvider(Class<?>... providers) { - Preconditions.checkState(!canHaveAnyProvider); - Collections.addAll(advertisedProviders, providers); + for (Class<?> provider : providers) { + advertisedProviders.addNative(provider); + } return this; } @@ -768,8 +766,7 @@ public final class RuleClass { * <code>bind</code> . */ public Builder canHaveAnyProvider() { - Preconditions.checkState(advertisedProviders.isEmpty()); - canHaveAnyProvider = true; + advertisedProviders.canHaveAnyProvider(); return this; } @@ -1003,9 +1000,7 @@ public final class RuleClass { /** * The list of transitive info providers this class advertises to aspects. */ - private final ImmutableSet<Class<?>> advertisedProviders; - - private final boolean canHaveAnyProvider; + private final AdvertisedProviderSet advertisedProviders; /** * The Skylark rule implementation of this RuleClass. Null for non Skylark executable RuleClasses. @@ -1073,8 +1068,7 @@ public final class RuleClass { ConfiguredTargetFactory<?, ?> configuredTargetFactory, PredicateWithMessage<Rule> validityPredicate, Predicate<String> preferredDependencyPredicate, - ImmutableSet<Class<?>> advertisedProviders, - boolean canHaveAnyProvider, + AdvertisedProviderSet advertisedProviders, @Nullable BaseFunction configuredTargetFunction, Function<? super Rule, Map<String, Label>> externalBindingsFunction, @Nullable Environment ruleDefinitionEnvironment, @@ -1096,7 +1090,6 @@ public final class RuleClass { this.validityPredicate = validityPredicate; this.preferredDependencyPredicate = preferredDependencyPredicate; this.advertisedProviders = advertisedProviders; - this.canHaveAnyProvider = canHaveAnyProvider; this.configuredTargetFunction = configuredTargetFunction; this.externalBindingsFunction = externalBindingsFunction; this.ruleDefinitionEnvironment = ruleDefinitionEnvironment; @@ -1266,23 +1259,10 @@ public final class RuleClass { * * <p>This is here so that we can do the loading phase overestimation required for "blaze query", * which does not have the configured targets available. - * - * <p>This should in theory only contain subclasses of - * {@link com.google.devtools.build.lib.analysis.TransitiveInfoProvider}, but - * our current dependency structure does not allow a reference to that class here. - */ - public ImmutableSet<Class<?>> getAdvertisedProviders() { + **/ + public AdvertisedProviderSet getAdvertisedProviders() { return advertisedProviders; } - - /** - * Returns true if this rule, when analyzed, can provide any provider. Used for "alias" rules, - * e.g. <code>bind()</code>. - */ - public boolean canHaveAnyProvider() { - return canHaveAnyProvider; - } - /** * For --compile_one_dependency: if multiple rules consume the specified target, * should we choose this one over the "unpreferred" options? diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java index 0a85af09be..4f11095490 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java @@ -94,7 +94,7 @@ public class TransitiveTraversalFunction // Retrieve the providers of the dep from the TransitiveTraversalValue, so we can avoid // issuing a dep on its defining Package. return AspectDefinition.visitAspectsIfRequired(fromRule, attr, - traversalVal.canHaveAnyProvider(), traversalVal.getProviders(), + traversalVal.getProviders(), DependencyFilter.ALL_DEPS).values(); } catch (NoSuchThingException e) { // Do nothing. This error was handled when we computed the corresponding diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalValue.java index 24ed4bb6c7..d0e9d4a79d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalValue.java @@ -13,11 +13,10 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; -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.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; +import com.google.devtools.build.lib.packages.AdvertisedProviderSet; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.Target; @@ -25,15 +24,9 @@ import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.util.StringCanonicalizer; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; import java.util.Objects; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; - import javax.annotation.Nullable; /** @@ -47,7 +40,7 @@ import javax.annotation.Nullable; @ThreadSafe public class TransitiveTraversalValue implements SkyValue { private static final TransitiveTraversalValue EMPTY_VALUE = - new TransitiveTraversalValue(false, ImmutableSet.<String>of(), null); + new TransitiveTraversalValue(AdvertisedProviderSet.EMPTY, null); // A quick-lookup cache that allows us to get the value for a given RuleClass, assuming no error // messages for the target. Only stores built-in RuleClass objects to avoid memory bloat. private static final ConcurrentMap<RuleClass, TransitiveTraversalValue> VALUES_BY_RULE_CLASS = @@ -65,23 +58,20 @@ public class TransitiveTraversalValue implements SkyValue { VALUE_INTERNER.intern(EMPTY_VALUE); } - private final boolean canHaveAnyProvider; - private final ImmutableSet<String> providers; + private final AdvertisedProviderSet advertisedProviders; @Nullable private final String firstErrorMessage; private TransitiveTraversalValue( - boolean canHaveAnyProvider, - ImmutableSet<String> providers, + AdvertisedProviderSet providers, @Nullable String firstErrorMessage) { - this.canHaveAnyProvider = canHaveAnyProvider; - this.providers = Preconditions.checkNotNull(providers); + this.advertisedProviders = Preconditions.checkNotNull(providers); this.firstErrorMessage = (firstErrorMessage == null) ? null : StringCanonicalizer.intern(firstErrorMessage); } static TransitiveTraversalValue unsuccessfulTransitiveTraversal(String firstErrorMessage) { return new TransitiveTraversalValue( - false, ImmutableSet.<String>of(), Preconditions.checkNotNull(firstErrorMessage)); + AdvertisedProviderSet.EMPTY, Preconditions.checkNotNull(firstErrorMessage)); } static TransitiveTraversalValue forTarget(Target target, @Nullable String firstErrorMessage) { @@ -93,8 +83,8 @@ public class TransitiveTraversalValue implements SkyValue { if (value != null) { return value; } - ImmutableSet<String> providers = canonicalSet(toList(ruleClass.getAdvertisedProviders())); - value = new TransitiveTraversalValue(ruleClass.canHaveAnyProvider(), providers, null); + AdvertisedProviderSet providers = ruleClass.getAdvertisedProviders(); + value = new TransitiveTraversalValue(providers, null); // May already be there from another RuleClass or a concurrent put. value = VALUE_INTERNER.intern(value); // May already be there from a concurrent put. @@ -104,25 +94,23 @@ public class TransitiveTraversalValue implements SkyValue { // If this is a Skylark rule, we may still get a cache hit from another RuleClass with the // same providers. return TransitiveTraversalValue.create( - ruleClass.canHaveAnyProvider(), - toList(rule.getRuleClassObject().getAdvertisedProviders()), + rule.getRuleClassObject().getAdvertisedProviders(), firstErrorMessage); } } if (firstErrorMessage == null) { return EMPTY_VALUE; } else { - return new TransitiveTraversalValue(false, ImmutableSet.<String>of(), firstErrorMessage); + return new TransitiveTraversalValue(AdvertisedProviderSet.EMPTY, firstErrorMessage); } } public static TransitiveTraversalValue create( - boolean canHaveAnyProvider, - Collection<String> providers, + AdvertisedProviderSet providers, @Nullable String firstErrorMessage) { TransitiveTraversalValue value = new TransitiveTraversalValue( - canHaveAnyProvider, canonicalSet(providers), firstErrorMessage); + providers, firstErrorMessage); if (firstErrorMessage == null) { TransitiveTraversalValue oldValue = VALUE_INTERNER.getCanonical(value); return oldValue == null ? value : oldValue; @@ -130,38 +118,19 @@ public class TransitiveTraversalValue implements SkyValue { return value; } - private static ImmutableSet<String> canonicalSet(Iterable<String> strIterable) { - ImmutableSet.Builder<String> builder = new ImmutableSet.Builder<>(); - for (String str : strIterable) { - builder.add(StringCanonicalizer.intern(str)); - } - return builder.build(); - } - - private static List<String> toList(Collection<Class<?>> providers) { - if (providers == null) { - return ImmutableList.of(); - } - List<String> strings = new ArrayList<>(providers.size()); - for (Class<?> clazz : providers) { - strings.add(clazz.getName()); - } - return strings; - } - /** * Returns if the associated target can have any provider. True for "alias" rules. */ public boolean canHaveAnyProvider() { - return canHaveAnyProvider; + return advertisedProviders.canHaveAnyProvider(); } /** * Returns the set of provider names from the target, if the target is a {@link Rule}. Otherwise * returns the empty set. */ - public Set<String> getProviders() { - return providers; + public AdvertisedProviderSet getProviders() { + return advertisedProviders; } /** @@ -182,14 +151,13 @@ public class TransitiveTraversalValue implements SkyValue { return false; } TransitiveTraversalValue that = (TransitiveTraversalValue) o; - return this.canHaveAnyProvider == that.canHaveAnyProvider - && Objects.equals(this.firstErrorMessage, that.firstErrorMessage) - && this.providers.equals(that.providers); + return Objects.equals(this.firstErrorMessage, that.firstErrorMessage) + && this.advertisedProviders.equals(that.advertisedProviders); } @Override public int hashCode() { - return Objects.hash(firstErrorMessage, providers, canHaveAnyProvider); + return Objects.hash(firstErrorMessage, advertisedProviders); } @ThreadSafe |