Add a PackageArgs class in preparation for REPO.bazel
- Created a PackageArgs class that stores the values passed to the attrs of the `package()` function, so that they can be applied to a package builder in one go
- These attr values used to be applied as soon as each attr is processed (i.e. evaluated). We can't do that anymore for repo(), since at the time of REPO.bazel evaluation, the package hasn't started being built yet.
- Instead of storing the individual fields, Package now just holds on to a PackageArgs object.
- In a follow-up CL, we store all the repo() values in a PackageArgs class, and apply those before the package() ones.
- Also removed `Package#isDefaultVisibilitySet` as nobody calls it anymore.
Work towards https://github.com/bazelbuild/bazel/issues/18077
PiperOrigin-RevId: 539728608
Change-Id: Ie4a081fa0e52e833ac2a6be50033f95728da3a9b
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/BuildGlobals.java
index 72097be..03c87ef 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BuildGlobals.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BuildGlobals.java
@@ -121,7 +121,7 @@
PackageContext context = PackageFactory.getContext(thread);
try {
License license = BuildType.LICENSE.convert(licensesList, "'licenses' operand");
- context.pkgBuilder.setDefaultLicense(license);
+ context.pkgBuilder.mergePackageArgsFrom(PackageArgs.builder().setLicense(license));
} catch (ConversionException e) {
context.eventHandler.handle(
Package.error(thread.getCallerLocation(), e.getMessage(), Code.LICENSE_PARSE_FAILURE));
@@ -144,7 +144,7 @@
try {
Set<DistributionType> distribs =
BuildType.DISTRIBUTIONS.convert(object, "'distribs' operand");
- context.pkgBuilder.setDefaultDistribs(distribs);
+ context.pkgBuilder.mergePackageArgsFrom(PackageArgs.builder().setDistribs(distribs));
} catch (ConversionException e) {
context.eventHandler.handle(
Package.error(
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index da4a123..a46dafd 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -44,7 +44,6 @@
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.EventKind;
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
-import com.google.devtools.build.lib.packages.License.DistributionType;
import com.google.devtools.build.lib.packages.Package.Builder.PackageSettings;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
@@ -66,7 +65,6 @@
import java.io.PrintStream;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
@@ -171,11 +169,7 @@
/** The collection of all targets defined in this package, indexed by name. */
private ImmutableSortedMap<String, Target> targets;
- /**
- * Default visibility for rules that do not specify it.
- */
- private RuleVisibility defaultVisibility;
- private boolean defaultVisibilitySet;
+ private PackageArgs packageArgs = PackageArgs.DEFAULT;
/**
* How to enforce config_setting visibility settings.
@@ -196,16 +190,6 @@
private ConfigSettingVisibilityPolicy configSettingVisibilityPolicy;
/**
- * Default package-level 'testonly' value for rules that do not specify it.
- */
- private boolean defaultTestOnly = false;
-
- /**
- * Default package-level 'deprecation' value for rules that do not specify it.
- */
- private String defaultDeprecation;
-
- /**
* Default header strictness checking for rules that do not specify it.
*/
private String defaultHdrsCheck;
@@ -232,18 +216,6 @@
*/
@Nullable private FailureDetail failureDetail;
- /** The package's default "package_metadata" attribute. */
- private ImmutableList<Label> defaultPackageMetadata = ImmutableList.of();
-
- /**
- * The package's default "licenses" and "distribs" attributes, as specified
- * in calls to licenses() and distribs() in the BUILD file.
- */
- // These sets contain the values specified by the most recent licenses() or
- // distribs() declarations encountered during package parsing:
- private License defaultLicense;
- private Set<License.DistributionType> defaultDistributionSet;
-
/**
* The map from each repository to that repository's remappings map. This is only used in the
* //external package, it is an empty map for all other packages. For example, an entry of {"@foo"
@@ -265,11 +237,6 @@
*/
private RepositoryMapping mainRepositoryMapping;
- private Set<Label> defaultCompatibleWith = ImmutableSet.of();
- private Set<Label> defaultRestrictedTo = ImmutableSet.of();
-
- private FeatureSet features;
-
private ImmutableList<TargetPattern> registeredExecutionPlatforms;
private ImmutableList<TargetPattern> registeredToolchains;
@@ -370,36 +337,6 @@
}
/**
- * Set the default 'testonly' value for this package.
- */
- private void setDefaultTestOnly(boolean testOnly) {
- defaultTestOnly = testOnly;
- }
-
- /**
- * Set the default 'deprecation' value for this package.
- */
- private void setDefaultDeprecation(String deprecation) {
- defaultDeprecation = deprecation;
- }
-
- /**
- * Sets the default value to use for a rule's {@link RuleClass#COMPATIBLE_ENVIRONMENT_ATTR}
- * attribute when not explicitly specified by the rule.
- */
- private void setDefaultCompatibleWith(Set<Label> environments) {
- defaultCompatibleWith = environments;
- }
-
- /**
- * Sets the default value to use for a rule's {@link RuleClass#RESTRICTED_ENVIRONMENT_ATTR}
- * attribute when not explicitly specified by the rule.
- */
- private void setDefaultRestrictedTo(Set<Label> environments) {
- defaultRestrictedTo = environments;
- }
-
- /**
* Returns the source root (a directory) beneath which this package's BUILD file was found, or
* {@link Optional#empty} if this package was derived from a workspace file.
*
@@ -471,15 +408,10 @@
this.makeEnv = ImmutableMap.copyOf(builder.makeEnv);
this.targets = ImmutableSortedMap.copyOf(builder.targets);
- this.defaultVisibility = builder.defaultVisibility;
- this.defaultVisibilitySet = builder.defaultVisibilitySet;
+ this.packageArgs = builder.packageArgs;
this.configSettingVisibilityPolicy = builder.configSettingVisibilityPolicy;
this.buildFile = builder.buildFile;
this.failureDetail = builder.getFailureDetail();
- this.defaultLicense = builder.defaultLicense;
- this.defaultDistributionSet = builder.defaultDistributionSet;
- this.defaultPackageMetadata = builder.defaultPackageMetadata;
- this.features = builder.features;
this.registeredExecutionPlatforms = ImmutableList.copyOf(builder.registeredExecutionPlatforms);
this.registeredToolchains = ImmutableList.copyOf(builder.registeredToolchains);
this.repositoryMapping = Preconditions.checkNotNull(builder.repositoryMapping);
@@ -744,7 +676,7 @@
/** Returns the features specified in the <code>package()</code> declaration. */
public FeatureSet getFeatures() {
- return features;
+ return packageArgs.features();
}
/**
@@ -826,7 +758,7 @@
* Returns the default visibility for this package.
*/
public RuleVisibility getDefaultVisibility() {
- return defaultVisibility;
+ return packageArgs.defaultVisibility();
}
/**
@@ -841,14 +773,14 @@
* Returns the default testonly value.
*/
public Boolean getDefaultTestOnly() {
- return defaultTestOnly;
+ return packageArgs.defaultTestOnly();
}
/**
* Returns the default deprecation value.
*/
public String getDefaultDeprecation() {
- return defaultDeprecation;
+ return packageArgs.defaultDeprecation();
}
/** Gets the default header checking mode. */
@@ -864,23 +796,19 @@
return defaultHdrsCheck != null;
}
- public boolean isDefaultVisibilitySet() {
- return defaultVisibilitySet;
- }
-
/** Gets the package metadata list for the default metadata declared by this package. */
ImmutableList<Label> getDefaultPackageMetadata() {
- return defaultPackageMetadata;
+ return packageArgs.defaultPackageMetadata();
}
/** Gets the parsed license object for the default license declared by this package. */
License getDefaultLicense() {
- return defaultLicense;
+ return packageArgs.license();
}
/** Returns the parsed set of distributions declared as the default for this package. */
Set<License.DistributionType> getDefaultDistribs() {
- return defaultDistributionSet;
+ return packageArgs.distribs();
}
/**
@@ -888,7 +816,7 @@
* attribute when not explicitly specified by the rule.
*/
public Set<Label> getDefaultCompatibleWith() {
- return defaultCompatibleWith;
+ return packageArgs.defaultCompatibleWith();
}
/**
@@ -896,7 +824,7 @@
* attribute when not explicitly specified by the rule.
*/
public Set<Label> getDefaultRestrictedTo() {
- return defaultRestrictedTo;
+ return packageArgs.defaultRestrictedTo();
}
public ImmutableList<TargetPattern> getRegisteredExecutionPlatforms() {
@@ -1072,10 +1000,8 @@
// TreeMap so that the iteration order of variables is predictable. This is useful so that the
// serialized representation is deterministic.
private final TreeMap<String, String> makeEnv = new TreeMap<>();
- private RuleVisibility defaultVisibility = RuleVisibility.PRIVATE;
+ private PackageArgs packageArgs = PackageArgs.DEFAULT;
private ConfigSettingVisibilityPolicy configSettingVisibilityPolicy;
- private boolean defaultVisibilitySet;
- private FeatureSet features = FeatureSet.EMPTY;
private final List<Event> events = Lists.newArrayList();
private final List<Postable> posts = Lists.newArrayList();
@Nullable private String ioExceptionMessage = null;
@@ -1094,10 +1020,6 @@
// serialize events emitted during its construction/evaluation.
@Nullable private FailureDetail failureDetailOverride = null;
- private ImmutableList<Label> defaultPackageMetadata = ImmutableList.of();
- private License defaultLicense = License.NO_LICENSE;
- private Set<License.DistributionType> defaultDistributionSet = License.DEFAULT_DISTRIB;
-
// All targets added to the package. We use SnapshottableBiMap to help track insertion order of
// Rule targets, for use by native.existing_rules().
private BiMap<String, Target> targets =
@@ -1201,7 +1123,7 @@
this.mainRepositoryMapping = mainRepositoryMapping;
this.labelConverter = new LabelConverter(id, repositoryMapping);
if (pkg.getName().startsWith("javatests/")) {
- setDefaultTestonly(true);
+ mergePackageArgsFrom(PackageArgs.builder().setDefaultTestOnly(true));
}
}
@@ -1347,22 +1269,19 @@
return this;
}
- /**
- * Sets the default visibility for this package. Called at most once per package from
- * PackageFactory.
- */
@CanIgnoreReturnValue
- public Builder setDefaultVisibility(RuleVisibility visibility) {
- this.defaultVisibility = visibility;
- this.defaultVisibilitySet = true;
+ public Builder mergePackageArgsFrom(PackageArgs packageArgs) {
+ this.packageArgs = this.packageArgs.mergeWith(packageArgs);
return this;
}
- /** Sets whether the default visibility is set in the BUILD file. */
@CanIgnoreReturnValue
- public Builder setDefaultVisibilitySet(boolean defaultVisibilitySet) {
- this.defaultVisibilitySet = defaultVisibilitySet;
- return this;
+ public Builder mergePackageArgsFrom(PackageArgs.Builder builder) {
+ return mergePackageArgsFrom(builder.build());
+ }
+
+ public PackageArgs getPackageArgs() {
+ return packageArgs;
}
/** Sets visibility enforcement policy for <code>config_setting</code>. */
@@ -1372,20 +1291,6 @@
return this;
}
- /** Sets the default value of 'testonly'. Rule-level 'testonly' will override this. */
- @CanIgnoreReturnValue
- Builder setDefaultTestonly(boolean defaultTestonly) {
- pkg.setDefaultTestOnly(defaultTestonly);
- return this;
- }
-
- /** Sets the default value of 'deprecation'. Rule-level 'deprecation' will append to this. */
- @CanIgnoreReturnValue
- Builder setDefaultDeprecation(String defaultDeprecation) {
- pkg.setDefaultDeprecation(defaultDeprecation);
- return this;
- }
-
/** Uses the workspace name from {@code //external} to set this package's workspace name. */
@CanIgnoreReturnValue
@VisibleForTesting
@@ -1420,12 +1325,6 @@
return this;
}
- @CanIgnoreReturnValue
- public Builder addFeatures(Iterable<String> features) {
- this.features = FeatureSet.merge(this.features, FeatureSet.parse(features));
- return this;
- }
-
Builder setIOException(IOException e, String message, DetailedExitCode detailedExitCode) {
this.ioException = e;
this.ioExceptionMessage = message;
@@ -1529,75 +1428,6 @@
}
/**
- * Sets the default value to use for a rule's {@link RuleClass#APPLICABLE_LICENSES_ATTR}
- * attribute when not explicitly specified by the rule. Records a package error if any labels
- * are duplicated.
- */
- void setDefaultPackageMetadata(List<Label> licenses, String attrName, Location location) {
- if (hasDuplicateLabels(
- licenses, "package " + pkg.getName(), attrName, location, this::addEvent)) {
- setContainsErrors();
- }
- this.defaultPackageMetadata = ImmutableList.copyOf(licenses);
- }
-
- ImmutableList<Label> getDefaultPackageMetadata() {
- return defaultPackageMetadata;
- }
-
- /**
- * Sets the default license for this package.
- */
- void setDefaultLicense(License license) {
- this.defaultLicense = license;
- }
-
- License getDefaultLicense() {
- return defaultLicense;
- }
-
- /**
- * Initializes the default set of distributions for targets in this package.
- *
- * <p> TODO(bazel-team): (2011) consider moving the license & distribs info into Metadata--maybe
- * even in the Build language.
- */
- void setDefaultDistribs(Set<DistributionType> dists) {
- this.defaultDistributionSet = dists;
- }
-
- Set<DistributionType> getDefaultDistribs() {
- return defaultDistributionSet;
- }
-
- /**
- * Sets the default value to use for a rule's {@link RuleClass#COMPATIBLE_ENVIRONMENT_ATTR}
- * attribute when not explicitly specified by the rule. Records a package error if any labels
- * are duplicated.
- */
- void setDefaultCompatibleWith(List<Label> environments, String attrName, Location location) {
- if (hasDuplicateLabels(
- environments, "package " + pkg.getName(), attrName, location, this::addEvent)) {
- setContainsErrors();
- }
- pkg.setDefaultCompatibleWith(ImmutableSet.copyOf(environments));
- }
-
- /**
- * Sets the default value to use for a rule's {@link RuleClass#RESTRICTED_ENVIRONMENT_ATTR}
- * attribute when not explicitly specified by the rule. Records a package error if
- * any labels are duplicated.
- */
- void setDefaultRestrictedTo(List<Label> environments, String attrName, Location location) {
- if (hasDuplicateLabels(
- environments, "package " + pkg.getName(), attrName, location, this::addEvent)) {
- setContainsErrors();
- }
-
- pkg.setDefaultRestrictedTo(ImmutableSet.copyOf(environments));
- }
-
- /**
* Creates a new {@link Rule} {@code r} where {@code r.getPackage()} is the {@link Package}
* associated with this {@link Builder}.
*
@@ -1989,8 +1819,6 @@
}
ruleLabels = null;
targets = Maps.unmodifiableBiMap(targets);
- defaultDistributionSet =
- Collections.unmodifiableSet(defaultDistributionSet);
// Now all targets have been loaded, so we validate the group's member environments.
for (EnvironmentGroup envGroup : ImmutableSet.copyOf(environmentGroups.values())) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageArgs.java b/src/main/java/com/google/devtools/build/lib/packages/PackageArgs.java
new file mode 100644
index 0000000..cd8b5f2
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageArgs.java
@@ -0,0 +1,226 @@
+// Copyright 2023 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 com.google.auto.value.AutoValue;
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedSet;
+import com.google.devtools.build.lib.analysis.config.FeatureSet;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.collect.CollectionUtils;
+import com.google.devtools.build.lib.packages.License.DistributionType;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import java.util.List;
+import java.util.Set;
+import javax.annotation.Nullable;
+import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.Starlark;
+
+/**
+ * A group of {@link Package} argument values that may be provided by `package()` or `repo()` calls.
+ */
+@AutoValue
+public abstract class PackageArgs {
+ public static final PackageArgs DEFAULT =
+ PackageArgs.builder()
+ .setDefaultVisibility(RuleVisibility.PRIVATE)
+ .setDefaultTestOnly(false)
+ .setFeatures(FeatureSet.EMPTY)
+ .setLicense(License.NO_LICENSE)
+ .setDistribs(License.DEFAULT_DISTRIB)
+ .setDefaultCompatibleWith(ImmutableSet.of())
+ .setDefaultRestrictedTo(ImmutableSet.of())
+ .setDefaultPackageMetadata(ImmutableList.of())
+ .build();
+
+ /** See {@link Package#getDefaultVisibility()}. */
+ @Nullable
+ abstract RuleVisibility defaultVisibility();
+
+ /** See {@link Package#getDefaultTestOnly()}. */
+ @Nullable
+ abstract Boolean defaultTestOnly();
+
+ /** See {@link Package#getDefaultDeprecation()}. */
+ @Nullable
+ abstract String defaultDeprecation();
+
+ /** See {@link Package#getFeatures()}. */
+ abstract FeatureSet features();
+
+ /** See {@link Package#getDefaultLicense()}. */
+ @Nullable
+ abstract License license();
+
+ /** See {@link Package#getDefaultDistribs()}. */
+ @Nullable
+ abstract ImmutableSet<DistributionType> distribs();
+
+ /** See {@link Package#getDefaultCompatibleWith()}. */
+ @Nullable
+ abstract ImmutableSet<Label> defaultCompatibleWith();
+
+ /** See {@link Package#getDefaultRestrictedTo()}. */
+ @Nullable
+ abstract ImmutableSet<Label> defaultRestrictedTo();
+
+ /** See {@link Package#getDefaultPackageMetadata()}. */
+ @Nullable
+ abstract ImmutableList<Label> defaultPackageMetadata();
+
+ public static Builder builder() {
+ return new AutoValue_PackageArgs.Builder().setFeatures(FeatureSet.EMPTY);
+ }
+
+ abstract Builder toBuilder();
+
+ /** Builder type for {@link PackageArgs}. */
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract Builder setDefaultVisibility(RuleVisibility x);
+
+ public abstract Builder setDefaultTestOnly(Boolean x);
+
+ public abstract Builder setDefaultDeprecation(String x);
+
+ abstract FeatureSet features();
+
+ public abstract Builder setFeatures(FeatureSet x);
+
+ @CanIgnoreReturnValue
+ public final Builder mergeFeatures(FeatureSet x) {
+ return setFeatures(FeatureSet.merge(features(), x));
+ }
+
+ public abstract Builder setLicense(License x);
+
+ public abstract Builder setDistribs(Set<DistributionType> x);
+
+ /** Note that we don't check dupes in this method. Check beforehand! */
+ public abstract Builder setDefaultCompatibleWith(Iterable<Label> x);
+
+ /** Note that we don't check dupes in this method. Check beforehand! */
+ public abstract Builder setDefaultRestrictedTo(Iterable<Label> x);
+
+ @Nullable
+ abstract ImmutableList<Label> defaultPackageMetadata();
+
+ /** Note that we don't check dupes in this method. Check beforehand! */
+ public abstract Builder setDefaultPackageMetadata(List<Label> x);
+
+ public abstract PackageArgs build();
+ }
+
+ private static void throwIfHasDupes(List<Label> labels, String what) throws EvalException {
+ var dupes = ImmutableSortedSet.copyOf(CollectionUtils.duplicatedElementsOf(labels));
+ if (!dupes.isEmpty()) {
+ throw Starlark.errorf("duplicate label(s) in %s: %s", what, Joiner.on(", ").join(dupes));
+ }
+ }
+
+ /**
+ * Processes the given Starlark parameter to the {@code package()/repo()} call into a field on a
+ * {@link Builder} object.
+ */
+ public static void processParam(
+ String name, Object rawValue, String what, LabelConverter labelConverter, Builder builder)
+ throws EvalException {
+ if (name.equals("default_visibility")) {
+ List<Label> value = BuildType.LABEL_LIST.convert(rawValue, what, labelConverter);
+ builder.setDefaultVisibility(RuleVisibility.parse(value));
+
+ } else if (name.equals("default_testonly")) {
+ Boolean value = Type.BOOLEAN.convert(rawValue, what, labelConverter);
+ builder.setDefaultTestOnly(value);
+
+ } else if (name.equals("default_deprecation")) {
+ String value = Type.STRING.convert(rawValue, what, labelConverter);
+ builder.setDefaultDeprecation(value);
+
+ } else if (name.equals("features")) {
+ List<String> value = Type.STRING_LIST.convert(rawValue, what, labelConverter);
+ builder.mergeFeatures(FeatureSet.parse(value));
+
+ } else if (name.equals("licenses")) {
+ License value = BuildType.LICENSE.convert(rawValue, what, labelConverter);
+ builder.setLicense(value);
+
+ } else if (name.equals("distribs")) {
+ Set<DistributionType> value = BuildType.DISTRIBUTIONS.convert(rawValue, what, labelConverter);
+ builder.setDistribs(value);
+
+ } else if (name.equals("default_compatible_with")) {
+ List<Label> value = BuildType.LABEL_LIST.convert(rawValue, what, labelConverter);
+ throwIfHasDupes(value, name);
+ builder.setDefaultCompatibleWith(value);
+
+ } else if (name.equals("default_restricted_to")) {
+ List<Label> value = BuildType.LABEL_LIST.convert(rawValue, what, labelConverter);
+ throwIfHasDupes(value, name);
+ builder.setDefaultRestrictedTo(value);
+
+ } else if (name.equals("default_applicable_licenses")
+ || name.equals("default_package_metadata")) {
+ List<Label> value = BuildType.LABEL_LIST.convert(rawValue, what, labelConverter);
+ if (builder.defaultPackageMetadata() != null) {
+ throw Starlark.errorf(
+ "Can not set both default_package_metadata and default_applicable_licenses."
+ + " Move all declarations to default_package_metadata.");
+ }
+ throwIfHasDupes(value, name);
+ builder.setDefaultPackageMetadata(value);
+
+ } else {
+ throw Starlark.errorf("unexpected keyword argument: %s", name);
+ }
+ }
+
+ /**
+ * Returns a new {@link PackageArgs} containing the result of merging {@code other} into {@code
+ * this}. {@code other}'s fields take precedence if specified.
+ */
+ public PackageArgs mergeWith(PackageArgs other) {
+ Builder builder = toBuilder();
+ if (other.defaultVisibility() != null) {
+ builder.setDefaultVisibility(other.defaultVisibility());
+ }
+ if (other.defaultTestOnly() != null) {
+ builder.setDefaultTestOnly(other.defaultTestOnly());
+ }
+ if (other.defaultDeprecation() != null) {
+ builder.setDefaultDeprecation(other.defaultDeprecation());
+ }
+ if (!other.features().equals(FeatureSet.EMPTY)) {
+ builder.mergeFeatures(other.features());
+ }
+ if (other.license() != null) {
+ builder.setLicense(other.license());
+ }
+ if (other.distribs() != null) {
+ builder.setDistribs(other.distribs());
+ }
+ if (other.defaultCompatibleWith() != null) {
+ builder.setDefaultCompatibleWith(other.defaultCompatibleWith());
+ }
+ if (other.defaultRestrictedTo() != null) {
+ builder.setDefaultRestrictedTo(other.defaultRestrictedTo());
+ }
+ if (other.defaultPackageMetadata() != null) {
+ builder.setDefaultPackageMetadata(other.defaultPackageMetadata());
+ }
+ return builder.build();
+ }
+}
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 3f8db53..3fae8da 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,18 +14,12 @@
package com.google.devtools.build.lib.packages;
-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.annot.Param;
import net.starlark.java.annot.StarlarkMethod;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkThread;
-import net.starlark.java.syntax.Location;
/**
* Utility class encapsulating the standard definition of the {@code package()} function of BUILD
@@ -54,12 +48,13 @@
throw new EvalException("at least one argument must be given to the 'package' function");
}
- Location loc = thread.getCallerLocation();
+ PackageArgs.Builder pkgArgsBuilder = PackageArgs.builder();
for (Map.Entry<String, Object> kwarg : kwargs.entrySet()) {
String name = kwarg.getKey();
Object rawValue = kwarg.getValue();
- processParam(name, rawValue, pkgBuilder, loc);
+ processParam(name, rawValue, pkgBuilder, pkgArgsBuilder);
}
+ pkgBuilder.mergePackageArgsFrom(pkgArgsBuilder);
return Starlark.NONE;
}
@@ -68,70 +63,13 @@
* back on the super method when the parameter does not match.
*/
protected void processParam(
- String name, Object rawValue, Package.Builder pkgBuilder, Location loc) throws EvalException {
- if (name.equals("default_visibility")) {
- List<Label> value = convert(BuildType.LABEL_LIST, rawValue, pkgBuilder);
- pkgBuilder.setDefaultVisibility(RuleVisibility.parse(value));
-
- } else if (name.equals("default_testonly")) {
- Boolean value = convert(Type.BOOLEAN, rawValue, pkgBuilder);
- pkgBuilder.setDefaultTestonly(value);
-
- } else if (name.equals("default_deprecation")) {
- String value = convert(Type.STRING, rawValue, pkgBuilder);
- pkgBuilder.setDefaultDeprecation(value);
-
- } else if (name.equals("features")) {
- List<String> value = convert(Type.STRING_LIST, rawValue, pkgBuilder);
- pkgBuilder.addFeatures(value);
-
- } else if (name.equals("licenses")) {
- License value = convert(BuildType.LICENSE, rawValue, pkgBuilder);
- pkgBuilder.setDefaultLicense(value);
-
- } else if (name.equals("distribs")) {
- Set<DistributionType> value = convert(BuildType.DISTRIBUTIONS, rawValue, pkgBuilder);
- pkgBuilder.setDefaultDistribs(value);
-
- } else if (name.equals("default_compatible_with")) {
- List<Label> value = convert(BuildType.LABEL_LIST, rawValue, pkgBuilder);
- pkgBuilder.setDefaultCompatibleWith(value, name, loc);
-
- } else if (name.equals("default_restricted_to")) {
- List<Label> value = convert(BuildType.LABEL_LIST, rawValue, pkgBuilder);
- pkgBuilder.setDefaultRestrictedTo(value, name, loc);
-
- } else if (name.equals("default_applicable_licenses")) {
- List<Label> value = convert(BuildType.LABEL_LIST, rawValue, pkgBuilder);
- if (!pkgBuilder.getDefaultPackageMetadata().isEmpty()) {
- pkgBuilder.addEvent(
- Package.error(
- loc,
- "Can not set both default_package_metadata and default_applicable_licenses."
- + " Move all declarations to default_package_metadata.",
- Code.INVALID_PACKAGE_SPECIFICATION));
- }
- pkgBuilder.setDefaultPackageMetadata(value, name, loc);
-
- } else if (name.equals("default_package_metadata")) {
- List<Label> value = convert(BuildType.LABEL_LIST, rawValue, pkgBuilder);
- if (!pkgBuilder.getDefaultPackageMetadata().isEmpty()) {
- pkgBuilder.addEvent(
- Package.error(
- loc,
- "Can not set both default_package_metadata and default_applicable_licenses."
- + " Move all declarations to default_package_metadata.",
- Code.INVALID_PACKAGE_SPECIFICATION));
- }
- pkgBuilder.setDefaultPackageMetadata(value, name, loc);
-
- } else {
- throw Starlark.errorf("unexpected keyword argument: %s", name);
- }
- }
-
- protected static <T> T convert(Type<T> type, Object value, Package.Builder pkgBuilder)
+ String name, Object rawValue, Package.Builder pkgBuilder, PackageArgs.Builder pkgArgsBuilder)
throws EvalException {
- return type.convert(value, "'package' argument", pkgBuilder.getLabelConverter());
+ PackageArgs.processParam(
+ name,
+ rawValue,
+ "package() argument '" + name + "'",
+ pkgBuilder.getLabelConverter(),
+ pkgArgsBuilder);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 8ee1d52..bac3724 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -2188,17 +2188,18 @@
if (rule.getRuleClassObject().isPackageMetadataRule()) {
// Do nothing
} else {
- rule.setAttributeValue(attr, pkgBuilder.getDefaultPackageMetadata(), /*explicit=*/ false);
+ rule.setAttributeValue(
+ attr, pkgBuilder.getPackageArgs().defaultPackageMetadata(), /* explicit= */ false);
}
} else if (attr.getName().equals("licenses") && attr.getType() == BuildType.LICENSE) {
rule.setAttributeValue(
attr,
- ignoreLicenses ? License.NO_LICENSE : pkgBuilder.getDefaultLicense(),
- /*explicit=*/ false);
+ ignoreLicenses ? License.NO_LICENSE : pkgBuilder.getPackageArgs().license(),
+ /* explicit= */ false);
} else if (attr.getName().equals("distribs") && attr.getType() == BuildType.DISTRIBUTIONS) {
- rule.setAttributeValue(attr, pkgBuilder.getDefaultDistribs(), /*explicit=*/ false);
+ rule.setAttributeValue(attr, pkgBuilder.getPackageArgs().distribs(), /* explicit= */ false);
}
// Don't store default values, querying materializes them at read time.
}
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java
index ed7dec9..210f7c2 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java
@@ -110,14 +110,13 @@
public List<PackageIdentifier> deletedPackages;
@Option(
- name = "default_visibility",
- defaultValue = "private",
- converter = DefaultVisibilityConverter.class,
- documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
- effectTags = {OptionEffectTag.UNKNOWN},
- help =
- "Default visibility for packages that don't set it explicitly ('public' or " + "'private')."
- )
+ name = "default_visibility",
+ defaultValue = "private",
+ converter = DefaultVisibilityConverter.class,
+ documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+ effectTags = {OptionEffectTag.UNKNOWN},
+ help =
+ "Default visibility for packages that don't set it explicitly ('public' or 'private').")
public RuleVisibility defaultVisibility;
@Option(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 30d84e8..00f1ac5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -49,6 +49,7 @@
import com.google.devtools.build.lib.packages.NonSkyframeGlobber;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Package.ConfigSettingVisibilityPolicy;
+import com.google.devtools.build.lib.packages.PackageArgs;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.PackageValidator.InvalidPackageException;
import com.google.devtools.build.lib.packages.RuleVisibility;
@@ -1372,13 +1373,11 @@
repositoryMapping,
mainRepositoryMappingValue.getRepositoryMapping())
.setFilename(buildFileRootedPath)
- .setDefaultVisibility(defaultVisibility)
- // "defaultVisibility" comes from the command line.
- // Let's give the BUILD file a chance to set default_visibility once,
- // by resetting the PackageBuilder.defaultVisibilitySet flag.
- .setDefaultVisibilitySet(false)
.setConfigSettingVisibilityPolicy(configSettingVisibilityPolicy);
+ pkgBuilder.mergePackageArgsFrom(
+ PackageArgs.builder().setDefaultVisibility(defaultVisibility));
+
Set<SkyKey> globDepKeys = ImmutableSet.of();
// OK to execute BUILD program?
