Automated rollback of commit 4eea5c62a566d21832c93e4c18ec559e75d5c1ce.
PiperOrigin-RevId: 234756779
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 3e911bc..08f8238 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,19 +41,6 @@
/** 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 3e3bd45..79f7fd9 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
@@ -1296,7 +1296,8 @@
withFeatureSetBuilder.add(withFeatureSetFromSkylark(withFeatureSetStruct));
}
- return new FlagSet(actions, withFeatureSetBuilder.build(), flagGroupsBuilder.build());
+ return new FlagSet(
+ actions, ImmutableSet.of(), 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 2747cb1..523ce51 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,6 +184,8 @@
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));
@@ -312,6 +314,26 @@
}
}
+ 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:
@@ -805,7 +827,8 @@
.addAllWithFeature(
flagSet.getWithFeatureSets().stream()
.map(withFeatureSet -> withFeatureSetToProto(withFeatureSet))
- .collect(ImmutableList.toImmutableList()));
+ .collect(ImmutableList.toImmutableList()))
+ .addAllExpandIfAllAvailable(flagSet.getExpandIfAllAvailable());
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 f0bc623..6566790 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,6 +516,7 @@
@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;
@@ -526,6 +527,7 @@
/** 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));
@@ -541,9 +543,11 @@
@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;
}
@@ -555,6 +559,11 @@
Set<String> enabledFeatureNames,
@Nullable ArtifactExpander expander,
List<String> commandLine) {
+ for (String variable : expandIfAllAvailable) {
+ if (!variables.isAvailable(variable, expander)) {
+ return;
+ }
+ }
if (!isWithFeaturesSatisfied(withFeatureSets, enabledFeatureNames)) {
return;
}
@@ -571,6 +580,7 @@
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);
}
@@ -579,13 +589,17 @@
@Override
public int hashCode() {
- return Objects.hash(actions, withFeatureSets, flagGroups);
+ return Objects.hash(actions, expandIfAllAvailable, 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 7dd60c9..c519d72 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
@@ -544,6 +544,10 @@
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 526e9b0..0720ff6 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
@@ -728,6 +728,20 @@
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,
@@ -863,6 +877,7 @@
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 a6dfb96..ba8d143 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,7 +148,17 @@
// unconditionally for every action specified.
repeated WithFeatureSet with_feature = 3;
- // Legacy field, ignored by Bazel.
+ // 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.
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 8f314b4..9b06bb3 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 feffd4f..c9f7baf 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,11 +1149,12 @@
" name: 'a'",
" flag_set {",
" action: 'c++-compile'",
- " flag_group { expand_if_all_available: 'v' flag: '%{v}' }",
+ " expand_if_all_available: 'v'",
+ " flag_group { flag: '%{v}' }",
" }",
" flag_set {",
- " action: 'c++-compile'",
- " flag_group { flag: 'unconditional' }",
+ " action: 'c++-compile'",
+ " flag_group { flag: 'unconditional' }",
" }",
"}")
.getFeatureConfiguration(ImmutableSet.of("a"));
@@ -1170,20 +1171,19 @@
"feature {",
" name: 'a'",
" flag_set {",
- " action: 'c++-compile'",
- " flag_group { expand_if_all_available: 'v' flag: '%{v}' }",
+ " action: 'c++-compile'",
+ " expand_if_all_available: 'v'",
+ " flag_group { flag: '%{v}' }",
" }",
" flag_set {",
- " action: 'c++-compile'",
- " flag_group { ",
- " expand_if_all_available: 'v'",
- " expand_if_all_available: 'w'",
- " flag: '%{v}%{w}' ",
- " }",
+ " action: 'c++-compile'",
+ " expand_if_all_available: 'v'",
+ " expand_if_all_available: 'w'",
+ " flag_group { flag: '%{v}%{w}' }",
" }",
" flag_set {",
- " action: 'c++-compile'",
- " flag_group { flag: 'unconditional' }",
+ " action: 'c++-compile'",
+ " flag_group { flag: 'unconditional' }",
" }",
"}")
.getFeatureConfiguration(ImmutableSet.of("a"));
@@ -1200,21 +1200,19 @@
"feature {",
" name: 'a'",
" flag_set {",
- " action: 'c++-compile'",
- " flag_group { expand_if_all_available: 'v' iterate_over: 'v' flag: '%{v}' }",
+ " action: 'c++-compile'",
+ " expand_if_all_available: 'v'",
+ " flag_group { iterate_over: 'v' flag: '%{v}' }",
" }",
" flag_set {",
- " action: 'c++-compile'",
- " flag_group { ",
- " iterate_over: 'v'",
- " expand_if_all_available: 'v'",
- " expand_if_all_available: 'w'",
- " flag: '%{v}%{w}'",
- " }",
+ " action: 'c++-compile'",
+ " expand_if_all_available: 'v'",
+ " expand_if_all_available: 'w'",
+ " flag_group { iterate_over: 'v' flag: '%{v}%{w}' }",
" }",
" flag_set {",
- " action: 'c++-compile'",
- " flag_group { flag: 'unconditional' }",
+ " action: 'c++-compile'",
+ " flag_group { flag: 'unconditional' }",
" }",
"}")
.getFeatureConfiguration(ImmutableSet.of("a"));
@@ -1234,16 +1232,15 @@
"feature {",
" name: 'a'",
" flag_set {",
- " action: 'c++-compile'",
- " flag_group { expand_if_all_available: 'v' iterate_over: 'v' flag: '%{v}' }",
+ " action: 'c++-compile'",
+ " expand_if_all_available: 'v'",
+ " flag_group { iterate_over: 'v' flag: '%{v}' }",
" }",
" flag_set {",
- " action: 'c++-compile'",
- " flag_group { ",
- " expand_if_all_available: 'v'",
- " expand_if_all_available: 'w'",
- " iterate_over: 'v' flag: '%{v}%{w}'",
- " }",
+ " action: 'c++-compile'",
+ " expand_if_all_available: 'v'",
+ " expand_if_all_available: 'w'",
+ " flag_group { 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 55a8fdd..cbbad1b 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,6 +267,32 @@
+ "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.
