diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build')
13 files changed, 300 insertions, 121 deletions
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 0000000000..7a43323362 --- /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 46fcc0295d..abbd401a37 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.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.Aspect; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.util.Preconditions; @@ -65,7 +64,7 @@ public abstract class Dependency { */ 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 @@ public abstract class Dependency { * <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 abstract class Dependency { */ 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 @@ public abstract class Dependency { * 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 @@ public abstract class Dependency { * * @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 @@ public abstract class Dependency { * * @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 @@ public abstract class Dependency { } @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 @@ public abstract class Dependency { */ 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 @@ public abstract class Dependency { } @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 @@ public abstract class Dependency { */ 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 @@ public abstract class Dependency { } @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 780e881660..02f8cd4e2c 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.AttributeMap; 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 @@ public abstract class DependencyResolver { } } - 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 d414dac236..349d6d451d 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.Maps; 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.Label; 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 @@ public final class BuildConfiguration { * 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 @@ public final class BuildConfiguration { } @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 @@ public final class BuildConfiguration { @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 @@ public final class BuildConfiguration { @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 34f3bfff3f..cff38f925e 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; + + 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, AspectParameters parameters) { - Preconditions.checkNotNull(aspect); - Preconditions.checkNotNull(parameters); - this.aspectClass = aspect; - this.parameters = parameters; + public static Aspect forNative( + NativeAspectClass<?> nativeAspectClass, AspectParameters parameters) { + return new Aspect(nativeAspectClass, nativeAspectClass.getDefinition(parameters), parameters); } - public Aspect(AspectClass aspect) { - this(aspect, AspectParameters.EMPTY); + 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 @@ public final class Aspect implements DependencyFilter.AttributeInfoProvider { } @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 1a3d52d5c6..6a7d24902e 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 @@ package com.google.devtools.build.lib.packages; /** * 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 @@ public interface AspectClass { * 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 ffe0369938..bf4a0a744e 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 final class Attribute implements Comparable<Attribute> { 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); + } + + @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); } - - Function<Rule, AspectParameters> getParametersExtractor() { - return parametersExtractor; + + @Override + public Aspect getAspect(Rule rule) { + return Aspect.forSkylark( + aspectClass, + aspectClass.getDefinition(), + parametersExtractor.apply(rule)); } } @@ -281,6 +301,15 @@ public final class Attribute implements Comparable<Attribute> { } } + 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 @@ public final class Attribute implements Comparable<Attribute> { * 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 @@ public final class Attribute implements Comparable<Attribute> { * 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 @@ public final class Attribute implements Comparable<Attribute> { * * @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 @@ public final class Attribute implements Comparable<Attribute> { * 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 @@ public final class Attribute implements Comparable<Attribute> { 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 final class Attribute implements Comparable<Attribute> { 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 62783dc111..febf653e2b 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 @@ public final class NativeAspectClass<T extends NativeAspectClass.NativeAspectFac 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 fa4fc5a978..935cb8f002 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 abstract class SkylarkAspectClass implements AspectClass { 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 6f0180abc1..4876b6a981 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.collect.nestedset.Order; 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 @@ public class SkylarkRuleClassFunctions { } @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..aa171b2732 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 @@ import javax.annotation.Nullable; /** * 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 @@ public final class AspectFunction implements SkyFunction { 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 @@ public final class AspectFunction implements SkyFunction { } 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 08410eee7d..48b046d0ef 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.Action; 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.concurrent.ThreadSafety.Immutable; 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 @@ final class ConfiguredTargetFunction implements SkyFunction { 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 @@ final class ConfiguredTargetFunction implements SkyFunction { 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 @@ final class ConfiguredTargetFunction implements SkyFunction { 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 @@ final class ConfiguredTargetFunction implements SkyFunction { // 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 @@ final class ConfiguredTargetFunction implements SkyFunction { 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 c21541185c..0232429ec4 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.Executor; 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.concurrent.ThreadSafety.ThreadCompatible; 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 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { 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 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { ((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; } |