From ff7b8783e451b9be9e214072019b5e81398aacb6 Mon Sep 17 00:00:00 2001 From: hlopko Date: Wed, 21 Nov 2018 05:28:53 -0800 Subject: [PATCH] Remove --make_variables_source, it's not needed, used, documented, or visible #6381 RELNOTES: None. PiperOrigin-RevId: 222393972 --- .../analysis/config/BuildConfiguration.java | 23 ------------------- .../lib/bazel/rules/BazelRulesModule.java | 8 +++++++ .../build/lib/rules/cpp/CcBinary.java | 5 +--- .../build/lib/rules/cpp/CcLibrary.java | 4 ---- .../build/lib/rules/cpp/CppConfiguration.java | 14 +---------- .../build/lib/rules/cpp/CppHelper.java | 10 +++----- .../build/lib/rules/ToolchainTypeTest.java | 3 +-- 7 files changed, 14 insertions(+), 53 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index fd0daaf6241859..1af93d2ad2e161 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -860,29 +860,6 @@ public static class Options extends FragmentOptions implements Cloneable { ) public Label autoCpuEnvironmentGroup; - /** The source of make variables for this configuration. */ - public enum MakeVariableSource { - CONFIGURATION, - TOOLCHAIN - } - - /** Converter for --make_variables_source. */ - public static class MakeVariableSourceConverter extends EnumConverter { - public MakeVariableSourceConverter() { - super(MakeVariableSource.class, "Make variable source"); - } - } - - @Option( - name = "make_variables_source", - converter = MakeVariableSourceConverter.class, - defaultValue = "configuration", - metadataTags = {OptionMetadataTag.HIDDEN}, - documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, - effectTags = {OptionEffectTag.UNKNOWN} - ) - public MakeVariableSource makeVariableSource; - /** Values for --experimental_dynamic_configs. */ public enum ConfigsMode { /** Only include the configuration fragments each rule needs. */ 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 c7c56d0b0b279f..0283eebbda4448 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 @@ -53,6 +53,14 @@ public static class GraveyardOptions extends OptionsBase { help = "Deprecated no-op.") public boolean disableMakeVariables; + @Option( + name = "make_variables_source", + defaultValue = "configuration", + metadataTags = {OptionMetadataTag.HIDDEN, OptionMetadataTag.DEPRECATED}, + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.UNKNOWN}) + public String makeVariableSource; + @Option( name = "incompatible_disable_legacy_flags_cc_toolchain_api", defaultValue = "true", diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java index fa433e5d325010..66e8e9f11e82c4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java @@ -241,15 +241,12 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont CcCommon common = new CcCommon(ruleContext); CcToolchainProvider ccToolchain = common.getToolchain(); - if (CppHelper.shouldUseToolchainForMakeVariables(ruleContext)) { ImmutableMap.Builder toolchainMakeVariables = ImmutableMap.builder(); ccToolchain.addGlobalMakeVariables(toolchainMakeVariables); ruleContext.initConfigurationMakeVariableContext( new MapBackedMakeVariableSupplier(toolchainMakeVariables.build()), new CcFlagsSupplier(ruleContext)); - } else { - ruleContext.initConfigurationMakeVariableContext(new CcFlagsSupplier(ruleContext)); - } + CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); PrecompiledFiles precompiledFiles = new PrecompiledFiles(ruleContext); LinkTargetType linkType = diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java index 3e00a999b148b2..f6b8b9710a2dba 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java @@ -121,15 +121,11 @@ public static void init( CcToolchainProvider ccToolchain = common.getToolchain(); - if (CppHelper.shouldUseToolchainForMakeVariables(ruleContext)) { ImmutableMap.Builder toolchainMakeVariables = ImmutableMap.builder(); ccToolchain.addGlobalMakeVariables(toolchainMakeVariables); ruleContext.initConfigurationMakeVariableContext( new MapBackedMakeVariableSupplier(toolchainMakeVariables.build()), new CcFlagsSupplier(ruleContext)); - } else { - ruleContext.initConfigurationMakeVariableContext(new CcFlagsSupplier(ruleContext)); - } FdoProvider fdoProvider = common.getFdoProvider(); FeatureConfiguration featureConfiguration = 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 12c4cddcd3468c..8c7a8b342e8eb6 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 @@ -21,7 +21,6 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.config.AutoCpuConverter; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options.MakeVariableSource; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.CompilationMode; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; @@ -187,7 +186,6 @@ public String toString() { // it here so that the output directory doesn't depend on the CToolchain. When we will eventually // verify that the two are the same, we can remove one of desiredCpu and targetCpu. private final String desiredCpu; - private final PathFragment crosstoolTopPathFragment; private final PathFragment fdoPath; private final Label fdoOptimizeLabel; @@ -210,18 +208,14 @@ public String toString() { private final boolean stripBinaries; private final CompilationMode compilationMode; - private final boolean shouldProvideMakeVariables; - private final CppToolchainInfo cppToolchainInfo; static CppConfiguration create(CppConfigurationParameters params) throws InvalidConfigurationException { CppOptions cppOptions = params.cppOptions; - PathFragment crosstoolTopPathFragment = - params.crosstoolTop.getPackageIdentifier().getPathUnderExecRoot(); CppToolchainInfo cppToolchainInfo = CppToolchainInfo.create( - crosstoolTopPathFragment, + params.crosstoolTop.getPackageIdentifier().getPathUnderExecRoot(), params.ccToolchainLabel, params.ccToolchainConfigInfo, cppOptions.disableLegacyCrosstoolFields, @@ -242,7 +236,6 @@ static CppConfiguration create(CppConfigurationParameters params) params.transformedCpu, params.compiler, Preconditions.checkNotNull(params.commonOptions.cpu), - crosstoolTopPathFragment, params.fdoPath, params.fdoOptimizeLabel, params.ccToolchainLabel, @@ -258,7 +251,6 @@ static CppConfiguration create(CppConfigurationParameters params) || (cppOptions.stripBinaries == StripMode.SOMETIMES && compilationMode == CompilationMode.FASTBUILD)), compilationMode, - params.commonOptions.makeVariableSource == MakeVariableSource.CONFIGURATION, cppToolchainInfo); } @@ -268,7 +260,6 @@ private CppConfiguration( String transformedCpuFromOptions, String compilerFromOptions, String desiredCpu, - PathFragment crosstoolTopPathFragment, PathFragment fdoPath, Label fdoOptimizeLabel, Label ccToolchainLabel, @@ -282,14 +273,12 @@ private CppConfiguration( CppOptions cppOptions, boolean stripBinaries, CompilationMode compilationMode, - boolean shouldProvideMakeVariables, CppToolchainInfo cppToolchainInfo) { this.crosstoolTop = crosstoolTop; this.crosstoolFromCcToolchainProtoAttribute = crosstoolFromCcToolchainProtoAttribute; this.transformedCpuFromOptions = transformedCpuFromOptions; this.compilerFromOptions = compilerFromOptions; this.desiredCpu = desiredCpu; - this.crosstoolTopPathFragment = crosstoolTopPathFragment; this.fdoPath = fdoPath; this.fdoOptimizeLabel = fdoOptimizeLabel; this.ccToolchainLabel = ccToolchainLabel; @@ -303,7 +292,6 @@ private CppConfiguration( this.cppOptions = cppOptions; this.stripBinaries = stripBinaries; this.compilationMode = compilationMode; - this.shouldProvideMakeVariables = shouldProvideMakeVariables; this.cppToolchainInfo = cppToolchainInfo; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index d9173229822f3d..3538f5cc0ea29c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java @@ -43,8 +43,6 @@ import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.actions.SymlinkAction; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options.MakeVariableSource; import com.google.devtools.build.lib.analysis.config.CompilationMode; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; @@ -116,11 +114,9 @@ public static TransitiveInfoCollection mallocForTarget(RuleContext ruleContext) */ public static boolean shouldUseToolchainForMakeVariables(RuleContext ruleContext) { Label toolchainType = getToolchainTypeFromRuleClass(ruleContext); - return (ruleContext.getConfiguration().getOptions().get(Options.class).makeVariableSource - == MakeVariableSource.TOOLCHAIN) - && (ruleContext - .getFragment(PlatformConfiguration.class) - .isToolchainTypeEnabled(toolchainType)); + return ruleContext + .getFragment(PlatformConfiguration.class) + .isToolchainTypeEnabled(toolchainType); } /** diff --git a/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java b/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java index 4fc5153e07a46e..2ae3c2ea1f67c0 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java @@ -76,8 +76,7 @@ public void testCcTargetsDependOnCcToolchainAutomatically() throws Exception { + TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type", "--experimental_platforms=//a:mock-platform", - "--extra_toolchains=//a:toolchain_b", - "--make_variables_source=toolchain"); + "--extra_toolchains=//a:toolchain_b"); // for cc_library, cc_binary, and cc_test, we check that $(TARGET_CPU) is a valid Make variable ConfiguredTarget cclibrary =