Remove ConfigurationFragmentFactory.requiredOptions().
This clears another obstacle toward removing ConfigurationFragmentFactory.
PiperOrigin-RevId: 342107364
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 765c8d5..6ee5d7f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -1671,9 +1671,11 @@
srcs = ["config/Fragment.java"],
deps = [
":config/build_options",
+ ":config/fragment_options",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/net/starlark/java/eval",
+ "//third_party:guava",
"//third_party:jsr305",
],
)
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 210c9f5..5e51012 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
@@ -204,7 +204,7 @@
* no two different configuration fragments can share the same name.
*/
public Builder addConfigurationFragment(ConfigurationFragmentFactory factory) {
- this.configurationOptions.addAll(factory.requiredOptions());
+ this.configurationOptions.addAll(Fragment.requiredOptions(factory.creates()));
configurationFragmentFactories.add(factory);
return this;
}
@@ -598,7 +598,8 @@
Map<String, Class<? extends Fragment>> result = new LinkedHashMap<>();
Map<Class<? extends FragmentOptions>, Integer> visitedOptionsClasses = new HashMap<>();
for (ConfigurationFragmentFactory factory : configurationFragments) {
- Set<Class<? extends FragmentOptions>> requiredOpts = factory.requiredOptions();
+ Set<Class<? extends FragmentOptions>> requiredOpts =
+ Fragment.requiredOptions(factory.creates());
for (Class<? extends FragmentOptions> optionsClass : requiredOpts) {
Integer previousBest = visitedOptionsClasses.get(optionsClass);
if (previousBest != null && previousBest <= requiredOpts.size()) {
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 26af17b..9b35cc4 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
@@ -345,7 +345,8 @@
fragmentToRequiredOptions = ArrayListMultimap.create();
for (ConfigurationFragmentFactory fragmentLoader :
((FragmentProvider) ruleClassProvider).getConfigurationFragments()) {
- fragmentToRequiredOptions.putAll(fragmentLoader.creates(), fragmentLoader.requiredOptions());
+ fragmentToRequiredOptions.putAll(
+ fragmentLoader.creates(), Fragment.requiredOptions(fragmentLoader.creates()));
}
Set<Class<? extends FragmentOptions>> options = new HashSet<>();
for (Class<? extends Fragment> fragmentClass : fragmentClasses) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFragmentFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFragmentFactory.java
index a797b65..025fe18 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFragmentFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFragmentFactory.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis.config;
-import com.google.common.collect.ImmutableSet;
import javax.annotation.Nullable;
/**
@@ -34,18 +33,4 @@
/** Returns the exact type of the fragment this factory creates. */
Class<? extends Fragment> creates();
-
- /**
- * Returns the option classes needed to create this fragment.
- *
- * <p>Don't override this method. We're intending to remove it entirely in preference for {@link
- * RequiresOptions}, which clears the way for removing {@link ConfigurationFragmentFactory}
- * entirely.
- */
- default ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() {
- Class<? extends Fragment> fragmentClass = creates();
- return fragmentClass.isAnnotationPresent(RequiresOptions.class)
- ? ImmutableSet.copyOf(fragmentClass.getAnnotation(RequiresOptions.class).options())
- : ImmutableSet.of();
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/Fragment.java b/src/main/java/com/google/devtools/build/lib/analysis/config/Fragment.java
index 1ce43ce..1ac1f6d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/Fragment.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/Fragment.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis.config;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.events.EventHandler;
import java.util.List;
@@ -51,4 +52,12 @@
public String getOutputDirectoryName() {
return null;
}
+
+ /** Returns the option classes needed to create a fragment. */
+ public static ImmutableSet<Class<? extends FragmentOptions>> requiredOptions(
+ Class<? extends Fragment> fragmentClass) {
+ return fragmentClass.isAnnotationPresent(RequiresOptions.class)
+ ? ImmutableSet.copyOf(fragmentClass.getAnnotation(RequiresOptions.class).options())
+ : ImmutableSet.of();
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index 4aa5d9a..998c8b8 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -32,6 +32,7 @@
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.FragmentOptions;
+import com.google.devtools.build.lib.analysis.config.RequiresOptions;
import com.google.devtools.build.lib.bazel.repository.LocalConfigPlatformRule;
import com.google.devtools.build.lib.bazel.rules.android.AndroidNdkRepositoryRule;
import com.google.devtools.build.lib.bazel.rules.android.AndroidSdkRepositoryRule;
@@ -175,6 +176,7 @@
* by the registered fragments. We create and register this fragment exclusively to ensure {@link
* StrictActionEnvOptions} is always available.
*/
+ @RequiresOptions(options = {StrictActionEnvOptions.class})
private static class StrictActionEnvConfiguration extends Fragment {
private StrictActionEnvConfiguration(BuildOptions buildOptions) {}
@@ -188,11 +190,6 @@
public Class<? extends Fragment> creates() {
return StrictActionEnvConfiguration.class;
}
-
- @Override
- public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() {
- return ImmutableSet.of(StrictActionEnvOptions.class);
- }
}
}
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 7fa1933..c270e35 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
@@ -384,7 +384,7 @@
fragments.put(
fragmentFactory.creates(),
ImmutableSortedSet.copyOf(
- (c1, c2) -> c1.getName().compareTo(c2.getName()), fragmentFactory.requiredOptions()));
+ comparing(Class::getName), Fragment.requiredOptions(fragmentFactory.creates())));
}
return fragments.build();
}
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProviderTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProviderTest.java
index 7e4c7d0..366a09c 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProviderTest.java
@@ -64,7 +64,8 @@
// Check that every created fragment is present.
assertThat(configurationFragments).contains(fragmentFactory.creates());
// Check that every options class required for fragment creation is provided.
- for (Class<? extends FragmentOptions> options : fragmentFactory.requiredOptions()) {
+ for (Class<? extends FragmentOptions> options :
+ Fragment.requiredOptions(fragmentFactory.creates())) {
assertThat(configOptions).contains(options);
}
}
