Skip to content

Commit

Permalink
Add incompatible flag to enable cc rules to use toolchain resolution.
Browse files Browse the repository at this point in the history
New flag: --incompatible_enable_cc_toolchain_resolution

This deprecates and replaces the previous flag, --enabled_toolchain_types.

Part of #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
  • Loading branch information
katre authored and Copybara-Service committed Jan 28, 2019
1 parent 8604c6a commit 2bfb470
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -131,16 +131,18 @@ public class PlatformOptions extends FragmentOptions {
public boolean toolchainResolutionDebug;

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

@Option(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,11 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.cpp;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.LicensesProvider;
import com.google.devtools.build.lib.analysis.MiddlemanProvider;
import com.google.devtools.build.lib.analysis.PlatformConfiguration;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.analysis.RuleContext;
Expand Down Expand Up @@ -57,10 +55,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
ruleConfiguredTargetBuilder.add(LicensesProvider.class, attributes.getLicensesProvider());
}

PlatformConfiguration platformConfig =
Preconditions.checkNotNull(ruleContext.getFragment(PlatformConfiguration.class));
if (!platformConfig.isToolchainTypeEnabled(
CppHelper.getToolchainTypeFromRuleClass(ruleContext))) {
if (!CppHelper.useToolchainResolution(ruleContext)) {
// This is not a platforms-backed build, let's provide CcToolchainAttributesProvider
// and have cc_toolchain_suite select one of its toolchains and create CcToolchainProvider
// from its attributes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@
package com.google.devtools.build.lib.rules.cpp;


import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.LicensesProvider;
import com.google.devtools.build.lib.analysis.MiddlemanProvider;
import com.google.devtools.build.lib.analysis.PlatformConfiguration;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.analysis.RuleContext;
Expand Down Expand Up @@ -66,10 +64,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
}
Label selectedCcToolchain = toolchains.get(key);
CcToolchainProvider ccToolchainProvider;
PlatformConfiguration platformConfig =
Preconditions.checkNotNull(ruleContext.getFragment(PlatformConfiguration.class));
if (platformConfig.isToolchainTypeEnabled(
CppHelper.getToolchainTypeFromRuleClass(ruleContext))) {

if (CppHelper.useToolchainResolution(ruleContext)) {
// This is a platforms build (and the user requested to build this suite explicitly).
// Cc_toolchains provide CcToolchainInfo already. Let's select the CcToolchainProvider from
// toolchains and provide it here as well.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,17 +106,6 @@ public static TransitiveInfoCollection mallocForTarget(RuleContext ruleContext)
return mallocForTarget(ruleContext, "malloc");
}

/**
* Returns true if this target should obtain c++ make variables from the toolchain instead of from
* the configuration.
*/
public static boolean shouldUseToolchainForMakeVariables(RuleContext ruleContext) {
Label toolchainType = getToolchainTypeFromRuleClass(ruleContext);
return ruleContext
.getFragment(PlatformConfiguration.class)
.isToolchainTypeEnabled(toolchainType);
}

/**
* Expands Make variables in a list of string and tokenizes the result. If the package feature
* no_copts_tokenization is set, tokenize only items consisting of a single make variable.
Expand Down Expand Up @@ -346,10 +335,7 @@ public static CcToolchainProvider getToolchain(
RuleContext ruleContext, TransitiveInfoCollection dep) {

Label toolchainType = getToolchainTypeFromRuleClass(ruleContext);
if (toolchainType != null
&& ruleContext
.getFragment(PlatformConfiguration.class)
.isToolchainTypeEnabled(toolchainType)) {
if (toolchainType != null && useToolchainResolution(ruleContext)) {
return getToolchainFromPlatformConstraints(ruleContext, toolchainType);
}
return getToolchainFromCrosstoolTop(ruleContext, dep);
Expand Down Expand Up @@ -866,4 +852,19 @@ public static void checkProtoLibrariesInDeps(RuleContext ruleContext,
}
}
}

static boolean useToolchainResolution(RuleContext ruleContext) {
CppOptions cppOptions =
Preconditions.checkNotNull(
ruleContext.getConfiguration().getOptions().get(CppOptions.class));

if (cppOptions.enableCcToolchainResolution) {
return true;
}

// TODO(https://github.com/bazelbuild/bazel/issues/7260): Remove this and the flag.
PlatformConfiguration platformConfig =
Preconditions.checkNotNull(ruleContext.getFragment(PlatformConfiguration.class));
return platformConfig.isToolchainTypeEnabled(getToolchainTypeFromRuleClass(ruleContext));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,18 @@ public Label getFdoPrefetchHintsLabel() {
// TODO(b/122328491): Document migration steps. See #7036.
public boolean disableLegacyCcProvider;

@Option(
name = "incompatible_enable_cc_toolchain_resolution",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help = "If true, cc rules use toolchain resolution to find the cc_toolchain.")
public boolean enableCcToolchainResolution;

@Override
public FragmentOptions getHost() {
CppOptions host = (CppOptions) getDefault();
Expand Down Expand Up @@ -896,6 +908,7 @@ public FragmentOptions getHost() {
host.disableLegacyCcProvider = disableLegacyCcProvider;
host.removeCpuCompilerCcToolchainAttributes = removeCpuCompilerCcToolchainAttributes;
host.disableLegacyCrosstoolFields = disableLegacyCrosstoolFields;
host.enableCcToolchainResolution = enableCcToolchainResolution;
return host;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,7 @@ public void testCcTargetsDependOnCcToolchainAutomatically() throws Exception {
")");

useConfiguration(
"--enabled_toolchain_types="
+ TestConstants.TOOLS_REPOSITORY
+ "//tools/cpp:toolchain_type",
"--incompatible_enable_cc_toolchain_resolution",
"--experimental_platforms=//a:mock-platform",
"--extra_toolchains=//a:toolchain_b");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public void testResolvedCcToolchain() throws Exception {
String crosstool = analysisMock.ccSupport().readCrosstoolFile();
getAnalysisMock().ccSupport().setupCrosstoolWithRelease(mockToolsConfig, crosstool);
useConfiguration(
"--enabled_toolchain_types=" + CPP_TOOLCHAIN_TYPE,
"--incompatible_enable_cc_toolchain_resolution",
"--experimental_platforms=//mock_platform:mock-piii-platform",
"--extra_toolchains=//mock_platform:toolchain_cc-compiler-piii");
ConfiguredTarget target =
Expand All @@ -70,7 +70,7 @@ public void testToolchainSelectionWithPlatforms() throws Exception {
String crosstool = analysisMock.ccSupport().readCrosstoolFile();
getAnalysisMock().ccSupport().setupCrosstoolWithRelease(mockToolsConfig, crosstool);
useConfiguration(
"--enabled_toolchain_types=" + CPP_TOOLCHAIN_TYPE,
"--incompatible_enable_cc_toolchain_resolution",
"--experimental_platforms=//mock_platform:mock-piii-platform",
"--extra_toolchains=//mock_platform:toolchain_cc-compiler-piii");
ConfiguredTarget target =
Expand Down Expand Up @@ -110,7 +110,7 @@ public void testIncompleteCcToolchain() throws Exception {
"filegroup(name = 'dummy_filegroup')");

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

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

useConfiguration(
"--enabled_toolchain_types=" + CPP_TOOLCHAIN_TYPE,
"--incompatible_enable_cc_toolchain_resolution",
"--experimental_platforms=//mock_platform:mock-piii-platform",
"--extra_toolchains=//mock_platform:toolchain_cc-compiler-piii");
ConfiguredTarget target =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@
*/
@RunWith(JUnit4.class)
public class CcToolchainTest extends BuildViewTestCase {
private static final String CPP_TOOLCHAIN_TYPE =
TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type";

@Test
public void testFilesToBuild() throws Exception {
Expand Down Expand Up @@ -635,7 +633,7 @@ public void testInlineCtoolchain_withToolchainResolution() throws Exception {
.setAbiVersion("orange")
.buildPartial());

useConfiguration("--enabled_toolchain_types=" + CPP_TOOLCHAIN_TYPE);
useConfiguration("--incompatible_enable_cc_toolchain_resolution");

ConfiguredTarget target = getConfiguredTarget("//a:b");
CcToolchainProvider toolchainProvider =
Expand Down

0 comments on commit 2bfb470

Please sign in to comment.