Skip to content

Commit

Permalink
Flip the default value of the incompatible_disable_genrule_cc_toolch…
Browse files Browse the repository at this point in the history 
…ain_dependency

    Flip --incompatible_disable_genrule_cc_toolchain_dependency flag.

    Fixes bazelbuild/bazel#6867.

    Removes and updates several tests of the old behavior.

    RELNOTES: The --incompatible_disable_genrule_cc_toolchain_dependency flag has been flipped (see bazelbuild/bazel#6867 for details).

    Closes #7878.

    PiperOrigin-RevId: 240993385
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 6027c20 commit 85e6e69
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 75 deletions.
21 changes: 21 additions & 0 deletions ...va/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@
import com.google.devtools.common.options.OptionEffectTag;
import com.google.devtools.common.options.OptionMetadataTag;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

/** Command-line options for C++. */
public class CppOptions extends FragmentOptions {
Expand Down Expand Up @@ -893,6 +896,24 @@ public FragmentOptions getHost() {
return host;
}

@Override
public Map<String, Set<Label>> getDefaultsLabels() {
Set<Label> crosstoolLabels = new LinkedHashSet<>();
crosstoolLabels.add(crosstoolTop);
if (hostCrosstoolTop != null) {
crosstoolLabels.add(hostCrosstoolTop);
}

if (libcTopLabel != null) {
Label libcLabel = libcTopLabel;
if (libcLabel != null) {
crosstoolLabels.add(libcLabel);
}
}

return ImmutableMap.of("CROSSTOOL", crosstoolLabels);
}

/**
* Returns true if targets under this configuration should apply FDO.
*/
Expand Down
2 changes: 1 addition & 1 deletion ...c/test/java/com/google/devtools/build/lib/analysis/platform/TemplateVariableInfoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
82 changes: 9 additions & 73 deletions ...ava/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleCommandSubstitutionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

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;
Expand All @@ -34,14 +33,14 @@
* but this test case exercises the composition of these various transformations.
*/
@RunWith(JUnit4.class)
public final class GenRuleCommandSubstitutionTest extends BuildViewTestCase {
public class GenRuleCommandSubstitutionTest extends BuildViewTestCase {

private static final Pattern SETUP_COMMAND_PATTERN =
Pattern.compile(".*/genrule-setup.sh;\\s+(?<command>.*)");

private String getGenruleCommand(String genrule) throws Exception {
return ((SpawnAction)
getGeneratingAction(getFilesToBuild(getConfiguredTarget(genrule)).toList().get(0)))
getGeneratingAction(getFilesToBuild(getConfiguredTarget(genrule)).iterator().next()))
.getArguments()
.get(2);
}
Expand All @@ -51,7 +50,7 @@ private void assertExpansionEquals(String expected, String genrule) throws Excep
assertCommandEquals(expected, command);
}

private static void assertCommandEquals(String expected, String command) {
private void assertCommandEquals(String expected, String command) {
// Ensure the command after the genrule setup is correct.
Matcher m = SETUP_COMMAND_PATTERN.matcher(command);
if (m.matches()) {
Expand All @@ -75,11 +74,6 @@ private void assertExpansionFails(String expectedErrorSuffix, String genrule) th
private void genrule(String command) throws Exception {
scratch.overwriteFile(
"test/BUILD",
// This is a horrible workaround for b/147306893:
// somehow, duplicate events (same location, same message)
// are being suppressed, so we must vary the location of the
// genrule by inserting a unique number of newlines.
new String(new char[seq++]).replace('\0', '\n'),
"genrule(name = 'test',",
" outs = ['out'],",
" cmd = '" + command + "')");
Expand All @@ -88,14 +82,14 @@ private void genrule(String command) throws Exception {
invalidatePackages();
}

private int seq = 0;

@Test
public void testLocationSyntaxErrors() throws Exception {
genrule("$(location )");
assertExpansionFails(
"invalid label in $(location) expression: empty package-relative label", "//test");

eventCollector.clear();

genrule("$(location foo bar");
assertExpansionFails("unterminated variable reference", "//test");

Expand Down Expand Up @@ -222,6 +216,8 @@ public void testLocationsSyntaxErrors() throws Exception {
assertExpansionFails(
"invalid label in $(locations) expression: empty package-relative label", "//test");

eventCollector.clear();

genrule("$(locations foo bar");
assertExpansionFails("unterminated variable reference", "//test");

Expand Down Expand Up @@ -453,66 +449,6 @@ public void testShellVariables() throws Exception {
assertContainsEvent("$(basename) not defined");
}

@Test
public void heuristicLabelExpansion_singletonFilegroupInTools_expandsToFile() throws Exception {
scratch.file(
"foo/BUILD",
"filegroup(name = 'fg', srcs = ['fg1.txt'])",
"genrule(",
" name = 'gen',",
" outs = ['gen.out'],",
" tools = [':fg'],",
" heuristic_label_expansion = True,",
" cmd = 'cp :fg $@',",
")");

useConfiguration("--experimental_enable_aggregating_middleman");
assertThat(getGenruleCommand("//foo:gen")).contains("foo/fg1.txt");

useConfiguration("--noexperimental_enable_aggregating_middleman");
assertThat(getGenruleCommand("//foo:gen")).contains("foo/fg1.txt");
}

@Test
public void heuristicLabelExpansion_emptyFilegroupInTools_fails() throws Exception {
scratch.file(
"foo/BUILD",
"filegroup(name = 'fg', srcs = [])",
"genrule(",
" name = 'gen',",
" outs = ['gen.out'],",
" tools = [':fg'],",
" heuristic_label_expansion = True,",
" cmd = 'cp :fg $@',",
")");

useConfiguration("--experimental_enable_aggregating_middleman");
assertExpansionFails("expands to 0 files", "//foo:gen");

useConfiguration("--noexperimental_enable_aggregating_middleman");
assertExpansionFails("expands to 0 files", "//foo:gen");
}

@Test
public void heuristicLabelExpansion_multiFilegroupInTools_fails() throws Exception {
scratch.file(
"foo/BUILD",
"filegroup(name = 'fg', srcs = ['fg1.txt', 'fg2.txt'])",
"genrule(",
" name = 'gen',",
" outs = ['gen.out'],",
" tools = [':fg'],",
" heuristic_label_expansion = True,",
" cmd = 'cp :fg $@',",
")");

useConfiguration("--experimental_enable_aggregating_middleman");
assertExpansionFails("expands to 2 files", "//foo:gen");

useConfiguration("--noexperimental_enable_aggregating_middleman");
assertExpansionFails("expands to 2 files", "//foo:gen");
}

@Test
public void testDollarFileFails() throws Exception {
checkError(
Expand All @@ -531,11 +467,11 @@ public void testDollarFile2Fails() throws Exception {
getBuildFileWithCommand("${file%:.*8}"));
}

private static String getBuildFileWithCommand(String command) {
private String getBuildFileWithCommand(String command) {
return Joiner.on("\n")
.join(
"genrule(name = 'test',",
" outs = ['out'],",
" cmd = '" + command + "')");
}
}
}
2 changes: 1 addition & 1 deletion ...t/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ private void assertNotStamped(String target) throws Exception {
private void assertStamped(ConfiguredTarget target) throws Exception {
Artifact out = Iterables.getFirst(getFilesToBuild(target), null);
List<String> inputs = ActionsTestUtil.baseArtifactNames(getGeneratingAction(out).getInputs());
assertThat(inputs).containsAtLeast("build-info.txt", "build-changelist.txt");
assertThat(inputs).containsAllOf("build-info.txt", "build-changelist.txt");
}

private void assertNotStamped(ConfiguredTarget target) throws Exception {
Expand Down

0 comments on commit 85e6e69

Please sign in to comment.