Extract a set of advertised providers into a separate class.
--
PiperOrigin-RevId: 143991903
MOS_MIGRATED_REVID=143991903
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 32469b5..f024fb2 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.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 @@
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 a835068..ef1428d 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 @@
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 0000000..78a963e
--- /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 24d28ae..18cd2eb 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 @@
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 bdddfec..b6b314b 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 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 @@
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 @@
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 @@
configuredTargetFactory,
validityPredicate,
preferredDependencyPredicate,
- ImmutableSet.copyOf(advertisedProviders),
- canHaveAnyProvider,
+ advertisedProviders.build(),
configuredTargetFunction,
externalBindingsFunction,
ruleDefinitionEnvironment,
@@ -758,8 +755,9 @@
* 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 @@
* <code>bind</code> .
*/
public Builder canHaveAnyProvider() {
- Preconditions.checkState(advertisedProviders.isEmpty());
- canHaveAnyProvider = true;
+ advertisedProviders.canHaveAnyProvider();
return this;
}
@@ -1003,9 +1000,7 @@
/**
* 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 @@
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 @@
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 @@
*
* <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 0a85af0..4f11095 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 @@
// 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 24ed4bb..d0e9d4a 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.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 @@
@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 @@
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 @@
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 @@
// 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 @@
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 @@
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
