diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java index 25187978c69e6f..8f8e5d64347368 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java +++ b/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 ruleClassMap = new HashMap<>(); private final Map ruleDefinitionMap = new HashMap<>(); private final Map nativeAspectClassMap = new HashMap<>(); - private final List> packageArgs = new ArrayList<>(); private final Map, RuleClass> ruleMap = new HashMap<>(); private final Digraph> dependencyGraph = new Digraph<>(); private final List> 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 ruleClassMap, ImmutableMap ruleDefinitionMap, ImmutableMap nativeAspectClassMap, - ImmutableList> 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, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/rules/BUILD index 158b0257f83326..ddb8968dcbb2fd 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BUILD +++ b/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", 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 b1b3ee498774be..0b640057aaf120 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 @@ -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 symbols = ImmutableMap.builder(); + Starlark.addMethods(symbols, PackageCallable.INSTANCE); + for (Map.Entry entry : symbols.buildOrThrow().entrySet()) { + builder.addBuildFileToplevel(entry.getKey(), entry.getValue()); + } } }; diff --git a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java b/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java index f17325db878536..b8b5f08c4690b1 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java +++ b/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 ruleFunctions, - Object packageCallable, ImmutableMap buildFileToplevels, ImmutableMap bzlToplevels, ImmutableMap 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 getBzlmodBzlEnv() { * injection didn't happen. */ private static ImmutableMap createUninjectedBuildBzlNativeBindings( - Map ruleFunctions, - Object packageCallable, - Map buildFileToplevels) { + Map ruleFunctions, Map buildFileToplevels) { ImmutableMap.Builder 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 createUninjectedBuildBzlEnv( } private static ImmutableMap createUninjectedBuildEnv( - Map ruleFunctions, - Object packageCallable, - Map buildFileToplevels) { + Map ruleFunctions, Map buildFileToplevels) { ImmutableMap.Builder 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(); diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageArgument.java b/src/main/java/com/google/devtools/build/lib/packages/PackageArgument.java deleted file mode 100644 index a23150a64c411c..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageArgument.java +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2020 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.packages; - -import net.starlark.java.eval.EvalException; -import net.starlark.java.syntax.Location; - -/** Defines an argument to the {@code package()} function. */ -public abstract class PackageArgument { - private final String name; - private final Type type; - - protected PackageArgument(String name, Type type) { - this.name = name; - this.type = type; - } - - String getName() { - return name; - } - - /** - * Converts an untyped argument to a typed one, then calls the user-provided {@link #process}. - * - * Note that the location is used not just for exceptions (for which null would do), but also for - * reporting events. - */ - final void convertAndProcess( - Package.Builder pkgBuilder, Location location, Object value) - throws EvalException { - T typedValue = type.convert(value, "'package' argument", pkgBuilder.getLabelConverter()); - process(pkgBuilder, location, typedValue); - } - - /** - * Processes an argument. - * - * @param pkgBuilder the package builder to be mutated - * @param location the location of the {@code package} function for error reporting - * @param value the value of the argument. Typically passed to {@link Type#convert} - */ - protected abstract void process( - Package.Builder pkgBuilder, Location location, T value) - throws EvalException; -} diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageCallable.java b/src/main/java/com/google/devtools/build/lib/packages/PackageCallable.java index 63909262a4ada0..3f8db53ca1fdd8 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageCallable.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageCallable.java @@ -14,276 +14,124 @@ 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.Label; import com.google.devtools.build.lib.packages.License.DistributionType; import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code; import java.util.List; import java.util.Map; import java.util.Set; -import net.starlark.java.eval.Dict; +import net.starlark.java.annot.Param; +import net.starlark.java.annot.StarlarkMethod; import net.starlark.java.eval.EvalException; -import net.starlark.java.eval.Printer; import net.starlark.java.eval.Starlark; -import net.starlark.java.eval.StarlarkCallable; import net.starlark.java.eval.StarlarkThread; -import net.starlark.java.eval.Tuple; import net.starlark.java.syntax.Location; /** - * Utility class encapsulating the definition of the {@code package()} function of BUILD files. - * - *

Also includes the definitions of those arguments to {@code package()} that are available in - * all Bazel environments. + * Utility class encapsulating the standard definition of the {@code package()} function of BUILD + * files. */ public class PackageCallable { - private PackageCallable() {} + protected PackageCallable() {} - /** - * Returns a {@link StarlarkCallable} that implements the {@code package()} functionality. - * - * @param packageArgs a list of {@link PackageArgument}s to support, beyond the standard ones - * included in every Bazel environment - */ - // TODO(b/280446865): Consider eliminating the package() extensibility mechanism altogether. - // There is currently only one use case: distinguishing the set of package() arguments available - // in OSS Bazel vs internally to Google. Instead of registering these arguments and passing them - // to this factory method to obtain a package() callable, we could instead define two - // @StarlarkMethod-annotated Java functions implementing the two versions of package(), and - // register the appropriate one with the ConfiguredRuleClassProvider.Builder. - public static StarlarkCallable newPackageCallable(List> packageArgs) { - // Construct a map of PackageArguments, which the returned callable closes over. - ImmutableMap.Builder> argsBuilder = ImmutableMap.builder(); - for (PackageArgument arg : getCommonArguments()) { - argsBuilder.put(arg.getName(), arg); - } - for (PackageArgument arg : packageArgs) { - argsBuilder.put(arg.getName(), arg); - } - final ImmutableMap> packageArguments = argsBuilder.buildOrThrow(); - - return new StarlarkCallable() { - @Override - public String getName() { - return "package"; - } - - @Override - public String toString() { - return "package(...)"; - } - - @Override - public boolean isImmutable() { - return true; - } + public static final PackageCallable INSTANCE = new PackageCallable(); - @Override - public void repr(Printer printer) { - printer.append(""); - } - - @Override - public Object call(StarlarkThread thread, Tuple args, Dict kwargs) - throws EvalException { - if (!args.isEmpty()) { - throw new EvalException("unexpected positional arguments"); - } - Package.Builder pkgBuilder = PackageFactory.getContext(thread).pkgBuilder; - - // Validate parameter list - if (pkgBuilder.isPackageFunctionUsed()) { - throw new EvalException("'package' can only be used once per BUILD file"); - } - pkgBuilder.setPackageFunctionUsed(); - - // Each supplied argument must name a PackageArgument. - if (kwargs.isEmpty()) { - throw new EvalException("at least one argument must be given to the 'package' function"); - } - Location loc = thread.getCallerLocation(); - for (Map.Entry kwarg : kwargs.entrySet()) { - String name = kwarg.getKey(); - PackageArgument pkgarg = packageArguments.get(name); - if (pkgarg == null) { - throw Starlark.errorf("unexpected keyword argument: %s", name); - } - pkgarg.convertAndProcess(pkgBuilder, loc, kwarg.getValue()); - } - return Starlark.NONE; - } - }; - } - - /** Returns the basic set of {@link PackageArgument}s. */ - private static ImmutableList> getCommonArguments() { - return ImmutableList.of( - new DefaultDeprecation(), - new DefaultDistribs(), - new DefaultApplicableLicenses(), - new DefaultPackageMetadata(), - new DefaultLicenses(), - new DefaultTestOnly(), - new DefaultVisibility(), - new Features(), - new DefaultCompatibleWith(), - new DefaultRestrictedTo()); - } + @StarlarkMethod( + name = "package", + documented = false, // documented in docgen/templates/be/functions.vm + extraKeywords = @Param(name = "kwargs", defaultValue = "{}"), + useStarlarkThread = true) + public Object packageCallable(Map kwargs, StarlarkThread thread) + throws EvalException { + Package.Builder pkgBuilder = PackageFactory.getContext(thread).pkgBuilder; + if (pkgBuilder.isPackageFunctionUsed()) { + throw new EvalException("'package' can only be used once per BUILD file"); + } + pkgBuilder.setPackageFunctionUsed(); - private static class DefaultVisibility extends PackageArgument> { - private DefaultVisibility() { - super("default_visibility", BuildType.LABEL_LIST); + if (kwargs.isEmpty()) { + throw new EvalException("at least one argument must be given to the 'package' function"); } - @Override - protected void process(Package.Builder pkgBuilder, Location location, List