diff options
Diffstat (limited to 'src/main/java/com')
22 files changed, 359 insertions, 128 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 ce726dcbed..deaddcbfa1 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 @@ -1062,7 +1062,9 @@ public class BuildView { @Override protected List<BuildConfiguration> getConfigurations( - FragmentClassSet fragments, Iterable<BuildOptions> buildOptions) { + FragmentClassSet fragments, + Iterable<BuildOptions> buildOptions, + BuildOptions defaultBuildOptions) { Preconditions.checkArgument( ct.getConfiguration().fragmentClasses().equals(fragments), "Mismatch: %s %s", @@ -1091,7 +1093,8 @@ public class BuildView { getConfigurableAttributeKeysForTesting(eventHandler, ctgNode), toolchainContext == null ? ImmutableSet.of() - : toolchainContext.getResolvedToolchainLabels()); + : toolchainContext.getResolvedToolchainLabels(), + skyframeExecutor.getDefaultBuildOptions()); } /** 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 fb13cfb218..1edf561a50 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 @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.actions.FailAction; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; 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.ConfigMatchingProvider; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; @@ -90,9 +91,12 @@ public final class ConfiguredTargetFactory { // in order to be accessible from the .view.skyframe package. private final ConfiguredRuleClassProvider ruleClassProvider; + private final BuildOptions defaultBuildOptions; - public ConfiguredTargetFactory(ConfiguredRuleClassProvider ruleClassProvider) { + public ConfiguredTargetFactory( + ConfiguredRuleClassProvider ruleClassProvider, BuildOptions defaultBuildOptions) { this.ruleClassProvider = ruleClassProvider; + this.defaultBuildOptions = defaultBuildOptions; } /** @@ -174,7 +178,8 @@ public final class ConfiguredTargetFactory { ArtifactOwner owner = ConfiguredTargetKey.of( rule.getLabel(), - getArtifactOwnerConfiguration(analysisEnvironment.getSkyframeEnv(), configuration)); + getArtifactOwnerConfiguration( + analysisEnvironment.getSkyframeEnv(), configuration, defaultBuildOptions)); if (analysisEnvironment.getSkyframeEnv().valuesMissing()) { return null; } @@ -190,11 +195,12 @@ public final class ConfiguredTargetFactory { } /** - * Returns the configuration's artifact owner (which may be null). Also returns null if the - * owning configuration isn't yet available from Skyframe. + * Returns the configuration's artifact owner (which may be null). Also returns null if the owning + * configuration isn't yet available from Skyframe. */ - public static BuildConfiguration getArtifactOwnerConfiguration(SkyFunction.Environment env, - BuildConfiguration fromConfig) throws InterruptedException { + public static BuildConfiguration getArtifactOwnerConfiguration( + SkyFunction.Environment env, BuildConfiguration fromConfig, BuildOptions defaultBuildOptions) + throws InterruptedException { if (fromConfig == null) { return null; } @@ -203,10 +209,14 @@ public final class ConfiguredTargetFactory { return fromConfig; } try { - BuildConfigurationValue ownerConfig = (BuildConfigurationValue) env.getValueOrThrow( - BuildConfigurationValue.key( - fromConfig.fragmentClasses(), ownerTransition.apply(fromConfig.getOptions())), - InvalidConfigurationException.class); + BuildConfigurationValue ownerConfig = + (BuildConfigurationValue) + env.getValueOrThrow( + BuildConfigurationValue.key( + fromConfig.fragmentClasses(), + BuildOptions.diffForReconstruction( + defaultBuildOptions, ownerTransition.apply(fromConfig.getOptions()))), + InvalidConfigurationException.class); return ownerConfig == null ? null : ownerConfig.getConfiguration(); } catch (InvalidConfigurationException e) { // We don't expect to have to handle an invalid configuration because in practice the owning 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 77fe0add2f..fbb23108ea 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 @@ -91,6 +91,8 @@ public abstract class DependencyResolver { * @param aspect the aspect applied to this target (if any) * @param configConditions resolver for config_setting labels * @param toolchainLabels required toolchain labels + * @param defaultBuildOptions default build options provided to the server to use for creating + * diffs during key construction * @return a mapping of each attribute in this rule or aspects to its dependent nodes */ public final OrderedSetMultimap<Attribute, Dependency> dependentNodeMap( @@ -98,7 +100,8 @@ public abstract class DependencyResolver { BuildConfiguration hostConfig, @Nullable Aspect aspect, ImmutableMap<Label, ConfigMatchingProvider> configConditions, - ImmutableSet<Label> toolchainLabels) + ImmutableSet<Label> toolchainLabels, + BuildOptions defaultBuildOptions) throws EvalException, InvalidConfigurationException, InterruptedException, InconsistentAspectOrderException { NestedSetBuilder<Label> rootCauses = NestedSetBuilder.<Label>stableOrder(); @@ -109,7 +112,8 @@ public abstract class DependencyResolver { aspect != null ? ImmutableList.of(aspect) : ImmutableList.<Aspect>of(), configConditions, toolchainLabels, - rootCauses); + rootCauses, + defaultBuildOptions); if (!rootCauses.isEmpty()) { throw new IllegalStateException(rootCauses.build().iterator().next().toString()); } @@ -144,6 +148,8 @@ public abstract class DependencyResolver { * @param toolchainLabels required toolchain labels * @param rootCauses collector for dep labels that can't be (loading phase) loaded @return a * mapping of each attribute in this rule or aspects to its dependent nodes + * @param defaultBuildOptions default build options provided by the server to use for creating + * diffs during key construction */ public final OrderedSetMultimap<Attribute, Dependency> dependentNodeMap( TargetAndConfiguration node, @@ -151,7 +157,8 @@ public abstract class DependencyResolver { Iterable<Aspect> aspects, ImmutableMap<Label, ConfigMatchingProvider> configConditions, ImmutableSet<Label> toolchainLabels, - NestedSetBuilder<Label> rootCauses) + NestedSetBuilder<Label> rootCauses, + BuildOptions defaultBuildOptions) throws EvalException, InvalidConfigurationException, InterruptedException, InconsistentAspectOrderException { Target target = node.getTarget(); @@ -168,7 +175,14 @@ public abstract class DependencyResolver { visitTargetVisibility(node, rootCauses, outgoingEdges.get(null)); } else if (target instanceof Rule) { visitRule( - node, hostConfig, aspects, configConditions, toolchainLabels, rootCauses, outgoingEdges); + node, + hostConfig, + aspects, + configConditions, + toolchainLabels, + rootCauses, + outgoingEdges, + defaultBuildOptions); } else if (target instanceof PackageGroup) { visitPackageGroup(node, (PackageGroup) target, rootCauses, outgoingEdges.get(null)); } else { @@ -185,7 +199,8 @@ public abstract class DependencyResolver { ImmutableMap<Label, ConfigMatchingProvider> configConditions, ImmutableSet<Label> toolchainLabels, NestedSetBuilder<Label> rootCauses, - OrderedSetMultimap<Attribute, Dependency> outgoingEdges) + OrderedSetMultimap<Attribute, Dependency> outgoingEdges, + BuildOptions defaultBuildOptions) throws EvalException, InvalidConfigurationException, InconsistentAspectOrderException, InterruptedException { Preconditions.checkArgument(node.getTarget() instanceof Rule); @@ -199,7 +214,7 @@ public abstract class DependencyResolver { visitTargetVisibility(node, rootCauses, outgoingEdges.get(null)); resolveEarlyBoundAttributes(depResolver); - resolveLateBoundAttributes(depResolver, ruleConfig, hostConfig); + resolveLateBoundAttributes(depResolver, ruleConfig, hostConfig, defaultBuildOptions); Attribute toolchainsAttribute = attributeMap.getAttributeDefinition(PlatformSemantics.TOOLCHAINS_ATTR); @@ -330,34 +345,38 @@ public abstract class DependencyResolver { /** * Resolves the dependencies for all late-bound attributes in this rule. * - * <p>Late-bound attributes need special handling because they require configuration - * transitions to determine their values. + * <p>Late-bound attributes need special handling because they require configuration transitions + * to determine their values. * * <p>In other words, the normal process of dependency resolution is: + * * <ol> - * <li>Find every label value in the rule's attributes</li> + * <li>Find every label value in the rule's attributes * <li>Apply configuration transitions over each value to get its dep configuration - * <li>Return each value with its dep configuration</li> + * <li>Return each value with its dep configuration * </ol> * - * This doesn't work for late-bound attributes because you can't get their values without - * knowing the configuration first. And that configuration may not be the owning rule's - * configuration. Specifically, {@link LateBoundDefault#useHostConfiguration()} switches to the - * host config and late-bound split attributes branch into multiple split configs. + * This doesn't work for late-bound attributes because you can't get their values without knowing + * the configuration first. And that configuration may not be the owning rule's configuration. + * Specifically, {@link LateBoundDefault#useHostConfiguration()} switches to the host config and + * late-bound split attributes branch into multiple split configs. * - * <p>This method implements that logic and makes sure the normal configuration - * transition logic mixes with it cleanly. + * <p>This method implements that logic and makes sure the normal configuration transition logic + * mixes with it cleanly. * * @param depResolver the resolver for this rule's deps * @param ruleConfig the rule's configuration * @param hostConfig the equivalent host configuration + * @param defaultBuildOptions default build options provided by the server to use for creating + * diffs during key construction */ private void resolveLateBoundAttributes( RuleResolver depResolver, BuildConfiguration ruleConfig, - BuildConfiguration hostConfig) + BuildConfiguration hostConfig, + BuildOptions defaultBuildOptions) throws EvalException, InvalidConfigurationException, InconsistentAspectOrderException, - InterruptedException{ + InterruptedException { ConfiguredAttributeMapper attributeMap = depResolver.attributeMap; for (AttributeAndOwner attributeAndOwner : depResolver.attributes) { Attribute attribute = attributeAndOwner.attribute; @@ -377,7 +396,7 @@ public abstract class DependencyResolver { // non-split branch, which calls TransitionResolver.evaluateTransition, which returns its // "host mode" result without ever looking at the split. Iterable<BuildConfiguration> splitConfigs = - getConfigurations(ruleConfig.fragmentClasses(), splitOptions); + getConfigurations(ruleConfig.fragmentClasses(), splitOptions, defaultBuildOptions); if (splitConfigs == null) { continue; // Need Skyframe deps. } @@ -868,7 +887,9 @@ public abstract class DependencyResolver { */ @Nullable protected abstract List<BuildConfiguration> getConfigurations( - FragmentClassSet fragments, Iterable<BuildOptions> buildOptions) + FragmentClassSet fragments, + Iterable<BuildOptions> buildOptions, + BuildOptions defaultBuildOptions) throws InvalidConfigurationException, InterruptedException; /** 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 9f5c6263b5..4308cd4f1c 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 @@ -1171,6 +1171,7 @@ public class BuildConfiguration { private final ActionEnvironment testEnv; private final BuildOptions buildOptions; + private final BuildOptions.OptionsDiffForReconstruction buildOptionsDiff; private final Options options; private final String mnemonic; @@ -1323,12 +1324,12 @@ public class BuildConfiguration { return fragmentsInterner.intern(ImmutableSortedMap.copyOf(fragmentsMap, lexicalFragmentSorter)); } - /** - * Constructs a new BuildConfiguration instance. - */ - public BuildConfiguration(BlazeDirectories directories, + /** Constructs a new BuildConfiguration instance. */ + public BuildConfiguration( + BlazeDirectories directories, Map<Class<? extends Fragment>, Fragment> fragmentsMap, BuildOptions buildOptions, + BuildOptions.OptionsDiffForReconstruction buildOptionsDiff, String repositoryName) { this.directories = directories; this.fragments = makeFragmentsMap(fragmentsMap); @@ -1337,6 +1338,7 @@ public class BuildConfiguration { this.skylarkVisibleFragments = buildIndexOfSkylarkVisibleFragments(); this.repositoryName = repositoryName; this.buildOptions = buildOptions.clone(); + this.buildOptionsDiff = buildOptionsDiff; this.actionsEnabled = buildOptions.enableActions(); this.options = buildOptions.get(Options.class); this.separateGenfilesDirectory = options.separateGenfilesDirectory; @@ -1419,7 +1421,9 @@ public class BuildConfiguration { * configuration is assumed to have). */ public BuildConfiguration clone( - FragmentClassSet fragmentClasses, RuleClassProvider ruleClassProvider) { + FragmentClassSet fragmentClasses, + RuleClassProvider ruleClassProvider, + BuildOptions defaultBuildOptions) { ClassToInstanceMap<Fragment> fragmentsMap = MutableClassToInstanceMap.create(); for (Fragment fragment : fragments.values()) { @@ -1434,6 +1438,7 @@ public class BuildConfiguration { directories, fragmentsMap, options, + BuildOptions.diffForReconstruction(defaultBuildOptions, options), mainRepositoryName.strippedName()); return newConfig; } @@ -1962,6 +1967,10 @@ public class BuildConfiguration { return buildOptions; } + public BuildOptions.OptionsDiffForReconstruction getBuildOptionsDiff() { + return buildOptionsDiff; + } + public String getCpu() { return options.cpu; } 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 ddfdc560ea..c4ce4e5ea3 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 @@ -291,10 +291,14 @@ public final class BuildOptions implements Cloneable, Serializable { this.fragmentOptionsMap = fragmentOptionsMap; } - BuildOptions applyDiff(OptionsDiffForReconstruction optionsDiff) { + public BuildOptions applyDiff(OptionsDiffForReconstruction optionsDiff) { if (optionsDiff.isEmpty()) { return this; } + maybeInitializeFingerprintAndHashCode(); + if (!Arrays.equals(fingerprint, optionsDiff.baseFingerprint)) { + throw new IllegalArgumentException("Can not reconstruct BuildOptions with a different base."); + } Builder builder = builder(); for (FragmentOptions options : fragmentOptionsMap.values()) { FragmentOptions newOptions = optionsDiff.transformOptions(options); @@ -342,6 +346,9 @@ public final class BuildOptions implements Cloneable, Serializable { /** Returns the difference between two BuildOptions in a {@link BuildOptions.OptionsDiff}. */ public static OptionsDiff diff(BuildOptions first, BuildOptions second) { + if (first == null || second == null) { + throw new IllegalArgumentException("Cannot diff null BuildOptions"); + } OptionsDiff diff = new OptionsDiff(); if (first.equals(second)) { return diff; @@ -390,25 +397,25 @@ public final class BuildOptions implements Cloneable, Serializable { BuildOptions first, BuildOptions second) { OptionsDiff diff = diff(first, second); if (diff.areSame()) { - return OptionsDiffForReconstruction.EMPTY; + return OptionsDiffForReconstruction.getEmpty(first.fingerprint); } - ImmutableMap.Builder<Class<? extends FragmentOptions>, ImmutableMap<String, Object>> - differingOptionsBuilder = - ImmutableMap.builderWithExpectedSize(diff.differingOptions.keySet().size()); + HashMap<Class<? extends FragmentOptions>, Map<String, Object>> differingOptions = + new HashMap<>(diff.differingOptions.keySet().size()); for (Class<? extends FragmentOptions> clazz : diff.differingOptions.keySet()) { Collection<OptionDefinition> fields = diff.differingOptions.get(clazz); - ImmutableMap.Builder<String, Object> valuesMapBuilder = - ImmutableMap.builderWithExpectedSize(fields.size()); + HashMap<String, Object> valueMap = new HashMap<>(fields.size()); for (OptionDefinition optionDefinition : fields) { - valuesMapBuilder.put( - optionDefinition.getField().getName(), diff.second.get(optionDefinition)); + Object secondValue = diff.second.get(optionDefinition); + valueMap.put(optionDefinition.getField().getName(), secondValue); } - differingOptionsBuilder.put(clazz, valuesMapBuilder.build()); + differingOptions.put(clazz, valueMap); } + first.maybeInitializeFingerprintAndHashCode(); return new OptionsDiffForReconstruction( - differingOptionsBuilder.build(), + differingOptions, ImmutableSet.copyOf(diff.extraFirstFragments), - ImmutableList.copyOf(diff.extraSecondFragments)); + ImmutableList.copyOf(diff.extraSecondFragments), + first.fingerprint); } /** @@ -492,35 +499,38 @@ public final class BuildOptions implements Cloneable, Serializable { * omitted, and the values of any fields that should be changed. */ @AutoCodec - static class OptionsDiffForReconstruction { - @AutoCodec - static final OptionsDiffForReconstruction EMPTY = - new OptionsDiffForReconstruction(ImmutableMap.of(), ImmutableSet.of(), ImmutableList.of()); - - private final ImmutableMap<Class<? extends FragmentOptions>, ImmutableMap<String, Object>> - differingOptions; + public static class OptionsDiffForReconstruction { + private final Map<Class<? extends FragmentOptions>, Map<String, Object>> differingOptions; private final ImmutableSet<Class<? extends FragmentOptions>> extraFirstFragmentClasses; private final ImmutableList<FragmentOptions> extraSecondFragments; + private final byte[] baseFingerprint; @AutoCodec.VisibleForSerialization OptionsDiffForReconstruction( - ImmutableMap<Class<? extends FragmentOptions>, ImmutableMap<String, Object>> - differingOptions, + Map<Class<? extends FragmentOptions>, Map<String, Object>> differingOptions, ImmutableSet<Class<? extends FragmentOptions>> extraFirstFragmentClasses, - ImmutableList<FragmentOptions> extraSecondFragments) { + ImmutableList<FragmentOptions> extraSecondFragments, + byte[] baseFingerprint) { this.differingOptions = differingOptions; this.extraFirstFragmentClasses = extraFirstFragmentClasses; this.extraSecondFragments = extraSecondFragments; + this.baseFingerprint = baseFingerprint; + } + + private static OptionsDiffForReconstruction getEmpty(byte[] baseFingerprint) { + return new OptionsDiffForReconstruction( + ImmutableMap.of(), ImmutableSet.of(), ImmutableList.of(), baseFingerprint); } @Nullable - private FragmentOptions transformOptions(FragmentOptions input) { + @VisibleForTesting + FragmentOptions transformOptions(FragmentOptions input) { Class<? extends FragmentOptions> clazz = input.getClass(); if (extraFirstFragmentClasses.contains(clazz)) { return null; } Map<String, Object> changedOptions = differingOptions.get(clazz); - if (changedOptions.isEmpty()) { + if (changedOptions == null || changedOptions.isEmpty()) { return input; } FragmentOptions newOptions = input.clone(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java index e67ea1e6c9..862bdbeade 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java @@ -94,7 +94,6 @@ public final class ConfigurationResolver { * @param originalDeps the transition requests for each dep under this target's attributes * @param hostConfiguration the host configuration * @param ruleClassProvider provider for determining the right configuration fragments for deps - * * @return a mapping from each attribute in the source target to the {@link BuildConfiguration}s * and {@link Label}s for the deps under that attribute. Returns null if not all Skyframe * dependencies are available. @@ -105,7 +104,8 @@ public final class ConfigurationResolver { TargetAndConfiguration ctgValue, OrderedSetMultimap<Attribute, Dependency> originalDeps, BuildConfiguration hostConfiguration, - RuleClassProvider ruleClassProvider) + RuleClassProvider ruleClassProvider, + BuildOptions defaultBuildOptions) throws ConfiguredTargetFunction.DependencyEvaluationException, InterruptedException { // Maps each Skyframe-evaluated BuildConfiguration to the dependencies that need that @@ -232,10 +232,16 @@ public final class ConfigurationResolver { // If we get here, we have to get the configuration from Skyframe. for (BuildOptions options : toOptions) { if (sameFragments) { - keysToEntries.put(BuildConfigurationValue.key(ctgFragments, options), depsEntry); + keysToEntries.put( + BuildConfigurationValue.key( + ctgFragments, BuildOptions.diffForReconstruction(defaultBuildOptions, options)), + depsEntry); } else { - keysToEntries.put(BuildConfigurationValue.key(depFragments, options), depsEntry); + keysToEntries.put( + BuildConfigurationValue.key( + depFragments, BuildOptions.diffForReconstruction(defaultBuildOptions, options)), + depsEntry); } } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java index ade1c0fc30..8594e6339c 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.bazel.rules; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; +import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.bazel.rules.cpp.BazelCppRuleClasses; import com.google.devtools.build.lib.rules.cpp.FdoSupportFunction; import com.google.devtools.build.lib.rules.cpp.FdoSupportValue; @@ -50,4 +51,10 @@ public class BazelRulesModule extends BlazeModule { BlazeRuntime runtime, BlazeDirectories directories, WorkspaceBuilder builder) { builder.addSkyFunction(FdoSupportValue.SKYFUNCTION, new FdoSupportFunction(directories)); } + + @Override + public BuildOptions getDefaultBuildOptions(BlazeRuntime blazeRuntime) { + return DefaultBuildOptionsForDiffing.getDefaultBuildOptionsForFragments( + blazeRuntime.getRuleClassProvider().getConfigurationOptions()); + } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/DefaultBuildOptionsForDiffing.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/DefaultBuildOptionsForDiffing.java new file mode 100644 index 0000000000..7e7a39f678 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/DefaultBuildOptionsForDiffing.java @@ -0,0 +1,50 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.bazel.rules; + +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.analysis.config.BuildOptions; +import com.google.devtools.build.lib.analysis.config.FragmentOptions; +import com.google.devtools.common.options.OptionsParsingException; +import java.util.ArrayList; +import java.util.List; + +/** + * Provides default {@link BuildOptions} based on the Fragment Classes provided. These default + * BuildOptions are used to create {@link BuildOptions.OptionsDiffForReconstruction} to store in + * {@link BuildConfigurationValue.Key}. + */ +public class DefaultBuildOptionsForDiffing { + + private static final ImmutableMap<Class<? extends FragmentOptions>, List<String>> + DEFAULT_OPTIONS_MAP = ImmutableMap.of(); + + public static BuildOptions getDefaultBuildOptionsForFragments( + List<Class<? extends FragmentOptions>> fragmentClasses) { + ArrayList<String> collector = new ArrayList<>(); + for (Class<? extends FragmentOptions> clazz : fragmentClasses) { + List<String> options = DEFAULT_OPTIONS_MAP.get(clazz); + if (options != null) { + collector.addAll(options); + } + } + try { + String[] stringCollector = new String[collector.size()]; + return BuildOptions.of(fragmentClasses, collector.toArray(stringCollector)); + } catch (OptionsParsingException e) { + throw new IllegalArgumentException("Failed to parse default options", e); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java index 4eb1908598..df96bb72f6 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java @@ -18,6 +18,7 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.BlazeVersionInfo; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.ServerDirectories; +import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory; import com.google.devtools.build.lib.buildtool.BuildRequest; import com.google.devtools.build.lib.clock.Clock; @@ -213,6 +214,15 @@ public abstract class BlazeModule { } /** + * Returns an instance of BuildOptions to be used to create {@link + * BuildOptions.OptionsDiffForReconstruction} with. Only one installed Module should override + * this. + */ + public BuildOptions getDefaultBuildOptions(BlazeRuntime runtime) { + return null; + } + + /** * Called when Bazel initializes the action execution subsystem. This is called once per build if * action execution is enabled. Modules can override this method to affect how execution is * performed. @@ -221,8 +231,7 @@ public abstract class BlazeModule { * @param request the build request * @param builder the builder to add action context providers and consumers to */ - public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) { - } + public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {} /** * Called after each command. diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java index 6e04c6170d..09467f1478 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java @@ -354,6 +354,25 @@ public final class BlazeRuntime { return blazeModules; } + public BuildOptions getDefaultBuildOptions() { + BuildOptions options = null; + for (BlazeModule module : blazeModules) { + BuildOptions optionsFromModule = module.getDefaultBuildOptions(this); + if (optionsFromModule != null) { + if (options == null) { + options = optionsFromModule; + } else { + throw new IllegalArgumentException( + "Two or more blaze modules contained default build options."); + } + } + } + if (options == null) { + throw new IllegalArgumentException("No default build options specified in any Blaze module"); + } + return options; + } + @SuppressWarnings("unchecked") public <T extends BlazeModule> T getBlazeModule(Class<T> moduleClass) { for (BlazeModule module : blazeModules) { diff --git a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java index 7e86341316..1e9ee5c7ac 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java @@ -65,7 +65,8 @@ public final class WorkspaceBuilder { SubscriberExceptionHandler eventBusExceptionHandler) throws AbruptExitException { // Set default values if none are set. if (skyframeExecutorFactory == null) { - skyframeExecutorFactory = new SequencedSkyframeExecutorFactory(); + skyframeExecutorFactory = + new SequencedSkyframeExecutorFactory(runtime.getDefaultBuildOptions()); } SkyframeExecutor skyframeExecutor = 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 917dfcb9dd..37ca17c17f 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 @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.analysis.DependencyResolver.InconsistentAsp import com.google.devtools.build.lib.analysis.TargetAndConfiguration; import com.google.devtools.build.lib.analysis.ToolchainContext; 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.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget; @@ -97,6 +98,7 @@ public final class AspectFunction implements SkyFunction { private final BuildViewProvider buildViewProvider; private final RuleClassProvider ruleClassProvider; private final Supplier<Boolean> removeActionsAfterEvaluation; + private final BuildOptions defaultBuildOptions; /** * Indicates whether the set of packages transitively loaded for a given {@link AspectValue} will * be needed for package root resolution later in the build. If not, they are not collected and @@ -108,12 +110,14 @@ public final class AspectFunction implements SkyFunction { BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider, Supplier<Boolean> removeActionsAfterEvaluation, - boolean storeTransitivePackagesForPackageRootResolution) { + boolean storeTransitivePackagesForPackageRootResolution, + BuildOptions defaultBuildOptions) { this.buildViewProvider = buildViewProvider; this.ruleClassProvider = ruleClassProvider; this.removeActionsAfterEvaluation = Preconditions.checkNotNull(removeActionsAfterEvaluation); this.storeTransitivePackagesForPackageRootResolution = storeTransitivePackagesForPackageRootResolution; + this.defaultBuildOptions = defaultBuildOptions; } /** @@ -382,7 +386,8 @@ public final class AspectFunction implements SkyFunction { ruleClassProvider, view.getHostConfiguration(originalTargetAndAspectConfiguration.getConfiguration()), transitivePackagesForPackageRootResolution, - transitiveRootCauses); + transitiveRootCauses, + defaultBuildOptions); } catch (ConfiguredTargetFunctionException e) { throw new AspectCreationException(e.getMessage()); } 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 e335ceaa71..6b39f2883b 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 @@ -21,6 +21,7 @@ import com.google.common.collect.MutableClassToInstanceMap; import com.google.devtools.build.lib.analysis.BlazeDirectories; 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.BuildOptions; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.skyframe.SkyFunction; @@ -39,11 +40,15 @@ public class BuildConfigurationFunction implements SkyFunction { private final BlazeDirectories directories; private final ConfiguredRuleClassProvider ruleClassProvider; + private final BuildOptions defaultBuildOptions; - public BuildConfigurationFunction(BlazeDirectories directories, - RuleClassProvider ruleClassProvider) { + public BuildConfigurationFunction( + BlazeDirectories directories, + RuleClassProvider ruleClassProvider, + BuildOptions defaultBuildOptions) { this.directories = directories; this.ruleClassProvider = (ConfiguredRuleClassProvider) ruleClassProvider; + this.defaultBuildOptions = defaultBuildOptions; } @Override @@ -71,12 +76,11 @@ public class BuildConfigurationFunction implements SkyFunction { fragmentsMap.put(fragment.getClass(), fragment); } + BuildOptions options = defaultBuildOptions.applyDiff(key.getOptionsDiff()); + BuildConfiguration config = new BuildConfiguration( - directories, - fragmentsMap, - key.getBuildOptions(), - workspaceNameValue.getName()); + directories, fragmentsMap, options, key.getOptionsDiff(), workspaceNameValue.getName()); return new BuildConfigurationValue(config); } @@ -85,9 +89,9 @@ public class BuildConfigurationFunction implements SkyFunction { // Get SkyKeys for the fragments we need to load. Set<SkyKey> fragmentKeys = new LinkedHashSet<>(); + BuildOptions options = defaultBuildOptions.applyDiff(key.getOptionsDiff()); for (Class<? extends BuildConfiguration.Fragment> fragmentClass : key.getFragments()) { - fragmentKeys.add( - ConfigurationFragmentValue.key(key.getBuildOptions(), fragmentClass, ruleClassProvider)); + fragmentKeys.add(ConfigurationFragmentValue.key(options, fragmentClass, ruleClassProvider)); } // Load them as Skyframe deps. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java index b445cc0a2b..8eed98ce64 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java @@ -13,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; -import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Interner; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; @@ -44,7 +43,6 @@ import java.util.Set; @AutoCodec @ThreadSafe public class BuildConfigurationValue implements SkyValue { - private static final Interner<Key> keyInterner = BlazeInterners.newWeakInterner(); private final BuildConfiguration configuration; @@ -60,40 +58,50 @@ public class BuildConfigurationValue implements SkyValue { * Returns the key for a requested configuration. * * @param fragments the fragments the configuration should contain - * @param buildOptions the build options the fragments should be built from + * @param optionsDiff the {@link BuildOptions.OptionsDiffForReconstruction} object the {@link + * BuildOptions} should be rebuilt from */ @ThreadSafe public static Key key( - Set<Class<? extends BuildConfiguration.Fragment>> fragments, BuildOptions buildOptions) { + Set<Class<? extends BuildConfiguration.Fragment>> fragments, + BuildOptions.OptionsDiffForReconstruction optionsDiff) { return key( FragmentClassSet.of( ImmutableSortedSet.copyOf(BuildConfiguration.lexicalFragmentSorter, fragments)), - buildOptions); + optionsDiff); } - public static Key key(FragmentClassSet fragmentClassSet, BuildOptions buildOptions) { - return keyInterner.intern(new Key(fragmentClassSet, buildOptions)); + public static Key key( + FragmentClassSet fragmentClassSet, BuildOptions.OptionsDiffForReconstruction optionsDiff) { + return Key.create(fragmentClassSet, optionsDiff); } /** {@link SkyKey} for {@link BuildConfigurationValue}. */ @VisibleForSerialization public static final class Key implements SkyKey, Serializable { + private static final Interner<Key> keyInterner = BlazeInterners.newWeakInterner(); + private final FragmentClassSet fragments; - private final BuildOptions buildOptions; + private final BuildOptions.OptionsDiffForReconstruction optionsDiff; // If hashCode really is -1, we'll recompute it from scratch each time. Oh well. private volatile int hashCode = -1; - private Key(FragmentClassSet fragments, BuildOptions buildOptions) { + private static Key create( + FragmentClassSet fragments, BuildOptions.OptionsDiffForReconstruction optionsDiff) { + return keyInterner.intern(new Key(fragments, optionsDiff)); + } + + Key(FragmentClassSet fragments, BuildOptions.OptionsDiffForReconstruction optionsDiff) { this.fragments = fragments; - this.buildOptions = Preconditions.checkNotNull(buildOptions); + this.optionsDiff = optionsDiff; } ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> getFragments() { return fragments.fragmentClasses(); } - BuildOptions getBuildOptions() { - return buildOptions; + BuildOptions.OptionsDiffForReconstruction getOptionsDiff() { + return optionsDiff; } @Override @@ -110,14 +118,14 @@ public class BuildConfigurationValue implements SkyValue { return false; } Key otherConfig = (Key) o; - return buildOptions.equals(otherConfig.buildOptions) + return optionsDiff.equals(otherConfig.optionsDiff) && Objects.equals(fragments, otherConfig.fragments); } @Override public int hashCode() { if (hashCode == -1) { - hashCode = Objects.hash(fragments, buildOptions); + hashCode = Objects.hash(fragments, optionsDiff); } return hashCode; } @@ -131,7 +139,7 @@ public class BuildConfigurationValue implements SkyValue { @Override public void serialize(SerializationContext context, Key obj, CodedOutputStream codedOut) throws SerializationException, IOException { - context.serialize(obj.buildOptions, codedOut); + context.serialize(obj.optionsDiff, codedOut); codedOut.writeInt32NoTag(obj.fragments.fragmentClasses().size()); for (Class<? extends BuildConfiguration.Fragment> fragment : obj.fragments.fragmentClasses()) { @@ -143,7 +151,7 @@ public class BuildConfigurationValue implements SkyValue { @SuppressWarnings("unchecked") // Class<? extends...> cast public Key deserialize(DeserializationContext context, CodedInputStream codedIn) throws SerializationException, IOException { - BuildOptions buildOptions = context.deserialize(codedIn); + BuildOptions.OptionsDiffForReconstruction optionsDiff = context.deserialize(codedIn); int fragmentsSize = codedIn.readInt32(); ImmutableSortedSet.Builder<Class<? extends BuildConfiguration.Fragment>> fragmentsBuilder = ImmutableSortedSet.orderedBy(BuildConfiguration.lexicalFragmentSorter); @@ -157,7 +165,7 @@ public class BuildConfigurationValue implements SkyValue { "Couldn't deserialize BuildConfigurationValue$Key fragment class", e); } } - return key(fragmentsBuilder.build(), buildOptions); + return key(fragmentsBuilder.build(), optionsDiff); } } } 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 2df73d1791..a4bb22fce0 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 @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.analysis.DependencyResolver.InconsistentAsp import com.google.devtools.build.lib.analysis.TargetAndConfiguration; import com.google.devtools.build.lib.analysis.ToolchainContext; 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.ConfigurationResolver; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; @@ -116,6 +117,7 @@ public final class ConfiguredTargetFunction implements SkyFunction { private final RuleClassProvider ruleClassProvider; private final Semaphore cpuBoundSemaphore; private final Supplier<Boolean> removeActionsAfterEvaluation; + private final BuildOptions defaultBuildOptions; /** * Indicates whether the set of packages transitively loaded for a given {@link * ConfiguredTargetValue} will be needed for package root resolution later in the build. If not, @@ -128,13 +130,15 @@ public final class ConfiguredTargetFunction implements SkyFunction { RuleClassProvider ruleClassProvider, Semaphore cpuBoundSemaphore, Supplier<Boolean> removeActionsAfterEvaluation, - boolean storeTransitivePackagesForPackageRootResolution) { + boolean storeTransitivePackagesForPackageRootResolution, + BuildOptions defaultBuildOptions) { this.buildViewProvider = buildViewProvider; this.ruleClassProvider = ruleClassProvider; this.cpuBoundSemaphore = cpuBoundSemaphore; this.removeActionsAfterEvaluation = Preconditions.checkNotNull(removeActionsAfterEvaluation); this.storeTransitivePackagesForPackageRootResolution = storeTransitivePackagesForPackageRootResolution; + this.defaultBuildOptions = defaultBuildOptions; } @Override @@ -269,7 +273,8 @@ public final class ConfiguredTargetFunction implements SkyFunction { ruleClassProvider, view.getHostConfiguration(configuration), transitivePackagesForPackageRootResolution, - transitiveLoadingRootCauses); + transitiveLoadingRootCauses, + defaultBuildOptions); if (env.valuesMissing()) { return null; } @@ -380,6 +385,9 @@ public final class ConfiguredTargetFunction implements SkyFunction { * @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 + * @param defaultBuildOptions the default build options provided by the server; these are used to + * create diffs for {@link BuildConfigurationValue.Key}s to prevent storing the entire + * BuildOptions object. */ @Nullable static OrderedSetMultimap<Attribute, ConfiguredTargetAndData> computeDependencies( @@ -392,7 +400,8 @@ public final class ConfiguredTargetFunction implements SkyFunction { RuleClassProvider ruleClassProvider, BuildConfiguration hostConfiguration, @Nullable NestedSetBuilder<Package> transitivePackagesForPackageRootResolution, - NestedSetBuilder<Label> transitiveLoadingRootCauses) + NestedSetBuilder<Label> transitiveLoadingRootCauses, + BuildOptions defaultBuildOptions) throws DependencyEvaluationException, ConfiguredTargetFunctionException, AspectCreationException, InterruptedException { // Create the map from attributes to set of (target, configuration) pairs. @@ -407,7 +416,8 @@ public final class ConfiguredTargetFunction implements SkyFunction { toolchainContext == null ? ImmutableSet.of() : toolchainContext.getResolvedToolchainLabels(), - transitiveLoadingRootCauses); + transitiveLoadingRootCauses, + defaultBuildOptions); } catch (EvalException e) { // EvalException can only be thrown by computed Skylark attributes in the current rule. env.getListener().handle(Event.error(e.getLocation(), e.getMessage())); @@ -423,8 +433,14 @@ public final class ConfiguredTargetFunction implements SkyFunction { // Trim each dep's configuration so it only includes the fragments needed by its transitive // closure. if (ctgValue.getConfiguration() != null) { - depValueNames = ConfigurationResolver.resolveConfigurations(env, ctgValue, depValueNames, - hostConfiguration, ruleClassProvider); + depValueNames = + ConfigurationResolver.resolveConfigurations( + env, + ctgValue, + depValueNames, + hostConfiguration, + ruleClassProvider, + defaultBuildOptions); // It's important that we don't use "if (env.missingValues()) { return null }" here (or // in the following lines). See the comments in getDynamicConfigurations' Skyframe call // for explanation. @@ -686,7 +702,8 @@ public final class ConfiguredTargetFunction implements SkyFunction { throws ConfiguredTargetFunctionException, InterruptedException { StoredEventHandler events = new StoredEventHandler(); BuildConfiguration ownerConfig = - ConfiguredTargetFactory.getArtifactOwnerConfiguration(env, configuration); + ConfiguredTargetFactory.getArtifactOwnerConfiguration( + env, configuration, defaultBuildOptions); if (env.valuesMissing()) { return null; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java index f9489794cc..e5692d3376 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java @@ -80,7 +80,7 @@ public class ConfiguredTargetKey extends ActionLookupKey { ? KeyAndHost.NULL_INSTANCE : new KeyAndHost( BuildConfigurationValue.key( - configuration.fragmentClasses(), configuration.getOptions()), + configuration.fragmentClasses(), configuration.getBuildOptionsDiff()), configuration.isHostConfiguration()); } 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 c8e61fe10a..2a35b9f186 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 @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.analysis.Dependency; import com.google.devtools.build.lib.analysis.DependencyResolver.InconsistentAspectOrderException; 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.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.analysis.config.ConfigurationResolver; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; @@ -64,12 +65,15 @@ public class PostConfiguredTargetFunction implements SkyFunction { private final SkyframeExecutor.BuildViewProvider buildViewProvider; private final RuleClassProvider ruleClassProvider; + private final BuildOptions defaultBuildOptions; public PostConfiguredTargetFunction( SkyframeExecutor.BuildViewProvider buildViewProvider, - RuleClassProvider ruleClassProvider) { + RuleClassProvider ruleClassProvider, + BuildOptions defaultBuildOptions) { this.buildViewProvider = Preconditions.checkNotNull(buildViewProvider); this.ruleClassProvider = ruleClassProvider; + this.defaultBuildOptions = defaultBuildOptions; } @Nullable @@ -125,10 +129,12 @@ public class PostConfiguredTargetFunction implements SkyFunction { hostConfiguration, /*aspect=*/ null, configConditions, - /*toolchainLabels=*/ ImmutableSet.of()); + /*toolchainLabels*/ ImmutableSet.of(), + defaultBuildOptions); if (ct.getConfiguration() != null) { - deps = ConfigurationResolver.resolveConfigurations(env, ctgValue, deps, hostConfiguration, - ruleClassProvider); + deps = + ConfigurationResolver.resolveConfigurations( + env, ctgValue, deps, hostConfiguration, ruleClassProvider, defaultBuildOptions); } } catch (EvalException | ConfiguredTargetFunction.DependencyEvaluationException | InvalidConfigurationException | InconsistentAspectOrderException e) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java index 88cced7588..3c25ef34d6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.analysis.BuildView; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Factory; import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory; +import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget; import com.google.devtools.build.lib.buildtool.BuildRequestOptions; import com.google.devtools.build.lib.cmdline.PackageIdentifier; @@ -147,7 +148,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { PathFragment additionalBlacklistedPackagePrefixesFile, CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, List<BuildFileName> buildFilesByPriority, - ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile) { + ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile, + BuildOptions defaultBuildOptions) { super( evaluatorSupplier, pkgFactory, @@ -162,7 +164,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { additionalBlacklistedPackagePrefixesFile, crossRepositoryLabelViolationStrategy, buildFilesByPriority, - actionOnIOExceptionReadingBuildFile); + actionOnIOExceptionReadingBuildFile, + defaultBuildOptions); this.diffAwarenessManager = new DiffAwarenessManager(diffAwarenessFactories); this.customDirtinessCheckers = customDirtinessCheckers; } @@ -181,7 +184,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { PathFragment additionalBlacklistedPackagePrefixesFile, CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, List<BuildFileName> buildFilesByPriority, - ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile) { + ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile, + BuildOptions defaultBuildOptions) { SequencedSkyframeExecutor skyframeExecutor = new SequencedSkyframeExecutor( InMemoryMemoizingEvaluator.SUPPLIER, @@ -198,7 +202,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { additionalBlacklistedPackagePrefixesFile, crossRepositoryLabelViolationStrategy, buildFilesByPriority, - actionOnIOExceptionReadingBuildFile); + actionOnIOExceptionReadingBuildFile, + defaultBuildOptions); skyframeExecutor.init(); return skyframeExecutor; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java index f8c5b8babb..ddb1a7c09d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java @@ -19,6 +19,7 @@ import com.google.devtools.build.lib.actions.ActionKeyContext; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Factory; import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory; +import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.skyframe.SkyFunction; @@ -29,6 +30,12 @@ import com.google.devtools.build.skyframe.SkyFunctionName; */ public class SequencedSkyframeExecutorFactory implements SkyframeExecutorFactory { + private final BuildOptions defaultBuildOptions; + + public SequencedSkyframeExecutorFactory(BuildOptions defaultBuildOptions) { + this.defaultBuildOptions = defaultBuildOptions; + } + @Override public SkyframeExecutor create( PackageFactory pkgFactory, @@ -54,6 +61,7 @@ public class SequencedSkyframeExecutorFactory implements SkyframeExecutorFactory BazelSkyframeExecutorConstants.ADDITIONAL_BLACKLISTED_PACKAGE_PREFIXES_FILE, BazelSkyframeExecutorConstants.CROSS_REPOSITORY_LABEL_VIOLATION_STRATEGY, BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY, - BazelSkyframeExecutorConstants.ACTION_ON_IO_EXCEPTION_READING_BUILD_FILE); + BazelSkyframeExecutorConstants.ACTION_ON_IO_EXCEPTION_READING_BUILD_FILE, + defaultBuildOptions); } } 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 3cb34515df..115c5c1fa8 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 @@ -126,7 +126,8 @@ public final class SkyframeBuildView { public SkyframeBuildView(BlazeDirectories directories, SkyframeExecutor skyframeExecutor, ConfiguredRuleClassProvider ruleClassProvider) { - this.factory = new ConfiguredTargetFactory(ruleClassProvider); + this.factory = + new ConfiguredTargetFactory(ruleClassProvider, skyframeExecutor.getDefaultBuildOptions()); this.artifactFactory = new ArtifactFactory(directories.getExecRoot(), directories.getRelativeOutputPath()); this.skyframeExecutor = skyframeExecutor; @@ -568,7 +569,8 @@ public final class SkyframeBuildView { // case. So further optimization is necessary to make that viable (proto_library in particular // contributes to much of the difference). BuildConfiguration trimmedConfig = - topLevelHostConfiguration.clone(fragmentClasses, ruleClassProvider); + topLevelHostConfiguration.clone( + fragmentClasses, ruleClassProvider, skyframeExecutor.getDefaultBuildOptions()); hostConfigurationCache.put(fragmentClasses, trimmedConfig); return trimmedConfig; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java index 8b3d23daf5..f4940066c8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java @@ -124,11 +124,15 @@ public final class SkyframeDependencyResolver extends DependencyResolver { @Nullable @Override protected List<BuildConfiguration> getConfigurations( - FragmentClassSet fragments, Iterable<BuildOptions> buildOptions) + FragmentClassSet fragments, + Iterable<BuildOptions> buildOptions, + BuildOptions defaultBuildOptions) throws InvalidConfigurationException, InterruptedException { List<SkyKey> keys = new ArrayList<>(); for (BuildOptions options : buildOptions) { - keys.add(BuildConfigurationValue.key(fragments, options)); + keys.add( + BuildConfigurationValue.key( + fragments, BuildOptions.diffForReconstruction(defaultBuildOptions, options))); } Map<SkyKey, ValueOrException<InvalidConfigurationException>> configValues = env.getValuesOrThrow(keys, InvalidConfigurationException.class); 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 d5c54471a3..d8686adf31 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 @@ -293,6 +293,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { private final ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile; + private final BuildOptions defaultBuildOptions; + private PerBuildSyscallCache perBuildSyscallCache; private int lastConcurrencyLevel = -1; @@ -312,7 +314,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { PathFragment additionalBlacklistedPackagePrefixesFile, CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, List<BuildFileName> buildFilesByPriority, - ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile) { + ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile, + BuildOptions defaultBuildOptions) { // Strictly speaking, these arguments are not required for initialization, but all current // callsites have them at hand, so we might as well set them during construction. this.evaluatorSupplier = evaluatorSupplier; @@ -338,6 +341,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { this.additionalBlacklistedPackagePrefixesFile = additionalBlacklistedPackagePrefixesFile; this.ruleClassProvider = pkgFactory.getRuleClassProvider(); + this.defaultBuildOptions = defaultBuildOptions; this.skyframeBuildView = new SkyframeBuildView( directories, this, @@ -423,19 +427,24 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { ruleClassProvider, cpuBoundSemaphore, removeActionsAfterEvaluation, - shouldStoreTransitivePackagesInLoadingAndAnalysis())); + shouldStoreTransitivePackagesInLoadingAndAnalysis(), + defaultBuildOptions)); map.put( SkyFunctions.ASPECT, new AspectFunction( new BuildViewProvider(), ruleClassProvider, removeActionsAfterEvaluation, - shouldStoreTransitivePackagesInLoadingAndAnalysis())); + shouldStoreTransitivePackagesInLoadingAndAnalysis(), + defaultBuildOptions)); map.put(SkyFunctions.LOAD_SKYLARK_ASPECT, new ToplevelSkylarkAspectFunction()); - map.put(SkyFunctions.POST_CONFIGURED_TARGET, - new PostConfiguredTargetFunction(new BuildViewProvider(), ruleClassProvider)); - map.put(SkyFunctions.BUILD_CONFIGURATION, - new BuildConfigurationFunction(directories, ruleClassProvider)); + map.put( + SkyFunctions.POST_CONFIGURED_TARGET, + new PostConfiguredTargetFunction( + new BuildViewProvider(), ruleClassProvider, defaultBuildOptions)); + map.put( + SkyFunctions.BUILD_CONFIGURATION, + new BuildConfigurationFunction(directories, ruleClassProvider, defaultBuildOptions)); map.put( SkyFunctions.CONFIGURATION_FRAGMENT, new ConfigurationFragmentFunction(configurationFragments, ruleClassProvider, directories)); @@ -907,7 +916,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { requiredToolchains, config == null ? null - : BuildConfigurationValue.key(config.fragmentClasses(), config.getOptions())); + : BuildConfigurationValue.key( + config.fragmentClasses(), + BuildOptions.diffForReconstruction(defaultBuildOptions, config.getOptions()))); } /** @@ -1299,6 +1310,11 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { return getConfiguredTargetMapForTesting(eventHandler, originalConfig, keys).values().asList(); } + @VisibleForTesting + public BuildOptions getDefaultBuildOptions() { + return defaultBuildOptions; + } + /** * Returns a map from {@link Dependency} inputs to the {@link ConfiguredTargetAndData}s * corresponding to those dependencies. @@ -1457,7 +1473,11 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { final ImmutableList<SkyKey> configSkyKeys = optionsList .stream() - .map(elem -> BuildConfigurationValue.key(allFragments, elem)) + .map( + elem -> + BuildConfigurationValue.key( + allFragments, + BuildOptions.diffForReconstruction(defaultBuildOptions, elem))) .collect(ImmutableList.toImmutableList()); // Skyframe-evaluate the configurations and throw errors if any. @@ -1554,7 +1574,10 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { if (depFragments != null) { for (BuildOptions toOptions : ConfigurationResolver.applyTransition( fromOptions, key.getTransition(), depFragments, ruleClassProvider, true)) { - configSkyKeys.add(BuildConfigurationValue.key(depFragments, toOptions)); + configSkyKeys.add( + BuildConfigurationValue.key( + depFragments, + BuildOptions.diffForReconstruction(defaultBuildOptions, toOptions))); } } } @@ -1569,7 +1592,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { if (depFragments != null) { for (BuildOptions toOptions : ConfigurationResolver.applyTransition( fromOptions, key.getTransition(), depFragments, ruleClassProvider, true)) { - SkyKey configKey = BuildConfigurationValue.key(depFragments, toOptions); + SkyKey configKey = + BuildConfigurationValue.key( + depFragments, BuildOptions.diffForReconstruction(defaultBuildOptions, toOptions)); BuildConfigurationValue configValue = ((BuildConfigurationValue) configsResult.get(configKey)); // configValue will be null here if there was an exception thrown during configuration @@ -1640,7 +1665,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { FragmentClassSet fragments, BuildOptions options) throws InterruptedException { - SkyKey key = BuildConfigurationValue.key(fragments, options); + SkyKey key = + BuildConfigurationValue.key( + fragments, BuildOptions.diffForReconstruction(defaultBuildOptions, options)); BuildConfigurationValue result = (BuildConfigurationValue) buildDriver .evaluate(ImmutableList.of(key), false, DEFAULT_THREAD_COUNT, eventHandler).get(key); return result.getConfiguration(); |