Flip --incompatible_disable_expand_if_all_available_in_flag_set

FlagSets in the CROSSTOOL no longer accept expand_if_all_available field

Fixes #7008

RELNOTES: `--incompatible_disable_expand_if_all_available_in_flag_set` has been flipped (#7008)
PiperOrigin-RevId: 234466411

src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java

@@ -40,6 +40,19 @@
 public class BazelRulesModule extends BlazeModule {
   /** This is where deprecated options go to die. */
   public static class GraveyardOptions extends OptionsBase {
+    @Option(
+        name = "incompatible_disable_expand_if_all_available_in_flag_set",
+        defaultValue = "false",
+        documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+        effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+        metadataTags = {
+          OptionMetadataTag.DEPRECATED,
+          OptionMetadataTag.INCOMPATIBLE_CHANGE,
+          OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+        },
+        help = "Deprecated no-op.")
+    public boolean disableExpandIfAllAvailableInFlagSet;
+
     @Option(
         name = "incompatible_dont_emit_static_libgcc",
         oldName = "experimental_dont_emit_static_libgcc",

src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java

@@ -1295,8 +1295,7 @@ static FlagSet flagSetFromSkylark(SkylarkInfo flagSetStruct, String actionName)
       withFeatureSetBuilder.add(withFeatureSetFromSkylark(withFeatureSetStruct));
     }
 
-    return new FlagSet(
-        actions, ImmutableSet.of(), withFeatureSetBuilder.build(), flagGroupsBuilder.build());
+    return new FlagSet(actions, withFeatureSetBuilder.build(), flagGroupsBuilder.build());
   }
 
   /**

src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainConfigInfo.java

@@ -184,8 +184,6 @@ public static CcToolchainConfigInfo fromToolchain(RuleContext ruleContext, CTool
       throws EvalException {
     CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
     boolean disableLegacyCrosstoolFields = cppConfiguration.disableLegacyCrosstoolFields();
-    boolean disableExpandIfAllAvailableInFlagSet =
-        cppConfiguration.disableExpandIfAllAvailableInFlagSet();
     ImmutableList.Builder<ActionConfig> actionConfigBuilder = ImmutableList.builder();
     for (CToolchain.ActionConfig actionConfig : toolchain.getActionConfigList()) {
       actionConfigBuilder.add(new ActionConfig(actionConfig));
@@ -314,26 +312,6 @@ public static CcToolchainConfigInfo fromToolchain(RuleContext ruleContext, CTool
       }
     }
 
-    if (disableExpandIfAllAvailableInFlagSet) {
-      toolchain
-          .getFeatureList()
-          .forEach(
-              (f) -> {
-                if (f.getFlagSetList().stream()
-                    .anyMatch((s) -> s.getExpandIfAllAvailableCount() != 0)) {
-                  ruleContext.ruleError(
-                      String.format(
-                          "Feature '%s' defines a flag_set with expand_if_all_available set. "
-                              + "This is disabled by "
-                              + "--incompatible_disable_expand_if_all_available_in_flag_set, "
-                              + "please migrate your CROSSTOOL (see "
-                              + "https://github.com/bazelbuild/bazel/issues/7008 "
-                              + "for migration instructions).",
-                          f.getName()));
-                }
-              });
-    }
-
     for (CompilationModeFlags flag : toolchain.getCompilationModeFlagsList()) {
       switch (flag.getMode()) {
         case OPT:
@@ -827,8 +805,7 @@ private static CToolchain.FlagSet flagSetToProto(FlagSet flagSet, boolean forAct
             .addAllWithFeature(
                 flagSet.getWithFeatureSets().stream()
                     .map(withFeatureSet -> withFeatureSetToProto(withFeatureSet))
-                    .collect(ImmutableList.toImmutableList()))
-            .addAllExpandIfAllAvailable(flagSet.getExpandIfAllAvailable());
+                    .collect(ImmutableList.toImmutableList()));
     if (!forActionConfig) {
       flagSetBuilder.addAllAction(flagSet.getActions());
     }

src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java

@@ -516,7 +516,6 @@ private static boolean isWithFeaturesSatisfied(
   @VisibleForSerialization
   public static class FlagSet implements Serializable {
     private final ImmutableSet<String> actions;
-    private final ImmutableSet<String> expandIfAllAvailable;
     private final ImmutableSet<WithFeatureSet> withFeatureSets;
     private final ImmutableList<FlagGroup> flagGroups;
 
@@ -527,7 +526,6 @@ private FlagSet(CToolchain.FlagSet flagSet) throws EvalException {
     /** Constructs a FlagSet for the given set of actions. */
     private FlagSet(CToolchain.FlagSet flagSet, ImmutableSet<String> actions) throws EvalException {
       this.actions = actions;
-      this.expandIfAllAvailable = ImmutableSet.copyOf(flagSet.getExpandIfAllAvailableList());
       ImmutableSet.Builder<WithFeatureSet> featureSetBuilder = ImmutableSet.builder();
       for (CToolchain.WithFeatureSet withFeatureSet : flagSet.getWithFeatureList()) {
         featureSetBuilder.add(new WithFeatureSet(withFeatureSet));
@@ -543,11 +541,9 @@ private FlagSet(CToolchain.FlagSet flagSet, ImmutableSet<String> actions) throws
     @AutoCodec.Instantiator
     public FlagSet(
         ImmutableSet<String> actions,
-        ImmutableSet<String> expandIfAllAvailable,
         ImmutableSet<WithFeatureSet> withFeatureSets,
         ImmutableList<FlagGroup> flagGroups) {
       this.actions = actions;
-      this.expandIfAllAvailable = expandIfAllAvailable;
       this.withFeatureSets = withFeatureSets;
       this.flagGroups = flagGroups;
     }
@@ -559,11 +555,6 @@ private void expandCommandLine(
         Set<String> enabledFeatureNames,
         @Nullable ArtifactExpander expander,
         List<String> commandLine) {
-      for (String variable : expandIfAllAvailable) {
-        if (!variables.isAvailable(variable, expander)) {
-          return;
-        }
-      }
       if (!isWithFeaturesSatisfied(withFeatureSets, enabledFeatureNames)) {
         return;
       }
@@ -580,7 +571,6 @@ public boolean equals(@Nullable Object object) {
       if (object instanceof FlagSet) {
         FlagSet that = (FlagSet) object;
         return Iterables.elementsEqual(actions, that.actions)
-            && Iterables.elementsEqual(expandIfAllAvailable, that.expandIfAllAvailable)
             && Iterables.elementsEqual(withFeatureSets, that.withFeatureSets)
             && Iterables.elementsEqual(flagGroups, that.flagGroups);
       }
@@ -589,17 +579,13 @@ public boolean equals(@Nullable Object object) {
 
     @Override
     public int hashCode() {
-      return Objects.hash(actions, expandIfAllAvailable, withFeatureSets, flagGroups);
+      return Objects.hash(actions, withFeatureSets, flagGroups);
     }
 
     ImmutableSet<String> getActions() {
       return actions;
     }
 
-    ImmutableSet<String> getExpandIfAllAvailable() {
-      return expandIfAllAvailable;
-    }
-
     ImmutableSet<WithFeatureSet> getWithFeatureSets() {
       return withFeatureSets;
     }

src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java

@@ -548,10 +548,6 @@ public boolean disableLegacyCrosstoolFields() {
     return cppOptions.disableLegacyCrosstoolFields;
   }
 
-  public boolean disableExpandIfAllAvailableInFlagSet() {
-    return cppOptions.disableExpandIfAllAvailableInFlagSet;
-  }
-
   public static String getLegacyCrosstoolFieldErrorMessage(String field) {
     Preconditions.checkNotNull(field);
     return field

src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java

@@ -743,20 +743,6 @@ public Label getFdoPrefetchHintsLabel() {
               + "(see https://github.com/bazelbuild/bazel/issues/7075 for migration instructions).")
   public boolean removeCpuCompilerCcToolchainAttributes;
 
-  @Option(
-      name = "incompatible_disable_expand_if_all_available_in_flag_set",
-      defaultValue = "false",
-      documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
-      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
-      metadataTags = {
-        OptionMetadataTag.INCOMPATIBLE_CHANGE,
-        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
-      },
-      help =
-          "If true, Bazel will not allow specifying expand_if_all_available in flag_sets"
-              + "(see https://github.com/bazelbuild/bazel/issues/7008 for migration instructions).")
-  public boolean disableExpandIfAllAvailableInFlagSet;
-
   @Option(
       name = "incompatible_disable_crosstool_file",
       defaultValue = "false",
@@ -893,7 +879,6 @@ public FragmentOptions getHost() {
     host.doNotUseCpuTransformer = doNotUseCpuTransformer;
     host.disableGenruleCcToolchainDependency = disableGenruleCcToolchainDependency;
     host.disableDepsetInUserFlags = disableDepsetInUserFlags;
-    host.disableExpandIfAllAvailableInFlagSet = disableExpandIfAllAvailableInFlagSet;
     host.disableLegacyCcProvider = disableLegacyCcProvider;
     host.removeCpuCompilerCcToolchainAttributes = removeCpuCompilerCcToolchainAttributes;
     host.disableLegacyCrosstoolFields = disableLegacyCrosstoolFields;

src/main/protobuf/crosstool_config.proto

@@ -16,11 +16,11 @@
 
 syntax = "proto2";
 
+package com.google.devtools.build.lib.view.config.crosstool;
+
 // option java_api_version = 2;
 option java_package = "com.google.devtools.build.lib.view.config.crosstool";
 
-package com.google.devtools.build.lib.view.config.crosstool;
-
 // A description of a toolchain, which includes all the tools generally expected
 // to be available for building C/C++ targets, based on the GNU C compiler.
 //
@@ -148,17 +148,7 @@ message CToolchain {
     // unconditionally for every action specified.
     repeated WithFeatureSet with_feature = 3;
 
-    // Deprecated (https://github.com/bazelbuild/bazel/issues/7008) - use
-    // expand_if_all_available in flag_group
-    //
-    // A list of build variables that this feature set needs, but which are
-    // allowed to not be set. If any of the build variables listed is not
-    // set, the feature set will not be expanded.
-    //
-    // NOTE: Consider alternatives before using this; usually tools should
-    // consistently create the same set of files, even if empty; use this
-    // only for backwards compatibility with already existing behavior in tools
-    // that are currently not worth changing.
+    // Legacy field, ignored by Bazel.
     repeated string expand_if_all_available = 4;
   }
 

src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java

@@ -237,12 +237,12 @@ public boolean apply(Artifact artifact) {
           + "  name: 'thin_lto'"
           + "  requires { feature: 'nonhost' }"
           + "  flag_set {"
-          + "    expand_if_all_available: 'thinlto_param_file'"
           + "    action: 'c++-link-executable'"
           + "    action: 'c++-link-dynamic-library'"
           + "    action: 'c++-link-nodeps-dynamic-library'"
           + "    action: 'c++-link-static-library'"
           + "    flag_group {"
+          + "      expand_if_all_available: 'thinlto_param_file'"
           + "      flag: 'thinlto_param_file=%{thinlto_param_file}'"
           + "    }"
           + "  }"

src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java

@@ -1149,12 +1149,11 @@ public void testFlagSetWithMissingVariableIsNotExpanded() throws Exception {
                 "  name: 'a'",
                 "  flag_set {",
                 "     action: 'c++-compile'",
-                "     expand_if_all_available: 'v'",
-                "     flag_group { flag: '%{v}' }",
+                "     flag_group { expand_if_all_available: 'v' flag: '%{v}' }",
                 "  }",
                 "  flag_set {",
-                "     action: 'c++-compile'",
-                "     flag_group { flag: 'unconditional' }",
+                "    action: 'c++-compile'",
+                "    flag_group { flag: 'unconditional' }",
                 "  }",
                 "}")
             .getFeatureConfiguration(ImmutableSet.of("a"));
@@ -1171,19 +1170,20 @@ public void testOnlyFlagSetsWithAllVariablesPresentAreExpanded() throws Exceptio
                 "feature {",
                 "  name: 'a'",
                 "  flag_set {",
-                "     action: 'c++-compile'",
-                "     expand_if_all_available: 'v'",
-                "     flag_group { flag: '%{v}' }",
+                "    action: 'c++-compile'",
+                "    flag_group { expand_if_all_available: 'v' flag: '%{v}' }",
                 "  }",
                 "  flag_set {",
-                "     action: 'c++-compile'",
-                "     expand_if_all_available: 'v'",
-                "     expand_if_all_available: 'w'",
-                "     flag_group { flag: '%{v}%{w}' }",
+                "    action: 'c++-compile'",
+                "    flag_group { ",
+                "      expand_if_all_available: 'v'",
+                "      expand_if_all_available: 'w'",
+                "      flag: '%{v}%{w}' ",
+                "    }",
                 "  }",
                 "  flag_set {",
-                "     action: 'c++-compile'",
-                "     flag_group { flag: 'unconditional' }",
+                "    action: 'c++-compile'",
+                "    flag_group { flag: 'unconditional' }",
                 "  }",
                 "}")
             .getFeatureConfiguration(ImmutableSet.of("a"));
@@ -1200,19 +1200,21 @@ public void testOnlyInnerFlagSetIsIteratedWithSequenceVariable() throws Exceptio
                 "feature {",
                 "  name: 'a'",
                 "  flag_set {",
-                "     action: 'c++-compile'",
-                "     expand_if_all_available: 'v'",
-                "     flag_group { iterate_over: 'v' flag: '%{v}' }",
+                "    action: 'c++-compile'",
+                "    flag_group { expand_if_all_available: 'v' iterate_over: 'v' flag: '%{v}' }",
                 "  }",
                 "  flag_set {",
-                "     action: 'c++-compile'",
-                "     expand_if_all_available: 'v'",
-                "     expand_if_all_available: 'w'",
-                "     flag_group { iterate_over: 'v' flag: '%{v}%{w}' }",
+                "    action: 'c++-compile'",
+                "    flag_group { ",
+                "      iterate_over: 'v'",
+                "      expand_if_all_available: 'v'",
+                "      expand_if_all_available: 'w'",
+                "      flag: '%{v}%{w}'",
+                "    }",
                 "  }",
                 "  flag_set {",
-                "     action: 'c++-compile'",
-                "     flag_group { flag: 'unconditional' }",
+                "    action: 'c++-compile'",
+                "    flag_group { flag: 'unconditional' }",
                 "  }",
                 "}")
             .getFeatureConfiguration(ImmutableSet.of("a"));
@@ -1232,15 +1234,16 @@ public void testFlagSetsAreIteratedIndividuallyForSequenceVariables() throws Exc
                 "feature {",
                 "  name: 'a'",
                 "  flag_set {",
-                "     action: 'c++-compile'",
-                "     expand_if_all_available: 'v'",
-                "     flag_group { iterate_over: 'v' flag: '%{v}' }",
+                "    action: 'c++-compile'",
+                "    flag_group { expand_if_all_available: 'v' iterate_over: 'v' flag: '%{v}' }",
                 "  }",
                 "  flag_set {",
-                "     action: 'c++-compile'",
-                "     expand_if_all_available: 'v'",
-                "     expand_if_all_available: 'w'",
-                "     flag_group { iterate_over: 'v' flag: '%{v}%{w}' }",
+                "    action: 'c++-compile'",
+                "    flag_group { ",
+                "      expand_if_all_available: 'v'",
+                "      expand_if_all_available: 'w'",
+                "      iterate_over: 'v' flag: '%{v}%{w}'",
+                "    }",
                 "  }",
                 "  flag_set {",
                 "     action: 'c++-compile'",

src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java

@@ -267,32 +267,6 @@ public void testDisablingLegacyCrosstoolFields() throws Exception {
             + "migrate your CROSSTOOL");
   }
 
-  @Test
-  public void testDisablingExpandIfAllAvailable() throws Exception {
-    reporter.removeHandler(failFastHandler);
-    AnalysisMock.get()
-        .ccSupport()
-        .setupCrosstool(
-            mockToolsConfig,
-            "feature { ",
-            "  name: 'foo'",
-            "  flag_set { expand_if_all_available: 'bar' }",
-            "}");
-
-    scratch.file("a/BUILD", "cc_library(name='a', srcs=['a.cc'])");
-
-    useConfiguration("--noincompatible_disable_expand_if_all_available_in_flag_set");
-    getConfiguredTarget("//a");
-    assertNoEvents();
-
-    useConfiguration("--incompatible_disable_expand_if_all_available_in_flag_set");
-    getConfiguredTarget("//a");
-
-    assertContainsEvent(
-        "defines a flag_set with expand_if_all_available set. This is disabled by "
-            + "--incompatible_disable_expand_if_all_available_in_flag_set");
-  }
-
   /*
    * Crosstools should load fine with or without 'gcov-tool'. Those that define 'gcov-tool'
    * should also add a make variable.