diff options
Diffstat (limited to 'src/main/java/com')
24 files changed, 1511 insertions, 223 deletions
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 a64e031e5f..289cabcfa1 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 @@ -20,12 +20,12 @@ import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; -import com.google.common.collect.Multimaps; import com.google.common.collect.Sets; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.Action; @@ -269,8 +269,9 @@ public class BuildView { public void run() { clear(); } - }, - binTools); + }, + binTools, + ruleClassProvider); skyframeExecutor.setSkyframeBuildView(skyframeBuildView); } @@ -298,6 +299,7 @@ public class BuildView { @VisibleForTesting public void setConfigurationsForTesting(BuildConfigurationCollection configurations) { this.configurations = configurations; + skyframeBuildView.setTopLevelHostConfiguration(configurations.getHostConfiguration()); } public BuildConfigurationCollection getConfigurationCollection() { @@ -352,8 +354,8 @@ public class BuildView { public Iterable<ConfiguredTarget> getDirectPrerequisites( ConfiguredTarget ct, @Nullable final LoadingCache<Label, Target> targetCache) { - return skyframeExecutor.getConfiguredTargets( - getDirectPrerequisiteDependencies(ct, targetCache)); + return skyframeExecutor.getConfiguredTargets(ct.getConfiguration(), + getDirectPrerequisiteDependencies(ct, targetCache), false); } public Iterable<Dependency> getDirectPrerequisiteDependencies( @@ -391,7 +393,8 @@ public class BuildView { DependencyResolver dependencyResolver = new SilentDependencyResolver(); TargetAndConfiguration ctgNode = new TargetAndConfiguration(ct.getTarget(), ct.getConfiguration()); - return dependencyResolver.dependentNodes(ctgNode, getConfigurableAttributeKeys(ctgNode)); + return dependencyResolver.dependentNodes(ctgNode, configurations.getHostConfiguration(), + getConfigurableAttributeKeys(ctgNode)); } /** @@ -603,6 +606,7 @@ public class BuildView { skyframeAnalysisWasDiscarded = false; ImmutableMap<PackageIdentifier, Path> packageRoots = loadingResult.getPackageRoots(); this.configurations = configurations; + skyframeBuildView.setTopLevelHostConfiguration(this.configurations.getHostConfiguration()); setArtifactRoots(packageRoots); // Determine the configurations. List<TargetAndConfiguration> nodes = nodesForTargets(targets); @@ -633,7 +637,7 @@ public class BuildView { } prepareToBuild(new SkyframePackageRootResolver(skyframeExecutor)); - skyframeExecutor.injectWorkspaceStatusData(); + skyframeExecutor.injectWorkspaceStatusData(configurations); SkyframeAnalysisResult skyframeAnalysisResult; try { skyframeAnalysisResult = @@ -843,7 +847,7 @@ public class BuildView { Label label, BuildConfiguration configuration) { return Iterables.getFirst( skyframeExecutor.getConfiguredTargets( - ImmutableList.of(new Dependency(label, configuration))), + configuration, ImmutableList.of(new Dependency(label, configuration)), true), null); } @@ -868,23 +872,21 @@ public class BuildView { TargetAndConfiguration ctNode = new TargetAndConfiguration(target); ListMultimap<Attribute, Dependency> depNodeNames; try { - depNodeNames = resolver.dependentNodeMap(ctNode, null, getConfigurableAttributeKeys(ctNode)); + depNodeNames = resolver.dependentNodeMap(ctNode, configurations.getHostConfiguration(), null, + getConfigurableAttributeKeys(ctNode)); } catch (EvalException e) { throw new IllegalStateException(e); } - final Map<LabelAndConfiguration, ConfiguredTarget> depMap = new HashMap<>(); - for (ConfiguredTarget dep : skyframeExecutor.getConfiguredTargets(depNodeNames.values())) { - depMap.put(LabelAndConfiguration.of(dep.getLabel(), dep.getConfiguration()), dep); - } + ImmutableMap<Dependency, ConfiguredTarget> cts = skyframeExecutor.getConfiguredTargetMap( + ctNode.getConfiguration(), ImmutableSet.copyOf(depNodeNames.values()), false); - return Multimaps.transformValues(depNodeNames, new Function<Dependency, ConfiguredTarget>() { - @Override - public ConfiguredTarget apply(Dependency depName) { - return depMap.get(LabelAndConfiguration.of(depName.getLabel(), - depName.getConfiguration())); - } - }); + ImmutableListMultimap.Builder<Attribute, ConfiguredTarget> builder = + ImmutableListMultimap.builder(); + for (Map.Entry<Attribute, Dependency> entry : depNodeNames.entries()) { + builder.put(entry.getKey(), cts.get(entry.getValue())); + } + return builder.build(); } /** @@ -955,7 +957,7 @@ public class BuildView { /*isSystemEnv=*/false, config.extendedSanityChecks(), eventHandler, /*skyframeEnv=*/null, config.isActionsEnabled(), binTools); return new RuleContext.Builder(analysisEnvironment, - (Rule) target.getTarget(), config, getHostConfigurationForTesting(config), + (Rule) target.getTarget(), config, configurations.getHostConfiguration(), ruleClassProvider.getPrerequisiteValidator()) .setVisibility(NestedSetBuilder.<PackageSpecification>create( Order.STABLE_ORDER, PackageSpecification.EVERYTHING)) @@ -972,7 +974,7 @@ public class BuildView { public RuleContext getRuleContextForTesting(ConfiguredTarget target, AnalysisEnvironment env) { BuildConfiguration targetConfig = target.getConfiguration(); return new RuleContext.Builder( - env, (Rule) target.getTarget(), targetConfig, getHostConfigurationForTesting(targetConfig), + env, (Rule) target.getTarget(), targetConfig, configurations.getHostConfiguration(), ruleClassProvider.getPrerequisiteValidator()) .setVisibility(NestedSetBuilder.<PackageSpecification>create( Order.STABLE_ORDER, PackageSpecification.EVERYTHING)) @@ -981,11 +983,6 @@ public class BuildView { .build(); } - private BuildConfiguration getHostConfigurationForTesting(BuildConfiguration config) { - // TODO(bazel-team): support dynamic transitions in tests. - return config == null ? null : config.getConfiguration(Attribute.ConfigurationTransition.HOST); - } - /** * For a configured target dependentTarget, returns the desired configured target * that is depended upon. Useful for obtaining the a target with aspects diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java index 14bb2a9695..14a79415a7 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.analysis; import com.google.common.cache.Cache; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigurationFactory; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; @@ -51,4 +52,16 @@ public interface ConfigurationCollectionFactory { BuildOptions buildOptions, EventHandler errorEventListener, boolean performSanityCheck) throws InvalidConfigurationException; + + /** + * Returns the module the given configuration should use for choosing dynamic transitions. + * + * <p>We can presumably factor away this method once static global configurations are properly + * deprecated. But for now we retain the + * {@link com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection.Transitions} + * interface since that's the same place where static transition logic is determined and + * {@link BuildConfigurationCollection.Transitions#configurationHook} + * is still used. + */ + BuildConfigurationCollection.Transitions getDynamicTransitionLogic(BuildConfiguration config); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java index b875a9db2b..f193298c3e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java @@ -281,11 +281,11 @@ public final class ConfiguredTargetFactory { AnalysisEnvironment env, RuleConfiguredTarget associatedTarget, ConfiguredAspectFactory aspectFactory, ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap, - Set<ConfigMatchingProvider> configConditions) { + Set<ConfigMatchingProvider> configConditions, BuildConfiguration hostConfiguration) { RuleContext.Builder builder = new RuleContext.Builder(env, associatedTarget.getTarget(), associatedTarget.getConfiguration(), - getHostConfiguration(associatedTarget.getConfiguration()), + hostConfiguration, ruleClassProvider.getPrerequisiteValidator()); RuleContext ruleContext = builder .setVisibility(convertVisibility( @@ -300,11 +300,6 @@ public final class ConfiguredTargetFactory { return aspectFactory.create(associatedTarget, ruleContext); } - private static BuildConfiguration getHostConfiguration(BuildConfiguration config) { - // TODO(bazel-team): support dynamic transitions. - return config == null ? null : config.getConfiguration(Attribute.ConfigurationTransition.HOST); - } - /** * A pseudo-implementation for configured targets that creates fail actions for all declared * outputs, both implicit and explicit. diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java index 4953176b45..9b6ea93ef7 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.analysis; import com.google.common.base.Function; import com.google.common.base.Preconditions; +import com.google.common.base.Verify; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -26,7 +27,6 @@ import com.google.devtools.build.lib.collect.ImmutableSortedKeyListMultimap; import com.google.devtools.build.lib.packages.AspectDefinition; import com.google.devtools.build.lib.packages.AspectFactory; import com.google.devtools.build.lib.packages.Attribute; -import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; import com.google.devtools.build.lib.packages.Attribute.SplitTransition; import com.google.devtools.build.lib.packages.AttributeMap; @@ -65,8 +65,12 @@ public abstract class DependencyResolver { /** * A dependency of a configured target through a label. * - * <p>Includes the target and the configuration of the dependency configured target and any - * aspects that may be required. + * <p>For static configurations: includes the target and the configuration of the dependency + * configured target and any aspects that may be required. + * + * <p>For dynamic configurations: includes the target and the desired configuration transitions + * that should be applied to produce the dependency's configuration. It's the caller's + * responsibility to construct an actual configuration out of that. * * <p>Note that the presence of an aspect here does not necessarily mean that it will be created. * They will be filtered based on the {@link TransitiveInfoProvider} instances their associated @@ -91,32 +95,76 @@ public abstract class DependencyResolver { private final Label label; + // Only one of the two below fields is set. Use hasStaticConfiguration to determine which. @Nullable private final BuildConfiguration configuration; + private final Attribute.Transition transition; + private final boolean hasStaticConfiguration; private final ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects; - public Dependency( - Label label, + /** + * Constructs a Dependency with a given configuration (suitable for static configuration + * builds). + */ + public Dependency(Label label, @Nullable BuildConfiguration configuration, ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects) { this.label = Preconditions.checkNotNull(label); this.configuration = configuration; + this.transition = null; + this.hasStaticConfiguration = true; this.aspects = Preconditions.checkNotNull(aspects); } + /** + * Constructs a Dependency with a given configuration (suitable for static configuration + * builds). + */ public Dependency(Label label, @Nullable BuildConfiguration configuration) { this(label, configuration, ImmutableSet.<Class<? extends ConfiguredAspectFactory>>of()); } + /** + * Constructs a Dependency with a given transition (suitable for dynamic configuration builds). + */ + public Dependency(Label label, Attribute.Transition transition, + ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects) { + this.label = Preconditions.checkNotNull(label); + this.configuration = null; + this.transition = Preconditions.checkNotNull(transition); + this.hasStaticConfiguration = false; + this.aspects = Preconditions.checkNotNull(aspects); + } + + /** + * Does this dependency represent a null configuration? + */ + public boolean isNull() { + return configuration == null && transition == null; + } + + /** + * Does this dependency specify a static configuration (vs. a dynamic transition)? + */ + public boolean hasStaticConfiguration() { + return hasStaticConfiguration; + } + public Label getLabel() { return label; } @Nullable public BuildConfiguration getConfiguration() { + Verify.verify(hasStaticConfiguration); return configuration; } + public Attribute.Transition getTransition() { + Verify.verify(!hasStaticConfiguration); + return transition; + } + public ImmutableSet<Class<? extends ConfiguredAspectFactory>> getAspects() { return aspects; } @@ -168,7 +216,7 @@ public abstract class DependencyResolver { * dependency. */ public final ListMultimap<Attribute, Dependency> dependentNodeMap( - TargetAndConfiguration node, AspectDefinition aspect, + TargetAndConfiguration node, BuildConfiguration hostConfig, AspectDefinition aspect, Set<ConfigMatchingProvider> configConditions) throws EvalException { Target target = node.getTarget(); @@ -188,7 +236,7 @@ public abstract class DependencyResolver { visitTargetVisibility(node, outgoingEdges.get(null)); Rule rule = (Rule) target; ListMultimap<Attribute, LabelAndConfiguration> labelMap = - resolveAttributes(rule, aspect, config, configConditions); + resolveAttributes(rule, aspect, config, hostConfig, configConditions); visitRule(rule, aspect, labelMap, outgoingEdges); } else if (target instanceof PackageGroup) { visitPackageGroup(node, (PackageGroup) target, outgoingEdges.get(null)); @@ -200,7 +248,7 @@ public abstract class DependencyResolver { private ListMultimap<Attribute, LabelAndConfiguration> resolveAttributes( Rule rule, AspectDefinition aspect, BuildConfiguration configuration, - Set<ConfigMatchingProvider> configConditions) + BuildConfiguration hostConfiguration, Set<ConfigMatchingProvider> configConditions) throws EvalException { ConfiguredAttributeMapper attributeMap = ConfiguredAttributeMapper.of(rule, configConditions); attributeMap.validateAttributes(); @@ -220,7 +268,8 @@ public abstract class DependencyResolver { resolveExplicitAttributes(rule, configuration, attributeMap, result); resolveImplicitAttributes(rule, configuration, attributeMap, attributes, result); - resolveLateBoundAttributes(rule, configuration, attributeMap, attributes, result); + resolveLateBoundAttributes(rule, configuration, hostConfiguration, attributeMap, attributes, + result); // Add the rule's visibility labels (which may come from the rule or from package defaults). addExplicitDeps(result, rule, "visibility", rule.getVisibility().getDependencyLabels(), @@ -377,6 +426,7 @@ public abstract class DependencyResolver { private void resolveLateBoundAttributes( Rule rule, BuildConfiguration configuration, + BuildConfiguration hostConfiguration, AttributeMap attributeMap, Iterable<Attribute> attributes, ImmutableSortedKeyListMultimap.Builder<Attribute, LabelAndConfiguration> builder) @@ -401,8 +451,7 @@ public abstract class DependencyResolver { LateBoundDefault<BuildConfiguration> lateBoundDefault = (LateBoundDefault<BuildConfiguration>) attribute.getLateBoundDefault(); if (lateBoundDefault.useHostConfiguration()) { - actualConfig = - actualConfig.getConfiguration(ConfigurationTransition.HOST); + actualConfig = hostConfiguration; } // TODO(bazel-team): This might be too expensive - can we cache this somehow? if (!lateBoundDefault.getRequiredConfigurationFragments().isEmpty()) { @@ -450,9 +499,11 @@ public abstract class DependencyResolver { * IllegalStateException}. */ public final Collection<Dependency> dependentNodes( - TargetAndConfiguration node, Set<ConfigMatchingProvider> configConditions) { + TargetAndConfiguration node, BuildConfiguration hostConfig, + Set<ConfigMatchingProvider> configConditions) { try { - return dependentNodeMap(node, null, configConditions).values(); + return ImmutableSet.copyOf( + dependentNodeMap(node, hostConfig, null, configConditions).values()); } catch (EvalException e) { throw new IllegalStateException(e); } @@ -550,12 +601,31 @@ public abstract class DependencyResolver { if (toTarget == null) { continue; } - Iterable<BuildConfiguration> toConfigurations = config.evaluateTransition( - rule, attribute, toTarget); - for (BuildConfiguration toConfiguration : toConfigurations) { + BuildConfiguration.TransitionApplier transitionApplier = config.getTransitionApplier(); + if (config.useDynamicConfigurations() && config.isHostConfiguration() + && !BuildConfiguration.usesNullConfiguration(toTarget)) { + // This condition is needed because resolveLateBoundAttributes may switch config to + // the host configuration, which is the only case DependencyResolver applies a + // configuration transition outside of this method. We need to reflect that + // transition in the results of this method, but config.evaluateTransition is hard-set + // to return a NONE transition when the input is a host config. Since the outside + // caller originally passed the *original* value of config (before the possible + // switch), it can mistakenly interpret the result as a NONE transition from the + // original value of config. This condition fixes that. Another fix would be to have + // config.evaluateTransition return a HOST transition when the input config is a host, + // but since this blemish is specific to DependencyResolver it seems best to keep the + // fix here. + // TODO(bazel-team): eliminate this special case by passing transitionApplier to + // resolveLateBoundAttributes, so that method uses the same interface for transitions. + transitionApplier.applyTransition(Attribute.ConfigurationTransition.HOST); + } else { + config.evaluateTransition(rule, attribute, toTarget, transitionApplier); + } + for (Dependency dependency : transitionApplier.getDependencies(label, + requiredAspects(aspect, attribute, toTarget))) { outgoingEdges.put( entry.getKey(), - new Dependency(label, toConfiguration, requiredAspects(aspect, attribute, toTarget))); + dependency); } } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index 4cf3b12122..29b5f4be1d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -15,10 +15,11 @@ package com.google.devtools.build.lib.analysis.config; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Function; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; +import com.google.common.base.Verify; import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ClassToInstanceMap; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -27,10 +28,14 @@ import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; +import com.google.common.collect.MutableClassToInstanceMap; import com.google.devtools.build.lib.actions.ArtifactFactory; import com.google.devtools.build.lib.actions.PackageRootResolver; import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.analysis.BlazeDirectories; +import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory; +import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; +import com.google.devtools.build.lib.analysis.DependencyResolver; import com.google.devtools.build.lib.analysis.ViewCreationFailedException; import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection.Transitions; import com.google.devtools.build.lib.events.Event; @@ -43,6 +48,7 @@ import com.google.devtools.build.lib.packages.InputFile; import com.google.devtools.build.lib.packages.PackageGroup; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; +import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.rules.test.TestActionBuilder; import com.google.devtools.build.lib.syntax.Label; @@ -70,7 +76,6 @@ import java.io.IOException; import java.io.Serializable; import java.lang.reflect.Field; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -799,6 +804,13 @@ public final class BuildConfiguration { category = "undocumented") public Label objcGcovBinary; + @Option(name = "experimental_dynamic_configs", + defaultValue = "false", + category = "undocumented", + help = "Dynamically instantiates build configurations instead of using the default " + + "static globally defined ones") + public boolean useDynamicConfigurations; + @Override public FragmentOptions getHost(boolean fallback) { Options host = (Options) getDefault(); @@ -806,6 +818,7 @@ public final class BuildConfiguration { host.outputDirectoryName = "host"; host.compilationMode = CompilationMode.OPT; host.isHost = true; + host.useDynamicConfigurations = useDynamicConfigurations; if (fallback) { // In the fallback case, we have already tried the target options and they didn't work, so @@ -888,10 +901,6 @@ public final class BuildConfiguration { } } - /** A list of build configurations that only contains the null element. */ - private static final List<BuildConfiguration> NULL_LIST = - Collections.unmodifiableList(Arrays.asList(new BuildConfiguration[] { null })); - private final String checksum; private Transitions transitions; @@ -902,7 +911,7 @@ public final class BuildConfiguration { /** * Directories in the output tree. - * + * * <p>The computation of the output directory should be a non-injective mapping from * BuildConfiguration instances to strings. The result should identify the aspects of the * configuration that should be reflected in the output file names. Furthermore the @@ -1051,10 +1060,26 @@ public final class BuildConfiguration { return builder.build(); } + /** + * Constructs a new BuildConfiguration instance. + */ public BuildConfiguration(BlazeDirectories directories, - Map<Class<? extends Fragment>, Fragment> fragmentsMap, - BuildOptions buildOptions, - boolean actionsDisabled) { + Map<Class<? extends Fragment>, Fragment> fragmentsMap, + BuildOptions buildOptions, + boolean actionsDisabled) { + this(null, directories, fragmentsMap, buildOptions, actionsDisabled); + } + + /** + * Constructor variation that uses the passed in output roots if non-null, else computes them + * from the directories. + */ + public BuildConfiguration(@Nullable OutputRoots outputRoots, + @Nullable BlazeDirectories directories, + Map<Class<? extends Fragment>, Fragment> fragmentsMap, + BuildOptions buildOptions, + boolean actionsDisabled) { + Preconditions.checkState(outputRoots == null ^ directories == null); this.actionsEnabled = !actionsDisabled; this.fragments = ImmutableMap.copyOf(fragmentsMap); @@ -1079,7 +1104,9 @@ public final class BuildConfiguration { this.shExecutable = collectExecutables().get("sh"); - this.outputRoots = new OutputRoots(directories, outputDirName); + this.outputRoots = outputRoots != null + ? outputRoots + : new OutputRoots(directories, outputDirName); ImmutableSet.Builder<Label> coverageLabelsBuilder = ImmutableSet.builder(); ImmutableSet.Builder<Label> coverageReportGeneratorLabelsBuilder = ImmutableSet.builder(); @@ -1124,6 +1151,50 @@ public final class BuildConfiguration { checksum = Fingerprint.md5Digest(buildOptions.computeCacheKey()); } + /** + * Returns a copy of this configuration only including the given fragments (which the current + * configuration is assumed to have). + */ + public BuildConfiguration clone( + Set<Class<? extends BuildConfiguration.Fragment>> fragmentClasses, + RuleClassProvider ruleClassProvider) { + + ClassToInstanceMap<Fragment> fragmentsMap = MutableClassToInstanceMap.create(); + for (Fragment fragment : fragments.values()) { + if (fragmentClasses.contains(fragment.getClass())) { + fragmentsMap.put(fragment.getClass(), fragment); + } + } + BuildOptions options = buildOptions.trim( + getOptionsClasses(fragmentsMap.keySet(), ruleClassProvider)); + BuildConfiguration newConfig = + new BuildConfiguration(outputRoots, null, fragmentsMap, options, !actionsEnabled); + newConfig.setConfigurationTransitions(this.transitions); + return newConfig; + } + + /** + * Returns the config fragment options classes used by the given fragment types. + */ + public static Set<Class<? extends FragmentOptions>> getOptionsClasses( + Iterable<Class<? extends Fragment>> fragmentClasses, RuleClassProvider ruleClassProvider) { + + Multimap<Class<? extends BuildConfiguration.Fragment>, Class<? extends FragmentOptions>> + fragmentToRequiredOptions = ArrayListMultimap.create(); + for (ConfigurationFragmentFactory fragmentLoader : + ((ConfiguredRuleClassProvider) ruleClassProvider).getConfigurationFragments()) { + fragmentToRequiredOptions.putAll(fragmentLoader.creates(), + fragmentLoader.requiredOptions()); + } + Set<Class<? extends FragmentOptions>> options = new HashSet<>(); + for (Class<? extends BuildConfiguration.Fragment> fragmentClass : fragmentClasses) { + options.addAll(fragmentToRequiredOptions.get(fragmentClass)); + } + return options; + } + + + private ImmutableMap<String, Class<? extends Fragment>> buildIndexOfVisibleFragments() { ImmutableMap.Builder<String, Class<? extends Fragment>> builder = ImmutableMap.builder(); SkylarkModuleNameResolver resolver = new SkylarkModuleNameResolver(); @@ -1199,9 +1270,10 @@ public final class BuildConfiguration { /** * Set the outgoing configuration transitions. During the lifetime of a given build configuration, * this must happen exactly once, shortly after the configuration is created. - * TODO(bazel-team): this makes the object mutable, get rid of it. */ public void setConfigurationTransitions(Transitions transitions) { + // TODO(bazel-team): This method makes the object mutable - get rid of it. Dynamic + // configurations should eventually make this obsolete. Preconditions.checkNotNull(transitions); Preconditions.checkState(this.transitions == null); this.transitions = transitions; @@ -1249,10 +1321,13 @@ public final class BuildConfiguration { * @param transition the configuration transition * @return the new configuration * @throws IllegalArgumentException if the transition is a {@link SplitTransition} + * + * TODO(bazel-team): remove this as part of the static -> dynamic configuration migration */ public BuildConfiguration getConfiguration(Transition transition) { Preconditions.checkArgument(!(transition instanceof SplitTransition)); - return transitions.getConfiguration(transition); + // The below call precondition-checks we're indeed using static configurations. + return transitions.getStaticConfiguration(transition); } /** @@ -1267,6 +1342,331 @@ public final class BuildConfiguration { } /** + * A common interface for static vs. dynamic configuration implementations that allows + * common configuration and transition-selection logic to seamlessly work with either. + * + * <p>The basic role of this interface is to "accept" a desired transition and produce + * an actual configuration change from it in an implementation-appropriate way. + */ + public interface TransitionApplier { + /** + * Creates a new instance of this transition applier bound to the specified source + * configuration. + */ + TransitionApplier create(BuildConfiguration config); + + /** + * Accepts the given configuration transition. The implementation decides how to turn + * this into an actual configuration. This may be called multiple times (representing a + * request for a sequence of transitions). + */ + void applyTransition(Transition transition); + + /** + * Accepts the given split transition. The implementation decides how to turn this into + * actual configurations. + */ + void split(SplitTransition<?> splitTransition); + + /** + * Returns whether or not all configuration(s) represented by the current state of this + * instance are null. + */ + boolean isNull(); + + /** + * Applies the given attribute configurator to the current configuration(s). + */ + void applyAttributeConfigurator(Attribute attribute, Rule fromRule, Target toTarget); + + /** + * Calls {@link Transitions#configurationHook} on the current configuration(s) represent by + * this instance. + */ + void applyConfigurationHook(Rule fromRule, Attribute attribute, Target toTarget); + + /** + * Returns the underlying {@Transitions} object for this instance's current configuration. + * Does not work for split configurations. + */ + Transitions getCurrentTransitions(); + + /** + * Populates a {@link com.google.devtools.build.lib.analysis.DependencyResolver.Dependency} + * for each configuration represented by this instance. + * TODO(bazel-team): this is a really ugly reverse dependency: factor this away. + */ + Iterable<DependencyResolver.Dependency> getDependencies(Label label, + ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects); + } + + /** + * Transition applier for static configurations. This implementation populates + * {@link com.google.devtools.build.lib.analysis.DependencyResolver.Dependency} objects with + * actual configurations. + * + * <p>Does not support split transitions (see {@link SplittableTransitionApplier}). + * TODO(bazel-team): remove this when dynamic configurations are fully production-ready. + */ + private static class StaticTransitionApplier implements TransitionApplier { + BuildConfiguration currentConfiguration; + + private StaticTransitionApplier(BuildConfiguration originalConfiguration) { + this.currentConfiguration = originalConfiguration; + } + + @Override + public TransitionApplier create(BuildConfiguration configuration) { + return new StaticTransitionApplier(configuration); + } + + @Override + public void applyTransition(Transition transition) { + if (transition == Attribute.ConfigurationTransition.NULL) { + currentConfiguration = null; + } else { + currentConfiguration = + currentConfiguration.getTransitions().getStaticConfiguration(transition); + } + } + + @Override + public void split(SplitTransition<?> splitTransition) { + throw new UnsupportedOperationException("This only works with SplittableTransitionApplier"); + } + + @Override + public boolean isNull() { + return currentConfiguration == null; + } + + @Override + public void applyAttributeConfigurator(Attribute attribute, Rule fromRule, Target toTarget) { + @SuppressWarnings("unchecked") + Configurator<BuildConfiguration, Rule> configurator = + (Configurator<BuildConfiguration, Rule>) attribute.getConfigurator(); + Verify.verifyNotNull(configurator); + currentConfiguration = + configurator.apply(fromRule, currentConfiguration, attribute, toTarget); + } + + @Override + public void applyConfigurationHook(Rule fromRule, Attribute attribute, Target toTarget) { + currentConfiguration.getTransitions().configurationHook(fromRule, attribute, toTarget, this); + + // Allow rule classes to override their own configurations. + Rule associatedRule = toTarget.getAssociatedRule(); + if (associatedRule != null) { + @SuppressWarnings("unchecked") + RuleClass.Configurator<BuildConfiguration, Rule> func = + associatedRule.getRuleClassObject().<BuildConfiguration, Rule>getConfigurator(); + currentConfiguration = func.apply(associatedRule, currentConfiguration); + } + } + + @Override + public Transitions getCurrentTransitions() { + return currentConfiguration.getTransitions(); + } + + @Override + public Iterable<DependencyResolver.Dependency> getDependencies(Label label, + ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects) { + return ImmutableList.of( + new DependencyResolver.Dependency(label, currentConfiguration, aspects)); + } + } + + /** + * Transition applier for dynamic configurations. This implementation populates + * {@link com.google.devtools.build.lib.analysis.DependencyResolver.Dependency} objects with + * transition definitions that the caller subsequently creates configurations out of. + * + * <p>Does not support split transitions (see {@link SplittableTransitionApplier}). + */ + private static class DynamicTransitionApplier implements TransitionApplier { + private final BuildConfiguration originalConfiguration; + private Transition transition = Attribute.ConfigurationTransition.NONE; + + private DynamicTransitionApplier(BuildConfiguration originalConfiguration) { + this.originalConfiguration = originalConfiguration; + } + + @Override + public TransitionApplier create(BuildConfiguration configuration) { + return new DynamicTransitionApplier(configuration); + } + + @Override + public void applyTransition(Transition transition) { + if (transition == Attribute.ConfigurationTransition.NONE) { + return; + } else if (this.transition != HostTransition.INSTANCE) { + // We don't currently support composed transitions (e.g. applyTransitions shouldn't be + // called multiple times). We can add support for this if needed by simply storing a list of + // transitions instead of a single transition. But we only want to do that if really + // necessary - if we can simplify BuildConfiguration's transition logic to not require + // scenarios like that, it's better to keep this simpler interface. + // + // The HostTransition exemption is because of limited cases where composition can + // occur. See relevant comments beginning with "BuildConfiguration.applyTransition NOTE" + // in the transition logic code if available. + + // Ensure we don't already have any mutating transitions registered. + // Note that for dynamic configurations, LipoDataTransition is equivalent to NONE. That's + // because dynamic transitions don't work with LIPO, so there's no LIPO context to change. + Verify.verify(this.transition == Attribute.ConfigurationTransition.NONE + || this.transition.toString().contains("LipoDataTransition")); + this.transition = getCurrentTransitions().getDynamicTransition(transition); + } + } + + @Override + public void split(SplitTransition<?> splitTransition) { + throw new UnsupportedOperationException("This only works with SplittableTransitionApplier"); + } + + @Override + public boolean isNull() { + return transition == Attribute.ConfigurationTransition.NULL; + } + + @Override + public void applyAttributeConfigurator(Attribute attribute, Rule fromRule, Target toTarget) { + // We don't support meaningful attribute configurators (since they produce configurations, + // and we're only interested in generating transitions so the calling code can realize + // configurations from them). So just check that the configurator is just a no-op. + @SuppressWarnings("unchecked") + Configurator<BuildConfiguration, Rule> configurator = + (Configurator<BuildConfiguration, Rule>) attribute.getConfigurator(); + Verify.verifyNotNull(configurator); + BuildConfiguration toConfiguration = + configurator.apply(fromRule, originalConfiguration, attribute, toTarget); + Verify.verify(toConfiguration == originalConfiguration); + } + + @Override + public void applyConfigurationHook(Rule fromRule, Attribute attribute, Target toTarget) { + if (isNull()) { + return; + } + getCurrentTransitions().configurationHook(fromRule, attribute, toTarget, this); + + // We don't support rule class configurators (which might imply composed transitions). + // The only current use of that is LIPO, which can't currently be invoked with dynamic + // configurations (e.g. this code can never get called for LIPO builds). So check that + // if there is a configurator, it's for LIPO, in which case we can ignore it. + Rule associatedRule = toTarget.getAssociatedRule(); + if (associatedRule != null) { + @SuppressWarnings("unchecked") + RuleClass.Configurator<?, ?> func = + associatedRule.getRuleClassObject().getConfigurator(); + Verify.verify(func == RuleClass.NO_CHANGE || func.getCategory().equals("lipo")); + } + } + + @Override + public Transitions getCurrentTransitions() { + return originalConfiguration.getTransitions(); + } + + @Override + public Iterable<DependencyResolver.Dependency> getDependencies(Label label, + ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects) { + return ImmutableList.of(new DependencyResolver.Dependency(label, transition, aspects)); + } + } + + /** + * Transition applier that wraps an underlying implementation with added support for + * split transitions. All external calls into BuildConfiguration should use this applier. + */ + private static class SplittableTransitionApplier implements TransitionApplier { + private List<TransitionApplier> appliers; + + private SplittableTransitionApplier(TransitionApplier original) { + appliers = ImmutableList.of(original); + } + + @Override + public TransitionApplier create(BuildConfiguration configuration) { + throw new UnsupportedOperationException("Not intended to be wrapped under another applier"); + } + + @Override + public void applyTransition(Transition transition) { + for (TransitionApplier applier : appliers) { + applier.applyTransition(transition); + } + } + + @Override + public void split(SplitTransition<?> splitTransition) { + TransitionApplier originalApplier = Iterables.getOnlyElement(appliers); + ImmutableList.Builder<TransitionApplier> splitAppliers = ImmutableList.builder(); + for (BuildConfiguration splitConfig : + originalApplier.getCurrentTransitions().getSplitConfigurations(splitTransition)) { + splitAppliers.add(originalApplier.create(splitConfig)); + } + appliers = splitAppliers.build(); + } + + @Override + public boolean isNull() { + throw new UnsupportedOperationException("Only for use from a Transitions instance"); + } + + + @Override + public void applyAttributeConfigurator(Attribute attribute, Rule fromRule, Target toTarget) { + for (TransitionApplier applier : appliers) { + applier.applyAttributeConfigurator(attribute, fromRule, toTarget); + } + } + + @Override + public void applyConfigurationHook(Rule fromRule, Attribute attribute, Target toTarget) { + for (TransitionApplier applier : appliers) { + applier.applyConfigurationHook(fromRule, attribute, toTarget); + } + } + + @Override + public Transitions getCurrentTransitions() { + throw new UnsupportedOperationException("Only for use from a Transitions instance"); + } + + + @Override + public Iterable<DependencyResolver.Dependency> getDependencies(Label label, + ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects) { + ImmutableList.Builder<DependencyResolver.Dependency> builder = ImmutableList.builder(); + for (TransitionApplier applier : appliers) { + builder.addAll(applier.getDependencies(label, aspects)); + } + return builder.build(); + } + } + + /** + * Returns the {@link TransitionApplier} that should be passed to {#evaluateTransition} calls. + */ + public TransitionApplier getTransitionApplier() { + TransitionApplier applier = useDynamicConfigurations() + ? new DynamicTransitionApplier(this) + : new StaticTransitionApplier(this); + return new SplittableTransitionApplier(applier); + } + + /** + * Returns true if the given target uses a null configuration, false otherwise. Consider + * this method the "source of truth" for determining this. + */ + public static boolean usesNullConfiguration(Target target) { + return target instanceof InputFile || target instanceof PackageGroup; + } + + /** * Calculates the configurations of a direct dependency. If a rule in some BUILD file refers * to a target (like another rule or a source file) using a label attribute, that target needs * to have a configuration, too. This method figures out the proper configuration for the @@ -1275,21 +1675,23 @@ public final class BuildConfiguration { * @param fromRule the rule that's depending on some target * @param attribute the attribute using which the rule depends on that target (eg. "srcs") * @param toTarget the target that's dependeded on - * @return the configuration that should be associated to {@code toTarget} + * @param transitionApplier the transition applier to accept transitions requests */ - public Iterable<BuildConfiguration> evaluateTransition(final Rule fromRule, - final Attribute attribute, final Target toTarget) { + public void evaluateTransition(final Rule fromRule, final Attribute attribute, + final Target toTarget, TransitionApplier transitionApplier) { // Fantastic configurations and where to find them: // I. Input files and package groups have no configurations. We don't want to duplicate them. - if (toTarget instanceof InputFile || toTarget instanceof PackageGroup) { - return NULL_LIST; + if (usesNullConfiguration(toTarget)) { + transitionApplier.applyTransition(Attribute.ConfigurationTransition.NULL); + return; } // II. Host configurations never switch to another. All prerequisites of host targets have the // same host configuration. if (isHostConfiguration()) { - return ImmutableList.of(this); + transitionApplier.applyTransition(Attribute.ConfigurationTransition.NONE); + return; } // Make sure config_setting dependencies are resolved in the referencing rule's configuration, @@ -1306,46 +1708,27 @@ public final class BuildConfiguration { // TODO(bazel-team): implement this more elegantly. This is far too hackish. Specifically: // don't reference the rule name explicitly and don't require special-casing here. if (toTarget instanceof Rule && ((Rule) toTarget).getRuleClass().equals("config_setting")) { - return ImmutableList.of(this); + transitionApplier.applyTransition(Attribute.ConfigurationTransition.NONE); // Unnecessary. + return; } - List<BuildConfiguration> toConfigurations; if (attribute.getConfigurationTransition() instanceof SplitTransition) { Preconditions.checkState(attribute.getConfigurator() == null); - toConfigurations = getSplitConfigurations( - (SplitTransition<?>) attribute.getConfigurationTransition()); + transitionApplier.split((SplitTransition<?>) attribute.getConfigurationTransition()); } else { // III. Attributes determine configurations. The configuration of a prerequisite is determined // by the attribute. @SuppressWarnings("unchecked") Configurator<BuildConfiguration, Rule> configurator = (Configurator<BuildConfiguration, Rule>) attribute.getConfigurator(); - toConfigurations = ImmutableList.of((configurator != null) - ? configurator.apply(fromRule, this, attribute, toTarget) - : getConfiguration(attribute.getConfigurationTransition())); - } - - return Iterables.transform(toConfigurations, - new Function<BuildConfiguration, BuildConfiguration>() { - @Override - public BuildConfiguration apply(BuildConfiguration input) { - // IV. Allow the transition object to perform an arbitrary switch. Blaze modules can inject - // configuration transition logic by extending the Transitions class. - BuildConfiguration actual = getTransitions().configurationHook( - fromRule, attribute, toTarget, input); - - // V. Allow rule classes to override their own configurations. - Rule associatedRule = toTarget.getAssociatedRule(); - if (associatedRule != null) { - @SuppressWarnings("unchecked") - RuleClass.Configurator<BuildConfiguration, Rule> func = - associatedRule.getRuleClassObject().<BuildConfiguration, Rule>getConfigurator(); - actual = func.apply(associatedRule, actual); - } - - return actual; + if (configurator != null) { + transitionApplier.applyAttributeConfigurator(attribute, fromRule, toTarget); + } else { + transitionApplier.applyTransition(attribute.getConfigurationTransition()); } - }); + } + + transitionApplier.applyConfigurationHook(fromRule, attribute, toTarget); } /** @@ -1677,6 +2060,13 @@ public final class BuildConfiguration { } /** + * Which fragments does this configuration contain? + */ + public Set<Class<? extends Fragment>> fragmentClasses() { + return fragments.keySet(); + } + + /** * Returns true if non-functional build stamps are enabled. */ public boolean stampBinaries() { @@ -1788,6 +2178,15 @@ public final class BuildConfiguration { } /** + * Returns whether we should use dynamically instantiated build configurations + * vs. static configurations (e.g. predefined in + * {@link com.google.devtools.build.lib.analysis.ConfigurationCollectionFactory}). + */ + public boolean useDynamicConfigurations() { + return options.useDynamicConfigurations; + } + + /** * Returns compilation mode. */ public CompilationMode getCompilationMode() { @@ -1801,7 +2200,22 @@ public final class BuildConfiguration { /** Returns a copy of the build configuration options for this configuration. */ public BuildOptions cloneOptions() { - return buildOptions.clone(); + BuildOptions clone = buildOptions.clone(); + return clone; + } + + /** + * Returns the actual options reference used by this configuration. + * + * <p><b>Be very careful using this method.</b> Options classes are mutable - no caller + * should ever call this method if there's any change the reference might be written to. + * This method only exists because {@link #cloneOptions} can be expensive when applied to + * every edge in a dependency graph, which becomes possible with dynamic configurations. + * + * <p>Do not use this method without careful review with other Bazel developers.. + */ + public BuildOptions getOptions() { + return buildOptions; } /** @@ -1907,7 +2321,12 @@ public final class BuildConfiguration { * See {@code BuildConfigurationCollection.Transitions.getArtifactOwnerConfiguration()}. */ public BuildConfiguration getArtifactOwnerConfiguration() { - return transitions.getArtifactOwnerConfiguration(); + // Dynamic configurations inherit transitions objects from other configurations exclusively + // for use of Transitions.getDynamicTransitions. No other calls to transitions should be + // made for dynamic configurations. + // TODO(bazel-team): enforce the above automatically (without having to explicitly check + // for dynamic configuration mode). + return useDynamicConfigurations() ? this : transitions.getArtifactOwnerConfiguration(); } /** diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java index 0374e8ea6b..f7713dfce1 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java @@ -153,7 +153,7 @@ public final class BuildConfigurationCollection { /** * The outgoing transitions for a build configuration. */ - public static class Transitions implements Serializable { + public abstract static class Transitions implements Serializable { protected final BuildConfiguration configuration; /** @@ -172,12 +172,6 @@ public final class BuildConfigurationCollection { this.splitTransitionTable = ImmutableListMultimap.copyOf(splitTransitionTable); } - public Transitions(BuildConfiguration configuration, - Map<? extends Transition, ConfigurationHolder> transitionTable) { - this(configuration, transitionTable, - ImmutableListMultimap.<SplitTransition<?>, BuildConfiguration>of()); - } - public Map<? extends Transition, ConfigurationHolder> getTransitionTable() { return transitionTable; } @@ -222,10 +216,13 @@ public final class BuildConfigurationCollection { * Returns the new configuration after traversing a dependency edge with a * given configuration transition. * + * <p>Only used for static configuration builds. + * * @param configurationTransition the configuration transition * @return the new configuration */ - public BuildConfiguration getConfiguration(Transition configurationTransition) { + public BuildConfiguration getStaticConfiguration(Transition configurationTransition) { + Preconditions.checkState(!configuration.useDynamicConfigurations()); ConfigurationHolder holder = transitionTable.get(configurationTransition); if (holder == null && configurationTransition.defaultsToSelf()) { return configuration; @@ -234,12 +231,41 @@ public final class BuildConfigurationCollection { } /** + * Translates a static configuration {@link Transition} reference into the corresponding + * dynamic configuration transition. + * + * <p>The difference is that with static configurations, the transition just models a desired + * type of transition that subsequently gets linked to a pre-built global configuration through + * custom logic in {@link BuildConfigurationCollection.Transitions} and + * {@link com.google.devtools.build.lib.analysis.ConfigurationCollectionFactory}. + * + * <p>With dynamic configurations, the transition directly embeds the semantics, e.g. + * it includes not just a name but also the logic of how it should transform its input + * configuration. + * + * <p>This is a connecting method meant to keep the two models in sync for the current time + * in which they must co-exist. Once dynamic configurations are production-ready, we'll remove + * the static configuration code entirely. + */ + protected Transition getDynamicTransition(Transition transition) { + Preconditions.checkState(configuration.useDynamicConfigurations()); + if (transition == Attribute.ConfigurationTransition.NONE) { + return transition; + } else if (transition == Attribute.ConfigurationTransition.NULL) { + return transition; + } else if (transition == Attribute.ConfigurationTransition.HOST) { + return HostTransition.INSTANCE; + } else { + throw new UnsupportedOperationException("No dynamic mapping for " + transition.toString()); + } + } + + /** * Arbitrary configuration transitions can be implemented by overriding this hook. */ @SuppressWarnings("unused") - public BuildConfiguration configurationHook(Rule fromTarget, - Attribute attribute, Target toTarget, BuildConfiguration toConfiguration) { - return toConfiguration; + public void configurationHook(Rule fromTarget, Attribute attribute, Target toTarget, + BuildConfiguration.TransitionApplier transitionApplier) { } /** diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java index cee3ceb0e1..e77df0c837 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.analysis.config; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Objects; import com.google.common.base.Preconditions; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; @@ -38,13 +39,14 @@ import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; +import javax.annotation.Nullable; + /** - * This is a collection of command-line options from all configuration fragments. Contains - * a single instance for all FragmentOptions classes provided by Blaze language modules. + * Stores the command-line options from a set of configuration fragments. */ public final class BuildOptions implements Cloneable, Serializable { /** - * Creates a BuildOptions object with all options set to its default value. + * Creates a BuildOptions object with all options set to their default values. */ public static BuildOptions createDefaults(Iterable<Class<? extends FragmentOptions>> options) { Builder builder = builder(); @@ -55,7 +57,7 @@ public final class BuildOptions implements Cloneable, Serializable { } /** - * This function creates a new BuildOptions instance for host. + * Creates a new BuildOptions instance for host. * * @param fallback if true, we have already tried the user specified hostCpu options * and it didn't work, so now we try the default options instead. @@ -81,7 +83,22 @@ public final class BuildOptions implements Cloneable, Serializable { } /** - * Creates an BuildOptions class by taking the option values from an options provider + * Returns an equivalent instance to this one with only options from the given + * {@link FragmentOptions} classes. + */ + public BuildOptions trim(Set<Class<? extends FragmentOptions>> optionsClasses) { + Builder builder = builder(); + for (FragmentOptions options : fragmentOptionsMap.values()) { + if (optionsClasses.contains(options.getClass()) + || options instanceof BuildConfiguration.Options) { + builder.add(options); + } + } + return builder.build(); + } + + /** + * Creates a BuildOptions class by taking the option values from an options provider * (eg. an OptionsParser). */ public static BuildOptions of(List<Class<? extends FragmentOptions>> optionsList, @@ -94,11 +111,21 @@ public final class BuildOptions implements Cloneable, Serializable { } /** - * Creates an BuildOptions class by taking the option values from command-line arguments + * Creates a BuildOptions class by taking the option values from command-line arguments */ @VisibleForTesting public static BuildOptions of(List<Class<? extends FragmentOptions>> optionsList, String... args) throws OptionsParsingException { + return of(optionsList, null, args); + } + + /** + * Creates a BuildOptions class by taking the option values from command-line arguments and + * applying the specified original options. + */ + @VisibleForTesting + static BuildOptions of(List<Class<?extends FragmentOptions>> optionsList, + BuildOptions originalOptions, String... args) throws OptionsParsingException { Builder builder = builder(); OptionsParser parser = OptionsParser.newOptionsParser( ImmutableList.<Class<? extends OptionsBase>>copyOf(optionsList)); @@ -106,6 +133,7 @@ public final class BuildOptions implements Cloneable, Serializable { for (Class<? extends FragmentOptions> optionsClass : optionsList) { builder.add(parser.getOptions(optionsClass)); } + builder.setOriginalOptions(originalOptions); return builder.build(); } @@ -200,19 +228,44 @@ public final class BuildOptions implements Cloneable, Serializable { */ @Override public BuildOptions clone() { + return clone(null); + } + + /** + * Creates a copy of the BuildOptions object that stores a set of original options. This can + * be used to power "reversion" of options changes. + */ + public BuildOptions clone(@Nullable BuildOptions originalOptions) { ImmutableMap.Builder<Class<? extends FragmentOptions>, FragmentOptions> builder = ImmutableMap.builder(); for (Map.Entry<Class<? extends FragmentOptions>, FragmentOptions> entry : fragmentOptionsMap.entrySet()) { builder.put(entry.getKey(), entry.getValue().clone()); } - return new BuildOptions(builder.build()); + // TODO(bazel-team): only store the diff between the current options and its original + // options. This may be easier with immutable options. + return new BuildOptions(builder.build(), originalOptions); + } + + /** + * Returns the original options these options were spawned from, or null if this info wasn't + * recorded. + */ + public BuildOptions getOriginal() { + return originalOptions; } @Override public boolean equals(Object other) { - return (this == other) || (other instanceof BuildOptions && - fragmentOptionsMap.equals(((BuildOptions) other).fragmentOptionsMap)); + if (this == other) { + return true; + } else if (!(other instanceof BuildOptions)) { + return false; + } else { + BuildOptions otherOptions = (BuildOptions) other; + return fragmentOptionsMap.equals(otherOptions.fragmentOptionsMap) + && Objects.equal(originalOptions, otherOptions.originalOptions); + } } @Override @@ -225,9 +278,17 @@ public final class BuildOptions implements Cloneable, Serializable { */ private final ImmutableMap<Class<? extends FragmentOptions>, FragmentOptions> fragmentOptionsMap; + /** + * Records an original set of options these options came from. When set, this + * provides the ability to "revert" options back to a previous form. + */ + @Nullable private final BuildOptions originalOptions; + private BuildOptions( - ImmutableMap<Class<? extends FragmentOptions>, FragmentOptions> fragmentOptionsMap) { + ImmutableMap<Class<? extends FragmentOptions>, FragmentOptions> fragmentOptionsMap, + BuildOptions originalOptions) { this.fragmentOptionsMap = fragmentOptionsMap; + this.originalOptions = originalOptions; } /** @@ -250,11 +311,21 @@ public final class BuildOptions implements Cloneable, Serializable { return this; } + /** + * Specify the original options these options were branched from. This should only be used + * when there's a desire to revert back to the old options, e.g. for a parent transition. + */ + public Builder setOriginalOptions(BuildOptions originalOptions) { + this.originalOptions = originalOptions; + return this; + } + public BuildOptions build() { - return new BuildOptions(ImmutableMap.copyOf(builderMap)); + return new BuildOptions(ImmutableMap.copyOf(builderMap), originalOptions); } private Map<Class<? extends FragmentOptions>, FragmentOptions> builderMap; + @Nullable private BuildOptions originalOptions; private Builder() { builderMap = new HashMap<>(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/HostTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/HostTransition.java new file mode 100644 index 0000000000..081f19c8ae --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/HostTransition.java @@ -0,0 +1,31 @@ +// Copyright 2015 Google Inc. 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.config; + +/** + * Dynamic transition to the host configuration. + */ +public final class HostTransition implements PatchTransition { + public static final HostTransition INSTANCE = new HostTransition(); + + private HostTransition() {} + + @Override + public BuildOptions apply(BuildOptions options) { + return options.createHostOptions(false); + } + + @Override + public boolean defaultsToSelf() { return false; } +} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/PatchTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/PatchTransition.java new file mode 100644 index 0000000000..30edd4a960 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/PatchTransition.java @@ -0,0 +1,45 @@ +// Copyright 2015 Google Inc. 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.config; + +import com.google.devtools.build.lib.packages.Attribute; + +/** + * Interface for a configuration transition using dynamic configurations. + * + * <p>The concept is simple: given the input configuration's build options, the + * transition does whatever it wants to them and returns the modified result. + * + * <p>Implementations must be stateless: the transformation logic cannot use + * any information from any data besides the input build options. + * + * <p>For performance reasons, the input options are passed in as a <i>reference</i>, + * not a <i>copy</i>. Transition implementations should <i>always</i> treat these + * options as immutable, and call + * {@link com.google.devtools.build.lib.analysis.config.BuildOptions#clone} + * before applying any mutations. Unfortunately, + * {@link com.google.devtools.build.lib.analysis.config.BuildOptions} does not currently + * enforce immutability, so care must be taken not to modify the wrong instance. + */ +public interface PatchTransition extends Attribute.Transition { + + /** + * Applies the transition. + * + * @param options the options representing the input configuration to this transition. DO NOT + * MODIFY THIS VARIABLE WITHOUT CLONING IT FIRST. + * @return the options representing the desired post-transition configuration + */ + BuildOptions apply(BuildOptions options); +} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java index 61ef8f4ef7..4aec2f2087 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java @@ -18,6 +18,7 @@ import com.google.common.cache.Cache; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.HashBasedTable; import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ListMultimap; import com.google.common.collect.Table; import com.google.devtools.build.lib.analysis.ConfigurationCollectionFactory; @@ -85,6 +86,17 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact ArrayListMultimap.create(); for (SplitTransition<BuildOptions> transition : buildOptions.getPotentialSplitTransitions()) { List<BuildOptions> splitOptionsList = transition.split(buildOptions); + + // While it'd be clearer to condition the below on "if (!splitOptionsList.empty())", + // IosExtension.ExtensionSplitArchTransition defaults to a single-value split. If we failed + // that case then no builds would work, whether or not they're iOS builds (since iOS + // configurations are unconditionally loaded). Once we have dynamic configuraiton support + // for split transitions, this will all go away. + if (splitOptionsList.size() > 1 && targetConfiguration.useDynamicConfigurations()) { + throw new InvalidConfigurationException( + "dynamic configurations don't yet support split transitions"); + } + for (BuildOptions splitOptions : splitOptionsList) { BuildConfiguration splitConfig = configurationFactory.getConfiguration( loadedPackageProvider, splitOptions, false, cache); @@ -120,6 +132,29 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact return result; } + private static class BazelTransitions extends BuildConfigurationCollection.Transitions { + public BazelTransitions(BuildConfiguration configuration, + Map<? extends Transition, ConfigurationHolder> transitionTable, + ListMultimap<? extends SplitTransition<?>, BuildConfiguration> splitTransitionTable) { + super(configuration, transitionTable, splitTransitionTable); + } + + @Override + protected Transition getDynamicTransition(Transition configurationTransition) { + if (configurationTransition == ConfigurationTransition.DATA) { + return ConfigurationTransition.NONE; + } else { + return super.getDynamicTransition(configurationTransition); + } + } + } + + @Override + public Transitions getDynamicTransitionLogic(BuildConfiguration config) { + return new BazelTransitions(config, ImmutableMap.<Transition, ConfigurationHolder>of(), + ImmutableListMultimap.<SplitTransition<?>, BuildConfiguration>of()); + } + /** * Gets the correct host configuration for this build. The behavior * depends on the value of the --distinct_host_configuration flag. @@ -193,7 +228,7 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact for (BuildConfiguration config : allConfigurations) { Transitions outgoingTransitions = - new BuildConfigurationCollection.Transitions(config, transitionBuilder.row(config), + new BazelTransitions(config, transitionBuilder.row(config), // Split transitions must not have their own split transitions because then they // would be applied twice due to a quirk in DependencyResolver. See the comment in // DependencyResolver.resolveLateBoundAttributes(). diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java index 2293beb98b..08250ed45f 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java @@ -117,6 +117,13 @@ public class BazelCppRuleClasses { new RuleClass.Configurator<BuildConfiguration, Rule>() { @Override public BuildConfiguration apply(Rule rule, BuildConfiguration configuration) { + if (configuration.useDynamicConfigurations()) { + // Dynamic configurations don't currently work with LIPO. partially because of lack of + // support for TARGET_CONFIG_FOR_LIPO. We can't check for LIPO here because we have + // to apply TARGET_CONFIG_FOR_LIPO to determine it, So we just assume LIPO is disabled. + // This is safe because Bazel errors out if the two options are combined. + return configuration; + } BuildConfiguration toplevelConfig = configuration.getConfiguration(CppTransition.TARGET_CONFIG_FOR_LIPO); // If LIPO is enabled, override the default configuration. @@ -128,10 +135,15 @@ public class BazelCppRuleClasses { return (rule.getLabel().equals( toplevelConfig.getFragment(CppConfiguration.class).getLipoContextLabel())) ? toplevelConfig - : configuration.getTransitions().getConfiguration(ConfigurationTransition.DATA); + : configuration.getTransitions().getStaticConfiguration(ConfigurationTransition.DATA); } return configuration; } + + @Override + public String getCategory() { + return "lipo"; + }; }; /** diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java index 65688bd2b4..7d06129e6b 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java @@ -99,6 +99,9 @@ public final class Attribute implements Comparable<Attribute> { /** Transition to the host configuration. */ HOST, + /** Transition to a null configuration (applies to, e.g., input files). */ + NULL, + /** Transition from the target configuration to the data configuration. */ // TODO(bazel-team): Move this elsewhere. DATA; diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java index 4bba5e8447..43a8ea0be9 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java @@ -160,6 +160,12 @@ public final class RuleClass { */ public interface Configurator<TConfig, TRule> { TConfig apply(TRule rule, TConfig configuration); + + /** + * Describes the Bazel feature this configurator is used for. Used for checking that dynamic + * configuration transitions are only applied to expected configurator types. + */ + String getCategory(); } /** @@ -181,6 +187,11 @@ public final class RuleClass { public Object apply(Object rule, Object configuration) { return configuration; } + + @Override + public String getCategory() { + return "core"; + }; }; /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java index 204c718092..c1b111da07 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java @@ -394,8 +394,13 @@ public final class CcLibraryHelper { */ public CcLibraryHelper addDeps(Iterable<? extends TransitiveInfoCollection> deps) { for (TransitiveInfoCollection dep : deps) { - Preconditions.checkArgument(dep.getConfiguration() == null - || dep.getConfiguration().equals(configuration)); + BuildConfiguration depConfig = dep.getConfiguration(); + if (depConfig != null && depConfig.hasFragment(CppConfiguration.class)) { + // We can't just check for configuration equality because the dep configuration may + // not have all the fragments that rule's configuration does. + Preconditions.checkState(depConfig.getFragment(CppConfiguration.class).equals( + configuration.getFragment(CppConfiguration.class))); + } this.deps.add(dep); } return this; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java index 23ee8f9e94..2ad010a89a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java @@ -18,6 +18,7 @@ import com.google.common.base.Function; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.RedirectChaser; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment; @@ -72,7 +73,13 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { if (params == null) { return null; } - return new CppConfiguration(params); + CppConfiguration cppConfig = new CppConfiguration(params); + if (options.get(BuildConfiguration.Options.class).useDynamicConfigurations + && cppConfig.getLipoMode() != CrosstoolConfig.LipoMode.OFF) { + throw new InvalidConfigurationException( + "LIPO does not currently work with dynamic configurations"); + } + return cppConfig; } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/LipoDataTransition.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/LipoDataTransition.java new file mode 100644 index 0000000000..14b8e92211 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/transitions/LipoDataTransition.java @@ -0,0 +1,62 @@ +// Copyright 2015 Google Inc. 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.rules.cpp.transitions; + +import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.analysis.config.BuildOptions; +import com.google.devtools.build.lib.analysis.config.PatchTransition; +import com.google.devtools.build.lib.rules.cpp.CppOptions; +import com.google.devtools.build.lib.rules.cpp.FdoSupport; +import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig; + +/** + * Dynamic transition that turns off LIPO/FDO settings. + * + * <p>This is suitable, for example, when visiting data dependencies of a C++ rule built with LIPO. + */ +public final class LipoDataTransition implements PatchTransition { + public static final LipoDataTransition INSTANCE = new LipoDataTransition(); + + private LipoDataTransition() {} + + @Override + public BuildOptions apply(BuildOptions options) { + if (options.get(BuildConfiguration.Options.class).isHost) { + return options; + } + + CppOptions cppOptions = options.get(CppOptions.class); + if (cppOptions.lipoMode == CrosstoolConfig.LipoMode.OFF) { + return options; + } + + options = options.clone(); + cppOptions = options.get(CppOptions.class); + + // Once autoFdoLipoData is on, it stays on (through all future transitions). + if (!cppOptions.autoFdoLipoData && cppOptions.fdoOptimize != null) { + cppOptions.autoFdoLipoData = FdoSupport.isAutoFdo(cppOptions.fdoOptimize); + } + cppOptions.lipoMode = CrosstoolConfig.LipoMode.OFF; + cppOptions.fdoInstrument = null; + cppOptions.fdoOptimize = null; + return options; + } + + @Override + public boolean defaultsToSelf() { + return false; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfigurationLoader.java index 15cfbfc7ff..9446909859 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfigurationLoader.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactor import com.google.devtools.build.lib.analysis.config.FragmentOptions; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.rules.cpp.CppConfiguration; +import com.google.devtools.build.lib.rules.cpp.CppOptions; import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaClasspathMode; import com.google.devtools.build.lib.syntax.Label; @@ -33,7 +34,9 @@ import com.google.devtools.build.lib.syntax.Label; public class JavaConfigurationLoader implements ConfigurationFragmentFactory { @Override public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() { - return ImmutableSet.<Class<? extends FragmentOptions>>of(JavaOptions.class); + // TODO(bazel-team): either require CppOptions only for dependency trees that use the JAVA_CPU + // make variable or break out CppConfiguration.getTargetCpu() into its own distinct fragment. + return ImmutableSet.of(JavaOptions.class, CppOptions.class); } 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 7db3e463fe..bcb6d3d688 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 @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.packages.AspectFactory; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.DependencyEvaluationException; @@ -48,9 +49,11 @@ import javax.annotation.Nullable; */ public final class AspectFunction implements SkyFunction { private final BuildViewProvider buildViewProvider; + private final RuleClassProvider ruleClassProvider; - public AspectFunction(BuildViewProvider buildViewProvider) { + public AspectFunction(BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) { this.buildViewProvider = buildViewProvider; + this.ruleClassProvider = ruleClassProvider; } @Nullable @@ -84,11 +87,14 @@ public final class AspectFunction implements SkyFunction { (ConfiguredTargetValue) env.getValue(ConfiguredTargetValue.key(key.getLabel(), key.getConfiguration())); if (configuredTargetValue == null) { + // TODO(bazel-team): remove this check when top-level targets also use dynamic configurations. + // Right now the key configuration may be dynamic while the original target's configuration + // is static, resulting in a Skyframe cache miss even though the original target is, in fact, + // precomputed. return null; } RuleConfiguredTarget associatedTarget = (RuleConfiguredTarget) configuredTargetValue.getConfiguredTarget(); - if (associatedTarget == null) { return null; } @@ -112,7 +118,8 @@ public final class AspectFunction implements SkyFunction { ListMultimap<Attribute, ConfiguredTarget> depValueMap = ConfiguredTargetFunction.computeDependencies(env, resolver, ctgValue, - aspectFactory.getDefinition(), configConditions); + aspectFactory.getDefinition(), configConditions, ruleClassProvider, + view.getHostConfiguration(ctgValue.getConfiguration())); return createAspect(env, key, associatedTarget, configConditions, depValueMap); } catch (DependencyEvaluationException e) { @@ -123,8 +130,7 @@ public final class AspectFunction implements SkyFunction { @Nullable private AspectValue createAspect(Environment env, AspectKey key, RuleConfiguredTarget associatedTarget, Set<ConfigMatchingProvider> configConditions, - ListMultimap<Attribute, ConfiguredTarget> directDeps) - throws AspectFunctionException { + ListMultimap<Attribute, ConfiguredTarget> directDeps) throws AspectFunctionException { SkyframeBuildView view = buildViewProvider.getSkyframeBuildView(); BuildConfiguration configuration = associatedTarget.getConfiguration(); @@ -137,8 +143,8 @@ public final class AspectFunction implements SkyFunction { ConfiguredAspectFactory aspectFactory = (ConfiguredAspectFactory) AspectFactory.Util.create(key.getAspect()); - Aspect aspect = view.createAspect( - analysisEnvironment, associatedTarget, aspectFactory, directDeps, configConditions); + Aspect aspect = view.createAspect(analysisEnvironment, associatedTarget, aspectFactory, + directDeps, configConditions); events.replayOn(env.getListener()); if (events.hasErrors()) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java index 3de1bc5c09..e3a6bd69d4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationFunction.java @@ -19,8 +19,11 @@ import com.google.common.collect.ClassToInstanceMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.MutableClassToInstanceMap; import com.google.devtools.build.lib.analysis.BlazeDirectories; +import com.google.devtools.build.lib.analysis.ConfigurationCollectionFactory; +import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; +import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; @@ -37,9 +40,13 @@ import java.util.Set; public class BuildConfigurationFunction implements SkyFunction { private final BlazeDirectories directories; + private final ConfigurationCollectionFactory collectionFactory; - public BuildConfigurationFunction(BlazeDirectories directories) { + public BuildConfigurationFunction(BlazeDirectories directories, + RuleClassProvider ruleClassProvider) { this.directories = directories; + collectionFactory = + ((ConfiguredRuleClassProvider) ruleClassProvider).getConfigurationCollectionFactory(); } @Override @@ -61,13 +68,22 @@ public class BuildConfigurationFunction implements SkyFunction { fragmentsMap.put(fragment.getClass(), fragment); } - return new BuildConfigurationValue( - new BuildConfiguration(directories, fragmentsMap, key.getBuildOptions(), - !key.actionsEnabled())); + BuildConfiguration config = new BuildConfiguration(directories, fragmentsMap, + key.getBuildOptions(), !key.actionsEnabled()); + // Unlike static configurations, dynamic configurations don't need to embed transition logic + // within the configuration itself. However we still use this interface to provide a mapping + // between Transition types (e.g. HOST) and the dynamic transitions that apply those + // transitions. Once static configurations are cleaned out we won't need this interface + // any more (all the centralized logic that maintains the transition logic can be distributed + // to the actual rule code that uses it). + config.setConfigurationTransitions(collectionFactory.getDynamicTransitionLogic(config)); + + return new BuildConfigurationValue(config); } private Set<Fragment> getConfigurationFragments(BuildConfigurationValue.Key key, Environment env) throws InvalidConfigurationException { + // Get SkyKeys for the fragments we need to load. Set<SkyKey> fragmentKeys = new LinkedHashSet<>(); for (Class<? extends BuildConfiguration.Fragment> fragmentClass : key.getFragments()) { @@ -84,7 +100,11 @@ public class BuildConfigurationFunction implements SkyFunction { // Collect and return the results. ImmutableSet.Builder<Fragment> fragments = ImmutableSet.builder(); for (ValueOrException<InvalidConfigurationException> value : fragmentDeps.values()) { - fragments.add(((ConfigurationFragmentValue) value.get()).getFragment()); + BuildConfiguration.Fragment fragment = + ((ConfigurationFragmentValue) value.get()).getFragment(); + if (fragment != null) { + fragments.add(fragment); + } } return fragments.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java index 3131868943..7883db2fe1 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.base.Supplier; -import com.google.common.base.Verify; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import com.google.common.collect.ImmutableSet; @@ -22,11 +21,12 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigurationFactory; +import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.PackageProviderForConfigurations; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.StoredEventHandler; -import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; +import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.skyframe.ConfigurationCollectionValue.ConfigurationCollectionKey; import com.google.devtools.build.skyframe.SkyFunction; @@ -61,7 +61,7 @@ public class ConfigurationCollectionFunction implements SkyFunction { ConfigurationCollectionKey collectionKey = (ConfigurationCollectionKey) skyKey.argument(); try { BuildConfigurationCollection result = - getConfigurations(env.getListener(), + getConfigurations(env, new SkyframePackageLoaderWithValueEnvironment(env, configurationPackages.get()), collectionKey.getBuildOptions(), collectionKey.getMultiCpu()); @@ -84,7 +84,7 @@ public class ConfigurationCollectionFunction implements SkyFunction { } /** Create the build configurations with the given options. */ - private BuildConfigurationCollection getConfigurations(EventHandler eventHandler, + private BuildConfigurationCollection getConfigurations(Environment env, PackageProviderForConfigurations loadedPackageProvider, BuildOptions buildOptions, ImmutableSet<String> multiCpu) throws InvalidConfigurationException { @@ -99,7 +99,7 @@ public class ConfigurationCollectionFunction implements SkyFunction { if (!multiCpu.isEmpty()) { for (String cpu : multiCpu) { BuildConfiguration targetConfiguration = createConfiguration( - cache, eventHandler, loadedPackageProvider, buildOptions, cpu); + cache, env.getListener(), loadedPackageProvider, buildOptions, cpu); if (targetConfiguration == null || targetConfigurations.contains(targetConfiguration)) { continue; } @@ -110,26 +110,49 @@ public class ConfigurationCollectionFunction implements SkyFunction { } } else { BuildConfiguration targetConfiguration = createConfiguration( - cache, eventHandler, loadedPackageProvider, buildOptions, null); + cache, env.getListener(), loadedPackageProvider, buildOptions, null); if (targetConfiguration == null) { return null; } targetConfigurations.add(targetConfiguration); } - - // Sanity check that all host configs are the same. This may not be true once we have - // better support for multi-host builds. - BuildConfiguration hostConfiguration = - targetConfigurations.get(0).getConfiguration(ConfigurationTransition.HOST); - for (BuildConfiguration targetConfig : - targetConfigurations.subList(1, targetConfigurations.size())) { - Verify.verify( - targetConfig.getConfiguration(ConfigurationTransition.HOST).equals(hostConfiguration)); + BuildConfiguration hostConfiguration = getHostConfiguration(env, targetConfigurations.get(0)); + if (hostConfiguration == null) { + return null; } return new BuildConfigurationCollection(targetConfigurations, hostConfiguration); } + /** + * Returns the host configuration, or null on missing Skyframe deps. + */ + private BuildConfiguration getHostConfiguration(Environment env, + BuildConfiguration targetConfiguration) throws InvalidConfigurationException { + if (targetConfiguration.useDynamicConfigurations()) { + BuildOptions hostOptions = HostTransition.INSTANCE.apply(targetConfiguration.getOptions()); + SkyKey hostConfigKey = + BuildConfigurationValue.key(targetConfiguration.fragmentClasses(), hostOptions); + BuildConfigurationValue skyValHost = (BuildConfigurationValue) + env.getValueOrThrow(hostConfigKey, InvalidConfigurationException.class); + + // Also preload the target configuration so the configured target functions for + // top-level targets don't have to waste cycles from a missing Skyframe dep. + BuildOptions targetOptions = targetConfiguration.getOptions(); + SkyKey targetConfigKey = + BuildConfigurationValue.key(targetConfiguration.fragmentClasses(), targetOptions); + BuildConfigurationValue skyValTarget = (BuildConfigurationValue) + env.getValueOrThrow(targetConfigKey, InvalidConfigurationException.class); + + if (skyValHost == null || skyValTarget == null) { + return null; + } + return skyValHost.getConfiguration(); + } else { + return targetConfiguration.getConfiguration(Attribute.ConfigurationTransition.HOST); + } + } + @Nullable private BuildConfiguration createConfiguration( Cache<String, BuildConfiguration> cache, 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 1132a7b02b..5df28bdedb 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 @@ -15,10 +15,13 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.base.Function; import com.google.common.base.Preconditions; +import com.google.common.base.Verify; import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; +import com.google.common.collect.Multimap; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.Actions; import com.google.devtools.build.lib.actions.Artifact; @@ -33,7 +36,11 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; +import com.google.devtools.build.lib.analysis.config.HostTransition; +import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; +import com.google.devtools.build.lib.analysis.config.PatchTransition; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.AspectDefinition; @@ -46,6 +53,7 @@ import com.google.devtools.build.lib.packages.NoSuchThingException; import com.google.devtools.build.lib.packages.PackageGroup; import com.google.devtools.build.lib.packages.RawAttributeMapper; import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.packages.Type; @@ -57,6 +65,7 @@ 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 com.google.devtools.build.skyframe.ValueOrException; import com.google.devtools.build.skyframe.ValueOrException2; import com.google.devtools.build.skyframe.ValueOrException3; @@ -64,6 +73,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Objects; import java.util.Set; import javax.annotation.Nullable; @@ -111,9 +121,12 @@ final class ConfiguredTargetFunction implements SkyFunction { }; private final BuildViewProvider buildViewProvider; + private final RuleClassProvider ruleClassProvider; - ConfiguredTargetFunction(BuildViewProvider buildViewProvider) { + ConfiguredTargetFunction(BuildViewProvider buildViewProvider, + RuleClassProvider ruleClassProvider) { this.buildViewProvider = buildViewProvider; + this.ruleClassProvider = ruleClassProvider; } @Override @@ -163,20 +176,16 @@ final class ConfiguredTargetFunction implements SkyFunction { // Get the configuration targets that trigger this rule's configurable attributes. Set<ConfigMatchingProvider> configConditions = getConfigConditions(ctgValue.getTarget(), env, resolver, ctgValue); - if (configConditions == null) { - // Those targets haven't yet been resolved. + if (env.valuesMissing()) { return null; } - ListMultimap<Attribute, ConfiguredTarget> depValueMap = - computeDependencies(env, resolver, ctgValue, null, configConditions); - - // TODO(bazel-team): Support dynamically created host configurations. - BuildConfiguration hostConfiguration = configuration == null - ? null : configuration.getConfiguration(Attribute.ConfigurationTransition.HOST); - - return createConfiguredTarget( - view, env, target, configuration, hostConfiguration, depValueMap, configConditions); + ListMultimap<Attribute, ConfiguredTarget> depValueMap = computeDependencies(env, resolver, + ctgValue, null, configConditions, ruleClassProvider, + view.getHostConfiguration(configuration)); + ConfiguredTargetValue ans = createConfiguredTarget( + view, env, target, configuration, depValueMap, configConditions); + return ans; } catch (DependencyEvaluationException e) { throw new ConfiguredTargetFunctionException(e.getRootCauseSkyKey(), e.getCause()); } @@ -195,42 +204,244 @@ final class ConfiguredTargetFunction implements SkyFunction { * @param aspectDefinition the aspect of the node (if null, the node is a configured target, * otherwise it's an asect) * @param configConditions the configuration conditions for evaluating the attributes of the node + * @param ruleClassProvider rule class provider for determining the right configuration fragments + * to apply to deps + * @param hostConfiguration the host configuration. There's a noticeable performance hit from + * instantiating this on demand for every dependency that wants it, so it's best to compute + * the host configuration as early as possible and pass this reference to all consumers + * without involving Skyframe. * @return an attribute -> direct dependency multimap */ @Nullable static ListMultimap<Attribute, ConfiguredTarget> computeDependencies( Environment env, SkyframeDependencyResolver resolver, TargetAndConfiguration ctgValue, - AspectDefinition aspectDefinition, Set<ConfigMatchingProvider> configConditions) + AspectDefinition aspectDefinition, Set<ConfigMatchingProvider> configConditions, + RuleClassProvider ruleClassProvider, BuildConfiguration hostConfiguration) throws DependencyEvaluationException { - // 1. Create the map from attributes to list of (target, configuration) pairs. + // Create the map from attributes to list of (target, configuration) pairs. ListMultimap<Attribute, Dependency> depValueNames; try { - depValueNames = resolver.dependentNodeMap(ctgValue, aspectDefinition, configConditions); + depValueNames = resolver.dependentNodeMap(ctgValue, hostConfiguration, aspectDefinition, + configConditions); } catch (EvalException e) { env.getListener().handle(Event.error(e.getLocation(), e.getMessage())); throw new DependencyEvaluationException(new ConfiguredValueCreationException(e.print())); } - // 2. Resolve configured target dependencies and handle errors. + // Trim each dep's configuration so it only includes the fragments needed by its transitive + // closure (only dynamic configurations support this). + if (ctgValue.getConfiguration() != null + && ctgValue.getConfiguration().useDynamicConfigurations()) { + depValueNames = trimConfigurations(env, ctgValue, depValueNames, hostConfiguration, + ruleClassProvider); + if (depValueNames == null) { + return null; + } + } + + // Resolve configured target dependencies and handle errors. Map<SkyKey, ConfiguredTarget> depValues = resolveConfiguredTargetDependencies(env, depValueNames.values(), ctgValue.getTarget()); if (depValues == null) { return null; } - // 3. Resolve required aspects. + // Resolve required aspects. ListMultimap<SkyKey, Aspect> depAspects = resolveAspectDependencies( env, depValues, depValueNames.values()); if (depAspects == null) { return null; } - // 3. Merge the dependent configured targets and aspects into a single map. + // Merge the dependent configured targets and aspects into a single map. return mergeAspects(depValueNames, depValues, depAspects); } /** + * Helper class for {@link #trimConfigurations} - encapsulates a set of config fragments and + * a dynamic transition. This can be used to determine the exact build options needed to + * set a dynamic configuration. + */ + private static final class FragmentsAndTransition { + // Treat this as immutable. The only reason this isn't an ImmutableSet is because it + // gets bound to a NestedSet.toSet() reference, which returns a Set interface. + final Set<Class<? extends BuildConfiguration.Fragment>> fragments; + final Attribute.Transition transition; + private final int hashCode; + + FragmentsAndTransition(Set<Class<? extends BuildConfiguration.Fragment>> fragments, + Attribute.Transition transition) { + this.fragments = fragments; + this.transition = transition; + hashCode = Objects.hash(this.fragments, this.transition); + } + + @Override + public boolean equals(Object o) { + if (o == this) { + return true; + } else if (o == null) { + return false; + } else { + FragmentsAndTransition other = (FragmentsAndTransition) o; + return other.transition.equals(transition) && other.fragments.equals(fragments); + } + } + + @Override + public int hashCode() { + return hashCode; + } + } + + /** + * Creates a dynamic configuration for each dep that's custom-fitted specifically for that dep. + * + * <p>More specifically: given a set of {@link Dependency} instances holding dynamic config + * transition requests (e.g. {@link Dependency#hasStaticConfiguration()} == false}), returns + * equivalent dependencies containing dynamically created configurations that a) apply those + * transitions and b) only contain the fragments needed by the dep and everything in its + * transitive closure. + * + * <p>This method is heavily performance-optimized. Because it, in aggregate, reads over every + * edge in the configured target graph, small inefficiencies can have observable impact on + * build analysis time. Keep this in mind when making modifications and performance-test any + * changes you make. + */ + @Nullable + static ListMultimap<Attribute, Dependency> trimConfigurations(Environment env, + TargetAndConfiguration ctgValue, ListMultimap<Attribute, Dependency> originalDeps, + BuildConfiguration hostConfiguration, RuleClassProvider ruleClassProvider) + throws DependencyEvaluationException { + + // Maps each Skyframe-evaluated BuildConfiguration to the dependencies that need that + // configuration. For cases where Skyframe isn't needed to get the configuration (e.g. when + // we just re-used the original rule's configuration), we should skip this outright. + Multimap<SkyKey, Map.Entry<Attribute, Dependency>> keysToEntries = ArrayListMultimap.create(); + + // Stores the result of applying a dynamic transition to the current configuration using a + // particular subset of fragments. By caching this, we save from redundantly computing the + // same transition for every dependency edge that requests that transition. This can have + // real effect on analysis time for commonly triggered transitions. + Map<FragmentsAndTransition, BuildOptions> transitionsMap = new HashMap<>(); + + // The fragments used by the current target's configuration. + Set<Class<? extends BuildConfiguration.Fragment>> ctgFragments = + ctgValue.getConfiguration().fragmentClasses(); + BuildOptions ctgOptions = ctgValue.getConfiguration().getOptions(); + + // Our output map. + ListMultimap<Attribute, Dependency> trimmedDeps = ArrayListMultimap.create(); + + for (Map.Entry<Attribute, Dependency> depsEntry : originalDeps.entries()) { + Dependency dep = depsEntry.getValue(); + + if (dep.hasStaticConfiguration()) { + // Certain targets (like output files) trivially pass their configurations to their deps. + // So no need to transform them in any way. + trimmedDeps.put(depsEntry.getKey(), + new Dependency(dep.getLabel(), dep.getConfiguration(), dep.getAspects())); + continue; + } else if (dep.getTransition() == Attribute.ConfigurationTransition.NULL) { + trimmedDeps.put(depsEntry.getKey(), + new Dependency(dep.getLabel(), (BuildConfiguration) null, dep.getAspects())); + continue; + } + + // Figure out the required fragments for this dep and its transitive closure. + SkyKey fragmentsKey = TransitiveTargetValue.key(dep.getLabel()); + TransitiveTargetValue transitiveDepInfo = (TransitiveTargetValue) env.getValue(fragmentsKey); + if (transitiveDepInfo == null) { + // This should only be possible for tests. In actual runs, this was already called + // as a routine part of the loading phase. + // TODO(bazel-team): check this only occurs in a test context. + return null; + } + Set<Class<? extends BuildConfiguration.Fragment>> depFragments = + transitiveDepInfo.getTransitiveConfigFragments().toSet(); + + boolean sameFragments = depFragments.equals(ctgFragments); + Attribute.Transition transition = dep.getTransition(); + + if (sameFragments) { + if (transition == Attribute.ConfigurationTransition.NONE) { + // The dep uses the same exact configuration. + trimmedDeps.put(depsEntry.getKey(), + new Dependency(dep.getLabel(), ctgValue.getConfiguration(), dep.getAspects())); + continue; + } else if (transition == HostTransition.INSTANCE) { + // The current rule's host configuration can also be used for the dep. We short-circuit + // the standard transition logic for host transitions because these transitions are + // uniquely frequent. It's possible, e.g., for every node in the configured target graph + // to incur multiple host transitions. So we aggressively optimize to avoid hurting + // analysis time. + trimmedDeps.put(depsEntry.getKey(), + new Dependency(dep.getLabel(), hostConfiguration, dep.getAspects())); + continue; + } + } + + // Apply the transition or use the cached result if it was already applied. + FragmentsAndTransition transitionKey = new FragmentsAndTransition(depFragments, transition); + BuildOptions toOptions = transitionsMap.get(transitionKey); + if (toOptions == null) { + Verify.verify(transition == Attribute.ConfigurationTransition.NONE + || transition instanceof PatchTransition); + BuildOptions fromOptions = ctgOptions; + if (!sameFragments) { + // TODO(bazel-team): pre-compute getOptionsClasses in the constructor. + fromOptions = fromOptions.trim(BuildConfiguration.getOptionsClasses( + transitiveDepInfo.getTransitiveConfigFragments(), ruleClassProvider)); + } + // TODO(bazel-team): safety-check that the below call never mutates fromOptions. + toOptions = transition == Attribute.ConfigurationTransition.NONE + ? fromOptions + : ((PatchTransition) transition).apply(fromOptions); + transitionsMap.put(transitionKey, toOptions); + } + + // If the transition doesn't change the configuration, trivially re-use the original + // configuration. + if (sameFragments && toOptions.equals(ctgOptions)) { + trimmedDeps.put(depsEntry.getKey(), + new Dependency(dep.getLabel(), ctgValue.getConfiguration(), dep.getAspects())); + continue; + } + + // If we get here, we have to get the configuration from Skyframe. + keysToEntries.put(BuildConfigurationValue.key(depFragments, toOptions), depsEntry); + } + + // Get all BuildConfigurations we need to get from Skyframe. + Map<SkyKey, ValueOrException<InvalidConfigurationException>> depConfigValues = + env.getValuesOrThrow(keysToEntries.keySet(), InvalidConfigurationException.class); + if (env.valuesMissing()) { + return null; + } + + // Now fill in the remaining unresolved deps with the now-resolved configurations. + try { + for (Map.Entry<SkyKey, ValueOrException<InvalidConfigurationException>> entry : + depConfigValues.entrySet()) { + SkyKey key = entry.getKey(); + BuildConfigurationValue trimmedConfig = (BuildConfigurationValue) entry.getValue().get(); + for (Map.Entry<Attribute, Dependency> info : keysToEntries.get(key)) { + Attribute attribute = info.getKey(); + Dependency originalDep = info.getValue(); + trimmedDeps.put(attribute, + new Dependency(originalDep.getLabel(), trimmedConfig.getConfiguration(), + originalDep.getAspects())); + } + } + } catch (InvalidConfigurationException e) { + throw new DependencyEvaluationException(e); + } + + return trimmedDeps; + } + + /** * 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 @@ -364,6 +575,20 @@ final class ConfiguredTargetFunction implements SkyFunction { // been computed yet. Collection<Dependency> configValueNames = resolver.resolveRuleLabels(ctgValue, null, configLabelMap); + + // No need to get new configs from Skyframe - config_setting rules always use the current + // target's config. + // TODO(bazel-team): remove the need for this special transformation. We can probably do this by + // simply passing this through trimConfigurations. + BuildConfiguration targetConfig = ctgValue.getConfiguration(); + if (targetConfig != null && targetConfig.useDynamicConfigurations()) { + ImmutableList.Builder<Dependency> staticConfigs = ImmutableList.builder(); + for (Dependency dep : configValueNames) { + staticConfigs.add(new Dependency(dep.getLabel(), targetConfig, dep.getAspects())); + } + configValueNames = staticConfigs.build(); + } + Map<SkyKey, ConfiguredTarget> configValues = resolveConfiguredTargetDependencies(env, configValueNames, target); if (configValues == null) { @@ -470,7 +695,7 @@ final class ConfiguredTargetFunction implements SkyFunction { @Nullable private ConfiguredTargetValue createConfiguredTarget(SkyframeBuildView view, Environment env, Target target, BuildConfiguration configuration, - BuildConfiguration hostConfiguration, ListMultimap<Attribute, ConfiguredTarget> depValueMap, + ListMultimap<Attribute, ConfiguredTarget> depValueMap, Set<ConfigMatchingProvider> configConditions) throws ConfiguredTargetFunctionException, InterruptedException { StoredEventHandler events = new StoredEventHandler(); @@ -484,7 +709,7 @@ final class ConfiguredTargetFunction implements SkyFunction { } ConfiguredTarget configuredTarget = view.createConfiguredTarget(target, configuration, - hostConfiguration, analysisEnvironment, depValueMap, configConditions); + analysisEnvironment, depValueMap, configConditions); events.replayOn(env.getListener()); if (events.hasErrors()) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java index 03920b7982..5f5d538956 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java @@ -18,24 +18,27 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.common.collect.ListMultimap; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.DependencyResolver.Dependency; import com.google.devtools.build.lib.analysis.LabelAndConfiguration; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.RawAttributeMapper; import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ConflictException; +import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.Label; 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.LinkedHashSet; import java.util.Map; import java.util.Set; @@ -56,10 +59,13 @@ public class PostConfiguredTargetFunction implements SkyFunction { }; private final SkyframeExecutor.BuildViewProvider buildViewProvider; + private final RuleClassProvider ruleClassProvider; public PostConfiguredTargetFunction( - SkyframeExecutor.BuildViewProvider buildViewProvider) { + SkyframeExecutor.BuildViewProvider buildViewProvider, + RuleClassProvider ruleClassProvider) { this.buildViewProvider = Preconditions.checkNotNull(buildViewProvider); + this.ruleClassProvider = ruleClassProvider; } @Nullable @@ -90,8 +96,22 @@ public class PostConfiguredTargetFunction implements SkyFunction { return null; } - Collection<Dependency> deps = resolver.dependentNodes(ctgValue, configConditions); - env.getValues(Iterables.transform(deps, TO_KEYS)); + ListMultimap<Attribute, Dependency> deps; + try { + BuildConfiguration hostConfiguration = + buildViewProvider.getSkyframeBuildView().getHostConfiguration(ct.getConfiguration()); + deps = resolver.dependentNodeMap(ctgValue, hostConfiguration, null, configConditions); + if (ct.getConfiguration() != null && ct.getConfiguration().useDynamicConfigurations()) { + deps = ConfiguredTargetFunction.trimConfigurations(env, ctgValue, deps, hostConfiguration, + ruleClassProvider); + } + } catch (EvalException e) { + throw new PostConfiguredTargetFunctionException(e); + } catch (ConfiguredTargetFunction.DependencyEvaluationException e) { + throw new PostConfiguredTargetFunctionException(e); + } + + env.getValues(Iterables.transform(deps.values(), TO_KEYS)); if (env.valuesMissing()) { return null; } @@ -142,4 +162,10 @@ public class PostConfiguredTargetFunction implements SkyFunction { super(e, Transience.PERSISTENT); } } + + private static class PostConfiguredTargetFunctionException extends SkyFunctionException { + public PostConfiguredTargetFunctionException(Exception e) { + super(e, Transience.PERSISTENT); + } + } } 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 8f1754d933..4d2010c55e 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 @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; +import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.Action; @@ -48,6 +49,7 @@ import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.packages.Attribute; +import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.skyframe.ActionLookupValue.ActionLookupKey; import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; @@ -100,14 +102,24 @@ public final class SkyframeBuildView { private Set<SkyKey> dirtiedConfiguredTargetKeys = Sets.newConcurrentHashSet(); private volatile boolean anyConfiguredTargetDeleted = false; - public SkyframeBuildView(ConfiguredTargetFactory factory, - ArtifactFactory artifactFactory, - SkyframeExecutor skyframeExecutor, Runnable legacyDataCleaner, BinTools binTools) { + private final RuleClassProvider ruleClassProvider; + + // The host configuration containing all fragments used by this build's transitive closure. + private BuildConfiguration topLevelHostConfiguration; + // Fragment-limited versions of the host configuration. It's faster to create/cache these here + // than to store them in Skyframe. + private Map<Set<Class<? extends BuildConfiguration.Fragment>>, BuildConfiguration> + hostConfigurationCache = Maps.newConcurrentMap(); + + public SkyframeBuildView(ConfiguredTargetFactory factory, ArtifactFactory artifactFactory, + SkyframeExecutor skyframeExecutor, Runnable legacyDataCleaner, BinTools binTools, + RuleClassProvider ruleClassProvider) { this.factory = factory; this.artifactFactory = artifactFactory; this.skyframeExecutor = skyframeExecutor; this.legacyDataCleaner = legacyDataCleaner; this.binTools = binTools; + this.ruleClassProvider = ruleClassProvider; skyframeExecutor.setArtifactFactoryAndBinTools(artifactFactory, binTools); } @@ -119,6 +131,21 @@ public final class SkyframeBuildView { return ImmutableSet.copyOf(evaluatedConfiguredTargets); } + /** + * Sets the host configuration consisting of all fragments that will be used by the top level + * targets' transitive closures. + * + * <p>This is used to power {@link #getHostConfiguration} during analysis, which computes + * fragment-trimmed host configurations from the top-level one. + */ + public void setTopLevelHostConfiguration(BuildConfiguration topLevelHostConfiguration) { + if (topLevelHostConfiguration.equals(this.topLevelHostConfiguration)) { + return; + } + hostConfigurationCache.clear(); + this.topLevelHostConfiguration = topLevelHostConfiguration; + } + private void setDeserializedArtifactOwners() throws ViewCreationFailedException { Map<PathFragment, Artifact> deserializedArtifactMap = artifactFactory.getDeserializedArtifacts(); @@ -393,13 +420,38 @@ public final class SkyframeBuildView { */ @Nullable ConfiguredTarget createConfiguredTarget(Target target, BuildConfiguration configuration, - BuildConfiguration hostConfiguration, CachingAnalysisEnvironment analysisEnvironment, + CachingAnalysisEnvironment analysisEnvironment, ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap, Set<ConfigMatchingProvider> configConditions) throws InterruptedException { Preconditions.checkState(enableAnalysis, "Already in execution phase %s %s", target, configuration); return factory.createConfiguredTarget(analysisEnvironment, artifactFactory, target, - configuration, hostConfiguration, prerequisiteMap, configConditions); + configuration, getHostConfiguration(configuration), prerequisiteMap, + configConditions); + } + + /** + * Returns the host configuration trimmed to the same fragments as the input configuration. If + * the input is null, returns the top-level host configuration. + * + * <p>For static configurations, this unconditionally returns the (sole) top-level configuration. + * + * <p>This may only be called after {@link #setTopLevelHostConfiguration} has set the + * correct host configuration at the top-level. + */ + public BuildConfiguration getHostConfiguration(BuildConfiguration config) { + if (config == null || !config.useDynamicConfigurations()) { + return topLevelHostConfiguration; + } + Set<Class<? extends BuildConfiguration.Fragment>> fragmentClasses = config.fragmentClasses(); + BuildConfiguration hostConfig = hostConfigurationCache.get(fragmentClasses); + if (hostConfig != null) { + return hostConfig; + } + BuildConfiguration trimmedConfig = + topLevelHostConfiguration.clone(fragmentClasses, ruleClassProvider); + hostConfigurationCache.put(fragmentClasses, trimmedConfig); + return trimmedConfig; } @Nullable @@ -408,8 +460,8 @@ public final class SkyframeBuildView { ConfiguredAspectFactory aspectFactory, ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap, Set<ConfigMatchingProvider> configConditions) { - return factory.createAspect( - env, associatedTarget, aspectFactory, prerequisiteMap, configConditions); + return factory.createAspect(env, associatedTarget, aspectFactory, prerequisiteMap, + configConditions, getHostConfiguration(associatedTarget.getConfiguration())); } @Nullable 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 2b7cb09c86..c6b5a22f82 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 @@ -64,10 +64,12 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigurationFactory; import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; +import com.google.devtools.build.lib.analysis.config.PatchTransition; import com.google.devtools.build.lib.concurrent.ThreadSafety; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Reporter; +import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchThingException; @@ -327,11 +329,12 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { map.put(SkyFunctions.TRANSITIVE_TARGET, new TransitiveTargetFunction(ruleClassProvider)); map.put(SkyFunctions.TRANSITIVE_TRAVERSAL, new TransitiveTraversalFunction()); map.put(SkyFunctions.CONFIGURED_TARGET, - new ConfiguredTargetFunction(new BuildViewProvider())); - map.put(SkyFunctions.ASPECT, new AspectFunction(new BuildViewProvider())); + new ConfiguredTargetFunction(new BuildViewProvider(), ruleClassProvider)); + map.put(SkyFunctions.ASPECT, new AspectFunction(new BuildViewProvider(), ruleClassProvider)); map.put(SkyFunctions.POST_CONFIGURED_TARGET, - new PostConfiguredTargetFunction(new BuildViewProvider())); - map.put(SkyFunctions.BUILD_CONFIGURATION, new BuildConfigurationFunction(directories)); + new PostConfiguredTargetFunction(new BuildViewProvider(), ruleClassProvider)); + map.put(SkyFunctions.BUILD_CONFIGURATION, + new BuildConfigurationFunction(directories, ruleClassProvider)); map.put(SkyFunctions.CONFIGURATION_COLLECTION, new ConfigurationCollectionFunction( configurationFactory, configurationPackages)); map.put(SkyFunctions.CONFIGURATION_FRAGMENT, new ConfigurationFragmentFunction( @@ -551,7 +554,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { PrecomputedValue.TOP_LEVEL_CONTEXT.set(injectable(), options); } - public void injectWorkspaceStatusData() { + public void injectWorkspaceStatusData(BuildConfigurationCollection configurations) { PrecomputedValue.WORKSPACE_STATUS_KEY.set(injectable(), workspaceStatusActionFactory.createWorkspaceStatusAction( artifactFactory.get(), WorkspaceStatusValue.ARTIFACT_OWNER, buildId)); @@ -1061,47 +1064,60 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { * returned list. */ @ThreadSafety.ThreadSafe - public ImmutableList<ConfiguredTarget> getConfiguredTargets(Iterable<Dependency> keys) { + public ImmutableList<ConfiguredTarget> getConfiguredTargets(BuildConfiguration originalConfig, + Iterable<Dependency> keys, boolean useOriginalConfig) { + return getConfiguredTargetMap(originalConfig, keys, useOriginalConfig).values().asList(); + } + + @ThreadSafety.ThreadSafe + public ImmutableMap<Dependency, ConfiguredTarget> getConfiguredTargetMap( + BuildConfiguration originalConfig, Iterable<Dependency> keys, boolean useOriginalConfig) { checkActive(); if (skyframeBuildView == null) { // If build view has not yet been initialized, no configured targets can have been created. // This is most likely to happen after a failed loading phase. - return ImmutableList.of(); + return ImmutableMap.of(); } + + Map<Dependency, BuildConfiguration> configs; + if (originalConfig != null) { + if (useOriginalConfig) { + // This flag is used because of some unfortunate complexity in the configuration machinery: + // Most callers of this method pass a <Label, Configuration> pair to directly create a + // ConfiguredTarget from, but happen to use the Dependency data structure to pass that + // info (even though the data has nothing to do with dependencies). If this configuration + // includes a split transition, a dynamic configuration created from it will *not* + // include that transition (because dynamic configurations don't embed transitions to + // other configurations. In that case, we need to preserve the original configuration. + // TODO(bazel-team); make this unnecessary once split transition logic is properly ported + // out of configurations. + configs = new HashMap<>(); + configs.put(Iterables.getOnlyElement(keys), originalConfig); + } else { + configs = getConfigurations(originalConfig.getOptions(), keys); + } + } else { + configs = new HashMap<>(); + for (Dependency key : keys) { + configs.put(key, null); + } + } + final List<SkyKey> skyKeys = new ArrayList<>(); for (Dependency key : keys) { - skyKeys.add(ConfiguredTargetValue.key(key.getLabel(), key.getConfiguration())); + skyKeys.add(ConfiguredTargetValue.key(key.getLabel(), configs.get(key))); for (Class<? extends ConfiguredAspectFactory> aspect : key.getAspects()) { - skyKeys.add(AspectValue.key(key.getLabel(), key.getConfiguration(), aspect)); + skyKeys.add(AspectValue.key(key.getLabel(), configs.get(key), aspect)); } } - EvaluationResult<SkyValue> result; - try { - result = callUninterruptibly(new Callable<EvaluationResult<SkyValue>>() { - @Override - public EvaluationResult<SkyValue> call() throws Exception { - synchronized (valueLookupLock) { - try { - skyframeBuildView.enableAnalysis(true); - return buildDriver.evaluate(skyKeys, false, DEFAULT_THREAD_COUNT, - errorEventListener); - } finally { - skyframeBuildView.enableAnalysis(false); - } - } - } - }); - } catch (Exception e) { - throw new IllegalStateException(e); // Should never happen. - } - - ImmutableList.Builder<ConfiguredTarget> cts = ImmutableList.builder(); + EvaluationResult<SkyValue> result = evaluateSkyKeys(skyKeys); + ImmutableMap.Builder<Dependency, ConfiguredTarget> cts = ImmutableMap.builder(); DependentNodeLoop: for (Dependency key : keys) { SkyKey configuredTargetKey = ConfiguredTargetValue.key( - key.getLabel(), key.getConfiguration()); + key.getLabel(), configs.get(key)); if (result.get(configuredTargetKey) == null) { continue; } @@ -1111,7 +1127,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { List<Aspect> aspects = new ArrayList<>(); for (Class<? extends ConfiguredAspectFactory> aspect : key.getAspects()) { - SkyKey aspectKey = AspectValue.key(key.getLabel(), key.getConfiguration(), aspect); + SkyKey aspectKey = AspectValue.key(key.getLabel(), configs.get(key), aspect); if (result.get(aspectKey) == null) { continue DependentNodeLoop; } @@ -1119,13 +1135,126 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { aspects.add(((AspectValue) result.get(aspectKey)).getAspect()); } - cts.add(RuleConfiguredTarget.mergeAspects(configuredTarget, aspects)); + cts.put(key, RuleConfiguredTarget.mergeAspects(configuredTarget, aspects)); } return cts.build(); } /** + * Retrieves the configurations needed for the given deps, trimming down their fragments + * to those only needed by their transitive closures. + */ + private Map<Dependency, BuildConfiguration> getConfigurations(BuildOptions fromOptions, + Iterable<Dependency> keys) { + Map<Dependency, BuildConfiguration> builder = new HashMap<>(); + Set<Dependency> depsToEvaluate = new HashSet<>(); + + // Check: if !Configuration.useDynamicConfigs then just return the original configs. + + // Get the fragments needed for dynamic configuration nodes. + final List<SkyKey> transitiveFragmentSkyKeys = new ArrayList<>(); + Map<Label, Set<Class<? extends BuildConfiguration.Fragment>>> fragmentsMap = new HashMap<>(); + Set<Label> labelsWithErrors = new HashSet<>(); + for (Dependency key : keys) { + if (key.hasStaticConfiguration()) { + builder.put(key, key.getConfiguration()); + } else if (key.getTransition() == Attribute.ConfigurationTransition.NULL) { + builder.put(key, null); + } else { + depsToEvaluate.add(key); + transitiveFragmentSkyKeys.add(TransitiveTargetValue.key(key.getLabel())); + } + } + EvaluationResult<SkyValue> fragmentsResult = evaluateSkyKeys(transitiveFragmentSkyKeys); + for (Dependency key : keys) { + if (!depsToEvaluate.contains(key)) { + // No fragments to compute here. + } else if (fragmentsResult.getError(TransitiveTargetValue.key(key.getLabel())) != null) { + labelsWithErrors.add(key.getLabel()); + } else { + TransitiveTargetValue ttv = + (TransitiveTargetValue) fragmentsResult.get(TransitiveTargetValue.key(key.getLabel())); + fragmentsMap.put(key.getLabel(), ttv.getTransitiveConfigFragments().toSet()); + } + } + + // Now get the configurations. + final List<SkyKey> configSkyKeys = new ArrayList<>(); + for (Dependency key : keys) { + if (!depsToEvaluate.contains(key) || labelsWithErrors.contains(key.getLabel())) { + continue; + } + configSkyKeys.add(BuildConfigurationValue.key(fragmentsMap.get(key.getLabel()), + getDynamicConfigOptions(key, fromOptions))); + } + EvaluationResult<SkyValue> configsResult = evaluateSkyKeys(configSkyKeys); + for (Dependency key : keys) { + if (!depsToEvaluate.contains(key) || labelsWithErrors.contains(key.getLabel())) { + continue; + } + SkyKey configKey = BuildConfigurationValue.key(fragmentsMap.get(key.getLabel()), + getDynamicConfigOptions(key, fromOptions)); + builder.put(key, ((BuildConfigurationValue) configsResult.get(configKey)).getConfiguration()); + } + + return builder; + } + + /** + * Computes the build options needed for the given key, accounting for transitions possibly + * specified in the key. + */ + private BuildOptions getDynamicConfigOptions(Dependency key, BuildOptions fromOptions) { + if (key.hasStaticConfiguration()) { + return key.getConfiguration().getOptions(); + } else if (key.getTransition() == Attribute.ConfigurationTransition.NONE) { + return fromOptions; + } else { + return ((PatchTransition) key.getTransition()).apply(fromOptions); + } + } + + /** + * Evaluates the given sky keys, blocks, and returns their evaluation results. + */ + private EvaluationResult<SkyValue> evaluateSkyKeys(final Iterable<SkyKey> skyKeys) { + EvaluationResult<SkyValue> result; + try { + result = callUninterruptibly(new Callable<EvaluationResult<SkyValue>>() { + @Override + public EvaluationResult<SkyValue> call() throws Exception { + synchronized (valueLookupLock) { + try { + skyframeBuildView.enableAnalysis(true); + return buildDriver.evaluate(skyKeys, false, DEFAULT_THREAD_COUNT, errorEventListener); + } finally { + skyframeBuildView.enableAnalysis(false); + } + } + } + }); + } catch (Exception e) { + throw new IllegalStateException(e); // Should never happen. + } + return result; + } + + /** + * Returns a dynamic configuration constructed from the given configuration fragments and build + * options. + */ + @VisibleForTesting + public BuildConfiguration getConfigurationForTesting( + Set<Class<? extends BuildConfiguration.Fragment>> fragments, BuildOptions options) + throws InterruptedException { + SkyKey key = BuildConfigurationValue.key(fragments, options); + BuildConfigurationValue result = (BuildConfigurationValue) buildDriver + .evaluate(ImmutableList.of(key), false, DEFAULT_THREAD_COUNT, errorEventListener).get(key); + return result.getConfiguration(); + } + + /** * Returns a particular configured target. * * <p>Used only for testing. @@ -1136,10 +1265,12 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { Label label, BuildConfiguration configuration) { if (memoizingEvaluator.getExistingValueForTesting( PrecomputedValue.WORKSPACE_STATUS_KEY.getKeyForTesting()) == null) { - injectWorkspaceStatusData(); + injectWorkspaceStatusData(null); } - return Iterables.getFirst(getConfiguredTargets(ImmutableList.of( - new Dependency(label, configuration))), null); + return Iterables.getFirst( + getConfiguredTargets(configuration, ImmutableList.of(new Dependency(label, configuration)), + true), + null); } /** |