Flip the default value of the incompatible_disable_genrule_cc_toolcha…

…in_dependency Flip --incompatible_disable_genrule_cc_toolchain_dependency flag. Fixes #6867. Removes and updates several tests of the old behavior. RELNOTES: The --incompatible_disable_genrule_cc_toolchain_dependency flag has been flipped (see #6867 for details). Closes #7878. PiperOrigin-RevId: 240993385
bazelbuild · Mar 29, 2019 · 5e3dcde · 5e3dcde
1 parent 30f6456
commit 5e3dcde
Show file tree

Hide file tree

Showing 4 changed files with 2 additions and 101 deletions.
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
@@ -803,7 +803,7 @@ public Label getFdoPrefetchHintsLabel() {
 
   @Option(
       name = "incompatible_disable_genrule_cc_toolchain_dependency",
-      defaultValue = "false",
+      defaultValue = "true",
       documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
       metadataTags = {

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/platform/TemplateVariableInfoTest.java b/src/test/java/com/google/devtools/build/lib/analysis/platform/TemplateVariableInfoTest.java
@@ -64,7 +64,7 @@ public void templateVariableInfo() throws Exception {
 
     @SuppressWarnings("unchecked")
     Map<String, String> makeVariables = (Map<String, String>) ct.get("variables");
-    assertThat(makeVariables).containsKey("CC_FLAGS");
+    assertThat(makeVariables).containsKey("CC");
   }
 
   @Test

diff --git a/...ava/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleCommandSubstitutionTest.java b/...ava/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleCommandSubstitutionTest.java
@@ -14,14 +14,11 @@
 
 package com.google.devtools.build.lib.bazel.rules.genrule;
 
-import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 
 import com.google.common.base.Joiner;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
-import com.google.devtools.build.lib.analysis.util.AnalysisMock;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
-import com.google.devtools.build.lib.packages.util.Crosstool.CcToolchainConfig;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.junit.Test;
@@ -477,21 +474,4 @@ private String getBuildFileWithCommand(String command) {
             "        outs = ['out'],",
             "        cmd = '" + command + "')");
   }
-
-  @Test
-  public void testCcFlagsFromFeatureConfiguration() throws Exception {
-    AnalysisMock.get()
-        .ccSupport()
-        .setupCcToolchainConfig(
-            mockToolsConfig,
-            CcToolchainConfig.builder().withActionConfigs("cc_flags_action_config_foo_bar_baz"));
-    useConfiguration();
-    scratch.file(
-        "foo/BUILD",
-        "genrule(name = 'foo',",
-        "        outs = ['out'],",
-        "        cmd = '$(CC_FLAGS)')");
-    String command = getGenruleCommand("//foo");
-    assertThat(command).endsWith("foo bar baz");
-  }
 }
diff --git a/...t/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java b/...t/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java
@@ -30,9 +30,6 @@
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.configuredtargets.FileConfiguredTarget;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
-import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
-import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
-import com.google.devtools.build.lib.rules.cpp.CppHelper;
 import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.List;
@@ -91,16 +88,6 @@ public void testToolchainOverridesJavabase() throws Exception {
     assertThat(cmd).endsWith("JAVABASE=REPLACED");
   }
 
-  @Test
-  public void testToolchainDoesNotOverrideCcFlags() throws Exception {
-    scratch.file("a/BUILD",
-        "genrule(name='gr', srcs=[], outs=['out'], cmd='CC_FLAGS=$(CC_FLAGS)', toolchains=[':v'])",
-        "make_variable_tester(name='v', variables={'CC_FLAGS': 'REPLACED'})");
-
-    String cmd = getCommand("//a:gr");
-    assertThat(cmd).doesNotContain("CC_FLAGS=REPLACED");
-  }
-
   @Test
   public void testD() throws Exception {
     createFiles();
@@ -303,51 +290,6 @@ public void testRuleDirExpansion() throws Exception {
     assertThat(getCommand("//foo:baz")).containsMatch(expectedRegex);
   }
 
-  /** Ensure that variable $(CC) gets expanded correctly in the genrule cmd. */
-  @Test
-  public void testMakeVarExpansion() throws Exception {
-    scratch.file(
-        "foo/BUILD",
-        "genrule(name = 'bar',",
-        "        srcs = ['bar.cc'],",
-        "        cmd = '$(CC) -o $(OUTS) $(SRCS) $$shellvar',",
-        "        outs = ['bar.o'])");
-    FileConfiguredTarget barOutTarget = getFileConfiguredTarget("//foo:bar.o");
-    FileConfiguredTarget barInTarget = getFileConfiguredTarget("//foo:bar.cc");
-
-    SpawnAction barAction = (SpawnAction) getGeneratingAction(barOutTarget.getArtifact());
-
-    CcToolchainProvider toolchain =
-        CppHelper.getToolchainUsingDefaultCcToolchainAttribute(
-            getRuleContext(getConfiguredTarget("//foo:bar")));
-    String cc = toolchain.getToolPathFragment(Tool.GCC).getPathString();
-    String expected =
-        cc
-            + " -o "
-            + barOutTarget.getArtifact().getExecPathString()
-            + " "
-            + barInTarget.getArtifact().getRootRelativePath().getPathString()
-            + " $shellvar";
-    assertCommandEquals(expected, barAction.getArguments().get(2));
-  }
-
-  @Test
-  public void onlyHasCcToolchainDepWhenCcMakeVariablesArePresent() throws Exception {
-    scratch.file(
-        "foo/BUILD",
-        "genrule(name = 'no_cc',",
-        "        srcs = [],",
-        "        cmd = 'echo no CC variables here > $@',",
-        "        outs = ['no_cc.out'])",
-        "genrule(name = 'cc',",
-        "        srcs = [],",
-        "        cmd = 'echo $(CC) > $@',",
-        "        outs = ['cc.out'])");
-    String ccToolchainAttr = ":cc_toolchain";
-    assertThat(getPrerequisites(getConfiguredTarget("//foo:no_cc"), ccToolchainAttr)).isEmpty();
-    assertThat(getPrerequisites(getConfiguredTarget("//foo:cc"), ccToolchainAttr)).isNotEmpty();
-  }
-
   // Returns the expansion of 'cmd' for the specified genrule.
   private String getCommand(String label) throws Exception {
     return getSpawnAction(label).getArguments().get(2);
@@ -672,25 +614,4 @@ public void testDuplicateLocalFlags() throws Exception {
     getConfiguredTarget("//foo:g");
     assertNoEvents();
   }
-
-  @Test
-  public void testDisableGenruleCcToolchainDependency() throws Exception {
-    reporter.removeHandler(failFastHandler);
-    scratch.file(
-        "a/BUILD",
-        "genrule(",
-        "    name = 'a',",
-        "    outs = ['out.log'],",
-        "    cmd = 'echo $(CC_FLAGS) > $@',",
-        ")");
-
-    // Legacy behavior: CC_FLAGS is implicitly defined.
-    getConfiguredTarget("//a:a");
-    assertDoesNotContainEvent("$(CC_FLAGS) not defined");
-
-    // Updated behavior: CC_FLAGS must be explicitly supplied by a dependency.
-    useConfiguration("--incompatible_disable_genrule_cc_toolchain_dependency");
-    getConfiguredTarget("//a:a");
-    assertContainsEvent("$(CC_FLAGS) not defined");
-  }
 }