Change default for rule classes that don't explicitly

declare required configuration fragments: from
*everything* to *nothing*.

Now that all builtin rules properly declare their fragments,
the "backwards compatibility" concern that inspired the
original behavior is no longer needed.

This impacts, for example, filegroup rules, which have
nothing to declare.

--
MOS_MIGRATED_REVID=106433791

3 files changed, 14 insertions, 42 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index a8cc9e4932..067f370f2c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -232,19 +232,19 @@ public final class ConfiguredTargetFactory {
     }
     ConfigurationFragmentPolicy configurationFragmentPolicy =
         rule.getRuleClassObject().getConfigurationFragmentPolicy();
-    if (!configurationFragmentPolicy.getRequiredConfigurationFragments().isEmpty()) {
-      MissingFragmentPolicy missingFragmentPolicy =
-          configurationFragmentPolicy.getMissingFragmentPolicy();
-      if (missingFragmentPolicy != MissingFragmentPolicy.IGNORE
-          && !configuration.hasAllFragments(
-              configurationFragmentPolicy.getRequiredConfigurationFragments())) {
-        if (missingFragmentPolicy == MissingFragmentPolicy.FAIL_ANALYSIS) {
-          ruleContext.ruleError(missingFragmentError(ruleContext, configurationFragmentPolicy));
-          return null;
-        }
-        return createFailConfiguredTarget(ruleContext);
+
+    MissingFragmentPolicy missingFragmentPolicy =
+        configurationFragmentPolicy.getMissingFragmentPolicy();
+    if (missingFragmentPolicy != MissingFragmentPolicy.IGNORE
+        && !configuration.hasAllFragments(
+            configurationFragmentPolicy.getRequiredConfigurationFragments())) {
+      if (missingFragmentPolicy == MissingFragmentPolicy.FAIL_ANALYSIS) {
+        ruleContext.ruleError(missingFragmentError(ruleContext, configurationFragmentPolicy));
+        return null;
       }
+      return createFailConfiguredTarget(ruleContext);
     }
+
     if (rule.getRuleClassObject().isSkylarkExecutable()) {
       // TODO(bazel-team): maybe merge with RuleConfiguredTargetBuilder?
       return SkylarkRuleConfiguredTargetBuilder.buildRule(
diff --git a/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java b/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java
index abad84899e..9833174066 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java
@@ -72,10 +72,6 @@ public final class ConfigurationFragmentPolicy {
      * Declares that the implementation of the associated rule class requires the given
      * configuration fragments to be present in the configuration. The value is inherited by
      * subclasses.
-     *
-     * <p>For backwards compatibility, if the set is empty, all fragments may be accessed. But note
-     * that this is only enforced in the {@link com.google.devtools.build.lib.analysis.RuleContext}
-     * class.
      */
     public Builder requiresConfigurationFragments(Collection<Class<?>> configurationFragments) {
       requiredConfigurationFragments.addAll(configurationFragments);
@@ -86,10 +82,6 @@ public final class ConfigurationFragmentPolicy {
      * Declares that the implementation of the associated rule class requires the given
      * configuration fragments to be present in the configuration. The value is inherited by
      * subclasses.
-     *
-     * <p>For backwards compatibility, if the set is empty, all fragments may be accessed. But note
-     * that this is only enforced in the {@link com.google.devtools.build.lib.analysis.RuleContext}
-     * class.
      */
     public Builder requiresConfigurationFragments(Class<?>... configurationFragments) {
       Collections.addAll(requiredConfigurationFragments, configurationFragments);
@@ -131,8 +123,7 @@ public final class ConfigurationFragmentPolicy {
 
   /**
    * The set of required configuration fragments; this should list all fragments that can be
-   * accessed by the rule implementation. If empty, all fragments are allowed to be accessed for
-   * backwards compatibility.
+   * accessed by the rule implementation.
    */
   private final ImmutableSet<Class<?>> requiredConfigurationFragments;
 
@@ -168,8 +159,7 @@ public final class ConfigurationFragmentPolicy {
 
   /**
    * The set of required configuration fragments; this contains all fragments that can be
-   * accessed by the rule implementation. If empty, all fragments are allowed to be accessed for
-   * backwards compatibility.
+   * accessed by the rule implementation.
    */
   public Set<Class<?>> getRequiredConfigurationFragments() {
     return requiredConfigurationFragments;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
index a5e6320970..bc39fe11e5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
@@ -14,11 +14,9 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
-import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -141,12 +139,7 @@ public class TransitiveTargetFunction
     if (target instanceof Rule) {
       ConfigurationFragmentPolicy configurationFragmentPolicy =
           target.getAssociatedRule().getRuleClassObject().getConfigurationFragmentPolicy();
-      Set<Class<?>> configFragments =
-          configurationFragmentPolicy.getRequiredConfigurationFragments();
-      // An empty result means this rule requires all fragments (which practically means
-      // the rule isn't yet declaring its actually needed fragments). So load everything.
-      configFragments = configFragments.isEmpty() ? getAllFragments() : configFragments;
-      for (Class<?> fragment : configFragments) {
+      for (Class<?> fragment : configurationFragmentPolicy.getRequiredConfigurationFragments()) {
         if (!builder.getConfigFragmentsFromDeps().contains(fragment)) {
           builder.getTransitiveConfigFragments().add(
               fragment.asSubclass(BuildConfiguration.Fragment.class));
@@ -212,17 +205,6 @@ public class TransitiveTargetFunction
   }
 
   /**
-   * Returns every configuration fragment known to the system.
-   */
-  private Set<Class<?>> getAllFragments() {
-    ImmutableSet.Builder<Class<?>> builder = ImmutableSet.builder();
-    for (ConfigurationFragmentFactory factory : ruleClassProvider.getConfigurationFragments()) {
-      builder.add(factory.creates());
-    }
-    return builder.build();
-  }
-
-  /**
    * Holds values accumulated across the given target and its transitive dependencies for the
    * purpose of constructing a {@link TransitiveTargetValue}.
    *