@@ -2295,9 +2295,6 @@ private void populateDefaultRuleAttributeValues(
2295
2295
2296
2296
} else if (attr .getName ().equals (APPLICABLE_LICENSES_ATTR )
2297
2297
&& attr .getType () == BuildType .LABEL_LIST ) {
2298
- // TODO(b/149505729): Determine the right semantics for someone trying to define their own
2299
- // attribute named applicable_licenses.
2300
- //
2301
2298
// The check here is preventing against an corner case where the license() rule can get
2302
2299
// itself as an applicable_license. This breaks the graph because there is now a self-edge.
2303
2300
//
@@ -2320,26 +2317,10 @@ private void populateDefaultRuleAttributeValues(
2320
2317
// have the self-edge problem, they would get all default_applicable_licenses and now the
2321
2318
// graph is inconsistent in that some license() rules have applicable_licenses while others
2322
2319
// do not.
2323
- //
2324
- // Another possible workaround is to leverage the fact that license() rules instantiated
2325
- // before the package() rule will not get default_applicable_licenses applied, and the
2326
- // self-edge problem cannot occur in that case. The semantics for how package() should
2327
- // impact rules instantiated prior are not clear and not well understood. If this
2328
- // modification is distasteful, leveraging the package() behavior and clarifying the
2329
- // semantics is an option. It's not recommended since BUILD files are not thought to be
2330
- // order-dependent, but they have always been, so fixing that behavior may be more important
2331
- // than some unfortunate code here.
2332
- //
2333
- // Breaking the encapsulation to recognize license() rules and treat them uniformly results
2334
- // fixes the self-edge problem and results in the simplest, semantically
2335
- // correct graph.
2336
- //
2337
- // TODO(b/183637322) consider this further
2338
- if (rule .getRuleClassObject ().isBazelLicense ()) {
2320
+ if (rule .getRuleClassObject ().isPackageMetadataRule ()) {
2339
2321
// Do nothing
2340
2322
} else {
2341
- rule .setAttributeValue (
2342
- attr , pkgBuilder .getDefaultApplicableLicenses (), /*explicit=*/ false );
2323
+ rule .setAttributeValue (attr , pkgBuilder .getDefaultPackageMetadata (), /*explicit=*/ false );
2343
2324
}
2344
2325
2345
2326
} else if (attr .getName ().equals ("licenses" ) && attr .getType () == BuildType .LICENSE ) {
@@ -2803,10 +2784,36 @@ public static boolean isThirdPartyPackage(PackageIdentifier packageIdentifier) {
2803
2784
&& packageIdentifier .getPackageFragment ().isMultiSegment ();
2804
2785
}
2805
2786
2806
- // Returns true if this rule is a license() rule as defined in
2807
- // https://docs.google.com/document/d/1uwBuhAoBNrw8tmFs-NxlssI6VRolidGYdYqagLqHWt8/edit#
2808
- // TODO(b/183637322) consider this further
2809
- public boolean isBazelLicense () {
2810
- return name .equals ("_license" ) && hasAttr ("license_kinds" , BuildType .LABEL_LIST );
2787
+ /**
2788
+ * Returns true if this rule is a <code>license()</code> as described in
2789
+ * https://docs.google.com/document/d/1uwBuhAoBNrw8tmFs-NxlssI6VRolidGYdYqagLqHWt8/edit# or
2790
+ * similar metadata.
2791
+ *
2792
+ * <p>The intended use is to detect if this rule is of a type which would be used in <code>
2793
+ * default_package_metadata</code>, so that we don't apply it to an instanced of itself when
2794
+ * <code>applicable_licenses</code> is left unset. Doing so causes a self-referential loop. To
2795
+ * prevent that, we are overly cautious at this time, treating all rules from <code>@rules_license
2796
+ * </code> as potential metadata rules.
2797
+ *
2798
+ * <p>Most users will only use declarations from <code>@rules_license</code>. If they which to
2799
+ * create organization local rules, they must be careful to avoid loops by explicitly setting
2800
+ * <code>applicable_licenses</code> on each of the metadata targets they define, so that default
2801
+ * processing is not an issue.
2802
+ */
2803
+ public boolean isPackageMetadataRule () {
2804
+ // If it was not defined in Starlark, it can not be a new style package metadata rule.
2805
+ if (ruleDefinitionEnvironmentLabel == null ) {
2806
+ return false ;
2807
+ }
2808
+ if (ruleDefinitionEnvironmentLabel .getRepository ().getName ().equals ("rules_license" )) {
2809
+ // For now, we treat all rules in rules_license as potenial metadate rules.
2810
+ // In the future we should add a way to disambiguate the two. The least invasive
2811
+ // thing is to add a hidden attribute to mark metadata rules. That attribute
2812
+ // could have a default value referencing @rules_license//<something>. That style
2813
+ // of checking would allow users to apply it to their own metadata rules. We are
2814
+ // not building it today because the exact needs are not clear.
2815
+ return true ;
2816
+ }
2817
+ return false ;
2811
2818
}
2812
2819
}
0 commit comments