Simplify config fragment creation.

Remove all unnecessesary accesses to ConfigurationEnvironment and
deprecate the accesses that actually need ConfigurationEnvironment.

For review, check out ConfigurationFragmentFactory first.

PiperOrigin-RevId: 195099768

15 files changed, 41 insertions, 53 deletions

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 df2e5f1d5b..a76290d06f 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
@@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -100,7 +99,7 @@ public class ShellConfiguration extends BuildConfiguration.Fragment {
 
     @Nullable
     @Override
-    public Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions) {
+    public Fragment create(BuildOptions buildOptions) {
         return new ShellConfiguration(shellExecutableProvider.getShellExecutable(buildOptions));
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationEnvironment.java
index 691cea22de..d7303f2686 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationEnvironment.java
@@ -31,13 +31,6 @@ import com.google.devtools.build.lib.vfs.Path;
  * be recorded for correct caching.
  */
 public interface ConfigurationEnvironment {
-
-  /**
-   * Returns an event handler to report errors to. Note that reporting an error does not cause the
-   * computation to abort - you also need to throw an exception.
-   */
-  ExtendedEventHandler getEventHandler();
-
   /**
    * Returns a target for the given label, loading it if necessary, and throwing an exception if it
    * does not exist.
@@ -93,11 +86,6 @@ public interface ConfigurationEnvironment {
     }
 
     @Override
-    public ExtendedEventHandler getEventHandler() {
-      return packageProvider.getEventHandler();
-    }
-
-    @Override
     public Target getTarget(final Label label)
         throws NoSuchPackageException, NoSuchTargetException, InterruptedException {
       return packageProvider.getLoadedTarget(label);
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 0c19502324..2d72a8be2c 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
@@ -24,13 +24,40 @@ public interface ConfigurationFragmentFactory {
   /**
    * Creates a configuration fragment.
    *
+   * <p>All implementations should override this method unless they have a really good reason
+   * to override {@link #create(ConfigurationEnvironment, BuildOptions)} instead. The latter
+   * interface is slated for removal once we detach legacy callers.
+   *
+   * @param buildOptions command-line options (see {@link FragmentOptions})
+   * @return the configuration fragment or null if some required dependencies are missing.
+   */
+  @Nullable
+  default BuildConfiguration.Fragment create(BuildOptions buildOptions)
+      throws InvalidConfigurationException, InterruptedException {
+    throw new IllegalStateException(
+        "One of this method's signatures must be overridden to have a valid fragment creator");
+  }
+
+  /**
+   * Creates a configuration fragment: <b>LEGACY VERSION</b>.
+   *
+   * <p>For implementations that cannot override {@link #create(BuildOptions)} because they really
+   * need access to {@link ConfigurationEnvironment}. {@link ConfigurationEnvironment} adds extra
+   * dependencies to fragment creation that makes the whole process more complicated and delicate.
+   * We're also working on Bazel enhancements that will make current calls unnecessary. So this
+   * version really only exists as a stopgap before we can migrate away the legacy calls.
+   *
    * @param env the ConfigurationEnvironment for querying targets and paths
    * @param buildOptions command-line options (see {@link FragmentOptions})
    * @return the configuration fragment or null if some required dependencies are missing.
    */
+  @Deprecated
   @Nullable
-  BuildConfiguration.Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions)
-      throws InvalidConfigurationException, InterruptedException;
+  default BuildConfiguration.Fragment create(ConfigurationEnvironment env,
+      BuildOptions buildOptions) throws InvalidConfigurationException, InterruptedException {
+    return create(buildOptions);
+  }
+
 
   /**
    * @return the exact type of the fragment this factory creates.
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 c31d95e472..026263cff3 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
@@ -19,7 +19,6 @@ import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -138,7 +137,7 @@ public class TestConfiguration extends Fragment {
   /** Configuration loader for test options */
   public static class Loader implements ConfigurationFragmentFactory {
     @Override
-    public Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions)
+    public Fragment create(BuildOptions buildOptions)
         throws InvalidConfigurationException {
       return new TestConfiguration(buildOptions.get(TestOptions.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 d35b3d6ba7..f9c73f0ad5 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
@@ -19,7 +19,6 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.LabelConverter;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -126,7 +125,7 @@ public class BazelPythonConfiguration extends BuildConfiguration.Fragment {
    */
   public static final class Loader implements ConfigurationFragmentFactory {
     @Override
-    public Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions)
+    public Fragment create(BuildOptions buildOptions)
         throws InvalidConfigurationException {
       BazelPythonConfiguration pythonConfiguration
           = new BazelPythonConfiguration(buildOptions.get(Options.class));
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 256d749b77..f1adf02199 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
@@ -24,7 +24,6 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration.EmptyToN
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.LabelConverter;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -849,7 +848,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
   /** Configuration loader for the Android fragment. */
   public static class Loader implements ConfigurationFragmentFactory {
     @Override
-    public Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions)
+    public Fragment create(BuildOptions buildOptions)
         throws InvalidConfigurationException, InterruptedException {
       return new AndroidConfiguration(buildOptions.get(Options.class));
     }
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 6cb3ea1ab1..9fa014d457 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
@@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -58,7 +57,7 @@ public class AndroidLocalTestConfiguration extends BuildConfiguration.Fragment {
 
     @Nullable
     @Override
-    public Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions)
+    public Fragment create(BuildOptions buildOptions)
         throws InvalidConfigurationException, InterruptedException {
       return new AndroidLocalTestConfiguration(buildOptions.get(Options.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 2d2d40e089..e91ad2214d 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
@@ -25,7 +25,6 @@ import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
-import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.NoSuchThingException;
 import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
@@ -151,7 +150,6 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory {
         try {
           fdoProfileLabel = Label.parseAbsolute(cppOptions.getFdoOptimize());
         } catch (LabelSyntaxException e) {
-          env.getEventHandler().handle(Event.error(e.getMessage()));
           throw new InvalidConfigurationException(e);
         }
       } else {
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 a096753189..477b3144a5 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
@@ -19,7 +19,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.skylark.annotations.SkylarkConfigurationField;
@@ -81,7 +80,7 @@ public class J2ObjcConfiguration extends Fragment {
    */
   public static class Loader implements ConfigurationFragmentFactory {
     @Override
-    public Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions) {
+    public Fragment create(BuildOptions buildOptions) {
       return new J2ObjcConfiguration(buildOptions.get(J2ObjcCommandLineOptions.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 c730a7b874..a5706d9bc2 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
@@ -20,7 +20,6 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.StrictDepsMode;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -177,7 +176,7 @@ public class ProtoConfiguration extends Fragment {
    */
   public static class Loader implements ConfigurationFragmentFactory {
     @Override
-    public Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions)
+    public Fragment create(BuildOptions buildOptions)
         throws InvalidConfigurationException {
       return new ProtoConfiguration(buildOptions.get(Options.class));
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java
index a35a2e45de..9a9f449610 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java
@@ -23,7 +23,6 @@ import com.google.devtools.build.lib.analysis.config.InvalidConfigurationExcepti
 import com.google.devtools.build.lib.analysis.config.PackageProviderForConfigurations;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
-import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.Package;
@@ -104,11 +103,6 @@ public final class ConfigurationFragmentFunction implements SkyFunction {
     }
 
     @Override
-    public ExtendedEventHandler getEventHandler() {
-      return packageProvider.getEventHandler();
-    }
-
-    @Override
     public Target getTarget(Label label)
         throws NoSuchPackageException, NoSuchTargetException, InterruptedException {
       return packageProvider.getTarget(label);
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 46f5b5a6ce..84a70a5d36 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
@@ -20,10 +20,8 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 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.transitions.SplitTransition;
 import com.google.devtools.build.lib.analysis.util.MockRule;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -74,9 +72,7 @@ public class LateBoundSplitUtil {
    */
   static class FragmentLoader implements ConfigurationFragmentFactory {
     @Override
-    public BuildConfiguration.Fragment create(ConfigurationEnvironment env,
-        BuildOptions buildOptions)
-        throws InvalidConfigurationException {
+    public BuildConfiguration.Fragment create(BuildOptions buildOptions) {
       return new TestFragment();
     }
 
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/TestConfigFragments.java b/src/test/java/com/google/devtools/build/lib/analysis/TestConfigFragments.java
index dbede30426..0366bf8221 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/TestConfigFragments.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/TestConfigFragments.java
@@ -17,10 +17,8 @@ package com.google.devtools.build.lib.analysis;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 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.transitions.PatchTransition;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -66,9 +64,7 @@ public class TestConfigFragments {
     }
 
     @Override
-    public BuildConfiguration.Fragment create(
-        ConfigurationEnvironment env, BuildOptions buildOptions)
-        throws InvalidConfigurationException {
+    public BuildConfiguration.Fragment create(BuildOptions buildOptions) {
       return fragment;
     }
 
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java
index cf7f1e6c56..aa36ebf127 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java
@@ -373,7 +373,7 @@ public class BuildConfigurationTest extends ConfigurationTestCase {
     return new ConfigurationFragmentFactory() {
       @Override
       public Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions)
-          throws InvalidConfigurationException, InterruptedException {
+          throws InterruptedException {
         try {
           env.getTarget(Label.parseAbsoluteUnchecked(label));
         } catch (NoSuchPackageException e) {
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 0103aa9e89..8faf64d8e4 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
@@ -21,10 +21,8 @@ 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.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
-import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 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.util.BuildViewTestCase;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Rule;
@@ -70,8 +68,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
 
   private static class LateBoundTestOptionsLoader implements ConfigurationFragmentFactory {
     @Override
-    public BuildConfiguration.Fragment create(ConfigurationEnvironment env,
-        BuildOptions buildOptions) throws InvalidConfigurationException {
+    public BuildConfiguration.Fragment create(BuildOptions buildOptions) {
       return new LateBoundTestOptionsFragment();
     }
 
@@ -107,8 +104,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
 
   private static class InternalTestOptionsLoader implements ConfigurationFragmentFactory {
     @Override
-    public BuildConfiguration.Fragment create(ConfigurationEnvironment env,
-        BuildOptions buildOptions) throws InvalidConfigurationException {
+    public BuildConfiguration.Fragment create(BuildOptions buildOptions) {
       return new InternalTestOptionsFragment();
     }