Refactor the missing fragment handling code to use a policy enum.

Flip the handling such that analysis failure is the default, rather than
execution failure (fail action creation).

--
MOS_MIGRATED_REVID=100020812

4 files changed, 65 insertions, 31 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 f582cb62c8..b875a9db2b 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
@@ -40,6 +40,7 @@ import com.google.devtools.build.lib.packages.PackageGroupsRuleVisibility;
 import com.google.devtools.build.lib.packages.PackageSpecification;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
+import com.google.devtools.build.lib.packages.RuleClass.MissingFragmentPolicy;
 import com.google.devtools.build.lib.packages.RuleVisibility;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.rules.SkylarkRuleConfiguredTargetBuilder;
@@ -225,9 +226,12 @@ public final class ConfiguredTargetFactory {
       return null;
     }
     if (!rule.getRuleClassObject().getRequiredConfigurationFragments().isEmpty()) {
-      if (!configuration.hasAllFragments(
-          rule.getRuleClassObject().getRequiredConfigurationFragments())) {
-        if (rule.getRuleClassObject().failIfMissingConfigurationFragment()) {
+      MissingFragmentPolicy missingFragmentPolicy =
+          rule.getRuleClassObject().missingFragmentPolicy();
+      if (missingFragmentPolicy != MissingFragmentPolicy.IGNORE
+          && !configuration.hasAllFragments(
+              rule.getRuleClassObject().getRequiredConfigurationFragments())) {
+        if (missingFragmentPolicy == MissingFragmentPolicy.FAIL_ANALYSIS) {
           ruleContext.ruleError(missingFragmentError(ruleContext));
           return null;
         }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java
index 63c485ef0f..dd09bea69b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java
@@ -627,7 +627,6 @@ public class BazelCppRuleClasses {
     public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
       return builder
           .requiresConfigurationFragments(CppConfiguration.class)
-          .failIfMissingConfigurationFragment()
           /*<!-- #BLAZE_RULE(cc_binary).IMPLICIT_OUTPUTS -->
           <ul>
           <li><code><var>name</var>.stripped</code> (only built if explicitly requested): A stripped
@@ -773,7 +772,6 @@ public class BazelCppRuleClasses {
           // deps, data, linkopts, defines, srcs; override here too?
 
           .requiresConfigurationFragments(CppConfiguration.class)
-          .failIfMissingConfigurationFragment()
           /*<!-- #BLAZE_RULE(cc_library).ATTRIBUTE(alwayslink) -->
           If 1, any binary that depends (directly or indirectly) on this C++
           library will link in all the object files for the files listed in
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 e7cbf9edec..e2bd958b48 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
@@ -183,6 +183,33 @@ public final class RuleClass {
   };
 
   /**
+   * How to handle the case if the configuration is missing fragments that are required according
+   * to the rule class.
+   */
+  public enum MissingFragmentPolicy {
+    /**
+     * Some rules are monolithic across languages, and we want them to continue to work even when
+     * individual languages are disabled. Use this policy if the rule implementation is handling
+     * missing fragments.
+     */
+    IGNORE,
+
+    /**
+     * Use this policy to generate fail actions for the target rather than failing the analysis
+     * outright. Again, this is used when rules are monolithic across languages, but we still need
+     * to analyze the dependent libraries. (Instead of this mechanism, consider annotating
+     * attributes as unused if certain fragments are unavailable.)
+     */
+    CREATE_FAIL_ACTIONS,
+
+    /**
+     * Use this policy to fail the analysis of that target with an error message; this is the
+     * default.
+     */
+    FAIL_ANALYSIS;
+  }
+
+  /**
    * For Bazel's constraint system: the attribute that declares the set of environments a rule
    * supports, overriding the defaults for their respective groups.
    */
@@ -461,7 +488,7 @@ public final class RuleClass {
         NO_EXTERNAL_BINDINGS;
     private SkylarkEnvironment ruleDefinitionEnvironment = null;
     private Set<Class<?>> configurationFragments = new LinkedHashSet<>();
-    private boolean failIfMissingConfigurationFragment;
+    private MissingFragmentPolicy missingFragmentPolicy = MissingFragmentPolicy.FAIL_ANALYSIS;
     private boolean supportsConstraintChecking = true;
 
     private final Map<String, Attribute> attributes = new LinkedHashMap<>();
@@ -489,7 +516,7 @@ public final class RuleClass {
           setPreferredDependencyPredicate(parent.preferredDependencyPredicate);
         }
         configurationFragments.addAll(parent.requiredConfigurationFragments);
-        failIfMissingConfigurationFragment |= parent.failIfMissingConfigurationFragment;
+        missingFragmentPolicy = parent.missingFragmentPolicy;
         supportsConstraintChecking = parent.supportsConstraintChecking;
 
         for (Attribute attribute : parent.getAttributes()) {
@@ -548,7 +575,7 @@ public final class RuleClass {
           configuredTargetFactory, validityPredicate, preferredDependencyPredicate,
           ImmutableSet.copyOf(advertisedProviders), configuredTargetFunction,
           externalBindingsFunction,
-          ruleDefinitionEnvironment, configurationFragments, failIfMissingConfigurationFragment,
+          ruleDefinitionEnvironment, configurationFragments, missingFragmentPolicy,
           supportsConstraintChecking, attributes.values().toArray(new Attribute[0]));
     }
 
@@ -565,8 +592,12 @@ public final class RuleClass {
       return this;
     }
 
-    public Builder failIfMissingConfigurationFragment() {
-      this.failIfMissingConfigurationFragment = true;
+    /**
+     * Sets the policy for the case where the configuration is missing required fragments (see
+     * {@link #requiresConfigurationFragments}).
+     */
+    public Builder setMissingFragmentPolicy(MissingFragmentPolicy missingFragmentPolicy) {
+      this.missingFragmentPolicy = missingFragmentPolicy;
       return this;
     }
 
@@ -917,12 +948,9 @@ public final class RuleClass {
   private final ImmutableSet<Class<?>> requiredConfigurationFragments;
 
   /**
-   * Whether to fail during analysis if a configuration fragment is missing. The default behavior is
-   * to create fail actions for all declared outputs, i.e., to fail during execution, if any of the
-   * outputs is actually attempted to be built.
+   * What to do during analysis if a configuration fragment is missing.
    */
-  private final boolean failIfMissingConfigurationFragment;
-
+  private final MissingFragmentPolicy missingFragmentPolicy;
 
   /**
    * Determines whether instances of this rule should be checked for constraint compatibility
@@ -965,7 +993,7 @@ public final class RuleClass {
       @Nullable BaseFunction configuredTargetFunction,
       Function<? super Rule, Map<String, Label>> externalBindingsFunction,
       @Nullable SkylarkEnvironment ruleDefinitionEnvironment,
-      Set<Class<?>> allowedConfigurationFragments, boolean failIfMissingConfigurationFragment,
+      Set<Class<?>> allowedConfigurationFragments, MissingFragmentPolicy missingFragmentPolicy,
       boolean supportsConstraintChecking,
       Attribute... attributes) {
     this.name = name;
@@ -988,7 +1016,7 @@ public final class RuleClass {
     this.workspaceOnly = workspaceOnly;
     this.outputsDefaultExecutable = outputsDefaultExecutable;
     this.requiredConfigurationFragments = ImmutableSet.copyOf(allowedConfigurationFragments);
-    this.failIfMissingConfigurationFragment = failIfMissingConfigurationFragment;
+    this.missingFragmentPolicy = missingFragmentPolicy;
     this.supportsConstraintChecking = supportsConstraintChecking;
 
     // create the index:
@@ -1158,8 +1186,8 @@ public final class RuleClass {
   /**
    * Whether to fail analysis if any of the required configuration fragments are missing.
    */
-  public boolean failIfMissingConfigurationFragment() {
-    return failIfMissingConfigurationFragment;
+  public MissingFragmentPolicy missingFragmentPolicy() {
+    return missingFragmentPolicy;
   }
 
   /**
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
index 2432ba70bc..4d1e7dade6 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
@@ -42,6 +42,7 @@ import com.google.devtools.build.lib.events.Location.LineAndColumn;
 import com.google.devtools.build.lib.packages.Attribute.ValidityPredicate;
 import com.google.devtools.build.lib.packages.Package.Builder;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
+import com.google.devtools.build.lib.packages.RuleClass.MissingFragmentPolicy;
 import com.google.devtools.build.lib.packages.util.PackageLoadingTestCase;
 import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.build.lib.syntax.Label.SyntaxException;
@@ -81,7 +82,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE,
         DUMMY_CONFIGURED_TARGET_FACTORY, PredicatesWithMessage.<Rule>alwaysTrue(),
         PREFERRED_DEPENDENCY_PREDICATE, ImmutableSet.<Class<?>>of(), null,
-        NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(), false, true,
+        NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
+        MissingFragmentPolicy.FAIL_ANALYSIS, true,
         attr("my-string-attr", STRING).mandatory().build(),
         attr("my-label-attr", LABEL).mandatory().legacyAllowAnyFileType()
             .value(Label.parseAbsolute("//default:label")).build(),
@@ -100,7 +102,7 @@ public class RuleClassTest extends PackageLoadingTestCase {
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
         ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
-        false, true, attributes.toArray(new Attribute[0]));
+        MissingFragmentPolicy.FAIL_ANALYSIS, true, attributes.toArray(new Attribute[0]));
   }
 
   public void testRuleClassBasics() throws Exception {
@@ -212,7 +214,7 @@ public class RuleClassTest extends PackageLoadingTestCase {
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
         ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
-        false, true,
+        MissingFragmentPolicy.FAIL_ANALYSIS, true,
         attr("list1", LABEL_LIST).mandatory().legacyAllowAnyFileType().build(),
         attr("list2", LABEL_LIST).mandatory().legacyAllowAnyFileType().build(),
         attr("list3", LABEL_LIST).mandatory().legacyAllowAnyFileType().build());
@@ -242,7 +244,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
         ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
-        false, true, attr("visibility", LABEL_LIST).legacyAllowAnyFileType().build());
+        MissingFragmentPolicy.FAIL_ANALYSIS, true,
+        attr("visibility", LABEL_LIST).legacyAllowAnyFileType().build());
     Map<String, Object> attributeValues = new HashMap<>();
     attributeValues.put("visibility", Arrays.asList("//visibility:legacy_public"));
 
@@ -327,7 +330,7 @@ public class RuleClassTest extends PackageLoadingTestCase {
         RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
         ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
-        false, true, attr("outs", OUTPUT_LIST).build());
+        MissingFragmentPolicy.FAIL_ANALYSIS, true, attr("outs", OUTPUT_LIST).build());
 
     Map<String, Object> attributeValues = new HashMap<>();
     attributeValues.put("outs", Collections.singletonList("explicit_out"));
@@ -348,7 +351,7 @@ public class RuleClassTest extends PackageLoadingTestCase {
         ImplicitOutputsFunction.fromTemplates("%{dirname}lib%{basename}.bar"), RuleClass.NO_CHANGE,
         DUMMY_CONFIGURED_TARGET_FACTORY, PredicatesWithMessage.<Rule>alwaysTrue(),
         PREFERRED_DEPENDENCY_PREDICATE, ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS,
-        null, ImmutableSet.<Class<?>>of(), false, true);
+        null, ImmutableSet.<Class<?>>of(), MissingFragmentPolicy.FAIL_ANALYSIS, true);
 
     Rule rule = createRule(ruleClass, "myRule", Collections.<String, Object>emptyMap(),
         testRuleLocation);
@@ -369,7 +372,7 @@ public class RuleClassTest extends PackageLoadingTestCase {
         ImplicitOutputsFunction.fromTemplates("empty"), RuleClass.NO_CHANGE,
         DUMMY_CONFIGURED_TARGET_FACTORY, PredicatesWithMessage.<Rule>alwaysTrue(),
         PREFERRED_DEPENDENCY_PREDICATE, ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS,
-        null, ImmutableSet.<Class<?>>of(), false, true,
+        null, ImmutableSet.<Class<?>>of(), MissingFragmentPolicy.FAIL_ANALYSIS, true,
         attr("condition", BOOLEAN).value(false).build(),
         attr("declared1", BOOLEAN).value(false).build(),
         attr("declared2", BOOLEAN).value(false).build(),
@@ -512,7 +515,7 @@ public class RuleClassTest extends PackageLoadingTestCase {
         RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
         ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
-        false, true, attr("outs", OUTPUT_LIST).build());
+        MissingFragmentPolicy.FAIL_ANALYSIS, true, attr("outs", OUTPUT_LIST).build());
 
     Map<String, Object> attributeValues = new HashMap<>();
     attributeValues.put("outs", ImmutableList.of("third", "fourth"));
@@ -535,7 +538,7 @@ public class RuleClassTest extends PackageLoadingTestCase {
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
         ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
-        false, true,
+        MissingFragmentPolicy.FAIL_ANALYSIS, true,
         attr("a", STRING_LIST).mandatory().build(),
         attr("b", STRING_LIST).mandatory().build(),
         attr("c", STRING_LIST).mandatory().build(),
@@ -621,7 +624,7 @@ public class RuleClassTest extends PackageLoadingTestCase {
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
         ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
-        false, true, attributes);
+        MissingFragmentPolicy.FAIL_ANALYSIS, true, attributes);
     return mandNonEmptyRuleClass;
   }
 
@@ -632,7 +635,7 @@ public class RuleClassTest extends PackageLoadingTestCase {
         ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
         ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null, ImmutableSet.<Class<?>>of(),
-        false, true, attr("list", LABEL_LIST)
+        MissingFragmentPolicy.FAIL_ANALYSIS, true, attr("list", LABEL_LIST)
         .nonEmpty().legacyAllowAnyFileType().value(emptyList).build());
 
     Map<String, Object> attributeValues = new LinkedHashMap<>();
@@ -718,7 +721,8 @@ public class RuleClassTest extends PackageLoadingTestCase {
         false, ImplicitOutputsFunction.NONE, RuleClass.NO_CHANGE, DUMMY_CONFIGURED_TARGET_FACTORY,
         PredicatesWithMessage.<Rule>alwaysTrue(), PREFERRED_DEPENDENCY_PREDICATE,
         ImmutableSet.<Class<?>>of(), null, NO_EXTERNAL_BINDINGS, null,
-        ImmutableSet.<Class<?>>of(DummyFragment.class), false, true, attr("attr", STRING).build());
+        ImmutableSet.<Class<?>>of(DummyFragment.class), MissingFragmentPolicy.FAIL_ANALYSIS, true,
+        attr("attr", STRING).build());
     return parentRuleClass;
   }