Remove code for providing make variables from CppConfiguration

Also make `--incompatible_disable_cc_configuration_make_variables` a noop.

#6381

RELNOTES: None.
PiperOrigin-RevId: 222280222

src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java

@@ -137,13 +137,6 @@ public abstract static class Fragment {
     public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOptions) {
     }
 
-    /**
-     * Adds mapping of names to values of "Make" variables defined by this configuration.
-     */
-    @SuppressWarnings("unused")
-    public void addGlobalMakeVariables(ImmutableMap.Builder<String, String> globalMakeEnvBuilder) {
-    }
-
     /**
      * Returns a fragment of the output directory name for this configuration. The output
      * directory for the whole configuration contains all the short names by all fragments.
@@ -1302,9 +1295,6 @@ public BuildConfiguration(
         TransitiveOptionDetails.forOptionsWithDefaults(buildOptions.getNativeOptions());
 
     ImmutableMap.Builder<String, String> globalMakeEnvBuilder = ImmutableMap.builder();
-    for (Fragment fragment : fragments.values()) {
-      fragment.addGlobalMakeVariables(globalMakeEnvBuilder);
-    }
 
     // TODO(configurability-team): Deprecate TARGET_CPU in favor of platforms.
     globalMakeEnvBuilder.put("TARGET_CPU", options.cpu);

src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java

@@ -40,6 +40,19 @@
 public class BazelRulesModule extends BlazeModule {
   /** This is where deprecated options go to die. */
   public static class GraveyardOptions extends OptionsBase {
+    @Option(
+        name = "incompatible_disable_cc_configuration_make_variables",
+        defaultValue = "true",
+        documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+        effectTags = {OptionEffectTag.UNKNOWN},
+        metadataTags = {
+          OptionMetadataTag.INCOMPATIBLE_CHANGE,
+          OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES,
+          OptionMetadataTag.DEPRECATED,
+        },
+        help = "Deprecated no-op.")
+    public boolean disableMakeVariables;
+
     @Option(
         name = "incompatible_disable_legacy_flags_cc_toolchain_api",
         defaultValue = "true",

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

@@ -1002,11 +1002,6 @@ public CcToolchainConfigInfo ccToolchainConfigInfoFromSkylark(
     if (!config.enableCcToolchainConfigInfoFromSkylark()) {
       throw new InvalidConfigurationException("Creating a CcToolchainConfigInfo is not enabled.");
     }
-    if (!config.disableMakeVariables()) {
-      throw new InvalidConfigurationException(
-          "--incompatible_disable_cc_configuration_make_variables must be set to true in "
-              + "order to configure the C++ toolchain from Starlark.");
-    }
 
     ImmutableList.Builder<Feature> featureBuilder = ImmutableList.builder();
     for (Object feature : features) {

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

@@ -82,7 +82,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
 
     TemplateVariableInfo templateVariableInfo =
         createMakeVariableProvider(
-            ccToolchainProvider.getCppConfiguration(),
             ccToolchainProvider,
             ccToolchainProvider.getSysrootPathFragment(),
             ruleContext.getRule().getLocation());
@@ -96,13 +95,12 @@ public ConfiguredTarget create(RuleContext ruleContext)
   }
 
   static TemplateVariableInfo createMakeVariableProvider(
-      CppConfiguration cppConfiguration,
       CcToolchainProvider toolchainProvider,
       PathFragment sysroot,
       Location location) {
 
     HashMap<String, String> makeVariables =
-        new HashMap<>(cppConfiguration.getAdditionalMakeVariables());
+        new HashMap<>(toolchainProvider.getAdditionalMakeVariables());
 
     // Add make variables from the toolchainProvider, also.
     ImmutableMap.Builder<String, String> ccProviderMakeVariables = new ImmutableMap.Builder<>();

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

@@ -132,7 +132,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
 
     TemplateVariableInfo templateVariableInfo =
         CcToolchain.createMakeVariableProvider(
-            ccToolchainProvider.getCppConfiguration(),
             ccToolchainProvider,
             ccToolchainProvider.getSysrootPathFragment(),
             ruleContext.getRule().getLocation());

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

@@ -414,18 +414,6 @@ public ImmutableList<String> getLtoBackendOptions() {
     return ltobackendOptions;
   }
 
-  /**
-   * Returns a map of additional make variables for use by {@link
-   * BuildConfiguration}. These are to used to allow some build rules to
-   * avoid the limits on stack frame sizes and variable-length arrays.
-   *
-   * <p>The returned map must contain an entry for {@code STACK_FRAME_UNLIMITED},
-   * though the entry may be an empty string.
-   */
-  public ImmutableMap<String, String> getAdditionalMakeVariables() {
-    return cppToolchainInfo.getAdditionalMakeVariables();
-  }
-
   @SkylarkCallable(
       name = "minimum_os_version",
       doc = "The minimum OS version for C/C++ compilation.")
@@ -643,27 +631,6 @@ public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOption
         "FDO state should not propagate to the host configuration");
   }
 
-  @Override
-  public void addGlobalMakeVariables(ImmutableMap.Builder<String, String> globalMakeEnvBuilder) {
-    if (cppOptions.disableMakeVariables) {
-      return;
-    }
-
-    if (!shouldProvideMakeVariables) {
-      return;
-    }
-    globalMakeEnvBuilder.putAll(
-        CcToolchainProvider.getCppBuildVariables(
-            this::getToolPathFragment,
-            cppToolchainInfo.getTargetLibc(),
-            cppToolchainInfo.getCompiler(),
-            desiredCpu,
-            crosstoolTopPathFragment,
-            cppToolchainInfo.getAbiGlibcVersion(),
-            cppToolchainInfo.getAbi(),
-            getAdditionalMakeVariables()));
-  }
-
   @Override
   public String getOutputDirectoryName() {
     String toolchainPrefix = desiredCpu;
@@ -725,10 +692,6 @@ public boolean disableLinkingModeFlags() {
     return cppOptions.disableLinkingModeFlags;
   }
 
-  public boolean disableMakeVariables() {
-    return cppOptions.disableMakeVariables;
-  }
-
   public boolean disableCcToolchainFromCrosstool() {
     return cppOptions.disableCcToolchainFromCrosstool;
   }

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

@@ -678,21 +678,6 @@ public Label getFdoPrefetchHintsLabel() {
   )
   public boolean strictSystemIncludes;
 
-  @Option(
-      name = "incompatible_disable_cc_configuration_make_variables",
-      defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      metadataTags = {
-        OptionMetadataTag.INCOMPATIBLE_CHANGE,
-        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
-      },
-      help =
-          "If enabled, the C++ configuration fragment supplies Make variables. This option "
-              + "is used in the migration to remove them in favor of requiring an explicit "
-              + "dependency on the C++ toolchain for rules that use them.")
-  public boolean disableMakeVariables;
-
   @Option(
     name = "experimental_use_llvm_covmap",
     defaultValue = "false",
@@ -851,8 +836,6 @@ public FragmentOptions getHost() {
       host.crosstoolTop = hostCrosstoolTop;
     }
 
-    host.disableMakeVariables = disableMakeVariables;
-
     // hostLibcTop doesn't default to the target's libcTop.
     // Only an explicit command-line option will change it.
     // The default is whatever the host's crosstool (which might have been specified

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

@@ -628,10 +628,7 @@ public void testToolchainFromStarlarkRule() throws Exception {
             mockToolsConfig,
             CrosstoolConfig.CToolchain.newBuilder().setAbiVersion("orange").buildPartial());
 
-    useConfiguration(
-        "--cpu=k8",
-        "--experimental_enable_cc_toolchain_config_info",
-        "--incompatible_disable_cc_configuration_make_variables");
+    useConfiguration("--cpu=k8", "--experimental_enable_cc_toolchain_config_info");
 
     ConfiguredTarget target = getConfiguredTarget("//a:b");
     CcToolchainProvider toolchainProvider =
@@ -649,10 +646,7 @@ public void testToolPathsInToolchainFromStarlarkRule() throws Exception {
 
     getAnalysisMock().ccSupport();
 
-    useConfiguration(
-        "--cpu=k8",
-        "--experimental_enable_cc_toolchain_config_info",
-        "--incompatible_disable_cc_configuration_make_variables");
+    useConfiguration("--cpu=k8", "--experimental_enable_cc_toolchain_config_info");
 
     ConfiguredTarget target = getConfiguredTarget("//a:b");
     CcToolchainProvider toolchainProvider =
@@ -663,28 +657,6 @@ public void testToolPathsInToolchainFromStarlarkRule() throws Exception {
         .isEqualTo("a/relative/path");
   }
 
-  @Test
-  public void testToolchainFromStarlarkRuleWithoutIncompatibleFlagsFlipped() throws Exception {
-    writeStarlarkRule();
-
-    getAnalysisMock()
-        .ccSupport()
-        .setupCrosstool(
-            mockToolsConfig,
-            CrosstoolConfig.CToolchain.newBuilder().setAbiVersion("orange").buildPartial());
-
-    useConfiguration("--experimental_enable_cc_toolchain_config_info");
-    try {
-      getConfiguredTarget("//a:b");
-    } catch (AssertionError e) {
-      assertThat(e)
-          .hasMessageThat()
-          .contains(
-              "--incompatible_disable_cc_configuration_make_variables must be set to true in "
-                  + "order to configure the C++ toolchain from Starlark.");
-    }
-  }
-
   private void writeStarlarkRule() throws IOException {
     scratch.file(
         "a/BUILD",

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

@@ -214,7 +214,7 @@ public void testSimpleCompleteConfiguration() throws Exception {
     assertThat(ccProvider.getObjCopyOptionsForEmbedding()).containsExactly("objcopy").inOrder();
     assertThat(ccProvider.getLdOptionsForEmbedding()).isEmpty();
 
-    assertThat(toolchain.getAdditionalMakeVariables().entrySet())
+    assertThat(ccProvider.getAdditionalMakeVariables().entrySet())
         .containsExactlyElementsIn(
             ImmutableMap.of(
                     "SOME_MAKE_VARIABLE", "make-variable-value",
@@ -585,9 +585,11 @@ public void testComprehensiveCompleteConfiguration() throws Exception {
     assertThat(ccProviderC.getObjCopyOptionsForEmbedding()).isEmpty();
     assertThat(ccProviderC.getLdOptionsForEmbedding()).isEmpty();
 
-    assertThat(toolchainC.getAdditionalMakeVariables()).containsExactlyEntriesIn(ImmutableMap.of(
-        "CC_FLAGS", "",
-        "STACK_FRAME_UNLIMITED", ""));
+    assertThat(ccProviderC.getAdditionalMakeVariables())
+        .containsExactlyEntriesIn(
+            ImmutableMap.of(
+                "CC_FLAGS", "",
+                "STACK_FRAME_UNLIMITED", ""));
     assertThat(ccProviderC.getBuiltInIncludeDirectories()).isEmpty();
     assertThat(ccProviderC.getSysroot()).isNull();
   }

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

@@ -4004,9 +4004,7 @@ public void testCcToolchainInfoFromSkylark() throws Exception {
         "load(':crosstool.bzl', 'cc_toolchain_config_rule')",
         "cc_toolchain_alias(name='alias')",
         "cc_toolchain_config_rule(name='r')");
-    useConfiguration(
-        "--experimental_enable_cc_toolchain_config_info",
-        "--incompatible_disable_cc_configuration_make_variables");
+    useConfiguration("--experimental_enable_cc_toolchain_config_info");
     ConfiguredTarget target = getConfiguredTarget("//foo:r");
     assertThat(target).isNotNull();
     CcToolchainConfigInfo ccToolchainConfigInfo =
@@ -4079,9 +4077,7 @@ public void testCcToolchainInfoFromSkylarkRequiredAbiLibcVersion() throws Except
   @Test
   public void testCcToolchainInfoFromSkylarkAllRequiredStringsPresent() throws Exception {
     setupSkylarkRuleForStringFieldsTesting("");
-    useConfiguration(
-        "--experimental_enable_cc_toolchain_config_info",
-        "--incompatible_disable_cc_configuration_make_variables");
+    useConfiguration("--experimental_enable_cc_toolchain_config_info");
     ConfiguredTarget target = getConfiguredTarget("//foo:r");
     assertThat(target).isNotNull();
     CcToolchainConfigInfo ccToolchainConfigInfo =
@@ -4173,9 +4169,7 @@ public void testCcToolchainInfoFromSkylarkNoLegacyFeatures() throws Exception {
         "load(':crosstool.bzl', 'cc_toolchain_config_rule')",
         "cc_toolchain_alias(name='alias')",
         "cc_toolchain_config_rule(name='r')");
-    useConfiguration(
-        "--experimental_enable_cc_toolchain_config_info",
-        "--incompatible_disable_cc_configuration_make_variables");
+    useConfiguration("--experimental_enable_cc_toolchain_config_info");
     ConfiguredTarget target = getConfiguredTarget("//foo:r");
     assertThat(target).isNotNull();
     CcToolchainConfigInfo ccToolchainConfigInfo =
@@ -4245,9 +4239,7 @@ public void testCcToolchainInfoFromSkylarkWithLegacyFeatures() throws Exception
         "load(':crosstool.bzl', 'cc_toolchain_config_rule')",
         "cc_toolchain_alias(name='alias')",
         "cc_toolchain_config_rule(name='r')");
-    useConfiguration(
-        "--experimental_enable_cc_toolchain_config_info",
-        "--incompatible_disable_cc_configuration_make_variables");
+    useConfiguration("--experimental_enable_cc_toolchain_config_info");
     ConfiguredTarget target = getConfiguredTarget("//foo:r");
     assertThat(target).isNotNull();
     CcToolchainConfigInfo ccToolchainConfigInfo =