Eliminate PackageContext

Its fields were already merged into Package.Builder in cc9b647. This CL just deletes the class and makes Package.Builder get stored directly as a threadlocal in the StarlarkThread.

Continued from unknown commit.

Work toward #19922.

PiperOrigin-RevId: 594142401
Change-Id: I235c2ccbdcaad6de478776e528ab42f3827a34f9

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -75,8 +75,8 @@
 import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.StarlarkImplicitOutputsFunctionWithCallback;
 import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.StarlarkImplicitOutputsFunctionWithMap;
 import com.google.devtools.build.lib.packages.LabelConverter;
+import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.Package.NameConflictException;
-import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
 import com.google.devtools.build.lib.packages.PredicateWithMessage;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
@@ -1037,8 +1037,8 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
       if (ruleClass == null) {
         throw new EvalException("Invalid rule class hasn't been exported by a bzl file");
       }
-      PackageContext pkgContext = thread.getThreadLocal(PackageContext.class);
-      if (pkgContext == null) {
+      Package.Builder pkgBuilder = thread.getThreadLocal(Package.Builder.class);
+      if (pkgBuilder == null) {
         throw new EvalException(
             "Cannot instantiate a rule when loading a .bzl file. "
                 + "Rules may be instantiated only in a BUILD thread.");
@@ -1086,7 +1086,7 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
                             currentRuleClass.getName(),
                             attr,
                             value,
-                            pkgContext.getBuilder().getLabelConverter());
+                            pkgBuilder.getLabelConverter());
                 initializerKwargs.put(attr.getName(), reifiedValue);
               }
             }
@@ -1141,15 +1141,15 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
           new BuildLangTypedAttributeValuesMap(kwargs);
       try {
         RuleFactory.createAndAddRule(
-            pkgContext.getBuilder(),
+            pkgBuilder,
             ruleClass,
             attributeValues,
             thread
                 .getSemantics()
                 .getBool(BuildLanguageOptions.INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES),
             // TODO(#19922): Delete this arg once it's definitely redundant (when the builder owns
             // its eventHandler).
-            pkgContext.getBuilder().getLocalEventHandler(),
+            pkgBuilder.getLocalEventHandler(),
             thread.getCallStack());
       } catch (InvalidRuleException | NameConflictException e) {
         throw new EvalException(e);

src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java

@@ -38,7 +38,6 @@
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.Package.NameConflictException;
 import com.google.devtools.build.lib.packages.PackageFactory;
-import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
@@ -244,18 +243,17 @@ private Object createRuleLegacy(StarlarkThread thread, Dict<String, Object> kwar
       String ruleClassName = getRuleClassName();
       try {
         RuleClass ruleClass = builder.build(ruleClassName, ruleClassName);
-        PackageContext context = PackageFactory.getContext(thread);
-        Package.Builder packageBuilder = context.getBuilder();
+        Package.Builder pkgBuilder = PackageFactory.getContext(thread);
 
         // TODO(adonovan): is this cast safe? Check.
         String name = (String) kwargs.get("name");
         if (name == null) {
           throw Starlark.errorf("argument 'name' is required");
         }
-        WorkspaceFactoryHelper.addMainRepoEntry(packageBuilder, name);
-        WorkspaceFactoryHelper.addRepoMappings(packageBuilder, kwargs, name);
+        WorkspaceFactoryHelper.addMainRepoEntry(pkgBuilder, name);
+        WorkspaceFactoryHelper.addRepoMappings(pkgBuilder, kwargs, name);
         return WorkspaceFactoryHelper.createAndAddRepositoryRule(
-            context.getBuilder(),
+            pkgBuilder,
             ruleClass,
             /* bindRuleClass= */ null,
             WorkspaceFactoryHelper.getFinalKwargs(kwargs),

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

@@ -18,7 +18,6 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.packages.License.DistributionType;
-import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
 import com.google.devtools.build.lib.packages.Type.ConversionException;
 import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code;
 import java.util.List;
@@ -77,23 +76,21 @@ public NoneType environmentGroup(
       StarlarkThread thread)
       throws EvalException {
     BazelStarlarkContext.checkLoadingPhase(thread, "environment_group");
-    PackageContext context = PackageFactory.getContext(thread);
+    Package.Builder pkgBuilder = PackageFactory.getContext(thread);
     List<Label> environments =
         BuildType.LABEL_LIST.convert(
-            environmentsList,
-            "'environment_group argument'",
-            context.pkgBuilder.getLabelConverter());
+            environmentsList, "'environment_group argument'", pkgBuilder.getLabelConverter());
     List<Label> defaults =
         BuildType.LABEL_LIST.convert(
-            defaultsList, "'environment_group argument'", context.pkgBuilder.getLabelConverter());
+            defaultsList, "'environment_group argument'", pkgBuilder.getLabelConverter());
 
     if (environments.isEmpty()) {
       throw Starlark.errorf("environment group %s must contain at least one environment", name);
     }
     try {
       Location loc = thread.getCallerLocation();
-      context.pkgBuilder.addEnvironmentGroup(
-          name, environments, defaults, context.getBuilder().getLocalEventHandler(), loc);
+      pkgBuilder.addEnvironmentGroup(
+          name, environments, defaults, pkgBuilder.getLocalEventHandler(), loc);
       return Starlark.NONE;
     } catch (LabelSyntaxException e) {
       throw Starlark.errorf("environment group has invalid name: %s: %s", name, e.getMessage());
@@ -120,18 +117,17 @@ public NoneType licenses(
       StarlarkThread thread)
       throws EvalException {
     BazelStarlarkContext.checkLoadingPhase(thread, "licenses");
-    PackageContext context = PackageFactory.getContext(thread);
+    Package.Builder pkgBuilder = PackageFactory.getContext(thread);
     try {
       License license = BuildType.LICENSE.convert(licensesList, "'licenses' operand");
-      context.pkgBuilder.mergePackageArgsFrom(PackageArgs.builder().setLicense(license));
+      pkgBuilder.mergePackageArgsFrom(PackageArgs.builder().setLicense(license));
     } catch (ConversionException e) {
-      context
-          .getBuilder()
+      pkgBuilder
           .getLocalEventHandler()
           .handle(
               Package.error(
                   thread.getCallerLocation(), e.getMessage(), Code.LICENSE_PARSE_FAILURE));
-      context.pkgBuilder.setContainsErrors();
+      pkgBuilder.setContainsErrors();
     }
     return Starlark.NONE;
   }
@@ -146,20 +142,19 @@ public NoneType licenses(
       useStarlarkThread = true)
   public NoneType distribs(Object object, StarlarkThread thread) throws EvalException {
     BazelStarlarkContext.checkLoadingPhase(thread, "distribs");
-    PackageContext context = PackageFactory.getContext(thread);
+    Package.Builder pkgBuilder = PackageFactory.getContext(thread);
 
     try {
       Set<DistributionType> distribs =
           BuildType.DISTRIBUTIONS.convert(object, "'distribs' operand");
-      context.pkgBuilder.mergePackageArgsFrom(PackageArgs.builder().setDistribs(distribs));
+      pkgBuilder.mergePackageArgsFrom(PackageArgs.builder().setDistribs(distribs));
     } catch (ConversionException e) {
-      context
-          .getBuilder()
+      pkgBuilder
           .getLocalEventHandler()
           .handle(
               Package.error(
                   thread.getCallerLocation(), e.getMessage(), Code.DISTRIBUTIONS_PARSE_FAILURE));
-      context.pkgBuilder.setContainsErrors();
+      pkgBuilder.setContainsErrors();
     }
     return Starlark.NONE;
   }

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

@@ -39,7 +39,7 @@ protected PackageCallable() {}
       useStarlarkThread = true)
   public Object packageCallable(Map<String, Object> kwargs, StarlarkThread thread)
       throws EvalException {
-    Package.Builder pkgBuilder = PackageFactory.getContext(thread).pkgBuilder;
+    Package.Builder pkgBuilder = PackageFactory.getContext(thread);
     if (pkgBuilder.isPackageFunctionUsed()) {
       throw new EvalException("'package' can only be used once per BUILD file");
     }

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

@@ -208,12 +208,16 @@ public RuleClassProvider getRuleClassProvider() {
     return ruleClassProvider;
   }
 
-  /** Get the PackageContext by looking up in the environment. */
-  // TODO(#19922): Eliminate PackageContext and retrieve the Package.Builder directly instead.
-  public static PackageContext getContext(StarlarkThread thread) throws EvalException {
-    PackageContext value = thread.getThreadLocal(PackageContext.class);
+  /**
+   * Retrieves the {@link Package.Builder} from the given {@link StarlarkThread}, or throws {@link
+   * EvalException} if unavailable.
+   */
+  // TODO(#19922): The name is a holdover from when we had PackageContext. Migrate this to a static
+  // fromOrFail method on Package.Builder or a new parent interface of it.
+  public static Package.Builder getContext(StarlarkThread thread) throws EvalException {
+    Package.Builder value = thread.getThreadLocal(Package.Builder.class);
     if (value == null) {
-      // if PackageContext is missing, we're not called from a BUILD file. This happens if someone
+      // if PackageBuilder is missing, we're not called from a BUILD file. This happens if someone
       // uses native.some_func() in the wrong place.
       throw Starlark.errorf(
           "The native module can be accessed only from a BUILD thread. "
@@ -282,31 +286,6 @@ public NonSkyframeGlobber createNonSkyframeGlobber(
             threadStateReceiverForMetrics));
   }
 
-  /**
-   * This class holds state associated with the construction of a single package for the duration of
-   * execution of one BUILD file. (We use a PackageContext object in preference to storing these
-   * values in mutable fields of the PackageFactory.)
-   *
-   * <p>PLEASE NOTE: the PackageContext is referred to by the StarlarkThread, but should become
-   * unreachable once the StarlarkThread is discarded at the end of evaluation. Please be aware of
-   * your memory footprint when making changes here!
-   */
-  // TODO(#19922): Merge this into Package.Builder, store the builder directly using
-  // StarlarkThread.setThreadLocal.
-  public static class PackageContext {
-    final Package.Builder pkgBuilder;
-
-    @VisibleForTesting
-    public PackageContext(Package.Builder pkgBuilder) {
-      this.pkgBuilder = pkgBuilder;
-    }
-
-    /** Returns the builder of this Package. */
-    public Package.Builder getBuilder() {
-      return pkgBuilder;
-    }
-  }
-
   /**
    * Runs final validation and administrative tasks on newly loaded package. Called by a caller of
    * {@link #executeBuildFile} after this caller has fully loaded the package.
@@ -423,7 +402,6 @@ private void executeBuildFileImpl(
     pkgBuilder.setLoads(loadedModules.values());
 
     StoredEventHandler eventHandler = new StoredEventHandler();
-    PackageContext pkgContext = new PackageContext(pkgBuilder);
     pkgBuilder.setLocalEventHandler(eventHandler);
 
     try (Mutability mu = Mutability.create("package", pkgBuilder.getFilename())) {
@@ -437,16 +415,14 @@ private void executeBuildFileImpl(
               new SymbolGenerator<>(pkgBuilder.getPackageIdentifier()))
           .storeInThread(thread);
 
-      // TODO(adonovan): save this as a field in BazelStarlarkContext.
-      // It needn't be a second thread-local.
-      thread.setThreadLocal(PackageContext.class, pkgContext);
+      // TODO(#19922): Have Package.Builder inherit from BazelStarlarkContext and only store one
+      // thread-local object.
+      thread.setThreadLocal(Package.Builder.class, pkgBuilder);
 
       // TODO(b/291752414): The rule definition environment shouldn't be needed at BUILD evaluation
       // time EXCEPT for analysis_test, which needs the tools repository for use in
       // StarlarkRuleClassFunctions#createRule. So we set it here as a thread-local to be retrieved
       // by StarlarkTestingModule#analysisTest.
-      // TODO(b/236456122): Though instead of being a separate thread-local, we should stick it and
-      // PackageContext on a new PackageThreadContext object.
       thread.setThreadLocal(RuleDefinitionEnvironment.class, ruleClassProvider);
 
       try {
@@ -456,7 +432,7 @@ private void executeBuildFileImpl(
             Package.error(null, ex.getMessageWithStack(), Code.STARLARK_EVAL_ERROR));
         pkgBuilder.setContainsErrors();
       } catch (InterruptedException ex) {
-        if (pkgContext.pkgBuilder.containsErrors()) {
+        if (pkgBuilder.containsErrors()) {
           // Suppress the interrupted exception: we have an error of our own to return.
           Thread.currentThread().interrupt();
           logger.atInfo().withCause(ex).log(

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

@@ -22,7 +22,6 @@
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.Attribute.StarlarkComputedDefaultTemplate.CannotPrecomputeDefaultsException;
 import com.google.devtools.build.lib.packages.Package.NameConflictException;
-import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
@@ -292,16 +291,16 @@ public NoneType call(StarlarkThread thread, Tuple args, Dict<String, Object> kwa
       }
       BazelStarlarkContext.checkLoadingOrWorkspacePhase(thread, ruleClass.getName());
       try {
-        PackageContext context = PackageFactory.getContext(thread);
+        Package.Builder pkgBuilder = PackageFactory.getContext(thread);
         RuleFactory.createAndAddRule(
-            context.pkgBuilder,
+            pkgBuilder,
             ruleClass,
             new BuildLangTypedAttributeValuesMap(kwargs),
             thread
                 .getSemantics()
                 .getBool(BuildLanguageOptions.INCOMPATIBLE_FAIL_ON_UNKNOWN_ATTRIBUTES),
             // TODO(#19922): createAndAddRule() should get this from the builder arg directly.
-            context.getBuilder().getLocalEventHandler(),
+            pkgBuilder.getLocalEventHandler(),
             thread.getCallStack());
       } catch (RuleFactory.InvalidRuleException | Package.NameConflictException e) {
         throw new EvalException(e);