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 (https://github.com/bazelbuild/bazel/issues/7008)
PiperOrigin-RevId: 234466411
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
index 08f8238..3e911bc 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
@@ -41,6 +41,19 @@
/** 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",
defaultValue = "true",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
index 8bfc0c8..ba7e48e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
@@ -1295,8 +1295,7 @@
withFeatureSetBuilder.add(withFeatureSetFromSkylark(withFeatureSetStruct));
}
- return new FlagSet(
- actions, ImmutableSet.of(), withFeatureSetBuilder.build(), flagGroupsBuilder.build());
+ return new FlagSet(actions, withFeatureSetBuilder.build(), flagGroupsBuilder.build());
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainConfigInfo.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainConfigInfo.java
index 523ce51..2747cb1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainConfigInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainConfigInfo.java
@@ -184,8 +184,6 @@
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 @@
}
}
- 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 @@
.addAllWithFeature(
flagSet.getWithFeatureSets().stream()
.map(withFeatureSet -> withFeatureSetToProto(withFeatureSet))
- .collect(ImmutableList.toImmutableList()))
- .addAllExpandIfAllAvailable(flagSet.getExpandIfAllAvailable());
+ .collect(ImmutableList.toImmutableList()));
if (!forActionConfig) {
flagSetBuilder.addAllAction(flagSet.getActions());
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
index 6566790..f0bc623 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
@@ -516,7 +516,6 @@
@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 @@
/** 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 @@
@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 @@
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 @@
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 @@
@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;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 5a614b4..804e5cb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -548,10 +548,6 @@
return cppOptions.disableLegacyCrosstoolFields;
}
- public boolean disableExpandIfAllAvailableInFlagSet() {
- return cppOptions.disableExpandIfAllAvailableInFlagSet;
- }
-
public static String getLegacyCrosstoolFieldErrorMessage(String field) {
Preconditions.checkNotNull(field);
return field
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
index 6e6415c..78fe766 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
@@ -744,20 +744,6 @@
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",
documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
@@ -893,7 +879,6 @@
host.doNotUseCpuTransformer = doNotUseCpuTransformer;
host.disableGenruleCcToolchainDependency = disableGenruleCcToolchainDependency;
host.disableDepsetInUserFlags = disableDepsetInUserFlags;
- host.disableExpandIfAllAvailableInFlagSet = disableExpandIfAllAvailableInFlagSet;
host.disableLegacyCcProvider = disableLegacyCcProvider;
host.removeCpuCompilerCcToolchainAttributes = removeCpuCompilerCcToolchainAttributes;
host.disableLegacyCrosstoolFields = disableLegacyCrosstoolFields;
diff --git a/src/main/protobuf/crosstool_config.proto b/src/main/protobuf/crosstool_config.proto
index ba8d143..a6dfb96 100644
--- a/src/main/protobuf/crosstool_config.proto
+++ b/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 @@
// 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;
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
index 9b06bb3..8f314b4 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
@@ -237,12 +237,12 @@
+ " 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}'"
+ " }"
+ " }"
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java
index c9f7baf..feffd4f 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java
@@ -1149,12 +1149,11 @@
" 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 @@
"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 @@
"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 @@
"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'",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java
index cbbad1b..55a8fdd 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java
@@ -267,32 +267,6 @@
+ "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.
