From 5e3dcde0c21fb4e979526d0a79f09aaba629ab61 Mon Sep 17 00:00:00 2001 From: John Cater Date: Fri, 29 Mar 2019 09:31:30 -0700 Subject: [PATCH] Flip the default value of the incompatible_disable_genrule_cc_toolchain_dependency Flip --incompatible_disable_genrule_cc_toolchain_dependency flag. Fixes https://github.com/bazelbuild/bazel/issues/6867. Removes and updates several tests of the old behavior. RELNOTES: The --incompatible_disable_genrule_cc_toolchain_dependency flag has been flipped (see https://github.com/bazelbuild/bazel/issues/6867 for details). Closes #7878. PiperOrigin-RevId: 240993385 --- .../build/lib/rules/cpp/CppOptions.java | 2 +- .../platform/TemplateVariableInfoTest.java | 2 +- .../GenRuleCommandSubstitutionTest.java | 20 ----- .../genrule/GenRuleConfiguredTargetTest.java | 79 ------------------- 4 files changed, 2 insertions(+), 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 index 6fcc8e662e085d..13469bc41d243c 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 @@ -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 index cfe030131b84e2..d3a16e2ef80482 100644 --- 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 makeVariables = (Map) ct.get("variables"); - assertThat(makeVariables).containsKey("CC_FLAGS"); + assertThat(makeVariables).containsKey("CC"); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleCommandSubstitutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleCommandSubstitutionTest.java index 2f871ac0fa1324..df74d717f38b2c 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleCommandSubstitutionTest.java +++ b/src/test/java/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"); - } } \ No newline at end of file diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java index a8e9bbbf0185f1..3c86efe40b2e17 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java +++ b/src/test/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"); - } }