diff options
author | hlopko <hlopko@google.com> | 2017-07-04 04:31:11 -0400 |
---|---|---|
committer | John Cater <jcater@google.com> | 2017-07-05 10:57:30 -0400 |
commit | ec41b8cea579e135eca18f808a710abb397994f6 (patch) | |
tree | f187ce5dd8cf767fb058cfb7e4761dad100c7322 /src/test/java/com | |
parent | 6446ffa1ec61f8cfd73edc540ac54e42d15169f9 (diff) |
Remove implicit iteration from Crosstool
Up until now we allowed implicit iteration, e.g.:
flag_group { flag: '%{some_sequence_variable}' }
From now on, snippet above will raise an error. We require explicit
'iterate_over' message, e.g.:
flag_group {
iterate_over: 'some_sequence_variable'
flag: '%{some_sequence_variable}'
}
RELNOTES: Implicit iteration in the CROSSTOOL has been removed, use explicit 'iterate_over' message.
PiperOrigin-RevId: 160871888
Diffstat (limited to 'src/test/java/com')
3 files changed, 144 insertions, 103 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java index 00ff1ca4ea..bd0bf1dfe0 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java @@ -76,9 +76,7 @@ public abstract class MockCcSupport { + " }" + "}"; - /** - * A feature configuration snippet useful for testing the layering check. - */ + /** A feature configuration snippet useful for testing the layering check. */ public static final String LAYERING_CHECK_FEATURE_CONFIGURATION = "" + "feature {" @@ -90,6 +88,7 @@ public abstract class MockCcSupport { + " action: 'c++-header-preprocessing'" + " action: 'c++-module-compile'" + " flag_group {" + + " iterate_over: 'dependent_module_map_files'" + " flag: 'dependent_module_map_file:%{dependent_module_map_files}'" + " }" + " }" @@ -146,6 +145,7 @@ public abstract class MockCcSupport { + " action: 'c++-header-preprocessing'" + " action: 'c++-modules-compile'" + " flag_group {" + + " iterate_over: 'module_files'" + " flag: 'module_file:%{module_files}'" + " }" + " }" diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java index 3beb3a6d13..9e804340d5 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java @@ -272,8 +272,8 @@ public class CcToolchainFeaturesTest { assertThat(getFlagParsingError("%{")).contains("expected variable name"); assertThat(getFlagParsingError("%{}")).contains("expected variable name"); assertThat( - getCommandLineForFlag( - "%{v}", + getCommandLineForFlagGroups( + "flag_group{ iterate_over: 'v' flag: '%{v}' }", new Variables.Builder() .addStringSequenceVariable("v", ImmutableList.<String>of()) .build())) @@ -422,19 +422,6 @@ public class CcToolchainFeaturesTest { .containsExactly("-Afirst -Bfoo", "-Afirst -Bbar", "-Asecond -Bfoo", "-Asecond -Bbar"); } - // <p>TODO(b/32655571): Get rid of this test once implicit iteration is not supported. - // It's there only to document a known limitation of the system. - @Test - public void testVariableLookupIsBrokenForImplicitStructFieldIteration() throws Exception { - assertThat( - getCommandLineForFlagGroups( - "flag_group { flag: '-A%{struct.sequence}' }", - createStructureVariables( - "struct", - new StructureBuilder().addField("sequence", ImmutableList.of("foo", "bar"))))) - .containsExactly("-Afoo", "-Abar"); - } - @Test public void testExpandIfAllAvailableWithStructsExpandsIfPresent() throws Exception { assertThat( @@ -660,18 +647,6 @@ public class CcToolchainFeaturesTest { } @Test - public void testLegacyListVariableExpansion() throws Exception { - assertThat(getCommandLineForFlag("%{v}", createVariables("v", "1", "v", "2"))) - .containsExactly("1", "2"); - assertThat(getCommandLineForFlag("%{v1} %{v2}", - createVariables("v1", "a1", "v1", "a2", "v2", "b"))) - .containsExactly("a1 b", "a2 b"); - assertThat(getFlagExpansionError("%{v1} %{v2}", - createVariables("v1", "a1", "v1", "a2", "v2", "b1", "v2", "b2"))) - .contains("'v1' and 'v2'"); - } - - @Test public void testListVariableExpansion() throws Exception { assertThat( getCommandLineForFlagGroups( @@ -715,42 +690,28 @@ public class CcToolchainFeaturesTest { } @Test - public void testListVariableExpansionMixedWithImplicitlyAccessedListVariableWithinFlagGroupWorks() - throws Exception { + public void testFlagGroupVariableExpansion() throws Exception { assertThat( getCommandLineForFlagGroups( - "flag_group {" - + " iterate_over: 'v1'" - + " flag_group {" - + " flag: '-A%{v1} -B%{v2}'" - + " }" - + "}", - createVariables("v1", "a1", "v1", "a2", "v2", "b1", "v2", "b2"))) - .containsExactly("-Aa1 -Bb1", "-Aa1 -Bb2", "-Aa2 -Bb1", "-Aa2 -Bb2"); - } - - @Test - public void testFlagGroupVariableExpansion() throws Exception { - assertThat(getCommandLineForFlagGroups( - "flag_group { flag: '-f' flag: '%{v}' } flag_group { flag: '-end' }", - createVariables("v", "1", "v", "2"))) + "" + + "flag_group { iterate_over: 'v' flag: '-f' flag: '%{v}' }" + + "flag_group { flag: '-end' }", + createVariables("v", "1", "v", "2"))) .containsExactly("-f", "1", "-f", "2", "-end"); - assertThat(getCommandLineForFlagGroups( - "flag_group { flag: '-f' flag: '%{v}' } flag_group { flag: '%{v}' }", - createVariables("v", "1", "v", "2"))) + assertThat( + getCommandLineForFlagGroups( + "" + + "flag_group { iterate_over: 'v' flag: '-f' flag: '%{v}' }" + + "flag_group { iterate_over: 'v' flag: '%{v}' }", + createVariables("v", "1", "v", "2"))) .containsExactly("-f", "1", "-f", "2", "1", "2"); - assertThat(getCommandLineForFlagGroups( - "flag_group { flag: '-f' flag: '%{v}' } flag_group { flag: '%{v}' }", - createVariables("v", "1", "v", "2"))) + assertThat( + getCommandLineForFlagGroups( + "" + + "flag_group { iterate_over: 'v' flag: '-f' flag: '%{v}' } " + + "flag_group { iterate_over: 'v' flag: '%{v}' }", + createVariables("v", "1", "v", "2"))) .containsExactly("-f", "1", "-f", "2", "1", "2"); - try { - getCommandLineForFlagGroups( - "flag_group { flag: '%{v1}' flag: '%{v2}' }", - createVariables("v1", "1", "v1", "2", "v2", "1", "v2", "2")); - fail("Expected ExpansionException"); - } catch (ExpansionException e) { - assertThat(e).hasMessageThat().contains("'v1' and 'v2'"); - } } private VariableValueBuilder createNestedSequence(int depth, int count, String prefix) { @@ -780,11 +741,11 @@ public class CcToolchainFeaturesTest { @Test public void testFlagTreeVariableExpansion() throws Exception { String nestedGroup = - "flag_group {" + "" + + "flag_group {" + + " iterate_over: 'v'" + " flag_group { flag: '-a' }" - + " flag_group {" - + " flag: '%{v}'" - + " }" + + " flag_group { iterate_over: 'v' flag: '%{v}' }" + " flag_group { flag: '-b' }" + "}"; assertThat(getCommandLineForFlagGroups(nestedGroup, createNestedVariables("v", 1, 3))) @@ -792,9 +753,6 @@ public class CcToolchainFeaturesTest { "-a", "00", "01", "02", "-b", "-a", "10", "11", "12", "-b", "-a", "20", "21", "22", "-b"); - assertThat(getCommandLineForFlagGroups(nestedGroup, createNestedVariables("v", 0, 3))) - .containsExactly("-a", "0", "-b", "-a", "1", "-b", "-a", "2", "-b"); - try { getCommandLineForFlagGroups(nestedGroup, createNestedVariables("v", 2, 3)); fail("Expected ExpansionException"); @@ -993,7 +951,29 @@ public class CcToolchainFeaturesTest { } @Test - public void testSuppressionViaMissingBuildVariable() throws Exception { + public void testFlagSetWithMissingVariableIsNotExpanded() throws Exception { + FeatureConfiguration configuration = + buildFeatures( + "feature {", + " name: 'a'", + " flag_set {", + " action: 'c++-compile'", + " expand_if_all_available: 'v'", + " flag_group { flag: '%{v}' }", + " }", + " flag_set {", + " action: 'c++-compile'", + " flag_group { flag: 'unconditional' }", + " }", + "}") + .getFeatureConfiguration(assumptionsFor("a")); + + assertThat(configuration.getCommandLine(CppCompileAction.CPP_COMPILE, createVariables())) + .containsExactly("unconditional"); + } + + @Test + public void testOnlyFlagSetsWithAllVariablesPresentAreExpanded() throws Exception { FeatureConfiguration configuration = buildFeatures( "feature {", @@ -1016,16 +996,66 @@ public class CcToolchainFeaturesTest { "}") .getFeatureConfiguration(assumptionsFor("a")); - assertThat(configuration.getCommandLine(CppCompileAction.CPP_COMPILE, createVariables())) - .containsExactly("unconditional"); assertThat( configuration.getCommandLine(CppCompileAction.CPP_COMPILE, createVariables("v", "1"))) .containsExactly("1", "unconditional"); + } + + @Test + public void testOnlyInnerFlagSetIsIteratedWithSequenceVariable() throws Exception { + FeatureConfiguration configuration = + buildFeatures( + "feature {", + " name: 'a'", + " flag_set {", + " action: 'c++-compile'", + " expand_if_all_available: 'v'", + " flag_group { iterate_over: 'v' flag: '%{v}' }", + " }", + " flag_set {", + " action: 'c++-compile'", + " expand_if_all_available: 'v'", + " expand_if_all_available: 'w'", + " flag_group { iterate_over: 'v' flag: '%{v}%{w}' }", + " }", + " flag_set {", + " action: 'c++-compile'", + " flag_group { flag: 'unconditional' }", + " }", + "}") + .getFeatureConfiguration(assumptionsFor("a")); + assertThat( configuration.getCommandLine( CppCompileAction.CPP_COMPILE, createVariables("v", "1", "v", "2"))) .containsExactly("1", "2", "unconditional") .inOrder(); + } + + @Test + public void testFlagSetsAreIteratedIndividuallyForSequenceVariables() throws Exception { + FeatureConfiguration configuration = + buildFeatures( + "feature {", + " name: 'a'", + " flag_set {", + " action: 'c++-compile'", + " expand_if_all_available: 'v'", + " flag_group { iterate_over: 'v' flag: '%{v}' }", + " }", + " flag_set {", + " action: 'c++-compile'", + " expand_if_all_available: 'v'", + " expand_if_all_available: 'w'", + " flag_group { iterate_over: 'v' flag: '%{v}%{w}' }", + " }", + " flag_set {", + " action: 'c++-compile'", + " flag_group { flag: 'unconditional' }", + " }", + "}") + .getFeatureConfiguration(assumptionsFor("a")); + assertThat( configuration.getCommandLine( CppCompileAction.CPP_COMPILE, createVariables("v", "1", "v", "2", "w", "3"))) diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java index 7b28ff2a1f..4f48d0d6cf 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java @@ -66,24 +66,44 @@ public class LinkBuildVariablesTest extends BuildViewTestCase { return getCppLinkAction(target, type).getLinkCommandLine().getBuildVariables(); } - /** Returns the value of a given variable in context of the given Variables instance. */ - protected List<String> getVariableValue(Variables variables, String variable) throws Exception { + /** Returns the value of a given sequence variable in context of the given Variables instance. */ + protected List<String> getSequenceVariableValue(Variables variables, String variable) + throws Exception { FeatureConfiguration mockFeatureConfiguration = CcToolchainFeaturesTest.buildFeatures( "feature {", - " name: 'a'", - " flag_set {", - " action: 'foo'", - " flag_group {", - " flag: '%{" + variable + "}'", - " }", - " }", + " name: 'a'", + " flag_set {", + " action: 'foo'", + " flag_group {", + " iterate_over: '" + variable + "'", + " flag: '%{" + variable + "}'", + " }", + " }", "}") .getFeatureConfiguration( FeatureSpecification.create(ImmutableSet.of("a"), ImmutableSet.<String>of())); return mockFeatureConfiguration.getCommandLine("foo", variables); } + /** Returns the value of a given string variable in context of the given Variables instance. */ + protected String getVariableValue(Variables variables, String variable) throws Exception { + FeatureConfiguration mockFeatureConfiguration = + CcToolchainFeaturesTest.buildFeatures( + "feature {", + " name: 'a'", + " flag_set {", + " action: 'foo'", + " flag_group {", + " flag: '%{" + variable + "}'", + " }", + " }", + "}") + .getFeatureConfiguration( + FeatureSpecification.create(ImmutableSet.of("a"), ImmutableSet.<String>of())); + return Iterables.getOnlyElement(mockFeatureConfiguration.getCommandLine("foo", variables)); + } + @Test public void testLinkstampBuildVariable() throws Exception { scratch.file( @@ -105,7 +125,7 @@ public class LinkBuildVariablesTest extends BuildViewTestCase { ConfiguredTarget target = getConfiguredTarget("//x:bin"); Variables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE); List<String> variableValue = - getVariableValue(variables, CppLinkActionBuilder.LINKSTAMP_PATHS_VARIABLE); + getSequenceVariableValue(variables, CppLinkActionBuilder.LINKSTAMP_PATHS_VARIABLE); assertThat(Iterables.getOnlyElement(variableValue)).contains("c.o"); } @@ -117,8 +137,7 @@ public class LinkBuildVariablesTest extends BuildViewTestCase { ConfiguredTarget target = getConfiguredTarget("//x:bin"); Variables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE); - List<String> variableValue = - getVariableValue(variables, CppLinkActionBuilder.FORCE_PIC_VARIABLE); + String variableValue = getVariableValue(variables, CppLinkActionBuilder.FORCE_PIC_VARIABLE); assertThat(variableValue).contains(""); } @@ -161,7 +180,8 @@ public class LinkBuildVariablesTest extends BuildViewTestCase { ConfiguredTarget target = getConfiguredTarget("//x:bin"); Variables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE); List<String> variableValue = - getVariableValue(variables, CppLinkActionBuilder.LIBRARY_SEARCH_DIRECTORIES_VARIABLE); + getSequenceVariableValue( + variables, CppLinkActionBuilder.LIBRARY_SEARCH_DIRECTORIES_VARIABLE); assertThat(Iterables.getOnlyElement(variableValue)).contains("some-dir"); } @@ -175,10 +195,9 @@ public class LinkBuildVariablesTest extends BuildViewTestCase { ConfiguredTarget target = getConfiguredTarget("//x:bin"); Variables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE); - List<String> variableValue = + String variableValue = getVariableValue(variables, CppLinkActionBuilder.LINKER_PARAM_FILE_VARIABLE); - assertThat(Iterables.getOnlyElement(variableValue)).matches(".*bin/x/bin" - + OsUtils.executableExtension() + "-2.params$"); + assertThat(variableValue).matches(".*bin/x/bin" + OsUtils.executableExtension() + "-2.params$"); } @Test @@ -197,17 +216,13 @@ public class LinkBuildVariablesTest extends BuildViewTestCase { Variables variables = getLinkBuildVariables(target, LinkTargetType.DYNAMIC_LIBRARY); String interfaceLibraryBuilder = - Iterables.getOnlyElement( - getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_BUILDER_VARIABLE)); + getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_BUILDER_VARIABLE); String interfaceLibraryInput = - Iterables.getOnlyElement( - getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_INPUT_VARIABLE)); + getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_INPUT_VARIABLE); String interfaceLibraryOutput = - Iterables.getOnlyElement( - getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_OUTPUT_VARIABLE)); + getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_OUTPUT_VARIABLE); String generateInterfaceLibrary = - Iterables.getOnlyElement( - getVariableValue(variables, CppLinkActionBuilder.GENERATE_INTERFACE_LIBRARY_VARIABLE)); + getVariableValue(variables, CppLinkActionBuilder.GENERATE_INTERFACE_LIBRARY_VARIABLE); assertThat(generateInterfaceLibrary).isEqualTo("yes"); assertThat(interfaceLibraryInput).endsWith("libfoo.so"); @@ -231,17 +246,13 @@ public class LinkBuildVariablesTest extends BuildViewTestCase { Variables variables = getLinkBuildVariables(target, LinkTargetType.STATIC_LIBRARY); String interfaceLibraryBuilder = - Iterables.getOnlyElement( - getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_BUILDER_VARIABLE)); + getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_BUILDER_VARIABLE); String interfaceLibraryInput = - Iterables.getOnlyElement( - getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_INPUT_VARIABLE)); + getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_INPUT_VARIABLE); String interfaceLibraryOutput = - Iterables.getOnlyElement( - getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_OUTPUT_VARIABLE)); + getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARY_OUTPUT_VARIABLE); String generateInterfaceLibrary = - Iterables.getOnlyElement( - getVariableValue(variables, CppLinkActionBuilder.GENERATE_INTERFACE_LIBRARY_VARIABLE)); + getVariableValue(variables, CppLinkActionBuilder.GENERATE_INTERFACE_LIBRARY_VARIABLE); assertThat(generateInterfaceLibrary).isEqualTo("no"); assertThat(interfaceLibraryInput).endsWith("ignored"); |