Add --incompatible_require_ctx_in_configure_features
In order to migrate C++ rules to platforms, we need the access to the C++ configuration fragment from the caller rule in Starlark APIs. All existing APIs have already access to it, but cc_common.configure_features doesn't. This incompatible change adds a ctx argument to configure_features.
https://github.com/bazelbuild/bazel/issues/7793
https://github.com/bazelbuild/bazel/issues/6516
RELNOTES: Added `--incompatible_require_ctx_in_configure_features`, see https://github.com/bazelbuild/bazel/issues/7793 for details.
PiperOrigin-RevId: 240099411
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index a46e459..88bff3d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -800,7 +800,10 @@
CcToolchainProvider toolchain) {
try {
return configureFeaturesOrThrowEvalException(
- requestedFeatures, unsupportedFeatures, toolchain);
+ requestedFeatures,
+ unsupportedFeatures,
+ toolchain,
+ ruleContext.getFragment(CppConfiguration.class));
} catch (EvalException e) {
ruleContext.ruleError(e.getMessage());
return FeatureConfiguration.EMPTY;
@@ -810,9 +813,9 @@
public static FeatureConfiguration configureFeaturesOrThrowEvalException(
ImmutableSet<String> requestedFeatures,
ImmutableSet<String> unsupportedFeatures,
- CcToolchainProvider toolchain)
+ CcToolchainProvider toolchain,
+ CppConfiguration cppConfiguration)
throws EvalException {
- CppConfiguration cppConfiguration = toolchain.getCppConfiguration();
ImmutableSet.Builder<String> allRequestedFeaturesBuilder = ImmutableSet.builder();
ImmutableSet.Builder<String> unsupportedFeaturesBuilder = ImmutableSet.builder();
unsupportedFeaturesBuilder.addAll(unsupportedFeatures);
@@ -1010,8 +1013,25 @@
private static List<String> computeCcFlagsFromFeatureConfig(
RuleContext ruleContext, CcToolchainProvider toolchainProvider) {
- FeatureConfiguration featureConfiguration =
- CcCommon.configureFeaturesOrReportRuleError(ruleContext, toolchainProvider);
+ FeatureConfiguration featureConfiguration = null;
+ CppConfiguration cppConfiguration =
+ toolchainProvider.getCppConfigurationEvenThoughItCanBeDifferentThatWhatTargetHas();
+ if (cppConfiguration.requireCtxInConfigureFeatures()) {
+ // When this is flipped, this whole method will go away. But I'm keeping it there
+ // so we can experiment with flags before they are flipped.
+ Preconditions.checkArgument(cppConfiguration.disableGenruleCcToolchainDependency());
+ cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
+ }
+ try {
+ featureConfiguration =
+ configureFeaturesOrThrowEvalException(
+ ruleContext.getFeatures(),
+ ruleContext.getDisabledFeatures(),
+ toolchainProvider,
+ cppConfiguration);
+ } catch (EvalException e) {
+ ruleContext.ruleError(e.getMessage());
+ }
if (featureConfiguration.actionIsConfigured(CppActionNames.CC_FLAGS_MAKE_VARIABLE)) {
CcToolchainVariables buildVariables = toolchainProvider.getBuildVariables();
return featureConfiguration.getCommandLine(
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 69e9785..6f35bc8 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
@@ -133,14 +133,30 @@
@Override
public FeatureConfiguration configureFeatures(
+ Object ruleContextOrNone,
CcToolchainProvider toolchain,
SkylarkList<String> requestedFeatures,
SkylarkList<String> unsupportedFeatures)
throws EvalException {
+ SkylarkRuleContext ruleContext = nullIfNone(ruleContextOrNone, SkylarkRuleContext.class);
+ if (ruleContext == null
+ && toolchain
+ .getCppConfigurationEvenThoughItCanBeDifferentThatWhatTargetHas()
+ .requireCtxInConfigureFeatures()) {
+ throw new EvalException(
+ Location.BUILTIN,
+ "Incompatible flag --incompatible_require_ctx_in_configure_features has been flipped, "
+ + "and the mandatory parameter 'ctx' of cc_common.configure_features is missing. "
+ + "Please add 'ctx' as a named parameter. See "
+ + "https://github.com/bazelbuild/bazel/issues/7793 for details.");
+ }
return CcCommon.configureFeaturesOrThrowEvalException(
ImmutableSet.copyOf(requestedFeatures),
ImmutableSet.copyOf(unsupportedFeatures),
- toolchain);
+ toolchain,
+ ruleContext == null
+ ? toolchain.getCppConfigurationEvenThoughItCanBeDifferentThatWhatTargetHas()
+ : ruleContext.getRuleContext().getFragment(CppConfiguration.class));
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java
index 930e76e..9b0a35b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java
@@ -626,11 +626,33 @@
|| featureConfiguration.isEnabled(CppRuleClasses.SUPPORTS_INTERFACE_SHARED_LIBRARIES);
}
+ /**
+ * Deprecated, do not use. Read the javadoc for {@link
+ * #getCppConfigurationEvenThoughItCanBeDifferentThatWhatTargetHas()}.
+ */
@Nullable
+ @Deprecated
public CppConfiguration getCppConfiguration() {
return cppConfiguration;
}
+ /**
+ * Return CppConfiguration instance that was used to configure CcToolchain.
+ *
+ * <p>If C++ rules use platforms/toolchains without
+ * https://github.com/bazelbuild/proposals/blob/master/designs/2019-02-12-toolchain-transitions.md
+ * implemented, CcToolchain is analyzed in the host configuration. This configuration is not what
+ * should be used by rules using the toolchain. This method should only be used to access stuff
+ * from CppConfiguration that is identical between host and target (e.g. incompatible flag
+ * values). Don't use it if you don't know what you're doing.
+ *
+ * <p>Once toolchain transitions are implemented, we can safely use the CppConfiguration from the
+ * toolchain in rules.
+ */
+ public CppConfiguration getCppConfigurationEvenThoughItCanBeDifferentThatWhatTargetHas() {
+ return cppConfiguration;
+ }
+
/** Returns build variables to be templated into the crosstool. */
public CcToolchainVariables getBuildVariables() {
return buildVariables;
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 defe913..221949f 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
@@ -605,4 +605,8 @@
public boolean disableCcContextQuoteIncludesHook() {
return cppOptions.disableCcContextQuoteIncludesHook;
}
+
+ public boolean requireCtxInConfigureFeatures() {
+ return cppOptions.requireCtxInConfigureFeatures;
+ }
}
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 21a49ee..fdadbce 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
@@ -708,6 +708,20 @@
public boolean dontEnableHostNonhost;
@Option(
+ name = "incompatible_require_ctx_in_configure_features",
+ 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 require 'ctx' parameter in to cc_common.configure_features "
+ + "(see https://github.com/bazelbuild/bazel/issues/7793 for more information).")
+ public boolean requireCtxInConfigureFeatures;
+
+ @Option(
name = "incompatible_disable_legacy_crosstool_fields",
oldName = "experimental_disable_legacy_crosstool_fields",
defaultValue = "true",
@@ -909,6 +923,7 @@
host.enableCcToolchainResolution = enableCcToolchainResolution;
host.removeLegacyWholeArchive = removeLegacyWholeArchive;
host.dontEnableHostNonhost = dontEnableHostNonhost;
+ host.requireCtxInConfigureFeatures = requireCtxInConfigureFeatures;
return host;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/BazelCcModuleApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/BazelCcModuleApi.java
index 3e454c6..f97bf0b 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/BazelCcModuleApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/BazelCcModuleApi.java
@@ -34,7 +34,7 @@
public interface BazelCcModuleApi<
FileT extends FileApi,
SkylarkRuleContextT extends SkylarkRuleContextApi,
- CcToolchainProviderT extends CcToolchainProviderApi,
+ CcToolchainProviderT extends CcToolchainProviderApi<FeatureConfigurationT>,
FeatureConfigurationT extends FeatureConfigurationApi,
CompilationInfoT extends CompilationInfoApi,
CcCompilationContextT extends CcCompilationContextApi,
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcBootstrap.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcBootstrap.java
index a06572d..d901b94 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcBootstrap.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcBootstrap.java
@@ -27,7 +27,7 @@
private final BazelCcModuleApi<
? extends FileApi,
? extends SkylarkRuleContextApi,
- ? extends CcToolchainProviderApi,
+ ? extends CcToolchainProviderApi<? extends FeatureConfigurationApi>,
? extends FeatureConfigurationApi,
? extends CompilationInfoApi,
? extends CcCompilationContextApi,
@@ -43,7 +43,7 @@
BazelCcModuleApi<
? extends FileApi,
? extends SkylarkRuleContextApi,
- ? extends CcToolchainProviderApi,
+ ? extends CcToolchainProviderApi<? extends FeatureConfigurationApi>,
? extends FeatureConfigurationApi,
? extends CompilationInfoApi,
? extends CcCompilationContextApi,
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcModuleApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcModuleApi.java
index fa3792b..8e82d72 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcModuleApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcModuleApi.java
@@ -68,6 +68,14 @@
doc = "Creates a feature_configuration instance.",
parameters = {
@Param(
+ name = "ctx",
+ positional = false,
+ named = true,
+ noneable = true,
+ defaultValue = "None",
+ type = SkylarkRuleContextApi.class,
+ doc = "The rule context."),
+ @Param(
name = "cc_toolchain",
doc = "cc_toolchain for which we configure features.",
positional = false,
@@ -89,6 +97,7 @@
type = SkylarkList.class),
})
FeatureConfigurationT configureFeatures(
+ Object ruleContextOrNone,
CcToolchainProviderT toolchain,
SkylarkList<String> requestedFeatures,
SkylarkList<String> unsupportedFeatures)
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/cpp/FakeCcModule.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/cpp/FakeCcModule.java
index ae7a17b..3fafa90 100644
--- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/cpp/FakeCcModule.java
+++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/cpp/FakeCcModule.java
@@ -43,7 +43,7 @@
implements BazelCcModuleApi<
FileApi,
SkylarkRuleContextApi,
- CcToolchainProviderApi,
+ CcToolchainProviderApi<FeatureConfigurationApi>,
FeatureConfigurationApi,
CompilationInfoApi,
CcCompilationContextApi,
@@ -60,8 +60,11 @@
}
@Override
- public FeatureConfigurationApi configureFeatures(CcToolchainProviderApi toolchain,
- SkylarkList<String> requestedFeatures, SkylarkList<String> unsupportedFeatures)
+ public FeatureConfigurationApi configureFeatures(
+ Object ruleContextOrNone,
+ CcToolchainProviderApi<FeatureConfigurationApi> toolchain,
+ SkylarkList<String> requestedFeatures,
+ SkylarkList<String> unsupportedFeatures)
throws EvalException {
return null;
}
@@ -168,7 +171,7 @@
public CompilationInfoApi compile(
SkylarkRuleContextApi skylarkRuleContext,
FeatureConfigurationApi skylarkFeatureConfiguration,
- CcToolchainProviderApi skylarkCcToolchainProvider,
+ CcToolchainProviderApi<FeatureConfigurationApi> skylarkCcToolchainProvider,
SkylarkList<FileApi> sources,
SkylarkList<FileApi> headers,
Object skylarkIncludes,
@@ -182,7 +185,7 @@
public LinkingInfoApi link(
SkylarkRuleContextApi skylarkRuleContext,
FeatureConfigurationApi skylarkFeatureConfiguration,
- CcToolchainProviderApi skylarkCcToolchainProvider,
+ CcToolchainProviderApi<FeatureConfigurationApi> skylarkCcToolchainProvider,
CcCompilationOutputsApi ccCompilationOutputs,
Object skylarkLinkopts,
Object dynamicLibrary,
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java
index a64c3e5..b0115fa 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java
@@ -187,7 +187,8 @@
CcCommon.configureFeaturesOrThrowEvalException(
/* requestedFeatures= */ ImmutableSet.of(),
/* unsupportedFeatures= */ ImmutableSet.of(),
- toolchainProvider);
+ toolchainProvider,
+ getRuleContext(target).getFragment(CppConfiguration.class));
return CppHelper.usePicForBinaries(toolchainProvider, featureConfiguration);
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelperTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelperTest.java
index 3fdde9b..29df277 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelperTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelperTest.java
@@ -231,7 +231,8 @@
CcCommon.configureFeaturesOrThrowEvalException(
/* requestedFeatures= */ ImmutableSet.of(),
/* unsupportedFeatures= */ ImmutableSet.of(),
- toolchain);
+ toolchain,
+ getRuleContext(target).getFragment(CppConfiguration.class));
boolean usePic = CppHelper.usePicForBinaries(toolchain, featureConfiguration);
CppLinkAction generatingAction = (CppLinkAction) getGeneratingAction(executable);
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
index e7f29e2..5b76f1d 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
@@ -135,7 +135,10 @@
ruleContext, ruleContext.getPrerequisite("$cc_toolchain", Mode.TARGET));
FeatureConfiguration featureConfiguration =
CcCommon.configureFeaturesOrThrowEvalException(
- ImmutableSet.of(), ImmutableSet.of(), toolchain);
+ ImmutableSet.of(),
+ ImmutableSet.of(),
+ toolchain,
+ ruleContext.getFragment(CppConfiguration.class));
assertThat(actionToolPath)
.isEqualTo(featureConfiguration.getToolPathForAction(CppActionNames.CPP_COMPILE));
}
@@ -251,7 +254,10 @@
ruleContext, ruleContext.getPrerequisite("$cc_toolchain", Mode.TARGET));
FeatureConfiguration featureConfiguration =
CcCommon.configureFeaturesOrThrowEvalException(
- ImmutableSet.of(), ImmutableSet.of(), toolchain);
+ ImmutableSet.of(),
+ ImmutableSet.of(),
+ toolchain,
+ ruleContext.getFragment(CppConfiguration.class));
assertThat(commandLine)
.containsExactlyElementsIn(
featureConfiguration.getCommandLine("c++-link-executable", CcToolchainVariables.EMPTY));
@@ -293,7 +299,10 @@
ruleContext, ruleContext.getPrerequisite("$cc_toolchain", Mode.TARGET));
FeatureConfiguration featureConfiguration =
CcCommon.configureFeaturesOrThrowEvalException(
- ImmutableSet.of(), ImmutableSet.of(), toolchain);
+ ImmutableSet.of(),
+ ImmutableSet.of(),
+ toolchain,
+ ruleContext.getFragment(CppConfiguration.class));
assertThat(environmentVariables)
.containsExactlyEntriesIn(
featureConfiguration.getEnvironmentVariables(
@@ -375,6 +384,34 @@
}
@Test
+ public void testFeatureConfigurationRequiresCtx() throws Exception {
+ scratch.file(
+ "a/BUILD",
+ "load(':rule.bzl', 'crule')",
+ "cc_toolchain_alias(name='alias')",
+ "crule(name='r')");
+
+ scratch.file(
+ "a/rule.bzl",
+ "def _impl(ctx):",
+ " toolchain = ctx.attr._cc_toolchain[cc_common.CcToolchainInfo]",
+ " feature_configuration = cc_common.configure_features(cc_toolchain = toolchain)",
+ " return struct()",
+ "crule = rule(",
+ " _impl,",
+ " attrs = { ",
+ " '_cc_toolchain': attr.label(default=Label('//a:alias'))",
+ " },",
+ " fragments = ['cpp'],",
+ ");");
+ useConfiguration("--incompatible_require_ctx_in_configure_features");
+ reporter.removeHandler(failFastHandler);
+
+ getConfiguredTarget("//a:r");
+ assertContainsEvent("mandatory parameter 'ctx' of cc_common.configure_features is missing");
+ }
+
+ @Test
public void testActionNames() throws Exception {
scratch.file(
"a/BUILD",
