Replace ConfigurationFragmentFactory.create() calls with direct Fragment constructors. Also tweaked the API for returning a "null" fragment. See Fragment.java for details. This clears another obstacle toward removing ConfigurationFragmentFactory. PiperOrigin-RevId: 342901534
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PlatformConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/analysis/PlatformConfigurationLoader.java index ca061fe..221822f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/PlatformConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/PlatformConfigurationLoader.java
@@ -14,20 +14,12 @@ package com.google.devtools.build.lib.analysis; -import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; import com.google.devtools.build.lib.analysis.config.Fragment; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; /** A loader that creates {@link PlatformConfiguration} instances based on command-line options. */ public class PlatformConfigurationLoader implements ConfigurationFragmentFactory { @Override - public PlatformConfiguration create(BuildOptions buildOptions) - throws InvalidConfigurationException { - return new PlatformConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return PlatformConfiguration.class; }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ShellConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/ShellConfiguration.java index e6ffb6d..80c78d0 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ShellConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ShellConfiguration.java
@@ -55,7 +55,7 @@ private final PathFragment shellExecutable; private final boolean useShBinaryStubScript; - private ShellConfiguration(BuildOptions buildOptions) { + public ShellConfiguration(BuildOptions buildOptions) { this.shellExecutable = shellExecutableFinder.apply(buildOptions); this.useShBinaryStubScript = buildOptions.get(Options.class).useShBinaryStubScript; } @@ -126,11 +126,6 @@ /** The loader for {@link ShellConfiguration}. */ public static class Loader implements ConfigurationFragmentFactory { @Override - public Fragment create(BuildOptions buildOptions) { - return new ShellConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return ShellConfiguration.class; }
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 025fe18..881d02e 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,24 +13,11 @@ // limitations under the License. package com.google.devtools.build.lib.analysis.config; -import javax.annotation.Nullable; - /** * A factory that instantiates configuration fragments, and which knows some "static" information * about these fragments. */ public interface ConfigurationFragmentFactory { - /** - * Creates a configuration fragment from the given command-line options. - * - * <p>{@code buildOptions} is only guaranteed to hold those {@link FragmentOptions} that are - * listed by {@link #requiredOptions}. - * - * @return the configuration fragment, or null if some required dependencies are missing. - */ - @Nullable - Fragment create(BuildOptions buildOptions) throws InvalidConfigurationException; - /** Returns the exact type of the fragment this factory creates. */ Class<? extends Fragment> creates(); }
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 1ac1f6d..a1c65fa 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
@@ -23,6 +23,10 @@ /** * An interface for language-specific configurations. * + * <p>Implementations must have a constructor that takes a single {@link BuildOptions} argument. If + * the constructor reads any {@link FragmentOptions} from this argument, the fragment must declare + * them via {@link RequiresOptions}. + * * <p>All implementations must be immutable and communicate this as clearly as possible (e.g. * declare {@link com.google.common.collect.ImmutableList} signatures on their interfaces vs. {@link * List}). This is because fragment instances may be shared across configurations. @@ -32,6 +36,15 @@ @Immutable public abstract class Fragment implements StarlarkValue { + /** + * When a fragment doesn't want to be part of the configuration (for example, when its required + * options are missing and the fragment determines this means the configuration doesn't need it), + * it should override this method. + */ + public boolean shouldInclude() { + return true; + } + @Override public boolean isImmutable() { return true; // immutable and Starlark-hashable
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java index b889399..367c664 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java
@@ -23,7 +23,6 @@ import com.google.devtools.build.lib.analysis.config.CoreOptionConverters.LabelConverter; 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.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.PerLabelOptions; import com.google.devtools.build.lib.analysis.config.RequiresOptions; import com.google.devtools.build.lib.analysis.test.TestShardingStrategy.ShardingStrategyConverter; @@ -299,16 +298,6 @@ /** Configuration loader for test options */ public static class Loader implements ConfigurationFragmentFactory { @Override - public Fragment create(BuildOptions buildOptions) - throws InvalidConfigurationException { - // TODO(gregce): Have TestConfiguration set itself to a non-functional state if this is true. - if (!buildOptions.contains(TestOptions.class)) { - return null; - } - return new TestConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return TestConfiguration.class; } @@ -316,11 +305,23 @@ private final TestOptions options; private final ImmutableMap<TestTimeout, Duration> testTimeout; + private final boolean shouldInclude; - private TestConfiguration(BuildOptions buildOptions) { - TestOptions options = buildOptions.get(TestOptions.class); - this.options = options; - this.testTimeout = ImmutableMap.copyOf(options.testTimeout); + public TestConfiguration(BuildOptions buildOptions) { + this.shouldInclude = buildOptions.contains(TestOptions.class); + if (shouldInclude) { + TestOptions options = buildOptions.get(TestOptions.class); + this.options = options; + this.testTimeout = ImmutableMap.copyOf(options.testTimeout); + } else { + this.options = null; + this.testTimeout = null; + } + } + + @Override + public boolean shouldInclude() { + return shouldInclude; } /** Returns test timeout mapping as set by --test_timeout options. */
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 998c8b8..cbb39ad 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
@@ -177,16 +177,12 @@ * StrictActionEnvOptions} is always available. */ @RequiresOptions(options = {StrictActionEnvOptions.class}) - private static class StrictActionEnvConfiguration extends Fragment { - private StrictActionEnvConfiguration(BuildOptions buildOptions) {} + public static class StrictActionEnvConfiguration extends Fragment { + public StrictActionEnvConfiguration(BuildOptions buildOptions) {} + /** Loader. */ public static class Loader implements ConfigurationFragmentFactory { @Override - public Fragment create(BuildOptions buildOptions) { - return new StrictActionEnvConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return StrictActionEnvConfiguration.class; }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfiguration.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfiguration.java index 32c148a..d5138c1 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfiguration.java
@@ -107,12 +107,6 @@ */ public static final class Loader implements ConfigurationFragmentFactory { @Override - public Fragment create(BuildOptions buildOptions) - throws InvalidConfigurationException { - return new BazelPythonConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return BazelPythonConfiguration.class; } @@ -120,7 +114,7 @@ private final Options options; - private BazelPythonConfiguration(BuildOptions buildOptions) throws InvalidConfigurationException { + public BazelPythonConfiguration(BuildOptions buildOptions) throws InvalidConfigurationException { this.options = buildOptions.get(Options.class); String pythonPath = getPythonPath(); if (!pythonPath.startsWith("python") && !PathFragment.create(pythonPath).isAbsolute()) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java index e261fde..8b1bce1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
@@ -954,11 +954,6 @@ /** Configuration loader for the Android fragment. */ public static class Loader implements ConfigurationFragmentFactory { @Override - public Fragment create(BuildOptions buildOptions) throws InvalidConfigurationException { - return new AndroidConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return AndroidConfiguration.class; } @@ -1011,7 +1006,7 @@ private final boolean disableInstrumentationManifestMerging; private final boolean incompatibleUseToolchainResolution; - private AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurationException { + public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurationException { Options options = buildOptions.get(Options.class); this.sdk = options.sdk; this.cpu = options.cpu;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestConfiguration.java index e482434..1b72a0e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestConfiguration.java
@@ -23,7 +23,6 @@ import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; -import javax.annotation.Nullable; /** Configuration fragment for android_local_test. */ @Immutable @@ -49,13 +48,6 @@ * com.google.devtools.build.lib.rules.android.AndroidLocalTestConfiguration}. */ public static final class Loader implements ConfigurationFragmentFactory { - - @Nullable - @Override - public Fragment create(BuildOptions buildOptions) { - return new AndroidLocalTestConfiguration(buildOptions); - } - @Override public Class<? extends Fragment> creates() { return AndroidLocalTestConfiguration.class; @@ -64,7 +56,7 @@ private final boolean androidLocalTestBinaryResources; - private AndroidLocalTestConfiguration(BuildOptions buildOptions) { + public AndroidLocalTestConfiguration(BuildOptions buildOptions) { this.androidLocalTestBinaryResources = buildOptions.get(Options.class).androidLocalTestBinaryResources; }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java index dd44e64..94f0cd8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
@@ -80,7 +80,7 @@ private final boolean mandatoryMinimumVersion; private final boolean objcProviderFromLinked; - private AppleConfiguration(BuildOptions buildOptions) { + public AppleConfiguration(BuildOptions buildOptions) { AppleCommandLineOptions options = buildOptions.get(AppleCommandLineOptions.class); String iosCpu = iosCpuFromCpu(buildOptions.get(CoreOptions.class).cpu); this.options = options; @@ -469,11 +469,6 @@ */ public static class Loader implements ConfigurationFragmentFactory { @Override - public AppleConfiguration create(BuildOptions buildOptions) { - return new AppleConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return AppleConfiguration.class; }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java index 08b1e37..7e787df 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java
@@ -71,8 +71,7 @@ private static CcToolchainVariables computeCcToolchainVariables( XcodeConfigInfo xcodeConfig, BuildOptions buildOptions) { - AppleConfiguration.Loader appleConfigurationLoader = new AppleConfiguration.Loader(); - AppleConfiguration appleConfiguration = appleConfigurationLoader.create(buildOptions); + AppleConfiguration appleConfiguration = new AppleConfiguration(buildOptions); ApplePlatform platform = appleConfiguration.getSingleArchPlatform(); String cpu = buildOptions.get(CoreOptions.class).cpu;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/swift/SwiftConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/apple/swift/SwiftConfiguration.java index b83a402..9516e7a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/apple/swift/SwiftConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/apple/swift/SwiftConfiguration.java
@@ -18,7 +18,6 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; import com.google.devtools.build.lib.analysis.config.Fragment; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.RequiresOptions; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.starlarkbuildapi.apple.SwiftConfigurationApi; @@ -50,12 +49,6 @@ /** Loads {@link SwiftConfiguration} from build options. */ public static class Loader implements ConfigurationFragmentFactory { @Override - public SwiftConfiguration create(BuildOptions buildOptions) - throws InvalidConfigurationException { - return new SwiftConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return SwiftConfiguration.class; }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfiguration.java index 9fb589a..0e09701 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfiguration.java
@@ -42,11 +42,6 @@ */ public static final class Loader implements ConfigurationFragmentFactory { @Override - public Fragment create(BuildOptions buildOptions) throws InvalidConfigurationException { - return new ConfigFeatureFlagConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return ConfigFeatureFlagConfiguration.class; }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java index f4c6388..0c7782f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java
@@ -14,10 +14,8 @@ package com.google.devtools.build.lib.rules.cpp; -import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; import com.google.devtools.build.lib.analysis.config.Fragment; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; /** * Loader for C++ configurations. @@ -30,9 +28,4 @@ /** Creates a new {@link CppConfigurationLoader} instance. */ public CppConfigurationLoader() {} - - @Override - public CppConfiguration create(BuildOptions options) throws InvalidConfigurationException { - return new CppConfiguration(options); - } }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQueryConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQueryConfiguration.java index 60a2e41..b4207c8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQueryConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQueryConfiguration.java
@@ -18,7 +18,6 @@ import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; 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.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.RequiresOptions; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; @@ -43,11 +42,6 @@ static class Loader implements ConfigurationFragmentFactory { @Override - public Fragment create(BuildOptions buildOptions) throws InvalidConfigurationException { - return new GenQueryConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return GenQueryConfiguration.class; }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfigurationLoader.java index c74461d..e256bbc 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfigurationLoader.java
@@ -13,10 +13,8 @@ // limitations under the License. package com.google.devtools.build.lib.rules.java; -import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; import com.google.devtools.build.lib.analysis.config.Fragment; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; /** * A loader that creates JavaConfiguration instances based on JavaBuilder configurations and @@ -24,11 +22,6 @@ */ public class JavaConfigurationLoader implements ConfigurationFragmentFactory { @Override - public JavaConfiguration create(BuildOptions buildOptions) throws InvalidConfigurationException { - return new JavaConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return JavaConfiguration.class; }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcConfiguration.java index 24ab99b..5cb50c4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcConfiguration.java
@@ -71,11 +71,6 @@ */ public static class Loader implements ConfigurationFragmentFactory { @Override - public Fragment create(BuildOptions buildOptions) { - return new J2ObjcConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return J2ObjcConfiguration.class; }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java index 7097c06..28db40a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
@@ -71,7 +71,7 @@ private final boolean compileInfoMigration; private final boolean avoidHardcodedCompilationFlags; - ObjcConfiguration(BuildOptions buildOptions) { + public ObjcConfiguration(BuildOptions buildOptions) { CoreOptions options = buildOptions.get(CoreOptions.class); CppOptions cppOptions = buildOptions.get(CppOptions.class); ObjcCommandLineOptions objcOptions = buildOptions.get(ObjcCommandLineOptions.class);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java index 7bb624e..ca4b691 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java
@@ -14,10 +14,8 @@ package com.google.devtools.build.lib.rules.objc; -import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; import com.google.devtools.build.lib.analysis.config.Fragment; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; /** * A loader that creates ObjcConfiguration instances based on Objective-C configurations and @@ -25,11 +23,6 @@ */ public class ObjcConfigurationLoader implements ConfigurationFragmentFactory { @Override - public ObjcConfiguration create(BuildOptions buildOptions) throws InvalidConfigurationException { - return new ObjcConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return ObjcConfiguration.class; }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java index ed64dc7..4da8c73 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java
@@ -21,7 +21,6 @@ import com.google.devtools.build.lib.analysis.config.CoreOptionConverters.StrictDepsMode; 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.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.RequiresOptions; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; @@ -198,12 +197,6 @@ */ public static class Loader implements ConfigurationFragmentFactory { @Override - public Fragment create(BuildOptions buildOptions) - throws InvalidConfigurationException { - return new ProtoConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return ProtoConfiguration.class; } @@ -214,7 +207,7 @@ private final ImmutableList<String> ccProtoLibrarySourceSuffixes; private final Options options; - private ProtoConfiguration(BuildOptions buildOptions) { + public ProtoConfiguration(BuildOptions buildOptions) { Options options = buildOptions.get(Options.class); this.protocOpts = ImmutableList.copyOf(options.protocOpts); this.ccProtoLibraryHeaderSuffixes = ImmutableList.copyOf(options.ccProtoLibraryHeaderSuffixes);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfigurationLoader.java index 5cc3721..f1a3f3310 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfigurationLoader.java
@@ -13,22 +13,14 @@ // limitations under the License. package com.google.devtools.build.lib.rules.python; -import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; import com.google.devtools.build.lib.analysis.config.Fragment; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; /** * A factory implementation for {@link PythonConfiguration} objects. */ public class PythonConfigurationLoader implements ConfigurationFragmentFactory { @Override - public PythonConfiguration create(BuildOptions buildOptions) - throws InvalidConfigurationException { - return new PythonConfiguration(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return PythonConfiguration.class; }
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 fcc4c38..9a8dde2 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
@@ -38,6 +38,7 @@ import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import java.lang.reflect.InvocationTargetException; import java.util.Set; import java.util.concurrent.ExecutionException; import net.starlark.java.eval.StarlarkSemantics; @@ -167,9 +168,20 @@ private Fragment makeFragment(FragmentKey fragmentKey) throws InvalidConfigurationException { BuildOptions buildOptions = fragmentKey.getBuildOptions(); - ConfigurationFragmentFactory factory = getFactory(fragmentKey.getFragmentClass()); - Fragment fragment = factory.create(buildOptions); - return fragment != null ? fragment : NULL_MARKER; + Class<? extends Fragment> fragmentClass = getFactory(fragmentKey.getFragmentClass()).creates(); + String noConstructorPattern = "%s lacks constructor(BuildOptions)"; + try { + Fragment fragment = + fragmentClass.getConstructor(BuildOptions.class).newInstance(buildOptions); + return fragment.shouldInclude() ? fragment : NULL_MARKER; + } catch (InvocationTargetException e) { + if (e.getCause() instanceof InvalidConfigurationException) { + throw (InvalidConfigurationException) e.getCause(); + } + throw new IllegalStateException(String.format(noConstructorPattern, fragmentClass), e); + } catch (ReflectiveOperationException e) { + throw new IllegalStateException(String.format(noConstructorPattern, fragmentClass), e); + } } private ConfigurationFragmentFactory getFactory(Class<? extends Fragment> fragmentType) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java index d1a6e45..b24fee5 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
@@ -610,19 +610,15 @@ } } + /** Test fragment. */ @StarlarkBuiltin(name = "test_diff_fragment", doc = "fragment for testing differy fragments") @RequiresOptions(options = {DiffResetOptions.class}) - private static final class DiffResetFragment extends Fragment implements StarlarkValue { + public static final class DiffResetFragment extends Fragment implements StarlarkValue { public DiffResetFragment(BuildOptions buildOptions) {} } private static final class DiffResetFactory implements ConfigurationFragmentFactory { @Override - public Fragment create(BuildOptions buildOptions) { - return new DiffResetFragment(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return DiffResetFragment.class; }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java index 4b03e5e..2886c3c 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java
@@ -46,7 +46,7 @@ /** The {@link Fragment} that contains the options. */ @AutoCodec @RequiresOptions(options = {TestOptions.class}) - static class TestFragment extends Fragment { + public static class TestFragment extends Fragment { private final BuildOptions buildOptions; public TestFragment(BuildOptions buildOptions) { @@ -63,11 +63,6 @@ */ static class FragmentLoader implements ConfigurationFragmentFactory { @Override - public Fragment create(BuildOptions buildOptions) { - return new TestFragment(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return TestFragment.class; }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java index a2cfc83..f34ff27 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
@@ -22,7 +22,6 @@ import com.google.devtools.build.lib.analysis.config.CoreOptionConverters.EmptyToNullLabelConverter; 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.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.RequiresOptions; import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition; import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions; @@ -47,7 +46,7 @@ * A fragment containing flags that exhibit different flag behaviors for easy testing purposes. */ @RequiresOptions(options = {DummyTestOptions.class}) - private static class DummyTestFragment extends Fragment { + public static class DummyTestFragment extends Fragment { public DummyTestFragment(BuildOptions buildOptions) {} } @@ -89,12 +88,6 @@ /** Loads a new {link @DummyTestFragment} instance. */ protected static class DummyTestLoader implements ConfigurationFragmentFactory { - - @Override - public Fragment create(BuildOptions buildOptions) throws InvalidConfigurationException { - return new DummyTestFragment(buildOptions); - } - @Override public Class<? extends Fragment> creates() { return DummyTestFragment.class;
diff --git a/src/test/java/com/google/devtools/build/lib/rules/LateBoundAliasTest.java b/src/test/java/com/google/devtools/build/lib/rules/LateBoundAliasTest.java index 6aafc4c..9a32796 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/LateBoundAliasTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/LateBoundAliasTest.java
@@ -28,7 +28,6 @@ import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.rules.LateBoundAlias.CommonAliasRule; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; -import javax.annotation.Nullable; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -39,7 +38,8 @@ @RunWith(JUnit4.class) public class LateBoundAliasTest extends BuildViewTestCase { - private static final class TestFragment extends Fragment { + /** Test fragment. */ + public static final class TestFragment extends Fragment { public TestFragment(BuildOptions buildOptions) {} } @@ -49,12 +49,6 @@ public Class<? extends Fragment> creates() { return TestFragment.class; } - - @Nullable - @Override - public Fragment create(BuildOptions buildOptions) { - return new TestFragment(buildOptions); - } } private static final class TestLateBoundDefault extends LabelLateBoundDefault<TestFragment> {
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java index 21468bd..6f2815e 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
@@ -109,9 +109,10 @@ } } + /** Test fragment. */ @AutoCodec @RequiresOptions(options = {DummyTestOptions.class}) - static class DummyTestOptionsFragment extends Fragment { + public static class DummyTestOptionsFragment extends Fragment { private final BuildOptions buildOptions; public DummyTestOptionsFragment(BuildOptions buildOptions) { @@ -125,11 +126,6 @@ private static class DummyTestOptionsLoader implements ConfigurationFragmentFactory { @Override - public Fragment create(BuildOptions buildOptions) { - return new DummyTestOptionsFragment(buildOptions); - } - - @Override public Class<? extends Fragment> creates() { return DummyTestOptionsFragment.class; }
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmableTestConfigurationFragments.java b/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmableTestConfigurationFragments.java index dbe1168..c3789a1 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmableTestConfigurationFragments.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/trimming/TrimmableTestConfigurationFragments.java
@@ -71,7 +71,6 @@ import com.google.devtools.common.options.OptionsParser; import java.io.IOException; import java.util.List; -import java.util.function.Function; import javax.annotation.Nullable; import net.starlark.java.annot.StarlarkBuiltin; import net.starlark.java.eval.EvalException; @@ -407,22 +406,15 @@ private static final class FragmentLoader<FragmentT extends Fragment> implements ConfigurationFragmentFactory { private final Class<FragmentT> fragmentType; - private final Function<BuildOptions, FragmentT> fragmentMaker; - FragmentLoader(Class<FragmentT> fragmentType, Function<BuildOptions, FragmentT> fragmentMaker) { + FragmentLoader(Class<FragmentT> fragmentType) { this.fragmentType = fragmentType; - this.fragmentMaker = fragmentMaker; } @Override public Class<? extends Fragment> creates() { return fragmentType; } - - @Override - public Fragment create(BuildOptions buildOptions) { - return fragmentMaker.apply(buildOptions); - } } /** Set of test options. */ @@ -439,8 +431,7 @@ @StarlarkBuiltin(name = "alpha", doc = "Test config fragment.") @RequiresOptions(options = {AOptions.class}) public static final class AConfig extends Fragment implements StarlarkValue { - public static final ConfigurationFragmentFactory FACTORY = - new FragmentLoader<>(AConfig.class, AConfig::new); + public static final ConfigurationFragmentFactory FACTORY = new FragmentLoader<>(AConfig.class); private final String value; @@ -468,8 +459,7 @@ @StarlarkBuiltin(name = "bravo", doc = "Test config fragment.") @RequiresOptions(options = {BOptions.class}) public static final class BConfig extends Fragment implements StarlarkValue { - public static final ConfigurationFragmentFactory FACTORY = - new FragmentLoader<>(BConfig.class, BConfig::new); + public static final ConfigurationFragmentFactory FACTORY = new FragmentLoader<>(BConfig.class); private final String value; @@ -497,8 +487,7 @@ @StarlarkBuiltin(name = "charlie", doc = "Test config fragment.") @RequiresOptions(options = {COptions.class}) public static final class CConfig extends Fragment implements StarlarkValue { - public static final ConfigurationFragmentFactory FACTORY = - new FragmentLoader<>(CConfig.class, CConfig::new); + public static final ConfigurationFragmentFactory FACTORY = new FragmentLoader<>(CConfig.class); private final String value; @@ -526,8 +515,7 @@ @StarlarkBuiltin(name = "delta", doc = "Test config fragment.") @RequiresOptions(options = {DOptions.class}) public static final class DConfig extends Fragment implements StarlarkValue { - public static final ConfigurationFragmentFactory FACTORY = - new FragmentLoader<>(DConfig.class, DConfig::new); + public static final ConfigurationFragmentFactory FACTORY = new FragmentLoader<>(DConfig.class); private final String value; @@ -558,8 +546,7 @@ @StarlarkBuiltin(name = "echo", doc = "Test config fragment.") @RequiresOptions(options = {EOptions.class}) public static final class EConfig extends Fragment implements StarlarkValue { - public static final ConfigurationFragmentFactory FACTORY = - new FragmentLoader<>(EConfig.class, EConfig::new); + public static final ConfigurationFragmentFactory FACTORY = new FragmentLoader<>(EConfig.class); private final String value;