Eliminate PackageArgument

The package() function is now implemented by a @StarlarkMethod rather than an anonymous subclass of StarlarkCallable.

The PackageArgument machinery is deleted. The convert-and-process approach is instead inlined into a series of if-elses for each argument (with the convert step using a helper to reduce repetition). The big if-else is factored out into an overridable method, so other implementations of package() can add their own parameters on top.

The package() symbol is now registered on the ConfiguredRuleClassProvider like any other BUILD toplevel. Removed its special casing in BazelStarlarkEnvironment, and the ability to register PackageArguments with the CRCP.

Fixed a copy-paste error where the "default_applicable_licenses" param would claim to be "default_package_metadata" for the purposes of error messages.

PiperOrigin-RevId: 532911239
Change-Id: I957354e0a4e32f7922b510c5537f56c6aff72ab9

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java

@@ -49,8 +49,6 @@
 import com.google.devtools.build.lib.graph.Node;
 import com.google.devtools.build.lib.packages.BazelStarlarkEnvironment;
 import com.google.devtools.build.lib.packages.NativeAspectClass;
-import com.google.devtools.build.lib.packages.PackageArgument;
-import com.google.devtools.build.lib.packages.PackageCallable;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.packages.RuleFactory;
@@ -157,7 +155,6 @@ public static final class Builder implements RuleDefinitionEnvironment {
     private final Map<String, RuleClass> ruleClassMap = new HashMap<>();
     private final Map<String, RuleDefinition> ruleDefinitionMap = new HashMap<>();
     private final Map<String, NativeAspectClass> nativeAspectClassMap = new HashMap<>();
-    private final List<PackageArgument<?>> packageArgs = new ArrayList<>();
     private final Map<Class<? extends RuleDefinition>, RuleClass> ruleMap = new HashMap<>();
     private final Digraph<Class<? extends RuleDefinition>> dependencyGraph = new Digraph<>();
     private final List<Class<? extends Fragment>> universalFragments = new ArrayList<>();
@@ -306,12 +303,6 @@ public Builder addNativeAspectClass(NativeAspectClass aspectFactoryClass) {
       return this;
     }
 
-    @CanIgnoreReturnValue
-    public Builder addPackageArg(PackageArgument<?> arg) {
-      packageArgs.add(arg);
-      return this;
-    }
-
     /**
      * Adds a configuration fragment and all build options required by its fragment.
      *
@@ -608,7 +599,6 @@ public ConfiguredRuleClassProvider build() {
           ImmutableMap.copyOf(ruleClassMap),
           ImmutableMap.copyOf(ruleDefinitionMap),
           ImmutableMap.copyOf(nativeAspectClassMap),
-          ImmutableList.copyOf(packageArgs),
           FragmentRegistry.create(
               configurationFragmentClasses, universalFragments, configurationOptions),
           defaultWorkspaceFilePrefix.toString(),
@@ -728,7 +718,6 @@ private ConfiguredRuleClassProvider(
       ImmutableMap<String, RuleClass> ruleClassMap,
       ImmutableMap<String, RuleDefinition> ruleDefinitionMap,
       ImmutableMap<String, NativeAspectClass> nativeAspectClassMap,
-      ImmutableList<PackageArgument<?>> packageArgs,
       FragmentRegistry fragmentRegistry,
       String defaultWorkspaceFilePrefix,
       String defaultWorkspaceFileSuffix,
@@ -780,10 +769,6 @@ private ConfiguredRuleClassProvider(
     this.bazelStarlarkEnvironment =
         new BazelStarlarkEnvironment(
             /* ruleFunctions= */ RuleFactory.buildRuleFunctions(ruleClassMap),
-            // TODO(b/280446865): Instead of exposing the {@code package()} symbol separately, just
-            // keep it part of the BUILD/native environments. Requires migrating {@code package()}
-            // to be a @StarlarkMethod that gets registered as a BUILD toplevel.
-            /* packageCallable= */ PackageCallable.newPackageCallable(packageArgs),
             buildFileToplevels,
             /* bzlToplevels= */ environment,
             /* nativeRuleSpecificBindings= */ nativeRuleSpecificBindings,

src/main/java/com/google/devtools/build/lib/bazel/rules/BUILD

@@ -64,6 +64,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/exec:module_action_context_registry",
         "//src/main/java/com/google/devtools/build/lib/exec:spawn_cache",
         "//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_registry",
+        "//src/main/java/com/google/devtools/build/lib/packages",
         "//src/main/java/com/google/devtools/build/lib/packages/semantics",
         "//src/main/java/com/google/devtools/build/lib/remote",
         "//src/main/java/com/google/devtools/build/lib/remote/options",

src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java

@@ -59,6 +59,7 @@
 import com.google.devtools.build.lib.bazel.rules.python.BazelPythonConfiguration;
 import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.packages.PackageCallable;
 import com.google.devtools.build.lib.rules.android.AarImportBaseRule;
 import com.google.devtools.build.lib.rules.android.AndroidApplicationResourceInfo;
 import com.google.devtools.build.lib.rules.android.AndroidAssetsInfo;
@@ -308,6 +309,15 @@ public void init(ConfiguredRuleClassProvider.Builder builder) {
           builder.addStarlarkBuiltinsInternal(CcStarlarkInternal.NAME, new CcStarlarkInternal());
           builder.addStarlarkBuiltinsInternal(
               BazelObjcStarlarkInternal.NAME, new BazelObjcStarlarkInternal());
+
+          // Add the package() function.
+          // TODO(bazel-team): Factor this into a group of similar BUILD definitions, or add a more
+          // convenient way of obtaining a BuiltinFunction than addMethods().
+          ImmutableMap.Builder<String, Object> symbols = ImmutableMap.builder();
+          Starlark.addMethods(symbols, PackageCallable.INSTANCE);
+          for (Map.Entry<String, Object> entry : symbols.buildOrThrow().entrySet()) {
+            builder.addBuildFileToplevel(entry.getKey(), entry.getValue());
+          }
         }
       };
 

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

@@ -81,7 +81,6 @@ public final class BazelStarlarkEnvironment {
    *
    * @param ruleFunctions a map from a rule class name (e.g. "java_library") to the (uninjected)
    *     Starlark callable that instantiates it
-   * @param packageCallable the symbol implementing the {@code package()} function in BUILD files
    * @param buildFileToplevels the map of non-{@link Starlark#UNIVERSE} top-level symbols available
    *     to BUILD files, prior to builtins injection
    * @param bzlToplevels the map of non-universe top-level symbols available to .bzl files, prior to
@@ -97,7 +96,6 @@ public final class BazelStarlarkEnvironment {
    */
   public BazelStarlarkEnvironment(
       ImmutableMap<String, ?> ruleFunctions,
-      Object packageCallable,
       ImmutableMap<String, Object> buildFileToplevels,
       ImmutableMap<String, Object> bzlToplevels,
       ImmutableMap<String, Object> nativeRuleSpecificBindings,
@@ -110,7 +108,7 @@ public BazelStarlarkEnvironment(
     this.workspaceBzlNativeBindings = workspaceBzlNativeBindings;
 
     this.uninjectedBuildBzlNativeBindings =
-        createUninjectedBuildBzlNativeBindings(ruleFunctions, packageCallable, buildFileToplevels);
+        createUninjectedBuildBzlNativeBindings(ruleFunctions, buildFileToplevels);
     this.uninjectedBuildBzlEnv =
         createUninjectedBuildBzlEnv(bzlToplevels, uninjectedBuildBzlNativeBindings);
     this.uninjectedWorkspaceBzlEnv =
@@ -126,8 +124,7 @@ public BazelStarlarkEnvironment(
             builtinsInternals,
             uninjectedBuildBzlNativeBindings,
             uninjectedBuildBzlEnv);
-    this.uninjectedBuildEnv =
-        createUninjectedBuildEnv(ruleFunctions, packageCallable, buildFileToplevels);
+    this.uninjectedBuildEnv = createUninjectedBuildEnv(ruleFunctions, buildFileToplevels);
   }
 
   /**
@@ -186,13 +183,10 @@ public ImmutableMap<String, Object> getBzlmodBzlEnv() {
    * injection didn't happen.
    */
   private static ImmutableMap<String, Object> createUninjectedBuildBzlNativeBindings(
-      Map<String, ?> ruleFunctions,
-      Object packageCallable,
-      Map<String, Object> buildFileToplevels) {
+      Map<String, ?> ruleFunctions, Map<String, Object> buildFileToplevels) {
     ImmutableMap.Builder<String, Object> env = new ImmutableMap.Builder<>();
     env.putAll(StarlarkNativeModule.BINDINGS_FOR_BUILD_FILES);
     env.putAll(ruleFunctions);
-    env.put("package", packageCallable);
     env.putAll(buildFileToplevels);
     return env.buildOrThrow();
   }
@@ -217,13 +211,10 @@ private static ImmutableMap<String, Object> createUninjectedBuildBzlEnv(
   }
 
   private static ImmutableMap<String, Object> createUninjectedBuildEnv(
-      Map<String, ?> ruleFunctions,
-      Object packageCallable,
-      Map<String, Object> buildFileToplevels) {
+      Map<String, ?> ruleFunctions, Map<String, Object> buildFileToplevels) {
     ImmutableMap.Builder<String, Object> env = ImmutableMap.builder();
     env.putAll(StarlarkLibrary.BUILD); // e.g. select, depset
     env.putAll(StarlarkNativeModule.BINDINGS_FOR_BUILD_FILES);
-    env.put("package", packageCallable);
     env.putAll(ruleFunctions);
     env.putAll(buildFileToplevels);
     return env.buildOrThrow();