Restrict aspects visible to other aspects according to their advertised providers.
--
PiperOrigin-RevId: 147526961
MOS_MIGRATED_REVID=147526961
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java
index 0018525..6427014 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java
@@ -213,6 +213,10 @@
}
}
+ public static AspectCollection createForTests(AspectDescriptor... descriptors) {
+ return createForTests(ImmutableSet.copyOf(descriptors));
+ }
+
public static AspectCollection createForTests(ImmutableSet<AspectDescriptor> descriptors) {
ImmutableSet.Builder<AspectDeps> depsBuilder = ImmutableSet.builder();
for (AspectDescriptor descriptor : descriptors) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 19e972c..10a9216 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -857,7 +857,7 @@
targetAndConfig.getLabel(),
Attribute.ConfigurationTransition.NONE,
// TODO(bazel-team): support top-level aspects
- ImmutableSet.<AspectDescriptor>of()));
+ AspectCollection.EMPTY));
}
}
@@ -1003,7 +1003,7 @@
Iterable<BuildOptions> buildOptions) {
Preconditions.checkArgument(ct.getConfiguration().fragmentClasses().equals(fragments));
Dependency asDep = Dependency.withTransitionAndAspects(ct.getLabel(),
- Attribute.ConfigurationTransition.NONE, ImmutableSet.<AspectDescriptor>of());
+ Attribute.ConfigurationTransition.NONE, AspectCollection.EMPTY);
ImmutableList.Builder<BuildConfiguration> builder = ImmutableList.builder();
for (BuildOptions options : buildOptions) {
builder.add(Iterables.getOnlyElement(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java b/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
index d72bced..9b67fe8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.analysis;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.AspectDescriptor;
@@ -62,7 +61,9 @@
*/
public static Dependency withConfiguration(Label label, BuildConfiguration configuration) {
return new StaticConfigurationDependency(
- label, configuration, ImmutableMap.<AspectDescriptor, BuildConfiguration>of());
+ label, configuration,
+ AspectCollection.EMPTY,
+ ImmutableMap.<AspectDescriptor, BuildConfiguration>of());
}
/**
@@ -72,13 +73,13 @@
* <p>The configuration and aspects must not be {@code null}.
*/
public static Dependency withConfigurationAndAspects(
- Label label, BuildConfiguration configuration, Iterable<AspectDescriptor> aspects) {
+ Label label, BuildConfiguration configuration, AspectCollection aspects) {
ImmutableMap.Builder<AspectDescriptor, BuildConfiguration> aspectBuilder =
new ImmutableMap.Builder<>();
- for (AspectDescriptor aspect : aspects) {
+ for (AspectDescriptor aspect : aspects.getAllAspects()) {
aspectBuilder.put(aspect, configuration);
}
- return new StaticConfigurationDependency(label, configuration, aspectBuilder.build());
+ return new StaticConfigurationDependency(label, configuration, aspects, aspectBuilder.build());
}
/**
@@ -90,9 +91,10 @@
*/
public static Dependency withConfiguredAspects(
Label label, BuildConfiguration configuration,
+ AspectCollection aspects,
Map<AspectDescriptor, BuildConfiguration> aspectConfigurations) {
return new StaticConfigurationDependency(
- label, configuration, ImmutableMap.copyOf(aspectConfigurations));
+ label, configuration, aspects, ImmutableMap.copyOf(aspectConfigurations));
}
/**
@@ -100,8 +102,8 @@
* configuration builds.
*/
public static Dependency withTransitionAndAspects(
- Label label, Attribute.Transition transition, Iterable<AspectDescriptor> aspects) {
- return new DynamicConfigurationDependency(label, transition, ImmutableSet.copyOf(aspects));
+ Label label, Attribute.Transition transition, AspectCollection aspects) {
+ return new DynamicConfigurationDependency(label, transition, aspects);
}
protected final Label label;
@@ -144,18 +146,16 @@
* Returns the set of aspects which should be evaluated and combined with the configured target
* pointed to by this dependency.
*
- * @see #getAspectConfigurations()
+ * @see #getAspectConfiguration(AspectDescriptor)
*/
- public abstract ImmutableSet<AspectDescriptor> getAspects();
+ public abstract AspectCollection getAspects();
/**
- * Returns the mapping from aspects to the static configurations they should be evaluated with.
- *
- * <p>The {@link Map#keySet()} of this map is equal to that returned by {@link #getAspects()}.
- *
+ * Returns the the static configuration an aspect should be evaluated with
+ **
* @throws IllegalStateException if {@link #hasStaticConfiguration()} returns false.
*/
- public abstract ImmutableMap<AspectDescriptor, BuildConfiguration> getAspectConfigurations();
+ public abstract BuildConfiguration getAspectConfiguration(AspectDescriptor aspect);
/**
* Implementation of a dependency with no configuration, suitable for static configuration
@@ -184,13 +184,13 @@
}
@Override
- public ImmutableSet<AspectDescriptor> getAspects() {
- return ImmutableSet.of();
+ public AspectCollection getAspects() {
+ return AspectCollection.EMPTY;
}
@Override
- public ImmutableMap<AspectDescriptor, BuildConfiguration> getAspectConfigurations() {
- return ImmutableMap.of();
+ public BuildConfiguration getAspectConfiguration(AspectDescriptor aspect) {
+ return null;
}
@Override
@@ -219,14 +219,17 @@
*/
private static final class StaticConfigurationDependency extends Dependency {
private final BuildConfiguration configuration;
+ private final AspectCollection aspects;
private final ImmutableMap<AspectDescriptor, BuildConfiguration> aspectConfigurations;
public StaticConfigurationDependency(
Label label, BuildConfiguration configuration,
- ImmutableMap<AspectDescriptor, BuildConfiguration> aspects) {
+ AspectCollection aspects,
+ ImmutableMap<AspectDescriptor, BuildConfiguration> aspectConfigurations) {
super(label);
this.configuration = Preconditions.checkNotNull(configuration);
- this.aspectConfigurations = Preconditions.checkNotNull(aspects);
+ this.aspects = Preconditions.checkNotNull(aspects);
+ this.aspectConfigurations = Preconditions.checkNotNull(aspectConfigurations);
}
@Override
@@ -246,13 +249,13 @@
}
@Override
- public ImmutableSet<AspectDescriptor> getAspects() {
- return aspectConfigurations.keySet();
+ public AspectCollection getAspects() {
+ return aspects;
}
@Override
- public ImmutableMap<AspectDescriptor, BuildConfiguration> getAspectConfigurations() {
- return aspectConfigurations;
+ public BuildConfiguration getAspectConfiguration(AspectDescriptor aspect) {
+ return aspectConfigurations.get(aspect);
}
@Override
@@ -268,6 +271,7 @@
StaticConfigurationDependency otherDep = (StaticConfigurationDependency) other;
return label.equals(otherDep.label)
&& configuration.equals(otherDep.configuration)
+ && aspects.equals(otherDep.aspects)
&& aspectConfigurations.equals(otherDep.aspectConfigurations);
}
@@ -285,10 +289,10 @@
*/
private static final class DynamicConfigurationDependency extends Dependency {
private final Attribute.Transition transition;
- private final ImmutableSet<AspectDescriptor> aspects;
+ private final AspectCollection aspects;
public DynamicConfigurationDependency(
- Label label, Attribute.Transition transition, ImmutableSet<AspectDescriptor> aspects) {
+ Label label, Attribute.Transition transition, AspectCollection aspects) {
super(label);
this.transition = Preconditions.checkNotNull(transition);
this.aspects = Preconditions.checkNotNull(aspects);
@@ -311,15 +315,15 @@
}
@Override
- public ImmutableSet<AspectDescriptor> getAspects() {
+ public AspectCollection getAspects() {
return aspects;
}
@Override
- public ImmutableMap<AspectDescriptor, BuildConfiguration> getAspectConfigurations() {
+ public BuildConfiguration getAspectConfiguration(AspectDescriptor aspect) {
throw new IllegalStateException(
"A dependency with a dynamic configuration transition does not have aspect "
- + "configurations.");
+ + "configurations.");
}
@Override
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 dc1f292..06d561b 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
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.analysis.AspectCollection.AspectCycleOnPathException;
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;
@@ -520,36 +521,86 @@
}
- private static ImmutableSet<AspectDescriptor> requiredAspects(
- Iterable<Aspect> aspects,
+ private static AspectCollection requiredAspects(
+ Iterable<Aspect> aspectPath,
AttributeAndOwner attributeAndOwner,
final Target target,
Rule originalRule) {
if (!(target instanceof Rule)) {
- return ImmutableSet.of();
+ return AspectCollection.EMPTY;
}
if (attributeAndOwner.ownerAspect != null) {
// Do not propagate aspects along aspect attributes.
- return ImmutableSet.of();
+ return AspectCollection.EMPTY;
}
- Iterable<Aspect> aspectCandidates =
- extractAspectCandidates(aspects, attributeAndOwner.attribute, originalRule);
- RuleClass ruleClass = ((Rule) target).getRuleClassObject();
- ImmutableSet.Builder<AspectDescriptor> result = ImmutableSet.builder();
+ ImmutableList.Builder<Aspect> filteredAspectPath = ImmutableList.builder();
+ ImmutableSet.Builder<AspectDescriptor> visibleAspects = ImmutableSet.builder();
- for (Aspect aspectCandidate : aspectCandidates) {
- if (aspectCandidate.getDefinition()
- .getRequiredProviders()
- .isSatisfiedBy(ruleClass.getAdvertisedProviders())) {
- result.add(
- new AspectDescriptor(
- aspectCandidate.getAspectClass(),
- aspectCandidate.getParameters()));
+ collectOriginatingAspects(originalRule, attributeAndOwner.attribute, (Rule) target,
+ filteredAspectPath, visibleAspects);
+
+ collectPropagatingAspects(aspectPath,
+ attributeAndOwner.attribute,
+ (Rule) target, filteredAspectPath, visibleAspects);
+ try {
+ return AspectCollection.create(filteredAspectPath.build(), visibleAspects.build());
+ } catch (AspectCycleOnPathException e) {
+ // TODO(dslomov): propagate the error and report to user.
+ throw new AssertionError(e);
+ }
+ }
+
+ /**
+ * Collects into {@code filteredAspectPath}
+ * aspects from {@code aspectPath} that propagate along {@code attribute}
+ * and apply to a given {@code target}.
+ *
+ * The last aspect in {@code aspectPath} is (potentially) visible and recorded
+ * in {@code visibleAspects}.
+ */
+ private static void collectPropagatingAspects(Iterable<Aspect> aspectPath,
+ Attribute attribute, Rule target,
+ ImmutableList.Builder<Aspect> filteredAspectPath,
+ ImmutableSet.Builder<AspectDescriptor> visibleAspects) {
+
+ Aspect lastAspect = null;
+ for (Aspect aspect : aspectPath) {
+ lastAspect = aspect;
+ if (aspect.getDefinition().propagateAlong(attribute)
+ && aspect.getDefinition().getRequiredProviders()
+ .isSatisfiedBy(target.getRuleClassObject().getAdvertisedProviders())) {
+ filteredAspectPath.add(aspect);
+ } else {
+ lastAspect = null;
}
}
- return result.build();
+
+ if (lastAspect != null) {
+ visibleAspects.add(lastAspect.getDescriptor());
+ }
+ }
+
+ /**
+ * Collect all aspects that originate on {@code attribute} of {@code originalRule}
+ * and are applicable to a {@code target}
+ *
+ * They are appended to {@code filteredAspectPath} and registered in {@code visibleAspects} set.
+ */
+ private static void collectOriginatingAspects(
+ Rule originalRule, Attribute attribute, Rule target,
+ ImmutableList.Builder<Aspect> filteredAspectPath,
+ ImmutableSet.Builder<AspectDescriptor> visibleAspects) {
+ ImmutableList<Aspect> baseAspects = attribute.getAspects(originalRule);
+ RuleClass ruleClass = target.getRuleClassObject();
+ for (Aspect baseAspect : baseAspects) {
+ if (baseAspect.getDefinition().getRequiredProviders()
+ .isSatisfiedBy(ruleClass.getAdvertisedProviders())) {
+ filteredAspectPath.add(baseAspect);
+ visibleAspects.add(baseAspect.getDescriptor());
+ }
+ }
}
private static Iterable<Aspect> extractAspectCandidates(
@@ -686,7 +737,7 @@
applyNullTransition = true;
}
- ImmutableSet<AspectDescriptor> aspects =
+ AspectCollection aspects =
requiredAspects(this.aspects, attributeAndOwner, toTarget, rule);
Dependency dep;
if (config.useDynamicConfigurations() && !applyNullTransition) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 5823a52..b16a4a6 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -34,6 +34,7 @@
import com.google.common.collect.Multimap;
import com.google.common.collect.MutableClassToInstanceMap;
import com.google.devtools.build.lib.actions.Root;
+import com.google.devtools.build.lib.analysis.AspectCollection;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.Dependency;
@@ -45,7 +46,6 @@
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.packages.AspectDescriptor;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
import com.google.devtools.build.lib.packages.Attribute.Configurator;
@@ -1612,7 +1612,7 @@
* for each configuration represented by this instance.
* TODO(bazel-team): this is a really ugly reverse dependency: factor this away.
*/
- Iterable<Dependency> getDependencies(Label label, ImmutableSet<AspectDescriptor> aspects);
+ Iterable<Dependency> getDependencies(Label label, AspectCollection aspects);
}
/**
@@ -1706,7 +1706,7 @@
@Override
public Iterable<Dependency> getDependencies(
- Label label, ImmutableSet<AspectDescriptor> aspects) {
+ Label label, AspectCollection aspects) {
ImmutableList.Builder<Dependency> deps = ImmutableList.builder();
for (BuildConfiguration config : toConfigurations) {
deps.add(config != null
@@ -1829,7 +1829,7 @@
@Override
public Iterable<Dependency> getDependencies(
- Label label, ImmutableSet<AspectDescriptor> aspects) {
+ Label label, AspectCollection aspects) {
return ImmutableList.of(
isNull()
// We can trivially set the final value for null-configured targets now. This saves
