Simplify EnvironmentExtension

This reduces the API of PackageFactory.EnvironmentExtension with an eye toward eliminating it by merging it into Bootstrap. This will help us get PackageFactory out of the business of managing the Starlark environment.

The extension's `updateWorkspace` method is removed, along with the workspace logic that called it. This feature has been unused for a long time.

The extension's `updateNative` method is also removed and merged into `update`. There are no call sites that need to differentially add a symbol to the BUILD environment but not the `native` object, or vice versa.

PackageFactory no longer needs to retain the list of extensions, since it was only retrieved for workspace logic.

PiperOrigin-RevId: 530917040
Change-Id: Ie0ff2be22e8f6c6c91a2c833dc0b50dd19b1ae85

src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java

@@ -152,7 +152,7 @@ private static ImmutableMap<String, Object> createUninjectedBuildBzlNativeBindin
     env.putAll(ruleFunctions);
     env.put("package", packageFunction);
     for (PackageFactory.EnvironmentExtension ext : environmentExtensions) {
-      ext.updateNative(env);
+      ext.update(env);
     }
     return env.buildOrThrow();
   }

src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java

@@ -97,15 +97,12 @@ public final class PackageFactory {
   // TODO(bazel-team): this should probably be renamed PackageFactory.RuntimeExtension
   //  since really we're extending the Runtime with more classes.
   public interface EnvironmentExtension {
-    /** Update the predeclared environment with the identifiers this extension contributes. */
+    /**
+     * Updates the predeclared BUILD environment and {@code native} object with the symbols this
+     * extension contributes.
+     */
     void update(ImmutableMap.Builder<String, Object> env);
 
-    /** Update the predeclared environment of WORKSPACE files. */
-    void updateWorkspace(ImmutableMap.Builder<String, Object> env);
-
-    /** Update the environment of the native module. */
-    void updateNative(ImmutableMap.Builder<String, Object> env);
-
     /** Returns the extra arguments to the {@code package()} statement. */
     Iterable<PackageArgument<?>> getPackageArguments();
   }
@@ -118,7 +115,6 @@ public interface EnvironmentExtension {
 
   private int maxDirectoriesToEagerlyVisitInGlobbing;
 
-  private final ImmutableList<EnvironmentExtension> environmentExtensions;
   private final PackageSettings packageSettings;
   private final PackageValidator packageValidator;
   private final PackageOverheadEstimator packageOverheadEstimator;
@@ -194,16 +190,15 @@ public PackageFactory(
       PackageLoadingListener packageLoadingListener) {
     this.ruleClassProvider = ruleClassProvider;
     this.executor = executorForGlobbing;
-    this.environmentExtensions = ImmutableList.copyOf(environmentExtensions);
     this.packageSettings = packageSettings;
     this.packageValidator = packageValidator;
     this.packageOverheadEstimator = packageOverheadEstimator;
     this.packageLoadingListener = packageLoadingListener;
     this.bazelStarlarkEnvironment =
         new BazelStarlarkEnvironment(
             ruleClassProvider,
-            this.environmentExtensions,
-            newPackageFunction(createPackageArguments(this.environmentExtensions)),
+            ImmutableList.copyOf(environmentExtensions),
+            newPackageFunction(createPackageArguments(ImmutableList.copyOf(environmentExtensions))),
             version);
   }
 
@@ -258,10 +253,6 @@ public RuleClassProvider getRuleClassProvider() {
     return ruleClassProvider;
   }
 
-  public ImmutableList<EnvironmentExtension> getEnvironmentExtensions() {
-    return environmentExtensions;
-  }
-
   public BazelStarlarkEnvironment getBazelStarlarkEnvironment() {
     return bazelStarlarkEnvironment;
   }

src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java

@@ -14,15 +14,13 @@
 
 package com.google.devtools.build.lib.packages;
 
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.NullEventHandler;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.Package.NameConflictException;
-import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension;
 import com.google.devtools.build.lib.server.FailureDetails;
 import com.google.devtools.build.lib.server.FailureDetails.PackageLoading;
 import com.google.devtools.build.lib.vfs.Path;
@@ -57,7 +55,6 @@ public class WorkspaceFactory {
 
   private final StarlarkSemantics starlarkSemantics;
   private final ImmutableMap<String, Object> workspaceFunctions;
-  private final ImmutableList<EnvironmentExtension> environmentExtensions;
 
   // Values accumulated from all previous WORKSPACE file parts.
   private final Map<String, Module> loadedModules = new HashMap<>();
@@ -67,7 +64,6 @@ public class WorkspaceFactory {
   /**
    * @param builder a builder for the Workspace
    * @param ruleClassProvider a provider for known rule classes
-   * @param environmentExtensions the Starlark environment extensions
    * @param mutability the Mutability for the current evaluation context
    * @param installDir the install directory
    * @param workspaceDir the workspace directory
@@ -76,7 +72,6 @@ public class WorkspaceFactory {
   public WorkspaceFactory(
       Package.Builder builder,
       RuleClassProvider ruleClassProvider,
-      ImmutableList<EnvironmentExtension> environmentExtensions,
       Mutability mutability,
       boolean allowOverride,
       @Nullable Path installDir,
@@ -88,7 +83,6 @@ public WorkspaceFactory(
     this.installDir = installDir;
     this.workspaceDir = workspaceDir;
     this.defaultSystemJavabaseDir = defaultSystemJavabaseDir;
-    this.environmentExtensions = environmentExtensions;
     this.starlarkSemantics = starlarkSemantics;
     this.workspaceFunctions =
         createWorkspaceFunctions(
@@ -338,9 +332,6 @@ private ImmutableMap<String, Object> getDefaultEnvironment() {
       env.put("__workspace_dir__", workspaceDir.getPathString());
     }
     env.put("DEFAULT_SYSTEM_JAVABASE", getDefaultSystemJavabase());
-    for (EnvironmentExtension ext : environmentExtensions) {
-      ext.updateWorkspace(env);
-    }
     return env.buildOrThrow();
   }
 

src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java

@@ -349,7 +349,6 @@ public SkyValue compute(SkyKey skyKey, Environment env)
           new WorkspaceFactory(
               builder,
               ruleClassProvider,
-              packageFactory.getEnvironmentExtensions(),
               mu,
               key.getIndex() == 0,
               directories.getEmbeddedBinariesRoot(),

src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTestHelper.java

@@ -18,7 +18,6 @@
 import static org.junit.Assert.fail;
 
 import com.google.common.base.Joiner;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.events.Event;
@@ -75,7 +74,6 @@ void parse(String... args) {
         new WorkspaceFactory(
             builder,
             TestRuleClassProvider.getRuleClassProvider(),
-            ImmutableList.<PackageFactory.EnvironmentExtension>of(),
             Mutability.create("test"),
             allowOverride,
             root.asPath(),