Roll forward of https://github.com/bazelbuild/bazel/commit/943c83aa58731c4f9561d79c458f254427a8f24c: Command line aspect-on-aspect Supports aspect-on-aspect for command line aspects. Command line aspects specified via `--aspects` option will support a top-level aspect requiring aspect providers via `required_aspect_providers` to get their values from other top-level aspects advertising it that come before it in the `--aspects` list. NEW: - Add `incompatible_ignore_duplicate_top_level_aspects` flag to allow duplicates in `--aspects` list. The flag is set to true by default, otherwise a validation error will be thrown in case of duplicates in top-level aspects. - Fix the error reporting for duplicate native aspects in `--aspects` list to be reported as a SkyFunction exception instead of crashing with assertion error. Automated rollback of commit 7b4f9826d2d38ac7d071a4ada7b8a40a7a78226d. *** Reason for rollback *** Guard the validation against duplicate aspects in `--aspects` list by a flag to avoid breaking builds with duplicate aspects. *** Original change description *** Automated rollback of commit 7649f610c45190735fd7de433b15679b21b2d91b. *** Reason for rollback *** The added validation to prevent duplicate aspects in --aspects list breaks //production/datapush/modular/implementations/build:buildtarget_test *** Original change description *** Roll forward of https://github.com/bazelbuild/bazel/commit/943c83aa58731c4f9561d79c458f254427a8f24c: Command line aspect-on-aspect Supports aspect-on-aspect for command line aspects. Command line aspects specified via `--aspects` option will support a top-level aspect requiring aspect providers via `required_a... *** PiperOrigin-RevId: 389217989
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisFailureEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisFailureEvent.java index 1e8a0ef..2a8445a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisFailureEvent.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisFailureEvent.java
@@ -30,7 +30,7 @@ import com.google.devtools.build.lib.causes.Cause; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; -import com.google.devtools.build.lib.skyframe.AspectValueKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import java.util.Collection; import javax.annotation.Nullable; @@ -40,7 +40,7 @@ * target cannot be completed because of an error in one of its dependencies. */ public class AnalysisFailureEvent implements BuildEvent { - @Nullable private final AspectValueKey failedAspect; + @Nullable private final AspectKey failedAspect; private final ConfiguredTargetKey failedTarget; private final BuildEventId configuration; private final NestedSet<Cause> rootCauses; @@ -48,12 +48,12 @@ public AnalysisFailureEvent( ActionLookupKey failedTarget, BuildEventId configuration, NestedSet<Cause> rootCauses) { Preconditions.checkArgument( - failedTarget instanceof ConfiguredTargetKey || failedTarget instanceof AspectValueKey); + failedTarget instanceof ConfiguredTargetKey || failedTarget instanceof AspectKey); if (failedTarget instanceof ConfiguredTargetKey) { this.failedAspect = null; this.failedTarget = (ConfiguredTargetKey) failedTarget; } else { - this.failedAspect = (AspectValueKey) failedTarget; + this.failedAspect = (AspectKey) failedTarget; this.failedTarget = failedAspect.getBaseConfiguredTargetKey(); } if (configuration != null) { @@ -65,7 +65,7 @@ } public AnalysisFailureEvent( - AspectValueKey failedAspect, BuildEventId configuration, NestedSet<Cause> rootCauses) { + AspectKey failedAspect, BuildEventId configuration, NestedSet<Cause> rootCauses) { this.failedAspect = failedAspect; this.failedTarget = failedAspect.getBaseConfiguredTargetKey(); if (configuration != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java index e8662a2..a39ddb6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java
@@ -166,4 +166,19 @@ + " be used. Example value: \"HOST_CPUS*0.5\".", converter = CpuResourceConverter.class) public int oomSensitiveSkyFunctionsSemaphoreSize; + + @Option( + name = "incompatible_ignore_duplicate_top_level_aspects", + defaultValue = "true", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + help = + "If true, remove duplicates from --aspects list by keeping only the first occurrence of" + + " every aspect. Otherwise, throw validation error if duplicate aspects are" + + " encountered.") + public boolean ignoreDuplicateTopLevelAspects; }
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 bec43a1..2e289f3 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
@@ -55,13 +55,12 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.AspectClass; -import com.google.devtools.build.lib.packages.AspectDescriptor; -import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.NativeAspectClass; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.packages.StarlarkAspectClass; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.pkgcache.PackageManager; @@ -75,6 +74,7 @@ import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns.Code; import com.google.devtools.build.lib.skyframe.AspectValueKey; import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.TopLevelAspectsKey; import com.google.devtools.build.lib.skyframe.BuildConfigurationValue; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.skyframe.CoverageReportValue; @@ -86,7 +86,6 @@ import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.RegexFilter; import com.google.devtools.build.skyframe.WalkableGraph; -import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -275,10 +274,11 @@ .map(TargetAndConfiguration::getConfiguredTargetKey) .collect(Collectors.toList()); - Multimap<Pair<Label, String>, BuildConfiguration> aspectConfigurations = - ArrayListMultimap.create(); - - List<AspectValueKey> aspectKeys = new ArrayList<>(); + ImmutableList.Builder<AspectClass> aspectClassesBuilder = ImmutableList.builder(); + if (viewOptions.ignoreDuplicateTopLevelAspects) { + // remove duplicates from aspects list + aspects = ImmutableSet.copyOf(aspects).asList(); + } for (String aspect : aspects) { // Syntax: label%aspect int delimiterPosition = aspect.indexOf('%'); @@ -318,38 +318,14 @@ createFailureDetail(errorMessage, Analysis.Code.ASPECT_LABEL_SYNTAX_ERROR), e); } - String starlarkFunctionName = aspect.substring(delimiterPosition + 1); - for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) { - aspectConfigurations.put( - Pair.of(targetSpec.getLabel(), aspect), targetSpec.getConfiguration()); - aspectKeys.add( - AspectValueKey.createStarlarkAspectKey( - targetSpec.getLabel(), - // For invoking top-level aspects, use the top-level configuration for both the - // aspect and the base target while the top-level configuration is untrimmed. - targetSpec.getConfiguration(), - targetSpec.getConfiguration(), - starlarkFileLabel, - starlarkFunctionName)); - } + aspectClassesBuilder.add(new StarlarkAspectClass(starlarkFileLabel, starlarkFunctionName)); } else { final NativeAspectClass aspectFactoryClass = ruleClassProvider.getNativeAspectClassMap().get(aspect); if (aspectFactoryClass != null) { - for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) { - // For invoking top-level aspects, use the top-level configuration for both the - // aspect and the base target while the top-level configuration is untrimmed. - BuildConfiguration configuration = targetSpec.getConfiguration(); - aspectConfigurations.put(Pair.of(targetSpec.getLabel(), aspect), configuration); - aspectKeys.add( - AspectValueKey.createAspectKey( - targetSpec.getLabel(), - configuration, - new AspectDescriptor(aspectFactoryClass, AspectParameters.EMPTY), - configuration)); - } + aspectClassesBuilder.add(aspectFactoryClass); } else { String errorMessage = "Aspect '" + aspect + "' is unknown"; throw new ViewCreationFailedException( @@ -358,6 +334,25 @@ } } + Multimap<Pair<Label, String>, BuildConfiguration> aspectConfigurations = + ArrayListMultimap.create(); + ImmutableList<AspectClass> aspectClasses = aspectClassesBuilder.build(); + ImmutableList.Builder<TopLevelAspectsKey> aspectsKeys = ImmutableList.builder(); + for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) { + BuildConfiguration configuration = targetSpec.getConfiguration(); + for (AspectClass aspectClass : aspectClasses) { + aspectConfigurations.put( + Pair.of(targetSpec.getLabel(), aspectClass.getName()), configuration); + } + // For invoking top-level aspects, use the top-level configuration for both the + // aspect and the base target while the top-level configuration is untrimmed. + if (!aspectClasses.isEmpty()) { + aspectsKeys.add( + AspectValueKey.createTopLevelAspectsKey( + aspectClasses, targetSpec.getLabel(), configuration)); + } + } + for (Pair<Label, String> target : aspectConfigurations.keys()) { eventBus.post( new AspectConfiguredEvent( @@ -382,7 +377,7 @@ skyframeBuildView.configureTargets( eventHandler, topLevelCtKeys, - aspectKeys, + aspectsKeys.build(), Suppliers.memoize(configurationLookupSupplier), topLevelOptions, eventBus,
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java index 6b62bd1..c25a910 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java
@@ -305,11 +305,18 @@ effectTags = {OptionEffectTag.UNKNOWN}, allowMultiple = true, help = - "Comma-separated list of aspects to be applied to top-level targets. All aspects " - + "are applied to all top-level targets independently. Aspects are specified in " - + "the form <bzl-file-label>%<aspect_name>, " - + "for example '//tools:my_def.bzl%my_aspect', where 'my_aspect' is a top-level " - + "value from from a file tools/my_def.bzl") + "Comma-separated list of aspects to be applied to top-level targets. All aspects are" + + " applied to all top-level targets. If aspect <code>some_aspect</code> specifies" + + " required aspect providers via <code>required_aspect_providers</code>," + + " <code>some_aspect</code> will run after every aspect that was mentioned before it" + + " in the aspects list and whose advertised providers satisfy" + + " <code>some_aspect</code> required aspect providers. <code>some_aspect</code> will" + + " then have access to the values of those aspects' providers. Aspects that do not" + + " have such dependency will run independently on the top-level targets." + + "" + + " Aspects are specified in the form <bzl-file-label>%<aspect_name>, for example" + + " '//tools:my_def.bzl%my_aspect', where 'my_aspect' is a top-level value from a" + + " file tools/my_def.bzl") public List<String> aspects; public BuildRequestOptions() throws OptionsParsingException {}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkAspectClass.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkAspectClass.java index 2b9f2a3..bf0b414 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkAspectClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkAspectClass.java
@@ -64,4 +64,9 @@ public int hashCode() { return Objects.hash(extensionLabel, exportedName); } + + @Override + public String toString() { + return getName(); + } }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java index 391a5b3..f97e08d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java
@@ -13,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; -import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -30,29 +29,15 @@ import com.google.devtools.build.skyframe.SkyFunctionName; import javax.annotation.Nullable; -/** A base class for keys that have AspectValue as a Sky value. */ -public abstract class AspectValueKey implements ActionLookupKey { +/** A wrapper class for sky keys needed to compute sky values for aspects. */ +public final class AspectValueKey { + + private AspectValueKey() {} private static final Interner<AspectKey> aspectKeyInterner = BlazeInterners.newWeakInterner(); - private static final Interner<StarlarkAspectLoadingKey> starlarkAspectKeyInterner = + private static final Interner<TopLevelAspectsKey> topLevelAspectsKeyInterner = BlazeInterners.newWeakInterner(); - /** - * Gets the name of the aspect that would be returned by the corresponding value's {@code - * aspectValue.getAspect().getAspectClass().getName()}, if the value could be produced. - * - * <p>Only needed for reporting errors in BEP when the key's AspectValue fails evaluation. - */ - public abstract String getAspectName(); - - public abstract String getDescription(); - - @Nullable - abstract BuildConfigurationValue.Key getAspectConfigurationKey(); - - /** Returns the key for the base configured target for this aspect. */ - public abstract ConfiguredTargetKey getBaseConfiguredTargetKey(); - public static AspectKey createAspectKey( Label label, @Nullable BuildConfiguration baseConfiguration, @@ -87,33 +72,48 @@ aspectConfiguration == null ? null : BuildConfigurationValue.key(aspectConfiguration)); } - public static StarlarkAspectLoadingKey createStarlarkAspectKey( + public static TopLevelAspectsKey createTopLevelAspectsKey( + ImmutableList<AspectClass> topLevelAspectsClasses, Label targetLabel, - @Nullable BuildConfiguration aspectConfiguration, - @Nullable BuildConfiguration targetConfiguration, - Label starlarkFileLabel, - String starlarkExportName) { - return StarlarkAspectLoadingKey.createInternal( + @Nullable BuildConfiguration configuration) { + return TopLevelAspectsKey.createInternal( + topLevelAspectsClasses, targetLabel, - aspectConfiguration == null ? null : BuildConfigurationValue.key(aspectConfiguration), ConfiguredTargetKey.builder() .setLabel(targetLabel) - .setConfiguration(targetConfiguration) - .build(), - starlarkFileLabel, - starlarkExportName); + .setConfiguration(configuration) + .build()); + } + + /** Common superclass for {@link AspectKey} and {@link TopLevelAspectsKey}. */ + public abstract static class AspectBaseKey implements ActionLookupKey { + private final ConfiguredTargetKey baseConfiguredTargetKey; + private final int hashCode; + + private AspectBaseKey(ConfiguredTargetKey baseConfiguredTargetKey, int hashCode) { + this.baseConfiguredTargetKey = baseConfiguredTargetKey; + this.hashCode = hashCode; + } + + /** Returns the key for the base configured target for this aspect. */ + public final ConfiguredTargetKey getBaseConfiguredTargetKey() { + return baseConfiguredTargetKey; + } + + @Override + public final int hashCode() { + return hashCode; + } } // Specific subtypes of aspect keys. /** Represents an aspect applied to a particular target. */ @AutoCodec - public static final class AspectKey extends AspectValueKey { - private final ConfiguredTargetKey baseConfiguredTargetKey; + public static final class AspectKey extends AspectBaseKey { private final ImmutableList<AspectKey> baseKeys; @Nullable private final BuildConfigurationValue.Key aspectConfigurationKey; private final AspectDescriptor aspectDescriptor; - private final int hashCode; private AspectKey( ConfiguredTargetKey baseConfiguredTargetKey, @@ -121,11 +121,10 @@ AspectDescriptor aspectDescriptor, @Nullable BuildConfigurationValue.Key aspectConfigurationKey, int hashCode) { + super(baseConfiguredTargetKey, hashCode); this.baseKeys = baseKeys; this.aspectConfigurationKey = aspectConfigurationKey; - this.baseConfiguredTargetKey = baseConfiguredTargetKey; this.aspectDescriptor = aspectDescriptor; - this.hashCode = hashCode; } @AutoCodec.VisibleForSerialization @@ -150,14 +149,19 @@ return SkyFunctions.ASPECT; } - @Override + /** + * Gets the name of the aspect that would be returned by the corresponding value's {@code + * aspectValue.getAspect().getAspectClass().getName()}, if the value could be produced. + * + * <p>Only needed for reporting errors in BEP when the key's AspectValue fails evaluation. + */ public String getAspectName() { return aspectDescriptor.getDescription(); } @Override public Label getLabel() { - return baseConfiguredTargetKey.getLabel(); + return getBaseConfiguredTargetKey().getLabel(); } public AspectClass getAspectClass() { @@ -187,7 +191,6 @@ return baseKeys; } - @Override public String getDescription() { if (baseKeys.isEmpty()) { return String.format("%s of %s", aspectDescriptor.getAspectClass().getName(), getLabel()); @@ -215,22 +218,10 @@ * base target's configuration. */ @Nullable - @Override BuildConfigurationValue.Key getAspectConfigurationKey() { return aspectConfigurationKey; } - /** Returns the key for the base configured target for this aspect. */ - @Override - public ConfiguredTargetKey getBaseConfiguredTargetKey() { - return baseConfiguredTargetKey; - } - - @Override - public int hashCode() { - return hashCode; - } - @Override public boolean equals(Object other) { if (this == other) { @@ -240,10 +231,10 @@ return false; } AspectKey that = (AspectKey) other; - return hashCode == that.hashCode + return hashCode() == that.hashCode() && Objects.equal(baseKeys, that.baseKeys) && Objects.equal(aspectConfigurationKey, that.aspectConfigurationKey) - && Objects.equal(baseConfiguredTargetKey, that.baseConfiguredTargetKey) + && Objects.equal(getBaseConfiguredTargetKey(), that.getBaseConfiguredTargetKey()) && Objects.equal(aspectDescriptor, that.aspectDescriptor); } @@ -266,7 +257,7 @@ + " " + aspectConfigurationKey + " " - + baseConfiguredTargetKey + + getBaseConfiguredTargetKey() + " " + aspectDescriptor.getParameters(); } @@ -280,7 +271,7 @@ return createAspectKey( ConfiguredTargetKey.builder() .setLabel(label) - .setConfigurationKey(baseConfiguredTargetKey.getConfigurationKey()) + .setConfigurationKey(getBaseConfiguredTargetKey().getConfigurationKey()) .build(), newBaseKeys.build(), aspectDescriptor, @@ -288,70 +279,43 @@ } } - /** The key for a Starlark aspect. */ + /** The key for top level aspects specified by --aspects option on a top level target. */ @AutoCodec - public static final class StarlarkAspectLoadingKey extends AspectValueKey { + public static final class TopLevelAspectsKey extends AspectBaseKey { + private final ImmutableList<AspectClass> topLevelAspectsClasses; private final Label targetLabel; - private final BuildConfigurationValue.Key aspectConfigurationKey; - private final ConfiguredTargetKey baseConfiguredTargetKey; - private final Label starlarkFileLabel; - private final String starlarkValueName; - private final int hashCode; @AutoCodec.Instantiator @AutoCodec.VisibleForSerialization - static StarlarkAspectLoadingKey createInternal( + static TopLevelAspectsKey createInternal( + ImmutableList<AspectClass> topLevelAspectsClasses, Label targetLabel, - BuildConfigurationValue.Key aspectConfigurationKey, - ConfiguredTargetKey baseConfiguredTargetKey, - Label starlarkFileLabel, - String starlarkValueName) { - return starlarkAspectKeyInterner.intern( - new StarlarkAspectLoadingKey( + ConfiguredTargetKey baseConfiguredTargetKey) { + return topLevelAspectsKeyInterner.intern( + new TopLevelAspectsKey( + topLevelAspectsClasses, targetLabel, - aspectConfigurationKey, baseConfiguredTargetKey, - starlarkFileLabel, - starlarkValueName, - Objects.hashCode( - targetLabel, - aspectConfigurationKey, - baseConfiguredTargetKey, - starlarkFileLabel, - starlarkValueName))); + Objects.hashCode(topLevelAspectsClasses, targetLabel, baseConfiguredTargetKey))); } - private StarlarkAspectLoadingKey( + private TopLevelAspectsKey( + ImmutableList<AspectClass> topLevelAspectsClasses, Label targetLabel, - BuildConfigurationValue.Key aspectConfigurationKey, ConfiguredTargetKey baseConfiguredTargetKey, - Label starlarkFileLabel, - String starlarkValueName, int hashCode) { + super(baseConfiguredTargetKey, hashCode); + this.topLevelAspectsClasses = topLevelAspectsClasses; this.targetLabel = targetLabel; - this.aspectConfigurationKey = aspectConfigurationKey; - this.baseConfiguredTargetKey = baseConfiguredTargetKey; - this.starlarkFileLabel = starlarkFileLabel; - this.starlarkValueName = starlarkValueName; - this.hashCode = hashCode; } @Override public SkyFunctionName functionName() { - return SkyFunctions.LOAD_STARLARK_ASPECT; + return SkyFunctions.TOP_LEVEL_ASPECTS; } - String getStarlarkValueName() { - return starlarkValueName; - } - - Label getStarlarkFileLabel() { - return starlarkFileLabel; - } - - @Override - public String getAspectName() { - return String.format("%s%%%s", starlarkFileLabel, starlarkValueName); + ImmutableList<AspectClass> getTopLevelAspectsClasses() { + return topLevelAspectsClasses; } @Override @@ -359,27 +323,8 @@ return targetLabel; } - @Override - public String getDescription() { - // Starlark aspects are referred to on command line with <file>%<value ame> - return String.format("%s%%%s of %s", starlarkFileLabel, starlarkValueName, targetLabel); - } - - @Nullable - @Override - BuildConfigurationValue.Key getAspectConfigurationKey() { - return aspectConfigurationKey; - } - - /** Returns the key for the base configured target for this aspect. */ - @Override - public ConfiguredTargetKey getBaseConfiguredTargetKey() { - return baseConfiguredTargetKey; - } - - @Override - public int hashCode() { - return hashCode; + String getDescription() { + return topLevelAspectsClasses + " on " + getLabel(); } @Override @@ -387,35 +332,14 @@ if (o == this) { return true; } - if (!(o instanceof StarlarkAspectLoadingKey)) { + if (!(o instanceof TopLevelAspectsKey)) { return false; } - StarlarkAspectLoadingKey that = (StarlarkAspectLoadingKey) o; - return hashCode == that.hashCode + TopLevelAspectsKey that = (TopLevelAspectsKey) o; + return hashCode() == that.hashCode() && Objects.equal(targetLabel, that.targetLabel) - && Objects.equal(aspectConfigurationKey, that.aspectConfigurationKey) - && Objects.equal(baseConfiguredTargetKey, that.baseConfiguredTargetKey) - && Objects.equal(starlarkFileLabel, that.starlarkFileLabel) - && Objects.equal(starlarkValueName, that.starlarkValueName); - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("targetLabel", targetLabel) - .add("aspectConfigurationKey", aspectConfigurationKey) - .add("baseConfiguredTargetKey", baseConfiguredTargetKey) - .add("starlarkFileLabel", starlarkFileLabel) - .add("starlarkValueName", starlarkValueName) - .toString(); - } - - AspectKey toAspectKey(AspectClass aspectClass) { - return AspectKey.createAspectKey( - baseConfiguredTargetKey, - ImmutableList.of(), - new AspectDescriptor(aspectClass, AspectParameters.EMPTY), - aspectConfigurationKey); + && Objects.equal(getBaseConfiguredTargetKey(), that.getBaseConfiguredTargetKey()) + && Objects.equal(topLevelAspectsClasses, that.topLevelAspectsClasses); } } }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index 39518ce..ac2ee83 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -26,6 +26,7 @@ "BazelSkyframeExecutorConstants.java", "BuildConfigurationFunction.java", "BuildInfoCollectionFunction.java", + "BuildTopLevelAspectsDetailsFunction.java", "BzlLoadFunction.java", "BzlmodRepoRuleFunction.java", "CompletionFunction.java", @@ -38,6 +39,7 @@ "ExternalFilesHelper.java", "ExternalPackageFunction.java", "FileStateFunction.java", + "LoadStarlarkAspectFunction.java", "LocalRepositoryLookupFunction.java", "NonRuleConfiguredTargetValue.java", "PackageFunction.java",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildTopLevelAspectsDetailsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildTopLevelAspectsDetailsFunction.java new file mode 100644 index 0000000..eb68e55 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildTopLevelAspectsDetailsFunction.java
@@ -0,0 +1,302 @@ +// Copyright 2021 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.skyframe; + +import com.google.common.base.Objects; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Interner; +import com.google.devtools.build.lib.analysis.AspectCollection; +import com.google.devtools.build.lib.analysis.AspectCollection.AspectCycleOnPathException; +import com.google.devtools.build.lib.concurrent.BlazeInterners; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.packages.Aspect; +import com.google.devtools.build.lib.packages.AspectClass; +import com.google.devtools.build.lib.packages.AspectDescriptor; +import com.google.devtools.build.lib.packages.AspectsListBuilder; +import com.google.devtools.build.lib.packages.NativeAspectClass; +import com.google.devtools.build.lib.packages.StarlarkAspect; +import com.google.devtools.build.lib.packages.StarlarkAspectClass; +import com.google.devtools.build.lib.server.FailureDetails.Analysis; +import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code; +import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; +import com.google.devtools.build.lib.skyframe.LoadStarlarkAspectFunction.StarlarkAspectLoadingKey; +import com.google.devtools.build.lib.skyframe.LoadStarlarkAspectFunction.StarlarkAspectLoadingValue; +import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; +import com.google.devtools.build.lib.util.DetailedExitCode; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyFunctionException; +import com.google.devtools.build.skyframe.SkyFunctionName; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import javax.annotation.Nullable; +import net.starlark.java.eval.EvalException; + +/** + * SkyFunction to load top level aspects, build the dependency relation between them based on the + * aspects required by the top level aspects and the aspect providers they require and advertise + * using {@link AspectCollection}. + * + * <p>This is needed to compute the relationship between top-level aspects once for all top-level + * targets in the command. The {@link SkyValue} of this function contains a list of {@link + * AspectDetails} objects which contain the aspect descriptor and a list of the used aspects this + * aspect depends on. Then {@link ToplevelStarlarkAspectFunction} adds the target information to + * create {@link AspectKey}. + */ +public class BuildTopLevelAspectsDetailsFunction implements SkyFunction { + BuildTopLevelAspectsDetailsFunction() {} + + private static final Interner<BuildTopLevelAspectsDetailsKey> + buildTopLevelAspectsDetailsKeyInterner = BlazeInterners.newWeakInterner(); + + @Nullable + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws BuildTopLevelAspectsDetailsFunctionException, InterruptedException { + BuildTopLevelAspectsDetailsKey topLevelAspectsDetailsKey = + (BuildTopLevelAspectsDetailsKey) skyKey.argument(); + + ImmutableList<Aspect> topLevelAspects = + getTopLevelAspects(env, topLevelAspectsDetailsKey.getTopLevelAspectsClasses()); + + if (topLevelAspects == null) { + return null; // some aspects are not loaded + } + + AspectCollection aspectCollection; + try { + aspectCollection = AspectCollection.create(topLevelAspects); + } catch (AspectCycleOnPathException e) { + // This exception should never happen because aspects duplicates are not allowed in top-level + // aspects and their existence should have been caught and reported by `getTopLevelAspects()`. + env.getListener().handle(Event.error(e.getMessage())); + throw new BuildTopLevelAspectsDetailsFunctionException( + new TopLevelAspectsDetailsBuildFailedException(e.getMessage())); + } + return new BuildTopLevelAspectsDetailsValue(getTopLevelAspectsDetails(aspectCollection)); + } + + @Nullable + @Override + public String extractTag(SkyKey skyKey) { + return null; + } + + @Nullable + private static ImmutableList<Aspect> getTopLevelAspects( + Environment env, ImmutableList<AspectClass> topLevelAspectsClasses) + throws InterruptedException, BuildTopLevelAspectsDetailsFunctionException { + AspectsListBuilder aspectsList = new AspectsListBuilder(); + + ImmutableList.Builder<StarlarkAspectLoadingKey> aspectLoadingKeys = ImmutableList.builder(); + for (AspectClass aspectClass : topLevelAspectsClasses) { + if (aspectClass instanceof StarlarkAspectClass) { + aspectLoadingKeys.add( + LoadStarlarkAspectFunction.createStarlarkAspectLoadingKey( + (StarlarkAspectClass) aspectClass)); + } + } + + Map<SkyKey, SkyValue> loadedAspects = env.getValues(aspectLoadingKeys.build()); + if (env.valuesMissing()) { + return null; + } + + for (AspectClass aspectClass : topLevelAspectsClasses) { + if (aspectClass instanceof StarlarkAspectClass) { + StarlarkAspectLoadingValue aspectLoadingValue = + (StarlarkAspectLoadingValue) + loadedAspects.get( + LoadStarlarkAspectFunction.createStarlarkAspectLoadingKey( + (StarlarkAspectClass) aspectClass)); + StarlarkAspect starlarkAspect = aspectLoadingValue.getAspect(); + try { + starlarkAspect.attachToAspectsList( + /** baseAspectName= */ + null, + aspectsList, + /** inheritedRequiredProviders= */ + ImmutableList.of(), + /** inheritedAttributeAspects= */ + ImmutableList.of(), + /** allowAspectsParameters= */ + false); + } catch (EvalException e) { + env.getListener().handle(Event.error(e.getMessage())); + throw new BuildTopLevelAspectsDetailsFunctionException( + new TopLevelAspectsDetailsBuildFailedException(e.getMessage())); + } + } else { + try { + aspectsList.addAspect((NativeAspectClass) aspectClass); + } catch (AssertionError ex) { + env.getListener().handle(Event.error(ex.getMessage())); + throw new BuildTopLevelAspectsDetailsFunctionException( + new TopLevelAspectsDetailsBuildFailedException(ex.getMessage())); + } + } + } + return aspectsList.buildAspects(); + } + + private static Collection<AspectDetails> getTopLevelAspectsDetails( + AspectCollection aspectCollection) { + Map<AspectDescriptor, AspectDetails> result = new HashMap<>(); + for (AspectCollection.AspectDeps aspectDeps : aspectCollection.getUsedAspects()) { + buildAspectDetails(aspectDeps, result); + } + return result.values(); + } + + private static AspectDetails buildAspectDetails( + AspectCollection.AspectDeps aspectDeps, Map<AspectDescriptor, AspectDetails> result) { + if (result.containsKey(aspectDeps.getAspect())) { + return result.get(aspectDeps.getAspect()); + } + + ImmutableList.Builder<AspectDetails> dependentAspects = ImmutableList.builder(); + for (AspectCollection.AspectDeps path : aspectDeps.getUsedAspects()) { + dependentAspects.add(buildAspectDetails(path, result)); + } + + AspectDetails aspectDetails = + new AspectDetails(dependentAspects.build(), aspectDeps.getAspect()); + result.put(aspectDetails.getAspectDescriptor(), aspectDetails); + return aspectDetails; + } + + public static BuildTopLevelAspectsDetailsKey createBuildTopLevelAspectsDetailsKey( + ImmutableList<AspectClass> aspectClasses) { + return BuildTopLevelAspectsDetailsKey.createInternal(aspectClasses); + } + + /** Exceptions thrown from BuildTopLevelAspectsDetailsFunction. */ + public static class BuildTopLevelAspectsDetailsFunctionException extends SkyFunctionException { + public BuildTopLevelAspectsDetailsFunctionException( + TopLevelAspectsDetailsBuildFailedException cause) { + super(cause, Transience.PERSISTENT); + } + } + + static final class TopLevelAspectsDetailsBuildFailedException extends Exception + implements SaneAnalysisException { + private final DetailedExitCode detailedExitCode; + + private TopLevelAspectsDetailsBuildFailedException(String errorMessage) { + super(errorMessage); + this.detailedExitCode = + DetailedExitCode.of( + FailureDetail.newBuilder() + .setMessage(errorMessage) + .setAnalysis(Analysis.newBuilder().setCode(Code.ASPECT_CREATION_FAILED)) + .build()); + } + + @Override + public DetailedExitCode getDetailedExitCode() { + return detailedExitCode; + } + } + + /** + * Details of the top-level aspects including the {@link AspectDescriptor} and a list of the + * aspects it depends on. This is used to build the {@link AspectKey} when combined with + * configured target details. + */ + public static final class AspectDetails { + private final ImmutableList<AspectDetails> usedAspects; + private final AspectDescriptor aspectDescriptor; + + AspectDetails(ImmutableList<AspectDetails> usedAspects, AspectDescriptor aspectDescriptor) { + this.usedAspects = usedAspects; + this.aspectDescriptor = aspectDescriptor; + } + + public AspectDescriptor getAspectDescriptor() { + return aspectDescriptor; + } + + public ImmutableList<AspectDetails> getUsedAspects() { + return usedAspects; + } + } + + /** SkyKey for building top-level aspects details. */ + public static final class BuildTopLevelAspectsDetailsKey implements SkyKey { + private final ImmutableList<AspectClass> topLevelAspectsClasses; + private final int hashCode; + + @AutoCodec.Instantiator + @AutoCodec.VisibleForSerialization + static BuildTopLevelAspectsDetailsKey createInternal( + ImmutableList<AspectClass> topLevelAspectsClasses) { + return buildTopLevelAspectsDetailsKeyInterner.intern( + new BuildTopLevelAspectsDetailsKey( + topLevelAspectsClasses, java.util.Objects.hashCode(topLevelAspectsClasses))); + } + + private BuildTopLevelAspectsDetailsKey( + ImmutableList<AspectClass> topLevelAspectsClasses, int hashCode) { + this.topLevelAspectsClasses = topLevelAspectsClasses; + this.hashCode = hashCode; + } + + @Override + public SkyFunctionName functionName() { + return SkyFunctions.BUILD_TOP_LEVEL_ASPECTS_DETAILS; + } + + ImmutableList<AspectClass> getTopLevelAspectsClasses() { + return topLevelAspectsClasses; + } + + @Override + public int hashCode() { + return hashCode; + } + + @Override + public boolean equals(Object o) { + if (o == this) { + return true; + } + if (!(o instanceof BuildTopLevelAspectsDetailsKey)) { + return false; + } + BuildTopLevelAspectsDetailsKey that = (BuildTopLevelAspectsDetailsKey) o; + return hashCode == that.hashCode + && Objects.equal(topLevelAspectsClasses, that.topLevelAspectsClasses); + } + } + + /** + * SkyValue for {@code BuildTopLevelAspectsDetailsKey} wraps a list of the {@code AspectDetails} + * of the top level aspects. + */ + public static final class BuildTopLevelAspectsDetailsValue implements SkyValue { + private final ImmutableList<AspectDetails> aspectsDetails; + + private BuildTopLevelAspectsDetailsValue(Collection<AspectDetails> aspectsDetails) { + this.aspectsDetails = ImmutableList.copyOf(aspectsDetails); + } + + public ImmutableList<AspectDetails> getAspectsDetails() { + return aspectsDetails; + } + } +}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/LoadStarlarkAspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/LoadStarlarkAspectFunction.java new file mode 100644 index 0000000..4d81910 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/LoadStarlarkAspectFunction.java
@@ -0,0 +1,169 @@ +// Copyright 2021 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.skyframe; + +import com.google.common.base.Objects; +import com.google.common.collect.Interner; +import com.google.devtools.build.lib.causes.LabelCause; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.concurrent.BlazeInterners; +import com.google.devtools.build.lib.packages.StarlarkAspect; +import com.google.devtools.build.lib.packages.StarlarkAspectClass; +import com.google.devtools.build.lib.server.FailureDetails.Analysis; +import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code; +import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; +import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; +import com.google.devtools.build.lib.util.DetailedExitCode; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyFunctionException; +import com.google.devtools.build.skyframe.SkyFunctionName; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import javax.annotation.Nullable; + +/** + * SkyFunction to load aspects from Starlark extensions and return StarlarkAspect. + * + * <p>Used for loading top-level aspects. At top level, in {@link + * com.google.devtools.build.lib.analysis.BuildView}, we cannot invoke two SkyFunctions one after + * another, so BuildView calls this function to do the work. + */ +public class LoadStarlarkAspectFunction implements SkyFunction { + private static final Interner<StarlarkAspectLoadingKey> starlarkAspectLoadingKeyInterner = + BlazeInterners.newWeakInterner(); + + LoadStarlarkAspectFunction() {} + + @Nullable + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws LoadStarlarkAspectFunctionException, InterruptedException { + StarlarkAspectLoadingKey aspectLoadingKey = (StarlarkAspectLoadingKey) skyKey.argument(); + + Label extensionLabel = aspectLoadingKey.getAspectClass().getExtensionLabel(); + String exportedName = aspectLoadingKey.getAspectClass().getExportedName(); + StarlarkAspect starlarkAspect; + try { + starlarkAspect = AspectFunction.loadStarlarkAspect(env, extensionLabel, exportedName); + if (starlarkAspect == null) { + return null; + } + if (!starlarkAspect.getParamAttributes().isEmpty()) { + String msg = + String.format( + "Cannot instantiate parameterized aspect %s at the top level.", + starlarkAspect.getName()); + throw new AspectCreationException( + msg, + new LabelCause( + extensionLabel, + createDetailedCode(msg, Code.PARAMETERIZED_TOP_LEVEL_ASPECT_INVALID))); + } + } catch (AspectCreationException e) { + throw new LoadStarlarkAspectFunctionException(e); + } + + return new StarlarkAspectLoadingValue(starlarkAspect); + } + + @Nullable + @Override + public String extractTag(SkyKey skyKey) { + return null; + } + + private static DetailedExitCode createDetailedCode(String msg, Code code) { + return DetailedExitCode.of( + FailureDetail.newBuilder() + .setMessage(msg) + .setAnalysis(Analysis.newBuilder().setCode(code)) + .build()); + } + + /** Exceptions thrown from LoadStarlarkAspectFunction. */ + public static class LoadStarlarkAspectFunctionException extends SkyFunctionException { + public LoadStarlarkAspectFunctionException(AspectCreationException cause) { + super(cause, Transience.PERSISTENT); + } + } + + public static StarlarkAspectLoadingKey createStarlarkAspectLoadingKey( + StarlarkAspectClass aspectClass) { + return StarlarkAspectLoadingKey.createInternal(aspectClass); + } + + /** Skykey for loading Starlark aspect. */ + @AutoCodec + public static final class StarlarkAspectLoadingKey implements SkyKey { + private final StarlarkAspectClass aspectClass; + private final int hashCode; + + @AutoCodec.Instantiator + @AutoCodec.VisibleForSerialization + static StarlarkAspectLoadingKey createInternal(StarlarkAspectClass aspectClass) { + return starlarkAspectLoadingKeyInterner.intern( + new StarlarkAspectLoadingKey(aspectClass, java.util.Objects.hashCode(aspectClass))); + } + + private StarlarkAspectLoadingKey(StarlarkAspectClass aspectClass, int hashCode) { + this.aspectClass = aspectClass; + this.hashCode = hashCode; + } + + @Override + public SkyFunctionName functionName() { + return SkyFunctions.LOAD_STARLARK_ASPECT; + } + + StarlarkAspectClass getAspectClass() { + return aspectClass; + } + + @Override + public int hashCode() { + return hashCode; + } + + @Override + public boolean equals(Object o) { + if (o == this) { + return true; + } + if (!(o instanceof StarlarkAspectLoadingKey)) { + return false; + } + StarlarkAspectLoadingKey that = (StarlarkAspectLoadingKey) o; + return hashCode == that.hashCode && Objects.equal(aspectClass, that.aspectClass); + } + + @Override + public String toString() { + return aspectClass.toString(); + } + } + + /** SkyValue for {@code StarlarkAspectLoadingKey} holds the loaded {@code StarlarkAspect}. */ + public static class StarlarkAspectLoadingValue implements SkyValue { + private final StarlarkAspect starlarkAspect; + + public StarlarkAspectLoadingValue(StarlarkAspect starlarkAspect) { + this.starlarkAspect = starlarkAspect; + } + + public StarlarkAspect getAspect() { + return starlarkAspect; + } + } +}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java index cb07796..c94ce04 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
@@ -88,6 +88,10 @@ public static final SkyFunctionName ASPECT = SkyFunctionName.createHermetic("ASPECT"); static final SkyFunctionName LOAD_STARLARK_ASPECT = SkyFunctionName.createHermetic("LOAD_STARLARK_ASPECT"); + static final SkyFunctionName TOP_LEVEL_ASPECTS = + SkyFunctionName.createHermetic("TOP_LEVEL_ASPECTS"); + static final SkyFunctionName BUILD_TOP_LEVEL_ASPECTS_DETAILS = + SkyFunctionName.createHermetic("BUILD_TOP_LEVEL_ASPECTS_DETAILS"); public static final SkyFunctionName TARGET_COMPLETION = SkyFunctionName.create( "TARGET_COMPLETION", ShareabilityOfValue.NEVER, FunctionHermeticity.HERMETIC);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java index c718762..792dc66 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -88,7 +88,9 @@ import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.skyframe.ArtifactConflictFinder.ConflictException; import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.TopLevelAspectsKey; import com.google.devtools.build.lib.skyframe.SkyframeExecutor.TopLevelActionConflictReport; +import com.google.devtools.build.lib.skyframe.ToplevelStarlarkAspectFunction.TopLevelAspectsValue; import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.OrderedSetMultimap; import com.google.devtools.build.lib.util.Pair; @@ -380,7 +382,7 @@ public SkyframeAnalysisResult configureTargets( ExtendedEventHandler eventHandler, List<ConfiguredTargetKey> ctKeys, - List<AspectValueKey> aspectKeys, + ImmutableList<TopLevelAspectsKey> topLevelAspectsKey, Supplier<Map<BuildConfigurationValue.Key, BuildConfiguration>> configurationLookupSupplier, TopLevelArtifactContext topLevelArtifactContextForConflictPruning, EventBus eventBus, @@ -397,7 +399,7 @@ skyframeExecutor.configureTargets( eventHandler, ctKeys, - aspectKeys, + topLevelAspectsKey, keepGoing, numThreads, cpuHeavySkyKeysThreadPoolSize); @@ -405,21 +407,33 @@ enableAnalysis(false); } - Map<AspectKey, ConfiguredAspect> aspects = Maps.newHashMapWithExpectedSize(aspectKeys.size()); + int numOfAspects = 0; + if (!topLevelAspectsKey.isEmpty()) { + numOfAspects = + topLevelAspectsKey.size() * topLevelAspectsKey.get(0).getTopLevelAspectsClasses().size(); + } + Map<AspectKey, ConfiguredAspect> aspects = Maps.newHashMapWithExpectedSize(numOfAspects); Root singleSourceRoot = skyframeExecutor.getForcedSingleSourceRootIfNoExecrootSymlinkCreation(); NestedSetBuilder<Package> packages = singleSourceRoot == null ? NestedSetBuilder.stableOrder() : null; - for (AspectValueKey aspectKey : aspectKeys) { - AspectValue value = (AspectValue) result.get(aspectKey); + ImmutableList.Builder<AspectKey> aspectKeysBuilder = ImmutableList.builder(); + + for (TopLevelAspectsKey key : topLevelAspectsKey) { + TopLevelAspectsValue value = (TopLevelAspectsValue) result.get(key); if (value == null) { // Skip aspects that couldn't be applied to targets. continue; } - aspects.put(value.getKey(), value.getConfiguredAspect()); - if (packages != null) { - packages.addTransitive(value.getTransitivePackagesForPackageRootResolution()); + for (SkyValue val : value.getTopLevelAspectsValues()) { + AspectValue aspectValue = (AspectValue) val; + aspects.put(aspectValue.getKey(), aspectValue.getConfiguredAspect()); + if (packages != null) { + packages.addTransitive(aspectValue.getTransitivePackagesForPackageRootResolution()); + } + aspectKeysBuilder.add(aspectValue.getKey()); } } + ImmutableList<AspectKey> aspectKeys = aspectKeysBuilder.build(); Collection<ConfiguredTarget> cts = Lists.newArrayListWithCapacity(ctKeys.size()); for (ConfiguredTargetKey value : ctKeys) { @@ -561,7 +575,7 @@ BuildConfigurationValue.Key configKey = ctKey instanceof ConfiguredTargetKey ? ((ConfiguredTargetKey) ctKey).getConfigurationKey() - : ((AspectValueKey) ctKey).getAspectConfigurationKey(); + : ((AspectKey) ctKey).getAspectConfigurationKey(); eventBus.post( new AnalysisFailureEvent( ctKey, @@ -597,13 +611,9 @@ .collect(toImmutableList()); aspects = - aspectKeys.stream() - .filter(topLevelActionConflictReport::isErrorFree) - .map(result::get) - .map(AspectValue.class::cast) - .collect( - ImmutableMap.toImmutableMap( - AspectValue::getKey, AspectValue::getConfiguredAspect)); + aspects.entrySet().stream() + .filter(e -> topLevelActionConflictReport.isErrorFree(e.getKey())) + .collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); } return new SkyframeAnalysisResult( @@ -745,17 +755,15 @@ .reportCycles(errorInfo.getCycleInfo(), errorKey, eventHandler); Exception cause = errorInfo.getException(); Preconditions.checkState(cause != null || !errorInfo.getCycleInfo().isEmpty(), errorInfo); - - if (errorKey.argument() instanceof AspectValueKey) { + if (errorKey.argument() instanceof TopLevelAspectsKey) { // We skip Aspects in the keepGoing case; the failures should already have been reported to // the event handler. if (!keepGoing && noKeepGoingException == null) { - AspectValueKey aspectKey = (AspectValueKey) errorKey.argument(); + TopLevelAspectsKey aspectKey = (TopLevelAspectsKey) errorKey.argument(); failedAspectLabel = aspectKey.getBaseConfiguredTargetKey(); - String errorMsg = String.format( - "Analysis of aspect '%s' failed; build aborted", aspectKey.getDescription()); + "Analysis of aspects '%s' failed; build aborted", aspectKey.getDescription()); noKeepGoingException = createViewCreationFailedException(cause, errorMsg); } continue; @@ -776,7 +784,7 @@ } Preconditions.checkState( errorKey.argument() instanceof ConfiguredTargetKey, - "expected '%s' to be a AspectValueKey or ConfiguredTargetKey", + "expected '%s' to be a TopLevelAspectsKey or ConfiguredTargetKey", errorKey.argument()); ConfiguredTargetKey label = (ConfiguredTargetKey) errorKey.argument(); Label topLevelLabel = label.getLabel();
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 d5e9066..74521b1 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
@@ -162,6 +162,7 @@ import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns; import com.google.devtools.build.lib.skyframe.ArtifactConflictFinder.ConflictException; import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.TopLevelAspectsKey; import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.FileDirtinessChecker; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.MetadataConsumerForMetrics.FilesMetricConsumer; @@ -574,7 +575,10 @@ new BuildViewProvider(), ruleClassProvider, shouldStoreTransitivePackagesInLoadingAndAnalysis())); - map.put(SkyFunctions.LOAD_STARLARK_ASPECT, new ToplevelStarlarkAspectFunction()); + map.put(SkyFunctions.LOAD_STARLARK_ASPECT, new LoadStarlarkAspectFunction()); + map.put(SkyFunctions.TOP_LEVEL_ASPECTS, new ToplevelStarlarkAspectFunction()); + map.put( + SkyFunctions.BUILD_TOP_LEVEL_ASPECTS_DETAILS, new BuildTopLevelAspectsDetailsFunction()); map.put(SkyFunctions.ACTION_LOOKUP_CONFLICT_FINDING, new ActionLookupConflictFindingFunction()); map.put( SkyFunctions.TOP_LEVEL_ACTION_LOOKUP_CONFLICT_FINDING, @@ -2345,7 +2349,7 @@ EvaluationResult<ActionLookupValue> configureTargets( ExtendedEventHandler eventHandler, List<ConfiguredTargetKey> values, - List<AspectValueKey> aspectKeys, + ImmutableList<TopLevelAspectsKey> aspectKeys, boolean keepGoing, int numThreads, int cpuHeavySkyKeysThreadPoolSize) @@ -3082,7 +3086,7 @@ } final AnalysisTraversalResult getActionLookupValuesInBuild( - List<ConfiguredTargetKey> topLevelCtKeys, List<AspectValueKey> aspectKeys) + List<ConfiguredTargetKey> topLevelCtKeys, ImmutableList<AspectKey> aspectKeys) throws InterruptedException { AnalysisTraversalResult result = new AnalysisTraversalResult(); if (!isAnalysisIncremental()) { @@ -3102,7 +3106,7 @@ for (ConfiguredTargetKey key : topLevelCtKeys) { findActionsRecursively(walkableGraph, key, seen, result); } - for (AspectValueKey key : aspectKeys) { + for (AspectKey key : aspectKeys) { findActionsRecursively(walkableGraph, key, seen, result); } return result;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCycleReporter.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCycleReporter.java index cf8f828..693bde7 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCycleReporter.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCycleReporter.java
@@ -39,7 +39,7 @@ Predicates.or( SkyFunctions.isSkyFunction(SkyFunctions.CONFIGURED_TARGET), SkyFunctions.isSkyFunction(SkyFunctions.ASPECT), - SkyFunctions.isSkyFunction(SkyFunctions.LOAD_STARLARK_ASPECT), + SkyFunctions.isSkyFunction(SkyFunctions.TOP_LEVEL_ASPECTS), SkyFunctions.isSkyFunction(TransitiveTargetKey.NAME), SkyFunctions.isSkyFunction(SkyFunctions.PREPARE_ANALYSIS_PHASE)); @@ -65,8 +65,6 @@ return ((ConfiguredTargetKey) key.argument()).prettyPrint(); } else if (key instanceof AspectKey) { return ((AspectKey) key.argument()).prettyPrint(); - } else if (key instanceof AspectValueKey) { - return ((AspectValueKey) key).getDescription(); } else { return getLabel(key).toString(); }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelStarlarkAspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelStarlarkAspectFunction.java index a858e35..4fd3bb1 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelStarlarkAspectFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelStarlarkAspectFunction.java
@@ -14,22 +14,26 @@ package com.google.devtools.build.lib.skyframe; -import com.google.devtools.build.lib.causes.LabelCause; -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.packages.StarlarkAspect; -import com.google.devtools.build.lib.server.FailureDetails.Analysis; -import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code; -import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; -import com.google.devtools.build.lib.skyframe.AspectValueKey.StarlarkAspectLoadingKey; -import com.google.devtools.build.lib.util.DetailedExitCode; +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; +import com.google.devtools.build.lib.actions.ActionLookupValue; +import com.google.devtools.build.lib.packages.AspectDescriptor; +import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; +import com.google.devtools.build.lib.skyframe.AspectValueKey.TopLevelAspectsKey; +import com.google.devtools.build.lib.skyframe.BuildTopLevelAspectsDetailsFunction.AspectDetails; +import com.google.devtools.build.lib.skyframe.BuildTopLevelAspectsDetailsFunction.BuildTopLevelAspectsDetailsValue; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; import javax.annotation.Nullable; /** - * SkyFunction to load aspects from Starlark extensions and calculate their values. + * SkyFunction to run the aspects path obtained from top-level aspects on the list of top-level + * targets. * * <p>Used for loading top-level aspects. At top level, in {@link * com.google.devtools.build.lib.analysis.BuildView}, we cannot invoke two SkyFunctions one after @@ -41,34 +45,29 @@ @Nullable @Override public SkyValue compute(SkyKey skyKey, Environment env) - throws LoadStarlarkAspectFunctionException, InterruptedException { - StarlarkAspectLoadingKey aspectLoadingKey = (StarlarkAspectLoadingKey) skyKey.argument(); - String starlarkValueName = aspectLoadingKey.getStarlarkValueName(); - Label starlarkFileLabel = aspectLoadingKey.getStarlarkFileLabel(); + throws TopLevelStarlarkAspectFunctionException, InterruptedException { + TopLevelAspectsKey topLevelAspectsKey = (TopLevelAspectsKey) skyKey.argument(); - StarlarkAspect starlarkAspect; - try { - starlarkAspect = AspectFunction.loadStarlarkAspect(env, starlarkFileLabel, starlarkValueName); - if (starlarkAspect == null) { - return null; - } - if (!starlarkAspect.getParamAttributes().isEmpty()) { - String msg = - String.format( - "Cannot instantiate parameterized aspect %s at the top level.", - starlarkAspect.getName()); - throw new AspectCreationException( - msg, - new LabelCause( - starlarkFileLabel, - createDetailedCode(msg, Code.PARAMETERIZED_TOP_LEVEL_ASPECT_INVALID))); - } - } catch (AspectCreationException e) { - throw new LoadStarlarkAspectFunctionException(e); + BuildTopLevelAspectsDetailsValue topLevelAspectsDetails = + (BuildTopLevelAspectsDetailsValue) + env.getValue( + BuildTopLevelAspectsDetailsFunction.createBuildTopLevelAspectsDetailsKey( + topLevelAspectsKey.getTopLevelAspectsClasses())); + if (topLevelAspectsDetails == null) { + return null; // some aspects details are not ready } - SkyKey aspectKey = aspectLoadingKey.toAspectKey(starlarkAspect.getAspectClass()); - return env.getValue(aspectKey); + Collection<AspectKey> aspectsKeys = + getTopLevelAspectsKeys( + topLevelAspectsDetails.getAspectsDetails(), + topLevelAspectsKey.getBaseConfiguredTargetKey()); + + Map<SkyKey, SkyValue> result = env.getValues(aspectsKeys); + if (env.valuesMissing()) { + return null; // some aspects keys are not evaluated + } + + return new TopLevelAspectsValue(result.values()); } @Nullable @@ -77,18 +76,63 @@ return null; } - private static DetailedExitCode createDetailedCode(String msg, Code code) { - return DetailedExitCode.of( - FailureDetail.newBuilder() - .setMessage(msg) - .setAnalysis(Analysis.newBuilder().setCode(code)) - .build()); + private static Collection<AspectKey> getTopLevelAspectsKeys( + ImmutableList<AspectDetails> aspectsDetails, ConfiguredTargetKey topLevelTargetKey) { + Map<AspectDescriptor, AspectKey> result = new HashMap<>(); + for (AspectDetails aspect : aspectsDetails) { + buildAspectKey(aspect, result, topLevelTargetKey); + } + return result.values(); + } + + private static AspectKey buildAspectKey( + AspectDetails aspect, + Map<AspectDescriptor, AspectKey> result, + ConfiguredTargetKey topLevelTargetKey) { + if (result.containsKey(aspect.getAspectDescriptor())) { + return result.get(aspect.getAspectDescriptor()); + } + + ImmutableList.Builder<AspectKey> dependentAspects = ImmutableList.builder(); + for (AspectDetails depAspect : aspect.getUsedAspects()) { + dependentAspects.add(buildAspectKey(depAspect, result, topLevelTargetKey)); + } + + AspectKey aspectKey = + AspectValueKey.createAspectKey( + aspect.getAspectDescriptor(), + dependentAspects.build(), + topLevelTargetKey.getConfigurationKey(), + topLevelTargetKey); + result.put(aspectKey.getAspectDescriptor(), aspectKey); + return aspectKey; } /** Exceptions thrown from ToplevelStarlarkAspectFunction. */ - public static class LoadStarlarkAspectFunctionException extends SkyFunctionException { - public LoadStarlarkAspectFunctionException(AspectCreationException cause) { + public static class TopLevelStarlarkAspectFunctionException extends SkyFunctionException { + public TopLevelStarlarkAspectFunctionException(AspectCreationException cause) { super(cause, Transience.PERSISTENT); } } + + /** + * SkyValue for {@code TopLevelAspectsKey} wraps a list of the {@code AspectValue} of the top + * level aspects applied on the same top level target. + */ + public static class TopLevelAspectsValue implements ActionLookupValue { + private final ImmutableList<SkyValue> topLevelAspectsValues; + + public TopLevelAspectsValue(Collection<SkyValue> topLevelAspectsValues) { + this.topLevelAspectsValues = ImmutableList.copyOf(topLevelAspectsValues); + } + + public ImmutableList<SkyValue> getTopLevelAspectsValues() { + return topLevelAspectsValues; + } + + @Override + public ImmutableList<ActionAnalysisMetadata> getActions() { + return ImmutableList.of(); + } + } }