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..f1a3f33 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;
