Skip to content

Commit 62488e9

Browse files
katreweixiao-huang
authored andcommitted
Add incompatible flag to enable cc rules to use toolchain resolution.
New flag: --incompatible_enable_cc_toolchain_resolution This deprecates and replaces the previous flag, --enabled_toolchain_types. Part of bazelbuild#7260. RELNOTES[INC]: Toolchain resolution for cc rules is now enabled via an incompatible flag, --incompatible_enable_cc_toolchain_resolution. The previous flag, --enabled_toolchain_types, is deprecated and will be removed. PiperOrigin-RevId: 231277726
1 parent 2810e08 commit 62488e9

File tree

8 files changed

+50
-47
lines changed

8 files changed

+50
-47
lines changed

src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,16 +131,18 @@ public class PlatformOptions extends FragmentOptions {
131131
public boolean toolchainResolutionDebug;
132132

133133
@Option(
134-
name = "enabled_toolchain_types",
135-
defaultValue = "",
136-
converter = LabelListConverter.class,
137-
documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
138-
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
139-
help =
140-
"Enable toolchain resolution for the given toolchain type, if the rules used support that. "
141-
+ "This does not directly change the core Blaze machinery, but is a signal to "
142-
+ "participating rule implementations that toolchain resolution should be used."
143-
)
134+
name = "enabled_toolchain_types",
135+
defaultValue = "",
136+
converter = LabelListConverter.class,
137+
documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
138+
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
139+
deprecationWarning =
140+
"Use --incompatible_enable_cc_toolchain_resolution to enable toolchain for cc rules. "
141+
+ "Other rules will define separate flags as needed.",
142+
help =
143+
"Enable toolchain resolution for the given toolchain type, if the rules used support "
144+
+ "that. This does not directly change the core Blaze machinery, but is a signal to "
145+
+ "participating rule implementations that toolchain resolution should be used.")
144146
public List<Label> enabledToolchainTypes;
145147

146148
@Option(

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,11 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.rules.cpp;
1515

16-
import com.google.common.base.Preconditions;
1716
import com.google.common.collect.ImmutableMap;
1817
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
1918
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
2019
import com.google.devtools.build.lib.analysis.LicensesProvider;
2120
import com.google.devtools.build.lib.analysis.MiddlemanProvider;
22-
import com.google.devtools.build.lib.analysis.PlatformConfiguration;
2321
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
2422
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
2523
import com.google.devtools.build.lib.analysis.RuleContext;
@@ -57,10 +55,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
5755
ruleConfiguredTargetBuilder.add(LicensesProvider.class, attributes.getLicensesProvider());
5856
}
5957

60-
PlatformConfiguration platformConfig =
61-
Preconditions.checkNotNull(ruleContext.getFragment(PlatformConfiguration.class));
62-
if (!platformConfig.isToolchainTypeEnabled(
63-
CppHelper.getToolchainTypeFromRuleClass(ruleContext))) {
58+
if (!CppHelper.useToolchainResolution(ruleContext)) {
6459
// This is not a platforms-backed build, let's provide CcToolchainAttributesProvider
6560
// and have cc_toolchain_suite select one of its toolchains and create CcToolchainProvider
6661
// from its attributes.

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,10 @@
1414
package com.google.devtools.build.lib.rules.cpp;
1515

1616

17-
import com.google.common.base.Preconditions;
1817
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
1918
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
2019
import com.google.devtools.build.lib.analysis.LicensesProvider;
2120
import com.google.devtools.build.lib.analysis.MiddlemanProvider;
22-
import com.google.devtools.build.lib.analysis.PlatformConfiguration;
2321
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
2422
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
2523
import com.google.devtools.build.lib.analysis.RuleContext;
@@ -66,10 +64,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
6664
}
6765
Label selectedCcToolchain = toolchains.get(key);
6866
CcToolchainProvider ccToolchainProvider;
69-
PlatformConfiguration platformConfig =
70-
Preconditions.checkNotNull(ruleContext.getFragment(PlatformConfiguration.class));
71-
if (platformConfig.isToolchainTypeEnabled(
72-
CppHelper.getToolchainTypeFromRuleClass(ruleContext))) {
67+
68+
if (CppHelper.useToolchainResolution(ruleContext)) {
7369
// This is a platforms build (and the user requested to build this suite explicitly).
7470
// Cc_toolchains provide CcToolchainInfo already. Let's select the CcToolchainProvider from
7571
// toolchains and provide it here as well.

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

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -106,17 +106,6 @@ public static TransitiveInfoCollection mallocForTarget(RuleContext ruleContext)
106106
return mallocForTarget(ruleContext, "malloc");
107107
}
108108

109-
/**
110-
* Returns true if this target should obtain c++ make variables from the toolchain instead of from
111-
* the configuration.
112-
*/
113-
public static boolean shouldUseToolchainForMakeVariables(RuleContext ruleContext) {
114-
Label toolchainType = getToolchainTypeFromRuleClass(ruleContext);
115-
return ruleContext
116-
.getFragment(PlatformConfiguration.class)
117-
.isToolchainTypeEnabled(toolchainType);
118-
}
119-
120109
/**
121110
* Expands Make variables in a list of string and tokenizes the result. If the package feature
122111
* no_copts_tokenization is set, tokenize only items consisting of a single make variable.
@@ -346,10 +335,7 @@ public static CcToolchainProvider getToolchain(
346335
RuleContext ruleContext, TransitiveInfoCollection dep) {
347336

348337
Label toolchainType = getToolchainTypeFromRuleClass(ruleContext);
349-
if (toolchainType != null
350-
&& ruleContext
351-
.getFragment(PlatformConfiguration.class)
352-
.isToolchainTypeEnabled(toolchainType)) {
338+
if (toolchainType != null && useToolchainResolution(ruleContext)) {
353339
return getToolchainFromPlatformConstraints(ruleContext, toolchainType);
354340
}
355341
return getToolchainFromCrosstoolTop(ruleContext, dep);
@@ -866,4 +852,19 @@ public static void checkProtoLibrariesInDeps(RuleContext ruleContext,
866852
}
867853
}
868854
}
855+
856+
static boolean useToolchainResolution(RuleContext ruleContext) {
857+
CppOptions cppOptions =
858+
Preconditions.checkNotNull(
859+
ruleContext.getConfiguration().getOptions().get(CppOptions.class));
860+
861+
if (cppOptions.enableCcToolchainResolution) {
862+
return true;
863+
}
864+
865+
// TODO(https://github.com/bazelbuild/bazel/issues/7260): Remove this and the flag.
866+
PlatformConfiguration platformConfig =
867+
Preconditions.checkNotNull(ruleContext.getFragment(PlatformConfiguration.class));
868+
return platformConfig.isToolchainTypeEnabled(getToolchainTypeFromRuleClass(ruleContext));
869+
}
869870
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,18 @@ public Label getFdoPrefetchHintsLabel() {
845845
// TODO(b/122328491): Document migration steps. See #7036.
846846
public boolean disableLegacyCcProvider;
847847

848+
@Option(
849+
name = "incompatible_enable_cc_toolchain_resolution",
850+
defaultValue = "false",
851+
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
852+
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
853+
metadataTags = {
854+
OptionMetadataTag.INCOMPATIBLE_CHANGE,
855+
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
856+
},
857+
help = "If true, cc rules use toolchain resolution to find the cc_toolchain.")
858+
public boolean enableCcToolchainResolution;
859+
848860
@Override
849861
public FragmentOptions getHost() {
850862
CppOptions host = (CppOptions) getDefault();
@@ -896,6 +908,7 @@ public FragmentOptions getHost() {
896908
host.disableLegacyCcProvider = disableLegacyCcProvider;
897909
host.removeCpuCompilerCcToolchainAttributes = removeCpuCompilerCcToolchainAttributes;
898910
host.disableLegacyCrosstoolFields = disableLegacyCrosstoolFields;
911+
host.enableCcToolchainResolution = enableCcToolchainResolution;
899912
return host;
900913
}
901914

src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,7 @@ public void testCcTargetsDependOnCcToolchainAutomatically() throws Exception {
7070
")");
7171

7272
useConfiguration(
73-
"--enabled_toolchain_types="
74-
+ TestConstants.TOOLS_REPOSITORY
75-
+ "//tools/cpp:toolchain_type",
73+
"--incompatible_enable_cc_toolchain_resolution",
7674
"--experimental_platforms=//a:mock-platform",
7775
"--extra_toolchains=//a:toolchain_b");
7876

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public void testResolvedCcToolchain() throws Exception {
4949
String crosstool = analysisMock.ccSupport().readCrosstoolFile();
5050
getAnalysisMock().ccSupport().setupCrosstoolWithRelease(mockToolsConfig, crosstool);
5151
useConfiguration(
52-
"--enabled_toolchain_types=" + CPP_TOOLCHAIN_TYPE,
52+
"--incompatible_enable_cc_toolchain_resolution",
5353
"--experimental_platforms=//mock_platform:mock-piii-platform",
5454
"--extra_toolchains=//mock_platform:toolchain_cc-compiler-piii");
5555
ConfiguredTarget target =
@@ -70,7 +70,7 @@ public void testToolchainSelectionWithPlatforms() throws Exception {
7070
String crosstool = analysisMock.ccSupport().readCrosstoolFile();
7171
getAnalysisMock().ccSupport().setupCrosstoolWithRelease(mockToolsConfig, crosstool);
7272
useConfiguration(
73-
"--enabled_toolchain_types=" + CPP_TOOLCHAIN_TYPE,
73+
"--incompatible_enable_cc_toolchain_resolution",
7474
"--experimental_platforms=//mock_platform:mock-piii-platform",
7575
"--extra_toolchains=//mock_platform:toolchain_cc-compiler-piii");
7676
ConfiguredTarget target =
@@ -110,7 +110,7 @@ public void testIncompleteCcToolchain() throws Exception {
110110
"filegroup(name = 'dummy_filegroup')");
111111

112112
useConfiguration(
113-
"--enabled_toolchain_types=" + CPP_TOOLCHAIN_TYPE,
113+
"--incompatible_enable_cc_toolchain_resolution",
114114
"--experimental_platforms=//mock_platform:mock-piii-platform",
115115
"--extra_toolchains=//incomplete_toolchain:incomplete_toolchain_cc-compiler-piii");
116116

@@ -129,7 +129,7 @@ public void testToolPaths() throws Exception {
129129
getAnalysisMock().ccSupport().setupCrosstoolWithRelease(mockToolsConfig, crosstoolWithPiiiLd);
130130

131131
useConfiguration(
132-
"--enabled_toolchain_types=" + CPP_TOOLCHAIN_TYPE,
132+
"--incompatible_enable_cc_toolchain_resolution",
133133
"--experimental_platforms=//mock_platform:mock-piii-platform",
134134
"--extra_toolchains=//mock_platform:toolchain_cc-compiler-piii");
135135
ConfiguredTarget target =

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,6 @@
4848
*/
4949
@RunWith(JUnit4.class)
5050
public class CcToolchainTest extends BuildViewTestCase {
51-
private static final String CPP_TOOLCHAIN_TYPE =
52-
TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type";
5351

5452
@Test
5553
public void testFilesToBuild() throws Exception {
@@ -635,7 +633,7 @@ public void testInlineCtoolchain_withToolchainResolution() throws Exception {
635633
.setAbiVersion("orange")
636634
.buildPartial());
637635

638-
useConfiguration("--enabled_toolchain_types=" + CPP_TOOLCHAIN_TYPE);
636+
useConfiguration("--incompatible_enable_cc_toolchain_resolution");
639637

640638
ConfiguredTarget target = getConfiguredTarget("//a:b");
641639
CcToolchainProvider toolchainProvider =

0 commit comments

Comments
 (0)