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);
