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 b54929e736d9b6..62bb02a18b39f3 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -292,6 +292,7 @@ java_library( ":config/fragment", ":config/fragment_class_set", ":config/fragment_options", + ":config/fragment_registry", ":config/host_transition", ":config/invalid_configuration_exception", ":config/per_label_options", @@ -1573,6 +1574,7 @@ java_library( ":config/fragment_options", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/common/options", "//third_party:guava", @@ -1715,6 +1717,18 @@ java_library( ], ) +java_library( + name = "config/fragment_registry", + srcs = ["config/FragmentRegistry.java"], + deps = [ + ":config/build_options", + ":config/fragment", + ":config/fragment_class_set", + ":config/fragment_options", + "//third_party:guava", + ], +) + java_library( name = "config/fragment_options", srcs = [ diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java index 4888c20ebd47c3..abff65362ffc7c 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.analysis.config.Fragment; import com.google.devtools.build.lib.analysis.config.FragmentClassSet; import com.google.devtools.build.lib.analysis.config.FragmentOptions; +import com.google.devtools.build.lib.analysis.config.FragmentRegistry; import com.google.devtools.build.lib.analysis.config.SymlinkDefinition; import com.google.devtools.build.lib.analysis.config.transitions.ComposingTransitionFactory; import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; @@ -59,19 +60,14 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; -import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDefinition; -import com.google.devtools.common.options.OptionsProvider; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.lang.reflect.Constructor; -import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -142,8 +138,7 @@ public static final class Builder implements RuleDefinitionEnvironment { @Nullable private String builtinsBzlPackagePathInSource; private final List> configurationFragmentClasses = new ArrayList<>(); private final List buildInfoFactories = new ArrayList<>(); - private final Set> configurationOptions = - new LinkedHashSet<>(); + private final List> configurationOptions = new ArrayList<>(); private final Map ruleClassMap = new HashMap<>(); private final Map ruleDefinitionMap = new HashMap<>(); @@ -286,7 +281,6 @@ public Builder addNativeAspectClass(NativeAspectClass aspectFactoryClass) { * no two different configuration fragments can share the same name. */ public Builder addConfigurationFragment(Class fragmentClass) { - this.configurationOptions.addAll(Fragment.requiredOptions(fragmentClass)); configurationFragmentClasses.add(fragmentClass); return this; } @@ -554,12 +548,11 @@ public ConfiguredRuleClassProvider build() { ImmutableMap.copyOf(ruleClassMap), ImmutableMap.copyOf(ruleDefinitionMap), ImmutableMap.copyOf(nativeAspectClassMap), + FragmentRegistry.create( + configurationFragmentClasses, universalFragments, configurationOptions), defaultWorkspaceFilePrefix.toString(), defaultWorkspaceFileSuffix.toString(), ImmutableList.copyOf(buildInfoFactories), - ImmutableList.copyOf(configurationOptions), - FragmentClassSet.of(configurationFragmentClasses), - FragmentClassSet.of(universalFragments), trimmingTransitionFactory, toolchainTaggedTrimmingTransition, shouldInvalidateCacheForOptionDiff, @@ -632,18 +625,7 @@ public Builder setNetworkAllowlistForTests(Label allowlist) { /** Maps aspect name to the aspect factory meta class. */ private final ImmutableMap nativeAspectClassMap; - /** The configuration options that affect the behavior of the rules. */ - private final ImmutableList> configurationOptions; - - /** The set of configuration fragment factories. */ - private final FragmentClassSet configurationFragmentClasses; - - /** - * Maps build option names to matching config fragments. This is used to determine correct - * fragment requirements for config_setting rules, which are unique in that their dependencies are - * triggered by string representations of option names. - */ - private final Map> optionsToFragmentMap; + private final FragmentRegistry fragmentRegistry; /** The transition factory used to produce the transition that will trim targets. */ @Nullable private final TransitionFactory trimmingTransitionFactory; @@ -654,12 +636,6 @@ public Builder setNetworkAllowlistForTests(Label allowlist) { /** The predicate used to determine whether a diff requires the cache to be invalidated. */ private final OptionsDiffPredicate shouldInvalidateCacheForOptionDiff; - /** - * Configuration fragments that should be available to all rules even when they don't explicitly - * require it. - */ - private final FragmentClassSet universalFragments; - private final ImmutableList buildInfoFactories; private final PrerequisiteValidator prerequisiteValidator; @@ -694,12 +670,10 @@ private ConfiguredRuleClassProvider( ImmutableMap ruleClassMap, ImmutableMap ruleDefinitionMap, ImmutableMap nativeAspectClassMap, + FragmentRegistry fragmentRegistry, String defaultWorkspaceFilePrefix, String defaultWorkspaceFileSuffix, ImmutableList buildInfoFactories, - ImmutableList> configurationOptions, - FragmentClassSet configurationFragmentClasses, - FragmentClassSet universalFragments, @Nullable TransitionFactory trimmingTransitionFactory, PatchTransition toolchainTaggedTrimmingTransition, OptionsDiffPredicate shouldInvalidateCacheForOptionDiff, @@ -721,13 +695,10 @@ private ConfiguredRuleClassProvider( this.ruleClassMap = ruleClassMap; this.ruleDefinitionMap = ruleDefinitionMap; this.nativeAspectClassMap = nativeAspectClassMap; + this.fragmentRegistry = fragmentRegistry; this.defaultWorkspaceFilePrefix = defaultWorkspaceFilePrefix; this.defaultWorkspaceFileSuffix = defaultWorkspaceFileSuffix; this.buildInfoFactories = buildInfoFactories; - this.configurationOptions = configurationOptions; - this.configurationFragmentClasses = configurationFragmentClasses; - this.optionsToFragmentMap = computeOptionsToFragmentMap(configurationFragmentClasses); - this.universalFragments = universalFragments; this.trimmingTransitionFactory = trimmingTransitionFactory; this.toolchainTaggedTrimmingTransition = toolchainTaggedTrimmingTransition; this.shouldInvalidateCacheForOptionDiff = shouldInvalidateCacheForOptionDiff; @@ -739,43 +710,12 @@ private ConfiguredRuleClassProvider( this.symlinkDefinitions = symlinkDefinitions; this.reservedActionMnemonics = reservedActionMnemonics; this.actionEnvironmentProvider = actionEnvironmentProvider; - this.configurationFragmentMap = createFragmentMap(configurationFragmentClasses); + this.configurationFragmentMap = createFragmentMap(fragmentRegistry.getAllFragments()); this.constraintSemantics = constraintSemantics; this.thirdPartyLicenseExistencePolicy = thirdPartyLicenseExistencePolicy; this.networkAllowlistForTests = networkAllowlistForTests; } - /** - * Computes the option name --> config fragments map. Note that this mapping is technically - * one-to-many: a single option may be required by multiple fragments (e.g. Java options are used - * by both JavaConfiguration and Jvm). In such cases, we arbitrarily choose one fragment since - * that's all that's needed to satisfy the config_setting. - */ - private static Map> computeOptionsToFragmentMap( - FragmentClassSet configurationFragments) { - Map> result = new LinkedHashMap<>(); - Map, Integer> visitedOptionsClasses = new HashMap<>(); - for (Class fragment : configurationFragments) { - Set> requiredOpts = Fragment.requiredOptions(fragment); - for (Class optionsClass : requiredOpts) { - Integer previousBest = visitedOptionsClasses.get(optionsClass); - if (previousBest != null && previousBest <= requiredOpts.size()) { - // Multiple config fragments may require the same options class, but we only need one of - // them to guarantee that class makes it into the configuration. Pick one that depends - // on as few options classes as possible (not necessarily unique). - continue; - } - visitedOptionsClasses.put(optionsClass, requiredOpts.size()); - for (Field field : optionsClass.getFields()) { - if (field.isAnnotationPresent(Option.class)) { - result.put(field.getAnnotation(Option.class).name(), fragment); - } - } - } - } - return result; - } - public PrerequisiteValidator getPrerequisiteValidator() { return prerequisiteValidator; } @@ -822,6 +762,10 @@ public NativeAspectClass getNativeAspectClass(String key) { return nativeAspectClassMap.get(key); } + public FragmentRegistry getFragmentRegistry() { + return fragmentRegistry; + } + public Map getBuildInfoFactoriesAsMap() { ImmutableMap.Builder factoryMapBuilder = ImmutableMap.builder(); for (BuildInfoFactory factory : buildInfoFactories) { @@ -830,16 +774,6 @@ public Map getBuildInfoFactoriesAsMap() { return factoryMapBuilder.build(); } - /** Returns the set of configuration fragments provided by this module. */ - public FragmentClassSet getConfigurationFragments() { - return configurationFragmentClasses; - } - - @Nullable - public Class getConfigurationFragmentForOption(String requiredOption) { - return optionsToFragmentMap.get(requiredOption); - } - /** * Returns the transition factory used to produce the transition to trim targets. * @@ -868,31 +802,11 @@ public boolean shouldInvalidateCacheForOptionDiff( return shouldInvalidateCacheForOptionDiff.apply(newOptions, changedOption, oldValue, newValue); } - /** Returns the set of configuration options that are supported in this module. */ - public ImmutableList> getConfigurationOptions() { - return configurationOptions; - } - /** Returns the definition of the rule class definition with the specified name. */ public RuleDefinition getRuleClassDefinition(String ruleClassName) { return ruleDefinitionMap.get(ruleClassName); } - /** - * Returns the configuration fragments that should be available to all rules even when not - * explicitly required. - * - *

This is a subset of {@link #getConfigurationFragments}. - */ - public FragmentClassSet getUniversalFragments() { - return universalFragments; - } - - /** Creates a BuildOptions class for the given options taken from an optionsProvider. */ - public BuildOptions createBuildOptions(OptionsProvider optionsProvider) { - return BuildOptions.of(configurationOptions, optionsProvider); - } - private static ImmutableMap createNativeRuleSpecificBindings( ImmutableMap starlarkAccessibleTopLevels, ImmutableList bootstraps) { 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 09199b545ad5e3..0f530705645bcf 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 @@ -303,7 +303,7 @@ private ConfiguredTarget createRule( .setVisibility(convertVisibility(prerequisiteMap, env.getEventHandler(), rule)) .setPrerequisites(transformPrerequisiteMap(prerequisiteMap)) .setConfigConditions(configConditions) - .setUniversalFragments(ruleClassProvider.getUniversalFragments()) + .setUniversalFragments(ruleClassProvider.getFragmentRegistry().getUniversalFragments()) .setToolchainContexts(toolchainContexts) .setExecGroupCollectionBuilder(execGroupCollectionBuilder) .setConstraintSemantics(ruleClassProvider.getConstraintSemantics()) @@ -311,7 +311,7 @@ private ConfiguredTarget createRule( RequiredFragmentsUtil.getRuleRequiredFragmentsIfEnabled( rule, configuration, - ruleClassProvider.getUniversalFragments(), + ruleClassProvider.getFragmentRegistry().getUniversalFragments(), configConditions.asProviders(), prerequisiteMap.values())) .build(); @@ -521,7 +521,7 @@ public ConfiguredAspect createAspect( .setPrerequisites(transformPrerequisiteMap(prerequisiteMap)) .setAspectAttributes(aspectAttributes) .setConfigConditions(configConditions) - .setUniversalFragments(ruleClassProvider.getUniversalFragments()) + .setUniversalFragments(ruleClassProvider.getFragmentRegistry().getUniversalFragments()) .setToolchainContext(toolchainContext) // TODO(b/161222568): Implement the exec_properties attr for aspects and read its value // here. @@ -534,7 +534,7 @@ public ConfiguredAspect createAspect( aspectFactory, associatedTarget.getTarget().getAssociatedRule(), aspectConfiguration, - ruleClassProvider.getUniversalFragments(), + ruleClassProvider.getFragmentRegistry().getUniversalFragments(), configConditions.asProviders(), prerequisiteMap.values())) .build(); 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 c1fc8126380fde..57a76ac6a5270c 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 @@ -22,7 +22,6 @@ import com.google.common.base.Strings; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableCollection; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; @@ -30,12 +29,12 @@ import com.google.common.collect.MapDifference; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; -import com.google.common.collect.Ordering; 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; import com.google.devtools.build.lib.skyframe.serialization.SerializationContext; +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; @@ -67,9 +66,10 @@ // TODO(janakr): If overhead of FragmentOptions class names is too high, add constructor that just // takes fragments and gets names from them. public final class BuildOptions implements Cloneable, Serializable { - private static final Comparator> - lexicalFragmentOptionsComparator = Comparator.comparing(Class::getName); - private static final Comparator

Order of elements in the given lists does not matter - the resulting registry will contain + * deterministically ordered sets. + * + * @param allFragments all registered fragment classes, including {@code universalFragments} + * @param universalFragments fragment classes that should be available to all rules even when not + * explicitly required + * @param additionalOptions any additional options classes not accounted for by a {@link + * RequiresOptions} annotation on a {@link Fragment} class in {@code allFragments} + */ + public static FragmentRegistry create( + List> allFragments, + List> universalFragments, + List> additionalOptions) { + FragmentClassSet allFragmentsSet = FragmentClassSet.of(allFragments); + FragmentClassSet universalFragmentsSet = FragmentClassSet.of(universalFragments); + if (!allFragmentsSet.containsAll(universalFragmentsSet)) { + throw new IllegalArgumentException( + "Missing universally required fragments: " + + Sets.difference(universalFragmentsSet, allFragmentsSet)); + } + + ImmutableSortedSet.Builder> optionsClasses = + ImmutableSortedSet.orderedBy(BuildOptions.LEXICAL_FRAGMENT_OPTIONS_COMPARATOR); + for (Class fragment : allFragmentsSet) { + optionsClasses.addAll(Fragment.requiredOptions(fragment)); + } + optionsClasses.addAll(additionalOptions); + + return new FragmentRegistry(allFragmentsSet, universalFragmentsSet, optionsClasses.build()); + } + + private final FragmentClassSet allFragments; + private final FragmentClassSet universalFragments; + private final ImmutableSortedSet> optionsClasses; + + private FragmentRegistry( + FragmentClassSet allFragments, + FragmentClassSet universalFragments, + ImmutableSortedSet> optionsClasses) { + this.allFragments = allFragments; + this.universalFragments = universalFragments; + this.optionsClasses = optionsClasses; + } + + /** Returns the set of all registered configuration fragments. */ + public FragmentClassSet getAllFragments() { + return allFragments; + } + + /** + * Returns a subset of {@link #getAllFragments} that should be available to all rules even when + * not explicitly required. + */ + public FragmentClassSet getUniversalFragments() { + return universalFragments; + } + + /** + * Returns the set of all registered {@link FragmentOptions} classes. + * + *

Includes at least all options classes {@linkplain RequiresOptions required} by fragments in + * {@link #getAllFragments}. + */ + public ImmutableSortedSet> getOptionsClasses() { + return optionsClasses; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandUtils.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandUtils.java index b7135bd7f1548d..6ff507698c9fbb 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandUtils.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandUtils.java @@ -53,8 +53,7 @@ private BlazeCommandUtils() {} public static ImmutableList> getStartupOptions( Iterable modules) { - Set> options = new HashSet<>(); - options.addAll(DEFAULT_STARTUP_OPTIONS); + Set> options = new HashSet<>(DEFAULT_STARTUP_OPTIONS); for (BlazeModule blazeModule : modules) { Iterables.addAll(options, blazeModule.getStartupOptions()); } @@ -73,11 +72,10 @@ public static ImmutableSet> getCommonOptions( } /** - * Returns the set of all options (including those inherited directly and - * transitively) for this AbstractCommand's @Command annotation. + * Returns the set of all options (including those inherited directly and transitively) for this + * AbstractCommand's @Command annotation. * - *

Why does metaprogramming always seem like such a bright idea in the - * beginning? + *

Why does metaprogramming always seem like such a bright idea in the beginning? */ public static ImmutableList> getOptions( Class clazz, @@ -88,12 +86,11 @@ public static ImmutableList> getOptions( throw new IllegalStateException("@Command missing for " + clazz.getName()); } - Set> options = new HashSet<>(); - options.addAll(getCommonOptions(modules)); + Set> options = new HashSet<>(getCommonOptions(modules)); Collections.addAll(options, commandAnnotation.options()); if (commandAnnotation.usesConfigurationOptions()) { - options.addAll(ruleClassProvider.getConfigurationOptions()); + options.addAll(ruleClassProvider.getFragmentRegistry().getOptionsClasses()); } for (BlazeModule blazeModule : modules) { @@ -116,7 +113,7 @@ public static ImmutableList> getOptions( * syntax, and full description. * @param productName the product name */ - public static final String expandHelpTopic( + public static String expandHelpTopic( String topic, String help, Class commandClass, @@ -178,7 +175,7 @@ public static String getUsage( commandAnnotation.name(), commandAnnotation.help(), commandClass, - BlazeCommandUtils.getOptions(commandClass, blazeModules, ruleClassProvider), + getOptions(commandClass, blazeModules, ruleClassProvider), verbosity, productName); } 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 04e08ccbecbed5..31fd54c0bf29f8 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 @@ -750,9 +750,10 @@ private void shutDownModulesOnCrash(DetailedExitCode exitCode) { } } - /** Creates a BuildOptions class for the given options taken from an optionsProvider. */ + /** Creates a BuildOptions class for the given options taken from an {@link OptionsProvider}. */ public BuildOptions createBuildOptions(OptionsProvider optionsProvider) { - return ruleClassProvider.createBuildOptions(optionsProvider); + return BuildOptions.of( + ruleClassProvider.getFragmentRegistry().getOptionsClasses(), optionsProvider); } /** diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD b/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD index c8cf4dafeca262..dd2f4c8257acc4 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD @@ -36,7 +36,9 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration", "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options", "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment", + "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_class_set", "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options", + "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_registry", "//src/main/java/com/google/devtools/build/lib/analysis:config/invalid_configuration_exception", "//src/main/java/com/google/devtools/build/lib/analysis:config/run_under", "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/ConfigCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/ConfigCommand.java index 4e5528d2d90601..38f73612c0a111 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/ConfigCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/ConfigCommand.java @@ -21,20 +21,23 @@ import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; +import com.google.common.base.Ascii; import com.google.common.base.Verify; import com.google.common.collect.HashBasedTable; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import com.google.common.collect.Ordering; import com.google.common.collect.Sets; import com.google.common.collect.Table; -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.CoreOptions; import com.google.devtools.build.lib.analysis.config.Fragment; +import com.google.devtools.build.lib.analysis.config.FragmentClassSet; import com.google.devtools.build.lib.analysis.config.FragmentOptions; +import com.google.devtools.build.lib.analysis.config.FragmentRegistry; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Event; @@ -61,7 +64,7 @@ import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.util.Collection; -import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -341,7 +344,8 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti : new JsonOutputFormatter(writer); ImmutableSortedMap< Class, ImmutableSortedSet>> - fragmentDefs = getFragmentDefs(env.getRuntime().getRuleClassProvider()); + fragmentDefs = + getFragmentDefs(env.getRuntime().getRuleClassProvider().getFragmentRegistry()); if (options.getResidue().isEmpty()) { if (configCommandOptions.dumpAll) { @@ -398,17 +402,15 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti */ private static ImmutableSortedMap< Class, ImmutableSortedSet>> - getFragmentDefs(ConfiguredRuleClassProvider ruleClassProvider) { - ImmutableSortedMap.Builder< - Class, ImmutableSortedSet>> - fragments = ImmutableSortedMap.orderedBy((c1, c2) -> c1.getName().compareTo(c2.getName())); - for (Class fragmentClass : ruleClassProvider.getConfigurationFragments()) { - fragments.put( - fragmentClass, - ImmutableSortedSet.copyOf( - comparing(Class::getName), Fragment.requiredOptions(fragmentClass))); - } - return fragments.build(); + getFragmentDefs(FragmentRegistry fragmentRegistry) { + return fragmentRegistry.getAllFragments().stream() + .collect( + toImmutableSortedMap( + FragmentClassSet.LEXICAL_FRAGMENT_SORTER, + fragment -> fragment, + fragment -> + ImmutableSortedSet.copyOf( + Comparator.comparing(Class::getName), Fragment.requiredOptions(fragment)))); } /** @@ -447,7 +449,7 @@ private static ConfigurationForOutput getConfigurationForOutput( fragments.add( new FragmentForOutput( entry.getKey().getName(), - entry.getValue().stream().map(clazz -> clazz.getName()).collect(toList()))); + entry.getValue().stream().map(Class::getName).collect(toList()))); } fragmentDefs.entrySet().stream() .filter(entry -> config.hasFragment(entry.getKey())) @@ -456,21 +458,18 @@ private static ConfigurationForOutput getConfigurationForOutput( fragments.add( new FragmentForOutput( entry.getKey().getName(), - entry.getValue().stream() - .map(clazz -> clazz.getName()) - .collect(toList())))); + entry.getValue().stream().map(Class::getName).collect(toList())))); ImmutableSortedSet.Builder fragmentOptions = ImmutableSortedSet.orderedBy(comparing(e -> e.name)); config.getOptions().getFragmentClasses().stream() .map(optionsClass -> config.getOptions().get(optionsClass)) .forEach( - fragmentOptionsInstance -> { - fragmentOptions.add( - new FragmentOptionsForOutput( - fragmentOptionsInstance.getClass().getName(), - getOrderedNativeOptions(fragmentOptionsInstance))); - }); + fragmentOptionsInstance -> + fragmentOptions.add( + new FragmentOptionsForOutput( + fragmentOptionsInstance.getClass().getName(), + getOrderedNativeOptions(fragmentOptionsInstance)))); fragmentOptions.add( new FragmentOptionsForOutput( UserDefinedFragment.DESCRIPTIVE_NAME, getOrderedUserDefinedOptions(config))); @@ -516,7 +515,7 @@ private static ConfigurationForOutput getConfiguration( } private static boolean doesConfigMatch(ConfigurationForOutput config, String configPrefix) { - if (configPrefix.toLowerCase().equals("host")) { + if (Ascii.equalsIgnoreCase(configPrefix, "host")) { return config.isHost; } return config.checksum().startsWith(configPrefix); @@ -538,7 +537,7 @@ private static ImmutableSortedMap getOrderedNativeOptions( !(options.getClass().equals(CoreOptions.class) && entry.getKey().equals("define"))) .collect( toImmutableSortedMap( - Ordering.natural(), e -> e.getKey(), e -> String.valueOf(e.getValue()))); + Ordering.natural(), Map.Entry::getKey, e -> String.valueOf(e.getValue()))); } /** @@ -578,7 +577,7 @@ private static BlazeCommandResult reportAllConfigurations( /** * Reports the result of blaze config and returns the appropriate command exit code. */ - private BlazeCommandResult reportConfigurationIds( + private static BlazeCommandResult reportConfigurationIds( ConfigCommandOutputFormatter writer, ImmutableSortedSet configurations) { writer.writeConfigurationIDs(configurations); @@ -656,10 +655,8 @@ private static Table> diffConfigurations( private static Map> diffOptions( @Nullable FragmentOptionsForOutput options1, @Nullable FragmentOptionsForOutput options2) { - Set optionNames1 = - options1 == null ? Collections.emptySet() : options1.optionNames(); - Set optionNames2 = - options2 == null ? Collections.emptySet() : options2.optionNames(); + Set optionNames1 = options1 == null ? ImmutableSet.of() : options1.optionNames(); + Set optionNames2 = options2 == null ? ImmutableSet.of() : options2.optionNames(); Map> diffs = new HashMap<>(); for (String optionName : Sets.union(optionNames1, optionNames2)) { @@ -678,7 +675,8 @@ private static ConfigurationDiffForOutput getConfigurationDiffForOutput( String configHash1, String configHash2, Table> diffs) { ImmutableSortedSet.Builder fragmentDiffs = ImmutableSortedSet.orderedBy(comparing(e -> e.name)); - diffs.rowKeySet().stream() + diffs + .rowKeySet() .forEach( fragmentName -> { ImmutableSortedMap> sortedOptionDiffs = diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/HelpCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/HelpCommand.java index 56326e513c81fc..7d9bfae410e8d2 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/HelpCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/HelpCommand.java @@ -181,7 +181,7 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti return BlazeCommandResult.success(); } - private void emitBlazeVersionInfo(OutErr outErr, String productName) { + private static void emitBlazeVersionInfo(OutErr outErr, String productName) { String releaseInfo = BlazeVersionInfo.instance().getReleaseName(); String line = String.format("[%s %s]", productName, releaseInfo); outErr.printOut(String.format("%80s\n", line)); @@ -199,7 +199,7 @@ private void emitStartupOptions( runtime.getProductName())); } - private void emitCompletionHelp(BlazeRuntime runtime, OutErr outErr) { + private static void emitCompletionHelp(BlazeRuntime runtime, OutErr outErr) { Map commandsByName = getSortedCommands(runtime); outErr.printOutLn("BAZEL_COMMAND_LIST=\"" + SPACE_JOINER.join(commandsByName.keySet()) + "\""); @@ -235,7 +235,7 @@ private void emitCompletionHelp(BlazeRuntime runtime, OutErr outErr) { visitAllOptions(runtime, startupOptionVisitor, commandOptionVisitor); } - private void emitFlagsAsProtoHelp(BlazeRuntime runtime, OutErr outErr) { + private static void emitFlagsAsProtoHelp(BlazeRuntime runtime, OutErr outErr) { Map flags = new HashMap<>(); Predicate allOptions = option -> true; @@ -250,13 +250,10 @@ private void emitFlagsAsProtoHelp(BlazeRuntime runtime, OutErr outErr) { info.addCommands(commandName); }; Consumer startupOptionVisitor = - parser -> { - parser.visitOptions(allOptions, option -> visitor.accept("startup", option)); - }; + parser -> parser.visitOptions(allOptions, option -> visitor.accept("startup", option)); CommandOptionVisitor commandOptionVisitor = - (commandName, commandAnnotation, parser) -> { - parser.visitOptions(allOptions, option -> visitor.accept(commandName, option)); - }; + (commandName, commandAnnotation, parser) -> + parser.visitOptions(allOptions, option -> visitor.accept(commandName, option)); visitAllOptions(runtime, startupOptionVisitor, commandOptionVisitor); @@ -268,7 +265,7 @@ private void emitFlagsAsProtoHelp(BlazeRuntime runtime, OutErr outErr) { outErr.printOut(Base64.getEncoder().encodeToString(collectionBuilder.build().toByteArray())); } - private BazelFlagsProto.FlagInfo.Builder createFlagInfo(OptionDefinition option) { + private static BazelFlagsProto.FlagInfo.Builder createFlagInfo(OptionDefinition option) { BazelFlagsProto.FlagInfo.Builder flagBuilder = BazelFlagsProto.FlagInfo.newBuilder(); flagBuilder.setName(option.getOptionName()); flagBuilder.setHasNegativeFlag(option.hasNegativeOption()); @@ -280,7 +277,7 @@ private BazelFlagsProto.FlagInfo.Builder createFlagInfo(OptionDefinition option) return flagBuilder; } - private void visitAllOptions( + private static void visitAllOptions( BlazeRuntime runtime, Consumer startupOptionVisitor, CommandOptionVisitor commandOptionVisitor) { @@ -312,19 +309,19 @@ private void emitTargetSyntaxHelp(OutErr outErr, String productName) { "target-syntax", "resource:target-syntax.txt", getClass(), - ImmutableList.>of(), + ImmutableList.of(), OptionsParser.HelpVerbosity.MEDIUM, productName)); } - private void emitInfoKeysHelp(CommandEnvironment env, OutErr outErr) { + private static void emitInfoKeysHelp(CommandEnvironment env, OutErr outErr) { for (InfoItem item : InfoCommand.getInfoItemMap(env, OptionsParser.builder().build()).values()) { outErr.printOut(String.format("%-23s %s\n", item.getName(), item.getDescription())); } } - private void emitGenericHelp(OutErr outErr, BlazeRuntime runtime) { + private static void emitGenericHelp(OutErr outErr, BlazeRuntime runtime) { outErr.printOut(String.format("Usage: %s ...\n\n", runtime.getProductName())); outErr.printOut("Available commands:\n"); @@ -430,7 +427,8 @@ private void emit(OutErr outErr) { options.clear(); Collections.addAll(options, annotation.options()); if (annotation.usesConfigurationOptions()) { - options.addAll(runtime.getRuleClassProvider().getConfigurationOptions()); + options.addAll( + runtime.getRuleClassProvider().getFragmentRegistry().getOptionsClasses()); } appendOptionsHtml(result, options); result.append("\n"); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunction.java index 9e8ac0e7ed55eb..38fb170d7bfec4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunction.java @@ -53,9 +53,9 @@ */ final class PlatformMappingFunction implements SkyFunction { - private final ImmutableList> optionsClasses; + private final ImmutableSet> optionsClasses; - PlatformMappingFunction(ImmutableList> optionsClasses) { + PlatformMappingFunction(ImmutableSet> optionsClasses) { this.optionsClasses = checkNotNull(optionsClasses); } @@ -109,7 +109,7 @@ public PlatformMappingValue compute(SkyKey skyKey, Environment env) if (!platformMappingKey.wasExplicitlySetByUser()) { // If no flag was passed and the default mapping file does not exist treat this as if the // mapping file was empty rather than an error. - return new PlatformMappingValue(ImmutableMap.of(), ImmutableMap.of(), ImmutableList.of()); + return new PlatformMappingValue(ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of()); } throw new PlatformMappingException( new MissingInputFileException( @@ -275,7 +275,7 @@ static final class Mappings { } PlatformMappingValue toPlatformMappingValue( - ImmutableList> optionsClasses) { + ImmutableSet> optionsClasses) { return new PlatformMappingValue(platformsToFlags, flagsToPlatforms, optionsClasses); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingValue.java index 1246bf996fb8b0..d2d56877e5eeb6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingValue.java @@ -100,7 +100,7 @@ public PathFragment getWorkspaceRelativeMappingPath() { return path; } - public boolean wasExplicitlySetByUser() { + boolean wasExplicitlySetByUser() { return wasExplicitlySetByUser; } @@ -138,7 +138,7 @@ public String toString() { private final ImmutableMap> platformsToFlags; private final ImmutableMap, Label> flagsToPlatforms; - private final ImmutableList> optionsClasses; + private final ImmutableSet> optionsClasses; private final LoadingCache, OptionsParsingResult> parserCache; private final LoadingCache mappingCache; @@ -155,7 +155,7 @@ public String toString() { PlatformMappingValue( ImmutableMap> platformsToFlags, ImmutableMap, Label> flagsToPlatforms, - ImmutableList> optionsClasses) { + ImmutableSet> optionsClasses) { this.platformsToFlags = checkNotNull(platformsToFlags); this.flagsToPlatforms = checkNotNull(flagsToPlatforms); this.optionsClasses = checkNotNull(optionsClasses); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareAnalysisPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareAnalysisPhaseFunction.java index d0894a517a1efe..1a92d5ea9b25e8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareAnalysisPhaseFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareAnalysisPhaseFunction.java @@ -256,7 +256,7 @@ private Multimap getConfigurations( throws InterruptedException, TransitionException, OptionsParsingException { Multimap builder = ArrayListMultimap.create(); - FragmentClassSet allFragments = ruleClassProvider.getConfigurationFragments(); + FragmentClassSet allFragments = ruleClassProvider.getFragmentRegistry().getAllFragments(); // Now get the configurations. PathFragment platformMappingPath = fromOptions.get(PlatformOptions.class).platformMappings; 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 31fb182bd82185..a8d02f6197ae5f 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 @@ -642,7 +642,7 @@ private ImmutableMap skyFunctions() { map.put(SkyFunctions.RESOLVED_FILE, new ResolvedFileFunction()); map.put( SkyFunctions.PLATFORM_MAPPING, - new PlatformMappingFunction(ruleClassProvider.getConfigurationOptions())); + new PlatformMappingFunction(ruleClassProvider.getFragmentRegistry().getOptionsClasses())); map.put( SkyFunctions.ARTIFACT_NESTED_SET, ArtifactNestedSetFunction.createInstance(valueBasedChangePruningEnabled())); @@ -2007,12 +2007,13 @@ private ImmutableList getConfigurations( // Prepare the Skyframe inputs. // TODO(gregce): support trimmed configs. - FragmentClassSet allFragments = ruleClassProvider.getConfigurationFragments(); + FragmentClassSet allFragments = ruleClassProvider.getFragmentRegistry().getAllFragments(); PlatformMappingValue platformMappingValue = getPlatformMappingValue(eventHandler, referenceBuildOptions); - ImmutableList.Builder configSkyKeysBuilder = ImmutableList.builder(); + ImmutableList.Builder configSkyKeysBuilder = + ImmutableList.builderWithExpectedSize(optionsList.size()); for (BuildOptions options : optionsList) { configSkyKeysBuilder.add(toConfigurationKey(platformMappingValue, allFragments, options)); } @@ -2059,7 +2060,7 @@ public ConfigurationsResult getConfigurations( throws InvalidConfigurationException, InterruptedException { ConfigurationsResult.Builder builder = ConfigurationsResult.newBuilder(); - FragmentClassSet allFragments = ruleClassProvider.getConfigurationFragments(); + FragmentClassSet allFragments = ruleClassProvider.getFragmentRegistry().getAllFragments(); PlatformMappingValue platformMappingValue = getPlatformMappingValue(eventHandler, fromOptions); // Now get the configurations. @@ -2493,7 +2494,7 @@ Optional getConflictException(ActionLookupKey k) { * no-keep_going evaluations) or indirectly by {@link #filterActionConflictsForTopLevelArtifacts} * in keep_going evaluations. */ - public void resetActionConflictsStoredInSkyframe() { + void resetActionConflictsStoredInSkyframe() { memoizingEvaluator.delete( SkyFunctionName.functionIs(SkyFunctions.ACTION_LOOKUP_CONFLICT_FINDING)); } @@ -3032,7 +3033,7 @@ public PrepareAnalysisPhaseValue prepareAnalysisPhase( Set multiCpu, Collection