Remove AspectClass.getDefinition Aspect becomes a triple (AspectClass, AspectDefinition, AspectParameters) and loses its equals() method. After this CL, SkylarkAspectClass.getDefintion still exists and is deprecated. -- MOS_MIGRATED_REVID=119159653
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectDescriptor.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectDescriptor.java new file mode 100644 index 0000000..7a43323 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectDescriptor.java
@@ -0,0 +1,70 @@ +// 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.analysis; + +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.packages.AspectClass; +import com.google.devtools.build.lib.packages.AspectParameters; + +import java.util.Objects; + +/** + * A pair of {@link AspectClass} and {@link AspectParameters}. + * + * Used for dependency resolution. + */ +@Immutable +public final class AspectDescriptor { + private final AspectClass aspectClass; + private final AspectParameters aspectParameters; + + public AspectDescriptor(AspectClass aspectClass, + AspectParameters aspectParameters) { + this.aspectClass = aspectClass; + this.aspectParameters = aspectParameters; + } + + public AspectDescriptor(AspectClass aspectClass) { + this(aspectClass, AspectParameters.EMPTY); + } + + public AspectClass getAspectClass() { + return aspectClass; + } + + public AspectParameters getParameters() { + return aspectParameters; + } + + @Override + public int hashCode() { + return Objects.hash(aspectClass, aspectParameters); + } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + + if (!(obj instanceof AspectDescriptor)) { + return false; + } + + AspectDescriptor that = (AspectDescriptor) obj; + return Objects.equals(aspectClass, that.aspectClass) + && Objects.equals(aspectParameters, that.aspectParameters); + } +}
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 46fcc029..abbd401 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
@@ -17,7 +17,6 @@ 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.Aspect; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.util.Preconditions; @@ -65,7 +64,7 @@ */ public static Dependency withConfiguration(Label label, BuildConfiguration configuration) { return new StaticConfigurationDependency( - label, configuration, ImmutableMap.<Aspect, BuildConfiguration>of()); + label, configuration, ImmutableMap.<AspectDescriptor, BuildConfiguration>of()); } /** @@ -75,9 +74,10 @@ * <p>The configuration and aspects must not be {@code null}. */ public static Dependency withConfigurationAndAspects( - Label label, BuildConfiguration configuration, Set<Aspect> aspects) { - ImmutableMap.Builder<Aspect, BuildConfiguration> aspectBuilder = new ImmutableMap.Builder<>(); - for (Aspect aspect : aspects) { + Label label, BuildConfiguration configuration, Set<AspectDescriptor> aspects) { + ImmutableMap.Builder<AspectDescriptor, BuildConfiguration> aspectBuilder = + new ImmutableMap.Builder<>(); + for (AspectDescriptor aspect : aspects) { aspectBuilder.put(aspect, configuration); } return new StaticConfigurationDependency(label, configuration, aspectBuilder.build()); @@ -92,7 +92,7 @@ */ public static Dependency withConfiguredAspects( Label label, BuildConfiguration configuration, - Map<Aspect, BuildConfiguration> aspectConfigurations) { + Map<AspectDescriptor, BuildConfiguration> aspectConfigurations) { return new StaticConfigurationDependency( label, configuration, ImmutableMap.copyOf(aspectConfigurations)); } @@ -102,7 +102,7 @@ * configuration builds. */ public static Dependency withTransitionAndAspects( - Label label, Attribute.Transition transition, Set<Aspect> aspects) { + Label label, Attribute.Transition transition, Set<AspectDescriptor> aspects) { return new DynamicConfigurationDependency(label, transition, ImmutableSet.copyOf(aspects)); } @@ -148,7 +148,7 @@ * * @see #getAspectConfigurations() */ - public abstract ImmutableSet<Aspect> getAspects(); + public abstract ImmutableSet<AspectDescriptor> getAspects(); /** * Returns the mapping from aspects to the static configurations they should be evaluated with. @@ -157,7 +157,7 @@ * * @throws IllegalStateException if {@link #hasStaticConfiguration()} returns false. */ - public abstract ImmutableMap<Aspect, BuildConfiguration> getAspectConfigurations(); + public abstract ImmutableMap<AspectDescriptor, BuildConfiguration> getAspectConfigurations(); /** * Implementation of a dependency with no configuration, suitable for static configuration @@ -186,12 +186,12 @@ } @Override - public ImmutableSet<Aspect> getAspects() { + public ImmutableSet<AspectDescriptor> getAspects() { return ImmutableSet.of(); } @Override - public ImmutableMap<Aspect, BuildConfiguration> getAspectConfigurations() { + public ImmutableMap<AspectDescriptor, BuildConfiguration> getAspectConfigurations() { return ImmutableMap.of(); } @@ -221,11 +221,11 @@ */ private static final class StaticConfigurationDependency extends Dependency { private final BuildConfiguration configuration; - private final ImmutableMap<Aspect, BuildConfiguration> aspectConfigurations; + private final ImmutableMap<AspectDescriptor, BuildConfiguration> aspectConfigurations; public StaticConfigurationDependency( Label label, BuildConfiguration configuration, - ImmutableMap<Aspect, BuildConfiguration> aspects) { + ImmutableMap<AspectDescriptor, BuildConfiguration> aspects) { super(label); this.configuration = Preconditions.checkNotNull(configuration); this.aspectConfigurations = Preconditions.checkNotNull(aspects); @@ -248,12 +248,12 @@ } @Override - public ImmutableSet<Aspect> getAspects() { + public ImmutableSet<AspectDescriptor> getAspects() { return aspectConfigurations.keySet(); } @Override - public ImmutableMap<Aspect, BuildConfiguration> getAspectConfigurations() { + public ImmutableMap<AspectDescriptor, BuildConfiguration> getAspectConfigurations() { return aspectConfigurations; } @@ -287,10 +287,10 @@ */ private static final class DynamicConfigurationDependency extends Dependency { private final Attribute.Transition transition; - private final ImmutableSet<Aspect> aspects; + private final ImmutableSet<AspectDescriptor> aspects; public DynamicConfigurationDependency( - Label label, Attribute.Transition transition, ImmutableSet<Aspect> aspects) { + Label label, Attribute.Transition transition, ImmutableSet<AspectDescriptor> aspects) { super(label); this.transition = Preconditions.checkNotNull(transition); this.aspects = Preconditions.checkNotNull(aspects); @@ -313,12 +313,12 @@ } @Override - public ImmutableSet<Aspect> getAspects() { + public ImmutableSet<AspectDescriptor> getAspects() { return aspects; } @Override - public ImmutableMap<Aspect, BuildConfiguration> getAspectConfigurations() { + public ImmutableMap<AspectDescriptor, BuildConfiguration> getAspectConfigurations() { throw new IllegalStateException( "A dependency with a dynamic configuration transition does not have aspect " + "configurations.");
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 780e881..02f8cd4 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
@@ -33,6 +33,7 @@ import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.EnvironmentGroup; import com.google.devtools.build.lib.packages.InputFile; +import com.google.devtools.build.lib.packages.NativeAspectClass; import com.google.devtools.build.lib.packages.NoSuchThingException; import com.google.devtools.build.lib.packages.OutputFile; import com.google.devtools.build.lib.packages.PackageGroup; @@ -449,38 +450,51 @@ } } - private ImmutableSet<Aspect> requiredAspects( + private ImmutableSet<AspectDescriptor> requiredAspects( Aspect aspect, Attribute attribute, Target target, Rule originalRule) { if (!(target instanceof Rule)) { return ImmutableSet.of(); } - Set<Aspect> aspectCandidates = extractAspectCandidates(aspect, attribute, originalRule); + Iterable<Aspect> aspectCandidates = extractAspectCandidates(aspect, attribute, originalRule); RuleClass ruleClass = ((Rule) target).getRuleClassObject(); - ImmutableSet.Builder<Aspect> result = ImmutableSet.builder(); - for (Aspect candidateClass : aspectCandidates) { + ImmutableSet.Builder<AspectDescriptor> result = ImmutableSet.builder(); + for (Aspect aspectCandidate : aspectCandidates) { if (Sets.difference( - candidateClass.getDefinition().getRequiredProviders(), + aspectCandidate.getDefinition().getRequiredProviders(), ruleClass.getAdvertisedProviders()) .isEmpty()) { - result.add(candidateClass); + result.add( + new AspectDescriptor( + aspectCandidate.getAspectClass(), + aspectCandidate.getParameters())); } } return result.build(); } - private static Set<Aspect> extractAspectCandidates( - Aspect aspectWithParameters, Attribute attribute, Rule originalRule) { + private static Iterable<Aspect> extractAspectCandidates( + Aspect aspect, Attribute attribute, Rule originalRule) { // The order of this set will be deterministic. This is necessary because this order eventually // influences the order in which aspects are merged into the main configured target, which in // turn influences which aspect takes precedence if two emit the same provider (maybe this // should be an error) Set<Aspect> aspectCandidates = new LinkedHashSet<>(); aspectCandidates.addAll(attribute.getAspects(originalRule)); - if (aspectWithParameters != null) { - for (AspectClass aspect : - aspectWithParameters.getDefinition().getAttributeAspects().get(attribute.getName())) { - aspectCandidates.add(new Aspect(aspect, aspectWithParameters.getParameters())); + if (aspect != null) { + for (AspectClass aspectClass : + aspect.getDefinition().getAttributeAspects().get(attribute.getName())) { + if (aspectClass.equals(aspect.getAspectClass())) { + aspectCandidates.add(aspect); + } else if (aspectClass instanceof NativeAspectClass<?>) { + aspectCandidates.add( + Aspect.forNative((NativeAspectClass<?>) aspectClass, aspect.getParameters())); + } else { + // If we ever want to support this specifying arbitrary aspects for Skylark aspects, + // we will need to do a Skyframe call here to load an up-to-date definition. + throw new IllegalStateException( + "Skylark aspect classes sending different aspects along attributes is not supported"); + } } } return aspectCandidates;
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 d414dac..349d6d4 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
@@ -30,6 +30,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.AspectDescriptor; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.Dependency; @@ -39,7 +40,6 @@ import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; -import com.google.devtools.build.lib.packages.Aspect; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.Attribute.Configurator; import com.google.devtools.build.lib.packages.Attribute.SplitTransition; @@ -1500,7 +1500,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<Aspect> aspects); + Iterable<Dependency> getDependencies(Label label, ImmutableSet<AspectDescriptor> aspects); } /** @@ -1573,7 +1573,8 @@ } @Override - public Iterable<Dependency> getDependencies(Label label, ImmutableSet<Aspect> aspects) { + public Iterable<Dependency> getDependencies( + Label label, ImmutableSet<AspectDescriptor> aspects) { return ImmutableList.of( currentConfiguration != null ? Dependency.withConfigurationAndAspects(label, currentConfiguration, aspects) @@ -1676,7 +1677,7 @@ @Override public Iterable<Dependency> getDependencies( - Label label, ImmutableSet<Aspect> aspects) { + Label label, ImmutableSet<AspectDescriptor> aspects) { return ImmutableList.of( Dependency.withTransitionAndAspects(label, transition, aspects)); } @@ -1743,7 +1744,8 @@ @Override - public Iterable<Dependency> getDependencies(Label label, ImmutableSet<Aspect> aspects) { + public Iterable<Dependency> getDependencies( + Label label, ImmutableSet<AspectDescriptor> aspects) { ImmutableList.Builder<Dependency> builder = ImmutableList.builder(); for (TransitionApplier applier : appliers) { builder.addAll(applier.getDependencies(label, aspects));
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java index 34f3bff..cff38f9 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
@@ -13,31 +13,49 @@ // limitations under the License. package com.google.devtools.build.lib.packages; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Preconditions; -import java.util.Objects; - /** - * Wrapper around {@link AspectClass} class and {@link AspectParameters}. Aspects are - * created with help of aspect factory instances and parameters are used to configure them, so we - * have to keep them together. + * An instance of a given {@code AspectClass} with loaded definition and parameters. + * + * This is an aspect equivalent of {@link Rule} class for build rules. + * + * Note: this class does not have {@code equals()} and {@code hashCode()} redefined, so should + * not be used in SkyKeys. */ +@Immutable public final class Aspect implements DependencyFilter.AttributeInfoProvider { // TODO(bazel-team): class objects are not really hashable or comparable for equality other than // by reference. We should identify the aspect here in a way that does not rely on comparison // by reference so that keys can be serialized and deserialized properly. private final AspectClass aspectClass; private final AspectParameters parameters; + private final AspectDefinition aspectDefinition; - public Aspect(AspectClass aspect, AspectParameters parameters) { - Preconditions.checkNotNull(aspect); - Preconditions.checkNotNull(parameters); - this.aspectClass = aspect; - this.parameters = parameters; + private Aspect( + AspectClass aspectClass, + AspectDefinition aspectDefinition, + AspectParameters parameters) { + this.aspectClass = Preconditions.checkNotNull(aspectClass); + this.aspectDefinition = Preconditions.checkNotNull(aspectDefinition); + this.parameters = Preconditions.checkNotNull(parameters); } - public Aspect(AspectClass aspect) { - this(aspect, AspectParameters.EMPTY); + public static Aspect forNative( + NativeAspectClass<?> nativeAspectClass, AspectParameters parameters) { + return new Aspect(nativeAspectClass, nativeAspectClass.getDefinition(parameters), parameters); + } + + public static Aspect forNative(NativeAspectClass<?> nativeAspectClass) { + return forNative(nativeAspectClass, AspectParameters.EMPTY); + } + + public static Aspect forSkylark( + SkylarkAspectClass skylarkAspectClass, + AspectDefinition definition, + AspectParameters parameters) { + return new Aspect(skylarkAspectClass, definition, parameters); } /** @@ -55,30 +73,12 @@ } @Override - public boolean equals(Object other) { - if (this == other) { - return true; - } - if (!(other instanceof Aspect)) { - return false; - } - Aspect that = (Aspect) other; - return Objects.equals(this.aspectClass, that.aspectClass) - && Objects.equals(this.parameters, that.parameters); - } - - @Override - public int hashCode() { - return Objects.hash(aspectClass, parameters); - } - - @Override public String toString() { return String.format("Aspect %s(%s)", aspectClass, parameters); } public AspectDefinition getDefinition() { - return aspectClass.getDefinition(parameters); + return aspectDefinition; } @Override
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java b/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java index 1a3d52d..6a7d249 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java
@@ -16,14 +16,54 @@ /** * A class of aspects. + * <p>Aspects might be defined natively, in Java ({@link NativeAspectClass}) + * or in Skylark ({@link SkylarkAspectClass}). * - * <p>This interface serves as a factory for {@code AspectFactory}. - * {@code AspectFactory} type argument is a placeholder for - * a {@link com.google.devtools.build.lib.analysis.ConfiguredAspectFactory}, which is - * an analysis-phase class. All loading-phase code uses {@code AspectClass<?>}, - * whereas analysis-phase code uses {@code AspectClass<ConfiguredAspectFactory>}. - * The latter is what all real implementations of this interface should implement. + * Bazel propagates aspects through a multistage process. The general pipeline is as follows: * + * <pre> + * {@link AspectClass} + * | + * V + * {@code AspectDescriptor} <- {@link AspectParameters} + * \ + * V + * {@link Aspect} <- {@link AspectDefinition} (might require loading Skylark files) + * | + * V + * {@code ConfiguredAspect} <- {@code ConfiguredTarget} + * </pre> + * + * <ul> + * <li>{@link AspectClass} is a moniker for "user" definition of the aspect, be it + * a native aspect or a Skylark aspect. It contains either a reference to + * the native class implementing the aspect or the location of the Skylark definition + * of the aspect in the source tree, i.e. label of .bzl file + symbol name. + * </li> + * <li>{@link AspectParameters} is a (key,value) pair list that can be used to + * parameterize aspect classes</li> + * <li>{@link com.google.devtools.build.lib.analysis.AspectDescriptor} is a pair + * of {@code AspectClass} and {@link AspectParameters}. It uniquely identifies + * the aspect and can be used in SkyKeys. + * </li> + * <li>{@link AspectDefinition} is a class encapsulating the aspect definition (what + * attributes aspoect has, and along which dependencies does it propagate. + * </li> + * <li>{@link Aspect} is a fully instantiated instance of an Aspect after it is loaded. + * Getting an {@code Aspect} from {@code AspectDescriptor} for Skylark aspects + * requires adding a Skyframe dependency. + * </li> + * <li>{@link com.google.devtools.build.lib.analysis.ConfiguredAspect} represents a result + * of application of an {@link Aspect} to a given + * {@link com.google.devtools.build.lib.analysis.ConfiguredTarget}. + * </li> + * </ul> + * + * {@link com.google.devtools.build.lib.analysis.AspectDescriptor}, or in general, a tuple + * of ({@link AspectClass}, {@link AspectParameters}) is an identifier that should be + * used in SkyKeys or in other contexts that need equality for aspects. + * See also {@link com.google.devtools.build.lib.skyframe.AspectFunction} for details + * on Skyframe treatment of Aspects. */ public interface AspectClass { @@ -31,6 +71,4 @@ * Returns an aspect name. */ String getName(); - - AspectDefinition getDefinition(AspectParameters aspectParameters); }
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 ffe0369..bf4a0a7 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
@@ -61,21 +61,41 @@ public static final Predicate<RuleClass> NO_RULE = Predicates.alwaysFalse(); - private static final class RuleAspect { - private final AspectClass aspectFactory; - private final Function<Rule, AspectParameters> parametersExtractor; + private abstract static class RuleAspect<C extends AspectClass> { + protected final C aspectClass; + protected final Function<Rule, AspectParameters> parametersExtractor; - RuleAspect(AspectClass aspectFactory, Function<Rule, AspectParameters> parametersExtractor) { - this.aspectFactory = aspectFactory; + protected RuleAspect(C aspectClass, Function<Rule, AspectParameters> parametersExtractor) { + this.aspectClass = aspectClass; this.parametersExtractor = parametersExtractor; } - AspectClass getAspectFactory() { - return aspectFactory; + public abstract Aspect getAspect(Rule rule); + } + + private static class NativeRuleAspect extends RuleAspect<NativeAspectClass<?>> { + public NativeRuleAspect(NativeAspectClass<?> aspectClass, + Function<Rule, AspectParameters> parametersExtractor) { + super(aspectClass, parametersExtractor); } - - Function<Rule, AspectParameters> getParametersExtractor() { - return parametersExtractor; + + @Override + public Aspect getAspect(Rule rule) { + return Aspect.forNative(aspectClass, parametersExtractor.apply(rule)); + } + } + + private static class SkylarkRuleAspect extends RuleAspect<SkylarkAspectClass> { + public SkylarkRuleAspect(SkylarkAspectClass aspectClass) { + super(aspectClass, NO_PARAMETERS); + } + + @Override + public Aspect getAspect(Rule rule) { + return Aspect.forSkylark( + aspectClass, + aspectClass.getDefinition(), + parametersExtractor.apply(rule)); } } @@ -281,6 +301,15 @@ } } + private static final Function<Rule, AspectParameters> NO_PARAMETERS = + new Function<Rule, AspectParameters>() { + @Override + public AspectParameters apply(Rule input) { + return AspectParameters.EMPTY; + } + }; + + /** * Creates a new attribute builder. * @@ -301,6 +330,7 @@ * already undocumented based on its name cannot be marked as undocumented. */ public static class Builder <TYPE> { + private String name; private final Type<TYPE> type; private Transition configTransition = ConfigurationTransition.NONE; @@ -724,13 +754,7 @@ * through this attribute. */ public <T extends NativeAspectFactory> Builder<TYPE> aspect(Class<T> aspect) { - Function<Rule, AspectParameters> noParameters = new Function<Rule, AspectParameters>() { - @Override - public AspectParameters apply(Rule input) { - return AspectParameters.EMPTY; - } - }; - return this.aspect(aspect, noParameters); + return this.aspect(aspect, NO_PARAMETERS); } /** @@ -750,8 +774,9 @@ * * @param evaluator function that extracts aspect parameters from rule. */ - public Builder<TYPE> aspect(AspectClass aspect, Function<Rule, AspectParameters> evaluator) { - this.aspects.add(new RuleAspect(aspect, evaluator)); + public Builder<TYPE> aspect( + NativeAspectClass<?> aspect, Function<Rule, AspectParameters> evaluator) { + this.aspects.add(new NativeRuleAspect(aspect, evaluator)); return this; } @@ -759,7 +784,7 @@ * Asserts that a particular parameterized aspect probably needs to be computed for all direct * dependencies through this attribute. */ - public Builder<TYPE> aspect(AspectClass aspect) { + public Builder<TYPE> aspect(NativeAspectClass<?> aspect) { Function<Rule, AspectParameters> noParameters = new Function<Rule, AspectParameters>() { @Override @@ -770,6 +795,11 @@ return this.aspect(aspect, noParameters); } + public Builder<TYPE> aspect(SkylarkAspectClass aspectClass) { + this.aspects.add(new SkylarkRuleAspect(aspectClass)); + return this; + } + /** * Sets the predicate-like edge validity checker. */ @@ -1400,8 +1430,7 @@ public ImmutableList<Aspect> getAspects(Rule rule) { ImmutableList.Builder<Aspect> builder = ImmutableList.builder(); for (RuleAspect aspect : aspects) { - builder.add( - new Aspect(aspect.getAspectFactory(), aspect.getParametersExtractor().apply(rule))); + builder.add(aspect.getAspect(rule)); } return builder.build(); }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/NativeAspectClass.java b/src/main/java/com/google/devtools/build/lib/packages/NativeAspectClass.java index 62783dc..febf653 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/NativeAspectClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/NativeAspectClass.java
@@ -33,7 +33,6 @@ return nativeClass.getSimpleName(); } - @Override public AspectDefinition getDefinition(AspectParameters aspectParameters) { return newInstance().getDefinition(aspectParameters); }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkAspectClass.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkAspectClass.java index fa4fc5a..935cb8f 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkAspectClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkAspectClass.java
@@ -52,4 +52,7 @@ public final int hashCode() { return Objects.hash(getExtensionLabel(), getExportedName()); } + + @Deprecated + public abstract AspectDefinition getDefinition(); }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java index 6f0180a..4876b6a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -49,7 +49,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.AspectDefinition; -import com.google.devtools.build.lib.packages.AspectParameters; 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.LateBoundLabel; @@ -992,7 +991,7 @@ } @Override - public AspectDefinition getDefinition(AspectParameters aspectParameters) { + public AspectDefinition getDefinition() { return aspectDefinition; }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java index c390a81499..aa171b2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -61,6 +61,15 @@ /** * The Skyframe function that generates aspects. + * + * {@link AspectFunction} takes a SkyKey containing an {@link AspectKey} [a tuple of + * (target label, configurations, aspect class and aspect parameters)], + * loads an {@link Aspect} from aspect class and aspect parameters, + * gets a {@link ConfiguredTarget} for label and configurations, and then creates + * a {@link ConfiguredAspect} for a given {@link AspectKey}. + * + * See {@link com.google.devtools.build.lib.packages.AspectClass} documentation + * for an overview of aspect-related classes */ public final class AspectFunction implements SkyFunction { private final BuildViewProvider buildViewProvider; @@ -116,7 +125,7 @@ NativeAspectClass<?> nativeAspectClass = (NativeAspectClass<?>) key.getAspectClass(); aspectFactory = (ConfiguredAspectFactory) nativeAspectClass.newInstance(); - aspect = new Aspect(nativeAspectClass, key.getParameters()); + aspect = Aspect.forNative(nativeAspectClass, key.getParameters()); } else if (key.getAspectClass() instanceof SkylarkAspectClass) { SkylarkAspectClass skylarkAspectClass = (SkylarkAspectClass) key.getAspectClass(); SkylarkAspect skylarkAspect; @@ -132,7 +141,10 @@ } aspectFactory = new SkylarkAspectFactory(skylarkAspect.getName(), skylarkAspect); - aspect = new Aspect(skylarkAspect.getAspectClass(), key.getParameters()); + aspect = Aspect.forSkylark( + skylarkAspect.getAspectClass(), + skylarkAspect.getAspectClass().getDefinition(), + key.getParameters()); } else { throw new IllegalStateException(); }
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 08410ee..48b046d 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
@@ -27,6 +27,7 @@ import com.google.devtools.build.lib.actions.Actions; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; +import com.google.devtools.build.lib.analysis.AspectDescriptor; import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment; import com.google.devtools.build.lib.analysis.ConfiguredAspect; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -49,7 +50,9 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.Aspect; +import com.google.devtools.build.lib.packages.AspectClass; import com.google.devtools.build.lib.packages.AspectDefinition; +import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.BuildType; @@ -598,9 +601,11 @@ ListMultimap<SkyKey, ConfiguredAspect> result = ArrayListMultimap.create(); Set<SkyKey> aspectKeys = new HashSet<>(); for (Dependency dep : deps) { - for (Entry<Aspect, BuildConfiguration> depAspect : dep.getAspectConfigurations().entrySet()) { + for (Entry<AspectDescriptor, BuildConfiguration> depAspect + : dep.getAspectConfigurations().entrySet()) { aspectKeys.add(createAspectKey( - dep.getLabel(), depAspect.getValue(), dep.getConfiguration(), depAspect.getKey())); + dep.getLabel(), depAspect.getValue(), dep.getConfiguration(), + depAspect.getKey().getAspectClass(), depAspect.getKey().getParameters())); } } @@ -615,13 +620,12 @@ continue; } ConfiguredTarget depConfiguredTarget = configuredTargetMap.get(depKey); - for (Entry<Aspect, BuildConfiguration> depAspect : dep.getAspectConfigurations().entrySet()) { - if (!aspectMatchesConfiguredTarget(depConfiguredTarget, depAspect.getKey())) { - continue; - } - + for (Entry<AspectDescriptor, BuildConfiguration> depAspect + : dep.getAspectConfigurations().entrySet()) { SkyKey aspectKey = createAspectKey( - dep.getLabel(), depAspect.getValue(), dep.getConfiguration(), depAspect.getKey()); + dep.getLabel(), depAspect.getValue(), dep.getConfiguration(), + depAspect.getKey().getAspectClass(), + depAspect.getKey().getParameters()); AspectValue aspectValue = null; try { // TODO(ulfjack): Catch all thrown AspectCreationException and NoSuchThingException @@ -631,7 +635,7 @@ throw new AspectCreationException( String.format( "Evaluation of aspect %s on %s failed: %s", - depAspect.getKey().getDefinition().getName(), + depAspect.getKey().getAspectClass().getName(), dep.getLabel(), e.toString())); } @@ -640,6 +644,10 @@ // Dependent aspect has either not been computed yet or is in error. return null; } + if (!aspectMatchesConfiguredTarget(depConfiguredTarget, aspectValue.getAspect())) { + continue; + } + result.put(depKey, aspectValue.getConfiguredAspect()); transitivePackages.addTransitive(aspectValue.getTransitivePackages()); } @@ -651,16 +659,17 @@ Label label, BuildConfiguration aspectConfiguration, BuildConfiguration baseConfiguration, - Aspect depAspect) { + AspectClass aspectClass, + AspectParameters parameters) { return AspectValue.key(label, aspectConfiguration, baseConfiguration, - depAspect.getAspectClass(), - depAspect.getParameters()); + aspectClass, + parameters); } - private static boolean aspectMatchesConfiguredTarget(ConfiguredTarget dep, Aspect aspectClass) { - AspectDefinition aspectDefinition = aspectClass.getDefinition(); + private static boolean aspectMatchesConfiguredTarget(ConfiguredTarget dep, Aspect aspect) { + AspectDefinition aspectDefinition = aspect.getDefinition(); for (Class<?> provider : aspectDefinition.getRequiredProviders()) { if (dep.getProvider(provider.asSubclass(TransitiveInfoProvider.class)) == null) { return false;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index c215411..0232429 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -47,6 +47,7 @@ import com.google.devtools.build.lib.actions.PackageRootResolutionException; import com.google.devtools.build.lib.actions.ResourceManager; import com.google.devtools.build.lib.actions.Root; +import com.google.devtools.build.lib.analysis.AspectDescriptor; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.BuildView.Options; import com.google.devtools.build.lib.analysis.ConfiguredAspect; @@ -75,7 +76,6 @@ import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.exec.OutputService; -import com.google.devtools.build.lib.packages.Aspect; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.NoSuchPackageException; @@ -1165,10 +1165,12 @@ final List<SkyKey> skyKeys = new ArrayList<>(); for (Dependency key : keys) { skyKeys.add(ConfiguredTargetValue.key(key.getLabel(), configs.get(key))); - for (Aspect aspect : key.getAspects()) { + for (AspectDescriptor aspectDescriptor : key.getAspects()) { skyKeys.add( ConfiguredTargetFunction.createAspectKey( - key.getLabel(), configs.get(key), configs.get(key), aspect)); + key.getLabel(), configs.get(key), configs.get(key), + aspectDescriptor.getAspectClass(), + aspectDescriptor.getParameters())); } } @@ -1187,10 +1189,12 @@ ((ConfiguredTargetValue) result.get(configuredTargetKey)).getConfiguredTarget(); List<ConfiguredAspect> configuredAspects = new ArrayList<>(); - for (Aspect aspect : key.getAspects()) { + for (AspectDescriptor aspectDescriptor : key.getAspects()) { SkyKey aspectKey = ConfiguredTargetFunction.createAspectKey( - key.getLabel(), configs.get(key), configs.get(key), aspect); + key.getLabel(), configs.get(key), configs.get(key), + aspectDescriptor.getAspectClass(), + aspectDescriptor.getParameters()); if (result.get(aspectKey) == null) { continue DependentNodeLoop; }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java index a740a1f..c87eb14 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -41,7 +41,6 @@ import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.analysis.util.BuildViewTestBase; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.packages.Aspect; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.pkgcache.LoadingFailedException; @@ -344,12 +343,12 @@ Dependency.withTransitionAndAspects( Label.parseAbsolute("//package:inner"), Attribute.ConfigurationTransition.NONE, - ImmutableSet.<Aspect>of()); + ImmutableSet.<AspectDescriptor>of()); fileDependency = Dependency.withTransitionAndAspects( Label.parseAbsolute("//package:file"), Attribute.ConfigurationTransition.NONE, - ImmutableSet.<Aspect>of()); + ImmutableSet.<AspectDescriptor>of()); } else { innerDependency = Dependency.withConfiguration(
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java index 2179908..13d31cc 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java
@@ -108,7 +108,7 @@ return dependencyResolver.dependentNodeMap( new TargetAndConfiguration(target, getTargetConfiguration()), getHostConfiguration(), - aspect != null ? new Aspect(new NativeAspectClass<T>(aspect)) : null, + aspect != null ? Aspect.forNative(new NativeAspectClass<T>(aspect)) : null, ImmutableSet.<ConfigMatchingProvider>of()); } @@ -117,7 +117,7 @@ ListMultimap<Attribute, Dependency> dependentNodeMap, String attrName, String dep, - Aspect... aspects) { + AspectDescriptor... aspects) { Attribute attr = null; for (Attribute candidate : dependentNodeMap.keySet()) { if (candidate.getName().equals(attrName)) { @@ -147,7 +147,8 @@ "aspect(name='b', foo=[])"); ListMultimap<Attribute, Dependency> map = dependentNodeMap("//a:a", null); assertDep( - map, "foo", "//a:b", new Aspect(new NativeAspectClass(TestAspects.SimpleAspect.class))); + map, "foo", "//a:b", + new AspectDescriptor(new NativeAspectClass<>(TestAspects.SimpleAspect.class))); } @Test @@ -159,7 +160,8 @@ ListMultimap<Attribute, Dependency> map = dependentNodeMap("//a:a", TestAspects.AttributeAspect.class); assertDep( - map, "foo", "//a:b", new Aspect(new NativeAspectClass(TestAspects.AttributeAspect.class))); + map, "foo", "//a:b", + new AspectDescriptor(new NativeAspectClass<>(TestAspects.AttributeAspect.class))); } @Test
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/DependencyTest.java b/src/test/java/com/google/devtools/build/lib/analysis/DependencyTest.java index 4ea7309..0e4908d 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/DependencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/DependencyTest.java
@@ -23,8 +23,9 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.util.AnalysisTestCase; import com.google.devtools.build.lib.analysis.util.TestAspects; +import com.google.devtools.build.lib.analysis.util.TestAspects.AttributeAspect; +import com.google.devtools.build.lib.analysis.util.TestAspects.SimpleAspect; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.packages.Aspect; import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.packages.NativeAspectClass; @@ -83,11 +84,11 @@ @Test public void withConfigurationAndAspects_BasicAccessors() throws Exception { update(); - Aspect simpleAspect = new Aspect( - new NativeAspectClass<TestAspects.SimpleAspect>(TestAspects.SimpleAspect.class)); - Aspect attributeAspect = new Aspect( - new NativeAspectClass<TestAspects.AttributeAspect>(TestAspects.AttributeAspect.class)); - ImmutableSet<Aspect> twoAspects = ImmutableSet.of(simpleAspect, attributeAspect); + AspectDescriptor simpleAspect = new AspectDescriptor( + new NativeAspectClass<SimpleAspect>(SimpleAspect.class)); + AspectDescriptor attributeAspect = new AspectDescriptor( + new NativeAspectClass<AttributeAspect>(AttributeAspect.class)); + ImmutableSet<AspectDescriptor> twoAspects = ImmutableSet.of(simpleAspect, attributeAspect); Dependency targetDep = Dependency.withConfigurationAndAspects( Label.parseAbsolute("//a"), getTargetConfiguration(), twoAspects); @@ -113,7 +114,7 @@ @Test public void withConfigurationAndAspects_RejectsNullAspectsWithNPE() throws Exception { update(); - Set<Aspect> nullSet = new LinkedHashSet<>(); + Set<AspectDescriptor> nullSet = new LinkedHashSet<>(); nullSet.add(null); try { @@ -129,11 +130,11 @@ public void withConfigurationAndAspects_RejectsNullConfigWithNPE() throws Exception { // Although the NullPointerTester should check this, this test invokes a different code path, // because it includes aspects (which the NPT test will not). - Aspect simpleAspect = new Aspect( - new NativeAspectClass<TestAspects.SimpleAspect>(TestAspects.SimpleAspect.class)); - Aspect attributeAspect = new Aspect( - new NativeAspectClass<TestAspects.AttributeAspect>(TestAspects.AttributeAspect.class)); - ImmutableSet<Aspect> twoAspects = ImmutableSet.of(simpleAspect, attributeAspect); + AspectDescriptor simpleAspect = new AspectDescriptor( + new NativeAspectClass<SimpleAspect>(SimpleAspect.class)); + AspectDescriptor attributeAspect = new AspectDescriptor( + new NativeAspectClass<AttributeAspect>(AttributeAspect.class)); + ImmutableSet<AspectDescriptor> twoAspects = ImmutableSet.of(simpleAspect, attributeAspect); try { Dependency.withConfigurationAndAspects(Label.parseAbsolute("//a"), null, twoAspects); @@ -148,7 +149,9 @@ update(); Dependency dep = Dependency.withConfigurationAndAspects( - Label.parseAbsolute("//a"), getTargetConfiguration(), ImmutableSet.<Aspect>of()); + Label.parseAbsolute("//a"), + getTargetConfiguration(), + ImmutableSet.<AspectDescriptor>of()); // Here we're also checking that this doesn't throw an exception. No boom? OK. Good. assertThat(dep.getAspects()).isEmpty(); assertThat(dep.getAspectConfigurations()).isEmpty(); @@ -157,11 +160,11 @@ @Test public void withConfiguredAspects_BasicAccessors() throws Exception { update(); - Aspect simpleAspect = new Aspect( + AspectDescriptor simpleAspect = new AspectDescriptor( new NativeAspectClass<TestAspects.SimpleAspect>(TestAspects.SimpleAspect.class)); - Aspect attributeAspect = new Aspect( - new NativeAspectClass<TestAspects.AttributeAspect>(TestAspects.AttributeAspect.class)); - ImmutableMap<Aspect, BuildConfiguration> twoAspectMap = ImmutableMap.of( + AspectDescriptor attributeAspect = new AspectDescriptor( + new NativeAspectClass<AttributeAspect>(AttributeAspect.class)); + ImmutableMap<AspectDescriptor, BuildConfiguration> twoAspectMap = ImmutableMap.of( simpleAspect, getTargetConfiguration(), attributeAspect, getHostConfiguration()); Dependency targetDep = Dependency.withConfiguredAspects( @@ -189,7 +192,7 @@ Dependency dep = Dependency.withConfiguredAspects( Label.parseAbsolute("//a"), getTargetConfiguration(), - ImmutableMap.<Aspect, BuildConfiguration>of()); + ImmutableMap.<AspectDescriptor, BuildConfiguration>of()); // Here we're also checking that this doesn't throw an exception. No boom? OK. Good. assertThat(dep.getAspects()).isEmpty(); assertThat(dep.getAspectConfigurations()).isEmpty(); @@ -197,11 +200,11 @@ @Test public void withTransitionAndAspects_BasicAccessors() throws Exception { - Aspect simpleAspect = new Aspect( - new NativeAspectClass<TestAspects.SimpleAspect>(TestAspects.SimpleAspect.class)); - Aspect attributeAspect = new Aspect( - new NativeAspectClass<TestAspects.AttributeAspect>(TestAspects.AttributeAspect.class)); - ImmutableSet<Aspect> twoAspects = ImmutableSet.of(simpleAspect, attributeAspect); + AspectDescriptor simpleAspect = new AspectDescriptor( + new NativeAspectClass<>(SimpleAspect.class)); + AspectDescriptor attributeAspect = new AspectDescriptor( + new NativeAspectClass<>(AttributeAspect.class)); + ImmutableSet<AspectDescriptor> twoAspects = ImmutableSet.of(simpleAspect, attributeAspect); Dependency hostDep = Dependency.withTransitionAndAspects( Label.parseAbsolute("//a"), ConfigurationTransition.HOST, twoAspects); @@ -232,7 +235,8 @@ update(); Dependency dep = Dependency.withTransitionAndAspects( - Label.parseAbsolute("//a"), ConfigurationTransition.HOST, ImmutableSet.<Aspect>of()); + Label.parseAbsolute("//a"), ConfigurationTransition.HOST, + ImmutableSet.<AspectDescriptor>of()); // Here we're also checking that this doesn't throw an exception. No boom? OK. Good. assertThat(dep.getAspects()).isEmpty(); } @@ -258,28 +262,28 @@ BuildConfiguration host = getHostConfiguration(); BuildConfiguration target = getTargetConfiguration(); - Aspect simpleAspect = new Aspect( - new NativeAspectClass<TestAspects.SimpleAspect>(TestAspects.SimpleAspect.class)); - Aspect attributeAspect = new Aspect( - new NativeAspectClass<TestAspects.AttributeAspect>(TestAspects.AttributeAspect.class)); - Aspect errorAspect = new Aspect( - new NativeAspectClass<TestAspects.ErrorAspect>(TestAspects.ErrorAspect.class)); + AspectDescriptor simpleAspect = new AspectDescriptor( + new NativeAspectClass<>(TestAspects.SimpleAspect.class)); + AspectDescriptor attributeAspect = new AspectDescriptor( + new NativeAspectClass<>(TestAspects.AttributeAspect.class)); + AspectDescriptor errorAspect = new AspectDescriptor( + new NativeAspectClass<>(TestAspects.ErrorAspect.class)); - ImmutableSet<Aspect> twoAspects = ImmutableSet.of(simpleAspect, attributeAspect); - ImmutableSet<Aspect> inverseAspects = ImmutableSet.of(attributeAspect, simpleAspect); - ImmutableSet<Aspect> differentAspects = ImmutableSet.of(attributeAspect, errorAspect); - ImmutableSet<Aspect> noAspects = ImmutableSet.<Aspect>of(); + ImmutableSet<AspectDescriptor> twoAspects = ImmutableSet.of(simpleAspect, attributeAspect); + ImmutableSet<AspectDescriptor> inverseAspects = ImmutableSet.of(attributeAspect, simpleAspect); + ImmutableSet<AspectDescriptor> differentAspects = ImmutableSet.of(attributeAspect, errorAspect); + ImmutableSet<AspectDescriptor> noAspects = ImmutableSet.<AspectDescriptor>of(); - ImmutableMap<Aspect, BuildConfiguration> twoAspectsHostMap = + ImmutableMap<AspectDescriptor, BuildConfiguration> twoAspectsHostMap = ImmutableMap.of(simpleAspect, host, attributeAspect, host); - ImmutableMap<Aspect, BuildConfiguration> twoAspectsTargetMap = + ImmutableMap<AspectDescriptor, BuildConfiguration> twoAspectsTargetMap = ImmutableMap.of(simpleAspect, target, attributeAspect, target); - ImmutableMap<Aspect, BuildConfiguration> differentAspectsHostMap = + ImmutableMap<AspectDescriptor, BuildConfiguration> differentAspectsHostMap = ImmutableMap.of(attributeAspect, host, errorAspect, host); - ImmutableMap<Aspect, BuildConfiguration> differentAspectsTargetMap = + ImmutableMap<AspectDescriptor, BuildConfiguration> differentAspectsTargetMap = ImmutableMap.of(attributeAspect, target, errorAspect, target); - ImmutableMap<Aspect, BuildConfiguration> noAspectsMap = - ImmutableMap.<Aspect, BuildConfiguration>of(); + ImmutableMap<AspectDescriptor, BuildConfiguration> noAspectsMap = + ImmutableMap.<AspectDescriptor, BuildConfiguration>of(); new EqualsTester() .addEqualityGroup(