From 0ea070be02e21c2418e967e3398251c3abba73e8 Mon Sep 17 00:00:00 2001
From: aiuto <aiuto@google.com>
Date: Thu, 1 Dec 2022 08:37:28 -0500
Subject: [PATCH] Backport recent package metadata and license check
 capabilities from Bazel 6.x. (#16892)

More context: https://docs.google.com/document/d/1uyJjkKbE8kV8EinakaR9q-Un25zCukhoH_dRBkWHSKQ/edit#

1. Rename default_applicable_licenses to default_package_metadata.

Leave default_applicable_licenses as an alias.  Don't allow both to be set.

Pick: https://github.com/bazelbuild/bazel/commit/ace99b575fe1e43f7591b75cf5d1c867175f9980

2.  Improve the check for being a package metadata rule

This allows the refactoring which will happen after default_applicable_licenses is renamed to default_package_metadata. The current behavior is intended to prevent `applicable_licenses` from being set on a `license` rule. The required behavior is that we don't set `applicable_licenses` on any of the metadata rules.

Pick: https://github.com/bazelbuild/bazel/commit/bbc221f60bc8c9177470529d85c3e47a5d9aaf21
---
 .../lib/analysis/LicensesProviderImpl.java    |  2 +-
 .../lib/packages/DefaultPackageArguments.java | 63 ++++++++++++++-----
 .../devtools/build/lib/packages/Package.java  | 22 +++----
 .../devtools/build/lib/packages/Rule.java     |  2 +-
 .../build/lib/packages/RuleClass.java         | 59 +++++++++--------
 5 files changed, 94 insertions(+), 54 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/LicensesProviderImpl.java b/src/main/java/com/google/devtools/build/lib/analysis/LicensesProviderImpl.java
index 36320ed5b2a336..63b33b53ade1c6 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/LicensesProviderImpl.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/LicensesProviderImpl.java
@@ -68,7 +68,7 @@ public static LicensesProvider of(RuleContext ruleContext) {
       ListMultimap<String, ? extends TransitiveInfoCollection> configuredMap =
           ruleContext.getConfiguredTargetMap();
 
-      if (rule.getRuleClassObject().isBazelLicense()) {
+      if (rule.getRuleClassObject().isPackageMetadataRule()) {
         // Don't crawl a new-style license, it's effectively a leaf.
         // The representation of the new-style rule is unfortunately hardcoded here,
         // but this is code in the old-style licensing path that will ultimately be removed.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/DefaultPackageArguments.java b/src/main/java/com/google/devtools/build/lib/packages/DefaultPackageArguments.java
index 4ec2aef24bdd66..927fd463dea8bb 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/DefaultPackageArguments.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/DefaultPackageArguments.java
@@ -17,6 +17,7 @@
 import com.google.common.collect.ImmutableList;
 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.Set;
 import net.starlark.java.eval.EvalException;
@@ -30,15 +31,16 @@ private DefaultPackageArguments() {}
   /** Returns the default set of {@link PackageArgument}s. */
   static ImmutableList<PackageArgument<?>> get() {
     return ImmutableList.of(
-            new DefaultDeprecation(),
-            new DefaultDistribs(),
-            new DefaultApplicableLicenses(),
-            new DefaultLicenses(),
-            new DefaultTestOnly(),
-            new DefaultVisibility(),
-            new Features(),
-            new DefaultCompatibleWith(),
-            new DefaultRestrictedTo());
+        new DefaultDeprecation(),
+        new DefaultDistribs(),
+        new DefaultApplicableLicenses(),
+        new DefaultPackageMetadata(),
+        new DefaultLicenses(),
+        new DefaultTestOnly(),
+        new DefaultVisibility(),
+        new Features(),
+        new DefaultCompatibleWith(),
+        new DefaultRestrictedTo());
   }
 
   private static class DefaultVisibility extends PackageArgument<List<Label>> {
@@ -96,17 +98,48 @@ protected void process(Package.Builder pkgBuilder, Location location,
    * specified.
    */
   private static class DefaultApplicableLicenses extends PackageArgument<List<Label>> {
-    private static final String DEFAULT_APPLICABLE_LICENSES_ATTRIBUTE =
-        "default_applicable_licenses";
-
     private DefaultApplicableLicenses() {
-      super(DEFAULT_APPLICABLE_LICENSES_ATTRIBUTE, BuildType.LABEL_LIST);
+      super("default_applicable_licenses", BuildType.LABEL_LIST);
+    }
+
+    @Override
+    protected void process(Package.Builder pkgBuilder, Location location, List<Label> value) {
+      if (!pkgBuilder.getDefaultPackageMetadata().isEmpty()) {
+        pkgBuilder.addEvent(
+            Package.error(
+                location,
+                "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, "default_package_metadata", location);
+    }
+  }
+
+  /**
+   * Declares the package() attribute specifying the default value for {@link
+   * com.google.devtools.build.lib.packages.RuleClass#APPLICABLE_LICENSES_ATTR} when not explicitly
+   * specified.
+   */
+  private static class DefaultPackageMetadata extends PackageArgument<List<Label>> {
+    private static final String DEFAULT_PACKAGE_METADATA_ATTRIBUTE = "default_package_metadata";
+
+    private DefaultPackageMetadata() {
+      super(DEFAULT_PACKAGE_METADATA_ATTRIBUTE, BuildType.LABEL_LIST);
     }
 
     @Override
     protected void process(Package.Builder pkgBuilder, Location location, List<Label> value) {
-      pkgBuilder.setDefaultApplicableLicenses(
-          value, DEFAULT_APPLICABLE_LICENSES_ATTRIBUTE, location);
+      if (!pkgBuilder.getDefaultPackageMetadata().isEmpty()) {
+        pkgBuilder.addEvent(
+            Package.error(
+                location,
+                "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, DEFAULT_PACKAGE_METADATA_ATTRIBUTE, location);
     }
   }
 
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 f6d8ea24b611a2..8dcb7a2bd81919 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
@@ -207,8 +207,8 @@ public enum ConfigSettingVisibilityPolicy {
   /** The list of transitive closure of the Starlark file dependencies. */
   private ImmutableList<Label> starlarkFileDependencies;
 
-  /** The package's default "applicable_licenses" attribute. */
-  private Set<Label> defaultApplicableLicenses = ImmutableSet.of();
+  /** The package's default "package_metadata" attribute. */
+  private ImmutableSet<Label> defaultPackageMetadata = ImmutableSet.of();
 
   /**
    * The package's default "licenses" and "distribs" attributes, as specified
@@ -461,7 +461,7 @@ private void finishInit(Builder builder) {
     this.starlarkFileDependencies = builder.starlarkFileDependencies;
     this.defaultLicense = builder.defaultLicense;
     this.defaultDistributionSet = builder.defaultDistributionSet;
-    this.defaultApplicableLicenses = ImmutableSortedSet.copyOf(builder.defaultApplicableLicenses);
+    this.defaultPackageMetadata = ImmutableSortedSet.copyOf(builder.defaultPackageMetadata);
     this.features = ImmutableSortedSet.copyOf(builder.features);
     this.registeredExecutionPlatforms = ImmutableList.copyOf(builder.registeredExecutionPlatforms);
     this.registeredToolchains = ImmutableList.copyOf(builder.registeredToolchains);
@@ -771,9 +771,9 @@ public boolean isDefaultVisibilitySet() {
     return defaultVisibilitySet;
   }
 
-  /** Gets the licenses list for the default applicable_licenses declared by this package. */
-  public Set<Label> getDefaultApplicableLicenses() {
-    return defaultApplicableLicenses;
+  /** Gets the package metadata list for the default metadata declared by this package. */
+  public Set<Label> getDefaultPackageMetadata() {
+    return defaultPackageMetadata;
   }
 
   /** Gets the parsed license object for the default license declared by this package. */
@@ -974,7 +974,7 @@ public boolean recordLoadedModules() {
     // serialize events emitted during its construction/evaluation.
     @Nullable private FailureDetail failureDetailOverride = null;
 
-    private ImmutableList<Label> defaultApplicableLicenses = ImmutableList.of();
+    private ImmutableList<Label> defaultPackageMetadata = ImmutableList.of();
     private License defaultLicense = License.NO_LICENSE;
     private Set<License.DistributionType> defaultDistributionSet = License.DEFAULT_DISTRIB;
 
@@ -1368,16 +1368,16 @@ public Builder setStarlarkFileDependencies(ImmutableList<Label> starlarkFileDepe
      * attribute when not explicitly specified by the rule. Records a package error if any labels
      * are duplicated.
      */
-    void setDefaultApplicableLicenses(List<Label> licenses, String attrName, Location location) {
+    void setDefaultPackageMetadata(List<Label> licenses, String attrName, Location location) {
       if (hasDuplicateLabels(
           licenses, "package " + pkg.getName(), attrName, location, this::addEvent)) {
         setContainsErrors();
       }
-      this.defaultApplicableLicenses = ImmutableList.copyOf(licenses);
+      this.defaultPackageMetadata = ImmutableList.copyOf(licenses);
     }
 
-    ImmutableList<Label> getDefaultApplicableLicenses() {
-      return defaultApplicableLicenses;
+    ImmutableList<Label> getDefaultPackageMetadata() {
+      return defaultPackageMetadata;
     }
 
     /**
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
index 59598f282b89e2..13d2a47cd5fdd9 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
@@ -883,7 +883,7 @@ public License getLicense() {
     // have old-style licenses. This is hardcoding the representation
     // of new-style rules, but it's in the old-style licensing code path
     // and will ultimately be removed.
-    if (ruleClass.isBazelLicense()) {
+    if (ruleClass.isPackageMetadataRule()) {
       return License.NO_LICENSE;
     } else if (isAttrDefined("licenses", BuildType.LICENSE)
         && isAttributeValueExplicitlySpecified("licenses")) {
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 b33d8e01f0d354..bbcc298827c25e 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
@@ -2295,9 +2295,6 @@ private void populateDefaultRuleAttributeValues(
 
       } else if (attr.getName().equals(APPLICABLE_LICENSES_ATTR)
           && attr.getType() == BuildType.LABEL_LIST) {
-        // TODO(b/149505729): Determine the right semantics for someone trying to define their own
-        // attribute named applicable_licenses.
-        //
         // The check here is preventing against an corner case where the license() rule can get
         // itself as an applicable_license. This breaks the graph because there is now a self-edge.
         //
@@ -2320,26 +2317,10 @@ private void populateDefaultRuleAttributeValues(
         // have the self-edge problem, they would get all default_applicable_licenses and now the
         // graph is inconsistent in that some license() rules have applicable_licenses while others
         // do not.
-        //
-        // Another possible workaround is to leverage the fact that license() rules instantiated
-        // before the package() rule will not get default_applicable_licenses applied, and the
-        // self-edge problem cannot occur in that case. The semantics for how package() should
-        // impact rules instantiated prior are not clear and not well understood. If this
-        // modification is distasteful, leveraging the package() behavior and clarifying the
-        // semantics is an option. It's not recommended since BUILD files are not thought to be
-        // order-dependent, but they have always been, so fixing that behavior may be more important
-        // than some unfortunate code here.
-        //
-        // Breaking the encapsulation to recognize license() rules and treat them uniformly results
-        // fixes the self-edge problem and results in the simplest, semantically
-        // correct graph.
-        //
-        // TODO(b/183637322) consider this further
-        if (rule.getRuleClassObject().isBazelLicense()) {
+        if (rule.getRuleClassObject().isPackageMetadataRule()) {
           // Do nothing
         } else {
-          rule.setAttributeValue(
-              attr, pkgBuilder.getDefaultApplicableLicenses(), /*explicit=*/ false);
+          rule.setAttributeValue(attr, pkgBuilder.getDefaultPackageMetadata(), /*explicit=*/ false);
         }
 
       } else if (attr.getName().equals("licenses") && attr.getType() == BuildType.LICENSE) {
@@ -2803,10 +2784,36 @@ public static boolean isThirdPartyPackage(PackageIdentifier packageIdentifier) {
         && packageIdentifier.getPackageFragment().isMultiSegment();
   }
 
-  // Returns true if this rule is a license() rule as defined in
-  // https://docs.google.com/document/d/1uwBuhAoBNrw8tmFs-NxlssI6VRolidGYdYqagLqHWt8/edit#
-  // TODO(b/183637322) consider this further
-  public boolean isBazelLicense() {
-    return name.equals("_license") && hasAttr("license_kinds", BuildType.LABEL_LIST);
+  /**
+   * Returns true if this rule is a <code>license()</code> as described in
+   * https://docs.google.com/document/d/1uwBuhAoBNrw8tmFs-NxlssI6VRolidGYdYqagLqHWt8/edit# or
+   * similar metadata.
+   *
+   * <p>The intended use is to detect if this rule is of a type which would be used in <code>
+   * default_package_metadata</code>, so that we don't apply it to an instanced of itself when
+   * <code>applicable_licenses</code> is left unset. Doing so causes a self-referential loop. To
+   * prevent that, we are overly cautious at this time, treating all rules from <code>@rules_license
+   * </code> as potential metadata rules.
+   *
+   * <p>Most users will only use declarations from <code>@rules_license</code>. If they which to
+   * create organization local rules, they must be careful to avoid loops by explicitly setting
+   * <code>applicable_licenses</code> on each of the metadata targets they define, so that default
+   * processing is not an issue.
+   */
+  public boolean isPackageMetadataRule() {
+    // If it was not defined in Starlark, it can not be a new style package metadata rule.
+    if (ruleDefinitionEnvironmentLabel == null) {
+      return false;
+    }
+    if (ruleDefinitionEnvironmentLabel.getRepository().getName().equals("rules_license")) {
+      // For now, we treat all rules in rules_license as potenial metadate rules.
+      // In the future we should add a way to disambiguate the two. The least invasive
+      // thing is to add a hidden attribute to mark metadata rules. That attribute
+      // could have a default value referencing @rules_license//<something>. That style
+      // of checking would allow users to apply it to their own metadata rules. We are
+      // not building it today because the exact needs are not clear.
+      return true;
+    }
+    return false;
   }
 }