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.

#7793
#6516

RELNOTES: Added `--incompatible_require_ctx_in_configure_features`, see #7793 for details.
PiperOrigin-RevId: 240099411

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

@@ -800,7 +800,10 @@ public static FeatureConfiguration configureFeaturesOrReportRuleError(
       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 configureFeaturesOrReportRuleError(
   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 String computeCcFlagForSysroot(CcToolchainProvider toolchainProvi
 
   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(

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

@@ -133,14 +133,30 @@ public Provider getCcToolchainProvider() {
 
   @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

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

@@ -626,11 +626,33 @@ public boolean supportsInterfaceSharedLibraries(FeatureConfiguration featureConf
         || 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;

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

@@ -605,4 +605,8 @@ public boolean dontEnableHostNonhost() {
   public boolean disableCcContextQuoteIncludesHook() {
     return cppOptions.disableCcContextQuoteIncludesHook;
   }
+
+  public boolean requireCtxInConfigureFeatures() {
+    return cppOptions.requireCtxInConfigureFeatures;
+  }
 }

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

@@ -707,6 +707,20 @@ public Label getFdoPrefetchHintsLabel() {
               + "(see https://github.com/bazelbuild/bazel/issues/7407 for more information).")
   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",
@@ -909,6 +923,7 @@ public FragmentOptions getHost() {
     host.enableCcToolchainResolution = enableCcToolchainResolution;
     host.removeLegacyWholeArchive = removeLegacyWholeArchive;
     host.dontEnableHostNonhost = dontEnableHostNonhost;
+    host.requireCtxInConfigureFeatures = requireCtxInConfigureFeatures;
     return host;
   }
 

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,

src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcBootstrap.java

@@ -27,7 +27,7 @@ public class CcBootstrap implements Bootstrap {
   private final BazelCcModuleApi<
           ? extends FileApi,
           ? extends SkylarkRuleContextApi,
-          ? extends CcToolchainProviderApi,
+          ? extends CcToolchainProviderApi<? extends FeatureConfigurationApi>,
           ? extends FeatureConfigurationApi,
           ? extends CompilationInfoApi,
           ? extends CcCompilationContextApi,
@@ -43,7 +43,7 @@ public CcBootstrap(
       BazelCcModuleApi<
               ? extends FileApi,
               ? extends SkylarkRuleContextApi,
-              ? extends CcToolchainProviderApi,
+              ? extends CcToolchainProviderApi<? extends FeatureConfigurationApi>,
               ? extends FeatureConfigurationApi,
               ? extends CompilationInfoApi,
               ? extends CcCompilationContextApi,

src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcModuleApi.java

@@ -67,6 +67,14 @@ default void compilerFlagExists() {}
       name = "configure_features",
       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.",
@@ -89,6 +97,7 @@ default void compilerFlagExists() {}
             type = SkylarkList.class),
       })
   FeatureConfigurationT configureFeatures(
+      Object ruleContextOrNone,
       CcToolchainProviderT toolchain,
       SkylarkList<String> requestedFeatures,
       SkylarkList<String> unsupportedFeatures)

src/main/java/com/google/devtools/build/skydoc/fakebuildapi/cpp/FakeCcModule.java

@@ -43,7 +43,7 @@ public class FakeCcModule
     implements BazelCcModuleApi<
         FileApi,
         SkylarkRuleContextApi,
-        CcToolchainProviderApi,
+        CcToolchainProviderApi<FeatureConfigurationApi>,
         FeatureConfigurationApi,
         CompilationInfoApi,
         CcCompilationContextApi,
@@ -60,8 +60,11 @@ public ProviderApi getCcToolchainProvider() {
   }
 
   @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 boolean isCcToolchainResolutionEnabled(SkylarkRuleContextApi ruleContext)
   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 CompilationInfoApi compile(
   public LinkingInfoApi link(
       SkylarkRuleContextApi skylarkRuleContext,
       FeatureConfigurationApi skylarkFeatureConfiguration,
-      CcToolchainProviderApi skylarkCcToolchainProvider,
+      CcToolchainProviderApi<FeatureConfigurationApi> skylarkCcToolchainProvider,
       CcCompilationOutputsApi ccCompilationOutputs,
       Object skylarkLinkopts,
       Object dynamicLibrary,

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

@@ -187,7 +187,8 @@ private boolean usePicForBinariesWithConfiguration(String... configuration) thro
         CcCommon.configureFeaturesOrThrowEvalException(
             /* requestedFeatures= */ ImmutableSet.of(),
             /* unsupportedFeatures= */ ImmutableSet.of(),
-            toolchainProvider);
+            toolchainProvider,
+            getRuleContext(target).getFragment(CppConfiguration.class));
     return CppHelper.usePicForBinaries(toolchainProvider, featureConfiguration);
   }
 

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

@@ -231,7 +231,8 @@ public void testLinkstampCompileDependsOnAllCcBinaryLinkingInputs() throws Excep
         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);

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

@@ -135,7 +135,10 @@ public void testGetToolForAction() throws Exception {
             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 @@ public void testGetCommandLine() throws Exception {
             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 @@ public void testGetEnvironment() throws Exception {
             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(
@@ -374,6 +383,34 @@ public void testIsEnabled() throws Exception {
     assertThat(disabledFeatureIsDisabled).isFalse();
   }
 
+  @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(