diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/analysis')
6 files changed, 124 insertions, 65 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); } } } |