Move aspect resolution out of ConfiguredTargetFunction. Part of the effort to simplify ConfiguredTargetFunction. PiperOrigin-RevId: 169453435
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java new file mode 100644 index 0000000..87a942d --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectResolver.java
@@ -0,0 +1,171 @@ +// Copyright 2017 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.common.collect.ImmutableList; +import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.packages.Aspect; +import com.google.devtools.build.lib.packages.AspectDescriptor; +import com.google.devtools.build.lib.packages.Attribute; +import com.google.devtools.build.lib.packages.NoSuchThingException; +import com.google.devtools.build.lib.packages.Package; +import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.skyframe.AspectFunction; +import com.google.devtools.build.lib.skyframe.AspectValue; +import com.google.devtools.build.lib.skyframe.ConfiguredTargetValue; +import com.google.devtools.build.lib.util.OrderedSetMultimap; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.ValueOrException2; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import javax.annotation.Nullable; + +/** + * Returns the aspects to attach to rule dependencies. + */ +public final class AspectResolver { + /** + * Given a list of {@link Dependency} objects, returns a multimap from the {@link Dependency}s to + * the {@link ConfiguredAspect} instances that should be merged with them. + * + * <p>Returns null if the required aspects are not yet available from Skyframe. + */ + @Nullable + public static OrderedSetMultimap<Dependency, ConfiguredAspect> resolveAspectDependencies( + SkyFunction.Environment env, + Map<SkyKey, ConfiguredTarget> configuredTargetMap, + Iterable<Dependency> deps, + NestedSetBuilder<Package> transitivePackages) + throws AspectFunction.AspectCreationException, InterruptedException { + OrderedSetMultimap<Dependency, ConfiguredAspect> result = OrderedSetMultimap.create(); + Set<SkyKey> allAspectKeys = new HashSet<>(); + for (Dependency dep : deps) { + allAspectKeys.addAll(getAspectKeys(dep).values()); + } + + Map<SkyKey, ValueOrException2<AspectFunction.AspectCreationException, NoSuchThingException>> + depAspects = env.getValuesOrThrow(allAspectKeys, + AspectFunction.AspectCreationException.class, NoSuchThingException.class); + + for (Dependency dep : deps) { + Map<AspectDescriptor, SkyKey> aspectToKeys = getAspectKeys(dep); + + for (AspectCollection.AspectDeps depAspect : dep.getAspects().getVisibleAspects()) { + SkyKey aspectKey = aspectToKeys.get(depAspect.getAspect()); + + AspectValue aspectValue; + try { + // TODO(ulfjack): Catch all thrown AspectCreationException and NoSuchThingException + // instances and merge them into a single Exception to get full root cause data. + aspectValue = (AspectValue) depAspects.get(aspectKey).get(); + } catch (NoSuchThingException e) { + throw new AspectFunction.AspectCreationException( + String.format( + "Evaluation of aspect %s on %s failed: %s", + depAspect.getAspect().getAspectClass().getName(), + dep.getLabel(), + e.toString())); + } + + if (aspectValue == null) { + // Dependent aspect has either not been computed yet or is in error. + return null; + } + + // Validate that aspect is applicable to "bare" configured target. + ConfiguredTarget associatedTarget = configuredTargetMap + .get(ConfiguredTargetValue.key(dep.getLabel(), dep.getConfiguration())); + if (!aspectMatchesConfiguredTarget(associatedTarget, aspectValue.getAspect())) { + continue; + } + + result.put(dep, aspectValue.getConfiguredAspect()); + transitivePackages.addTransitive(aspectValue.getTransitivePackages()); + } + } + return result; + } + + /** + * Merges each direct dependency configured target with the aspects associated with it. + * + * <p>Note that the combination of a configured target and its associated aspects are not + * represented by a Skyframe node. This is because there can possibly be many different + * combinations of aspects for a particular configured target, so it would result in a + * combinatorial explosion of Skyframe nodes. + */ + public static OrderedSetMultimap<Attribute, ConfiguredTarget> mergeAspects( + OrderedSetMultimap<Attribute, Dependency> depValueNames, + Map<SkyKey, ConfiguredTarget> depConfiguredTargetMap, + OrderedSetMultimap<Dependency, ConfiguredAspect> depAspectMap) + throws MergedConfiguredTarget.DuplicateException { + OrderedSetMultimap<Attribute, ConfiguredTarget> result = OrderedSetMultimap.create(); + + for (Map.Entry<Attribute, Dependency> entry : depValueNames.entries()) { + Dependency dep = entry.getValue(); + SkyKey depKey = ConfiguredTargetValue.key(dep.getLabel(), dep.getConfiguration()); + ConfiguredTarget depConfiguredTarget = depConfiguredTargetMap.get(depKey); + + result.put(entry.getKey(), + MergedConfiguredTarget.of(depConfiguredTarget, depAspectMap.get(dep))); + } + + return result; + } + + private static Map<AspectDescriptor, SkyKey> getAspectKeys(Dependency dep) { + HashMap<AspectDescriptor, SkyKey> result = new HashMap<>(); + AspectCollection aspects = dep.getAspects(); + for (AspectCollection.AspectDeps aspectDeps : aspects.getVisibleAspects()) { + buildAspectKey(aspectDeps, result, dep); + } + return result; + } + + private static AspectValue.AspectKey buildAspectKey(AspectCollection.AspectDeps aspectDeps, + HashMap<AspectDescriptor, SkyKey> result, Dependency dep) { + if (result.containsKey(aspectDeps.getAspect())) { + return (AspectValue.AspectKey) result.get(aspectDeps.getAspect()).argument(); + } + + ImmutableList.Builder<AspectValue.AspectKey> dependentAspects = ImmutableList.builder(); + for (AspectCollection.AspectDeps path : aspectDeps.getDependentAspects()) { + dependentAspects.add(buildAspectKey(path, result, dep)); + } + AspectValue.AspectKey aspectKey = AspectValue.createAspectKey( + dep.getLabel(), dep.getConfiguration(), + dependentAspects.build(), + aspectDeps.getAspect(), + dep.getAspectConfiguration(aspectDeps.getAspect())); + result.put(aspectKey.getAspectDescriptor(), aspectKey.getSkyKey()); + return aspectKey; + } + + public static boolean aspectMatchesConfiguredTarget(final ConfiguredTarget dep, Aspect aspect) { + if (!aspect.getDefinition().applyToFiles() && !(dep.getTarget() instanceof Rule)) { + return false; + } + if (dep.getTarget().getAssociatedRule() == null) { + // even aspects that 'apply to files' cannot apply to input files. + return false; + } + return dep.satisfies(aspect.getDefinition().getRequiredProviders()); + } +} \ No newline at end of file
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 2beb088..f67a4e8 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
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.ActionLookupValue; import com.google.devtools.build.lib.analysis.AliasProvider; +import com.google.devtools.build.lib.analysis.AspectResolver; import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment; import com.google.devtools.build.lib.analysis.ConfiguredAspect; import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory; @@ -466,7 +467,7 @@ } ConfiguredAspect configuredAspect; - if (ConfiguredTargetFunction.aspectMatchesConfiguredTarget(associatedTarget, aspect)) { + if (AspectResolver.aspectMatchesConfiguredTarget(associatedTarget, aspect)) { configuredAspect = view.getConfiguredTargetFactory() .createAspect(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java index ee23308..8a80192 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -372,7 +372,7 @@ transitivePackages = null; } - NestedSet<Package> getTransitivePackages() { + public NestedSet<Package> getTransitivePackages() { return Preconditions.checkNotNull(transitivePackages); }
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 c157351..d1c0eae 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
@@ -22,8 +22,7 @@ import com.google.devtools.build.lib.actions.Actions; import com.google.devtools.build.lib.actions.Actions.GeneratingActions; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; -import com.google.devtools.build.lib.analysis.AspectCollection; -import com.google.devtools.build.lib.analysis.AspectCollection.AspectDeps; +import com.google.devtools.build.lib.analysis.AspectResolver; import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment; import com.google.devtools.build.lib.analysis.ConfiguredAspect; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -37,7 +36,6 @@ import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.analysis.config.ConfigurationResolver; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; -import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget; import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget.DuplicateException; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; @@ -46,7 +44,6 @@ 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.AspectDescriptor; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.NoSuchTargetException; @@ -58,7 +55,6 @@ import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.skyframe.AspectFunction.AspectCreationException; -import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; import com.google.devtools.build.lib.skyframe.SkyframeExecutor.BuildViewProvider; import com.google.devtools.build.lib.skyframe.ToolchainUtil.ToolchainContextException; import com.google.devtools.build.lib.syntax.EvalException; @@ -69,11 +65,8 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.ValueOrException; -import com.google.devtools.build.skyframe.ValueOrException2; import java.util.Collection; -import java.util.HashMap; -import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; @@ -403,15 +396,16 @@ } // Resolve required aspects. - OrderedSetMultimap<Dependency, ConfiguredAspect> depAspects = resolveAspectDependencies( - env, depValues, depValueNames.values(), transitivePackages); + OrderedSetMultimap<Dependency, ConfiguredAspect> depAspects = + AspectResolver.resolveAspectDependencies(env, depValues, depValueNames.values(), + transitivePackages); if (depAspects == null) { return null; } // Merge the dependent configured targets and aspects into a single map. try { - return mergeAspects(depValueNames, depValues, depAspects); + return AspectResolver.mergeAspects(depValueNames, depValues, depAspects); } catch (DuplicateException e) { env.getListener().handle( Event.error(ctgValue.getTarget().getLocation(), e.getMessage())); @@ -422,134 +416,6 @@ } /** - * Merges the each direct dependency configured target with the aspects associated with it. - * - * <p>Note that the combination of a configured target and its associated aspects are not - * represented by a Skyframe node. This is because there can possibly be many different - * combinations of aspects for a particular configured target, so it would result in a - * combinatiorial explosion of Skyframe nodes. - */ - private static OrderedSetMultimap<Attribute, ConfiguredTarget> mergeAspects( - OrderedSetMultimap<Attribute, Dependency> depValueNames, - Map<SkyKey, ConfiguredTarget> depConfiguredTargetMap, - OrderedSetMultimap<Dependency, ConfiguredAspect> depAspectMap) - throws DuplicateException { - OrderedSetMultimap<Attribute, ConfiguredTarget> result = OrderedSetMultimap.create(); - - for (Map.Entry<Attribute, Dependency> entry : depValueNames.entries()) { - Dependency dep = entry.getValue(); - SkyKey depKey = ConfiguredTargetValue.key(dep.getLabel(), dep.getConfiguration()); - ConfiguredTarget depConfiguredTarget = depConfiguredTargetMap.get(depKey); - - result.put(entry.getKey(), - MergedConfiguredTarget.of(depConfiguredTarget, depAspectMap.get(dep))); - } - - return result; - } - - /** - * Given a list of {@link Dependency} objects, returns a multimap from the - * {@link Dependency}s too the {@link ConfiguredAspect} instances that should be merged into it. - * - * <p>Returns null if the required aspects are not computed yet. - */ - @Nullable - private static OrderedSetMultimap<Dependency, ConfiguredAspect> resolveAspectDependencies( - Environment env, - Map<SkyKey, ConfiguredTarget> configuredTargetMap, - Iterable<Dependency> deps, - NestedSetBuilder<Package> transitivePackages) - throws AspectCreationException, InterruptedException { - OrderedSetMultimap<Dependency, ConfiguredAspect> result = OrderedSetMultimap.create(); - Set<SkyKey> allAspectKeys = new HashSet<>(); - for (Dependency dep : deps) { - allAspectKeys.addAll(getAspectKeys(dep).values()); - } - - Map<SkyKey, ValueOrException2<AspectCreationException, NoSuchThingException>> depAspects = - env.getValuesOrThrow(allAspectKeys, - AspectCreationException.class, NoSuchThingException.class); - - for (Dependency dep : deps) { - Map<AspectDescriptor, SkyKey> aspectToKeys = getAspectKeys(dep); - - for (AspectDeps depAspect : dep.getAspects().getVisibleAspects()) { - SkyKey aspectKey = aspectToKeys.get(depAspect.getAspect()); - - AspectValue aspectValue; - try { - // TODO(ulfjack): Catch all thrown AspectCreationException and NoSuchThingException - // instances and merge them into a single Exception to get full root cause data. - aspectValue = (AspectValue) depAspects.get(aspectKey).get(); - } catch (NoSuchThingException e) { - throw new AspectCreationException( - String.format( - "Evaluation of aspect %s on %s failed: %s", - depAspect.getAspect().getAspectClass().getName(), - dep.getLabel(), - e.toString())); - } - - if (aspectValue == null) { - // Dependent aspect has either not been computed yet or is in error. - return null; - } - - // Validate that aspect is applicable to "bare" configured target. - ConfiguredTarget associatedTarget = configuredTargetMap - .get(ConfiguredTargetValue.key(dep.getLabel(), dep.getConfiguration())); - if (!aspectMatchesConfiguredTarget(associatedTarget, aspectValue.getAspect())) { - continue; - } - - result.put(dep, aspectValue.getConfiguredAspect()); - transitivePackages.addTransitive(aspectValue.getTransitivePackages()); - } - } - return result; - } - - private static Map<AspectDescriptor, SkyKey> getAspectKeys(Dependency dep) { - HashMap<AspectDescriptor, SkyKey> result = new HashMap<>(); - AspectCollection aspects = dep.getAspects(); - for (AspectDeps aspectDeps : aspects.getVisibleAspects()) { - buildAspectKey(aspectDeps, result, dep); - } - return result; - } - - private static AspectKey buildAspectKey(AspectDeps aspectDeps, - HashMap<AspectDescriptor, SkyKey> result, Dependency dep) { - if (result.containsKey(aspectDeps.getAspect())) { - return (AspectKey) result.get(aspectDeps.getAspect()).argument(); - } - - ImmutableList.Builder<AspectKey> dependentAspects = ImmutableList.builder(); - for (AspectDeps path : aspectDeps.getDependentAspects()) { - dependentAspects.add(buildAspectKey(path, result, dep)); - } - AspectKey aspectKey = AspectValue.createAspectKey( - dep.getLabel(), dep.getConfiguration(), - dependentAspects.build(), - aspectDeps.getAspect(), - dep.getAspectConfiguration(aspectDeps.getAspect())); - result.put(aspectKey.getAspectDescriptor(), aspectKey.getSkyKey()); - return aspectKey; - } - - static boolean aspectMatchesConfiguredTarget(final ConfiguredTarget dep, Aspect aspect) { - if (!aspect.getDefinition().applyToFiles() && !(dep.getTarget() instanceof Rule)) { - return false; - } - if (dep.getTarget().getAssociatedRule() == null) { - // even aspects that 'apply to files' cannot apply to input files. - return false; - } - return dep.satisfies(aspect.getDefinition().getRequiredProviders()); - } - - /** * Returns the set of {@link ConfigMatchingProvider}s that key the configurable attributes used by * this rule. * @@ -587,7 +453,7 @@ // Collect the corresponding Skyframe configured target values. Abort early if they haven't // been computed yet. - Collection<Dependency> configValueNames = null; + Collection<Dependency> configValueNames; try { configValueNames = resolver.resolveRuleLabels( ctgValue, configLabelMap, transitiveLoadingRootCauses);