diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index 64993b41dd59e5..bcd2429c2a7d6b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -1655,6 +1655,7 @@ java_library( ":config/fragment_registry", ":config/invalid_configuration_exception", ":config/optioninfo", + ":config/options_diff", ":config/run_under", ":config/starlark_defined_config_transition", ":platform_options", @@ -1954,6 +1955,21 @@ java_library( ], ) +java_library( + name = "config/options_diff", + srcs = [ + "config/OptionsDiff.java", + ], + deps = [ + ":config/build_options", + ":config/fragment_options", + "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/util", + "//src/main/java/com/google/devtools/common/options", + "//third_party:guava", + ], +) + java_library( name = "config/per_label_options", srcs = ["config/PerLabelOptions.java"], @@ -2480,6 +2496,7 @@ java_library( ":config/core_options", ":config/fragment_options", ":config/optioninfo", + ":config/options_diff", ":config/starlark_defined_config_transition", ":config/transitions/configuration_transition", ":test/test_configuration", 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 08065c4457bb31..bdb2a826038fa0 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 @@ -14,13 +14,11 @@ package com.google.devtools.build.lib.analysis.config; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.collect.ImmutableMap.toImmutableMap; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; -import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -28,8 +26,6 @@ import com.google.common.collect.Iterables; import com.google.common.collect.MapDifference; import com.google.common.collect.Maps; -import com.google.common.collect.Multimap; -import com.google.common.collect.Sets; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.skyframe.serialization.DeserializationContext; import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec; @@ -37,7 +33,6 @@ import com.google.devtools.build.lib.skyframe.serialization.SerializationException; import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant; import com.google.devtools.build.lib.util.Fingerprint; -import com.google.devtools.build.lib.util.OrderedSetMultimap; import com.google.devtools.common.options.OptionDefinition; import com.google.devtools.common.options.OptionsBase; import com.google.devtools.common.options.OptionsParser; @@ -49,12 +44,9 @@ import com.google.protobuf.CodedInputStream; import com.google.protobuf.CodedOutputStream; import java.io.IOException; -import java.util.ArrayList; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedHashMap; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -481,232 +473,6 @@ public BuildOptions build() { private Builder() {} } - /** Returns the difference between two BuildOptions in a new {@link BuildOptions.OptionsDiff}. */ - public static OptionsDiff diff(BuildOptions first, BuildOptions second) { - return diff(new OptionsDiff(), first, second); - } - - /** - * Returns the difference between two BuildOptions in a pre-existing {@link - * BuildOptions.OptionsDiff}. - * - *

In a single pass through this method, the method can only compare a single "first" {@link - * BuildOptions} and single "second" BuildOptions; but an OptionsDiff instance can store the diff - * between a single "first" BuildOptions and multiple "second" BuildOptions. Being able to - * maintain a single OptionsDiff over multiple calls to diff is useful for, for example, - * aggregating the difference between a single BuildOptions and the results of applying a {@link - * com.google.devtools.build.lib.analysis.config.transitions.SplitTransition}) to it. - */ - @SuppressWarnings("ReferenceEquality") // See comment above == comparison. - public static OptionsDiff diff(OptionsDiff diff, BuildOptions first, BuildOptions second) { - checkArgument( - !diff.hasStarlarkOptions, - "OptionsDiff cannot handle multiple 'second' BuildOptions with Starlark options and is" - + " trying to diff against %s", - diff); - checkNotNull(first); - checkNotNull(second); - if (first.equals(second)) { - return diff; - } - - // Check and report if either class has been trimmed of an options class that exists in the - // other. - ImmutableSet> firstOptionClasses = - first.getNativeOptions().stream() - .map(FragmentOptions::getClass) - .collect(ImmutableSet.toImmutableSet()); - ImmutableSet> secondOptionClasses = - second.getNativeOptions().stream() - .map(FragmentOptions::getClass) - .collect(ImmutableSet.toImmutableSet()); - Sets.difference(firstOptionClasses, secondOptionClasses).forEach(diff::addExtraFirstFragment); - Sets.difference(secondOptionClasses, firstOptionClasses).stream() - .map(second::get) - .forEach(diff::addExtraSecondFragment); - - // For fragments in common, report differences. - for (Class clazz : - Sets.intersection(firstOptionClasses, secondOptionClasses)) { - FragmentOptions firstOptions = first.get(clazz); - FragmentOptions secondOptions = second.get(clazz); - // We avoid calling #equals because we are going to do a field-by-field comparison anyway. - if (firstOptions == secondOptions) { - continue; - } - for (OptionDefinition definition : OptionsParser.getOptionDefinitions(clazz)) { - Object firstValue = firstOptions.getValueFromDefinition(definition); - Object secondValue = secondOptions.getValueFromDefinition(definition); - if (!Objects.equals(firstValue, secondValue)) { - diff.addDiff(clazz, definition, firstValue, secondValue); - } - } - } - - // Compare Starlark options for the two classes. - Map starlarkFirst = first.starlarkOptionsMap; - Map starlarkSecond = second.starlarkOptionsMap; - for (Label buildSetting : Sets.union(starlarkFirst.keySet(), starlarkSecond.keySet())) { - if (starlarkFirst.get(buildSetting) == null) { - diff.addExtraSecondStarlarkOption(buildSetting, starlarkSecond.get(buildSetting)); - } else if (starlarkSecond.get(buildSetting) == null) { - diff.addExtraFirstStarlarkOption(buildSetting); - } else if (!starlarkFirst.get(buildSetting).equals(starlarkSecond.get(buildSetting))) { - diff.putStarlarkDiff( - buildSetting, starlarkFirst.get(buildSetting), starlarkSecond.get(buildSetting)); - } - } - return diff; - } - - /** - * A diff class for BuildOptions. Fields are meant to be populated and returned by {@link - * BuildOptions#diff}. - */ - public static final class OptionsDiff { - private final Multimap, OptionDefinition> differingOptions = - ArrayListMultimap.create(); - // The keyset for the {@link first} and {@link second} maps are identical and indicate which - // specific options differ between the first and second built options. - private final Map first = new LinkedHashMap<>(); - // Since this class can be used to track the result of transitions, {@link second} is a multimap - // to be able to handle {@link SplitTransition}s. - private final Multimap second = OrderedSetMultimap.create(); - // List of "extra" fragments for each BuildOption aka fragments that were trimmed off one - // BuildOption but not the other. - private final Set> extraFirstFragments = new HashSet<>(); - private final Set extraSecondFragments = new HashSet<>(); - - private final Map starlarkFirst = new LinkedHashMap<>(); - // TODO(b/112041323): This should also be multimap but we don't diff multiple times with - // Starlark options anywhere yet so add that feature when necessary. - private final Map starlarkSecond = new LinkedHashMap<>(); - - private final List

In a single pass through this method, the method can only compare a single "first" {@link + * BuildOptions} and single "second" BuildOptions; but an OptionsDiff instance can store the diff + * between a single "first" BuildOptions and multiple "second" BuildOptions. Being able to + * maintain a single OptionsDiff over multiple calls to diff is useful for, for example, + * aggregating the difference between a single BuildOptions and the results of applying a {@link + * com.google.devtools.build.lib.analysis.config.transitions.SplitTransition}) to it. + */ + @SuppressWarnings("ReferenceEquality") // See comment above == comparison. + public static OptionsDiff diff(OptionsDiff diff, BuildOptions first, BuildOptions second) { + checkArgument( + !diff.hasStarlarkOptions, + "OptionsDiff cannot handle multiple 'second' BuildOptions with Starlark options and is" + + " trying to diff against %s", + diff); + checkNotNull(first); + checkNotNull(second); + if (first.equals(second)) { + return diff; + } + + // Check and report if either class has been trimmed of an options class that exists in the + // other. + ImmutableSet> firstOptionClasses = + first.getNativeOptions().stream() + .map(FragmentOptions::getClass) + .collect(ImmutableSet.toImmutableSet()); + ImmutableSet> secondOptionClasses = + second.getNativeOptions().stream() + .map(FragmentOptions::getClass) + .collect(ImmutableSet.toImmutableSet()); + Sets.difference(firstOptionClasses, secondOptionClasses).forEach(diff::addExtraFirstFragment); + Sets.difference(secondOptionClasses, firstOptionClasses).stream() + .map(second::get) + .forEach(diff::addExtraSecondFragment); + + // For fragments in common, report differences. + for (Class clazz : + Sets.intersection(firstOptionClasses, secondOptionClasses)) { + FragmentOptions firstOptions = first.get(clazz); + FragmentOptions secondOptions = second.get(clazz); + // We avoid calling #equals because we are going to do a field-by-field comparison anyway. + if (firstOptions == secondOptions) { + continue; + } + for (OptionDefinition definition : OptionsParser.getOptionDefinitions(clazz)) { + Object firstValue = firstOptions.getValueFromDefinition(definition); + Object secondValue = secondOptions.getValueFromDefinition(definition); + if (!Objects.equals(firstValue, secondValue)) { + diff.addDiff(clazz, definition, firstValue, secondValue); + } + } + } + + // Compare Starlark options for the two classes. + ImmutableMap starlarkFirst = first.getStarlarkOptions(); + ImmutableMap starlarkSecond = second.getStarlarkOptions(); + for (Label buildSetting : Sets.union(starlarkFirst.keySet(), starlarkSecond.keySet())) { + if (starlarkFirst.get(buildSetting) == null) { + diff.addExtraSecondStarlarkOption(buildSetting, starlarkSecond.get(buildSetting)); + } else if (starlarkSecond.get(buildSetting) == null) { + diff.addExtraFirstStarlarkOption(buildSetting); + } else if (!starlarkFirst.get(buildSetting).equals(starlarkSecond.get(buildSetting))) { + diff.putStarlarkDiff( + buildSetting, starlarkFirst.get(buildSetting), starlarkSecond.get(buildSetting)); + } + } + return diff; + } + + private final ListMultimap, OptionDefinition> differingOptions = + ArrayListMultimap.create(); + // The keyset for the {@link first} and {@link second} maps are identical and indicate which + // specific options differ between the first and second built options. + private final Map first = new LinkedHashMap<>(); + // Since this class can be used to track the result of transitions, {@link second} is a multimap + // to be able to handle {@link SplitTransition}s. + private final SetMultimap second = OrderedSetMultimap.create(); + // List of "extra" fragments for each BuildOption aka fragments that were trimmed off one + // BuildOption but not the other. + private final Set> extraFirstFragments = new HashSet<>(); + private final Set extraSecondFragments = new HashSet<>(); + + private final Map starlarkFirst = new LinkedHashMap<>(); + // TODO(b/112041323): This should also be multimap but we don't diff multiple times with + // Starlark options anywhere yet so add that feature when necessary. + private final Map starlarkSecond = new LinkedHashMap<>(); + + private final List