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.