diff options
author | Damien Martin-Guillerez <dmarting@google.com> | 2016-07-25 14:06:45 +0000 |
---|---|---|
committer | Damien Martin-Guillerez <dmarting@google.com> | 2016-07-25 14:28:06 +0000 |
commit | 7326e15c113ecb9d43e7e091a1c427ea39e13914 (patch) | |
tree | 62b76ae78a36db1d02b276142b70024ed70d6322 /src/test | |
parent | 3862e521eed9c58e0bbb22c163d86c0d5e373b00 (diff) |
Rollback of commit b669406789dd452161875d407d0ce6a3502de5f6.
*** Reason for rollback ***
Breaks TensorFlow and rules_go on ci.bazel.io
Fixes #1562.
Fixes #1558.
*** Original change description ***
Refactor CppLinkAction to construct its command line using the crosstool instead of a hardcoded switch. Add action configs to CppConfiguration for each link type.
--
MOS_MIGRATED_REVID=128352435
Diffstat (limited to 'src/test')
-rw-r--r-- | src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java | 120 | ||||
-rw-r--r-- | src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java | 134 |
2 files changed, 12 insertions, 242 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java index 7531632e8a..c4ea2be93f 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java @@ -17,8 +17,8 @@ package com.google.devtools.build.lib.rules.cpp; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Action; @@ -34,17 +34,18 @@ import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; -import com.google.devtools.build.lib.rules.cpp.CppLinkActionConfigs.CppLinkPlatform; import com.google.devtools.build.lib.rules.cpp.Link.LinkStaticness; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; import com.google.devtools.build.lib.vfs.PathFragment; -import java.io.IOException; + import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** Tests for {@link CppLinkAction}. */ +/** + * Tests for {@link CppLinkAction}. + */ @RunWith(JUnit4.class) public class CppLinkActionTest extends BuildViewTestCase { private RuleContext createDummyRuleContext() throws Exception { @@ -65,18 +66,6 @@ public class CppLinkActionTest extends BuildViewTestCase { } }, masterConfig); } - - private final FeatureConfiguration getMockFeatureConfiguration() throws Exception { - return CcToolchainFeaturesTest.buildFeatures( - CppLinkActionConfigs.getCppLinkActionConfigs(CppLinkPlatform.LINUX)) - .getFeatureConfiguration( - Link.LinkTargetType.EXECUTABLE.getActionName(), - Link.LinkTargetType.DYNAMIC_LIBRARY.getActionName(), - Link.LinkTargetType.STATIC_LIBRARY.getActionName(), - Link.LinkTargetType.PIC_STATIC_LIBRARY.getActionName(), - Link.LinkTargetType.ALWAYS_LINK_STATIC_LIBRARY.getActionName(), - Link.LinkTargetType.ALWAYS_LINK_PIC_STATIC_LIBRARY.getActionName()); - } @Test public void testToolchainFeatureFlags() throws Exception { @@ -104,9 +93,9 @@ public class CppLinkActionTest extends BuildViewTestCase { "out", ImmutableList.<Artifact>of(), ImmutableList.<LibraryToLink>of(), - featureConfiguration, - false) + featureConfiguration) .build(); + assertThat(Joiner.on(" ").join(linkAction.getArgv())).contains("mock_tool"); assertThat(linkAction.getArgv()).contains("some_flag"); } @@ -129,8 +118,7 @@ public class CppLinkActionTest extends BuildViewTestCase { "out", ImmutableList.<Artifact>of(), ImmutableList.<LibraryToLink>of(), - featureConfiguration, - false) + featureConfiguration) .build(); assertThat(linkAction.getEnvironment()).containsEntry("foo", "bar"); } @@ -147,8 +135,6 @@ public class CppLinkActionTest extends BuildViewTestCase { final Artifact oFile = getSourceArtifact("cc/a.o"); final Artifact oFile2 = getSourceArtifact("cc/a2.o"); final Artifact interfaceSoBuilder = getBinArtifactWithNoOwner("foo/build_interface_so"); - final FeatureConfiguration featureConfiguration = getMockFeatureConfiguration(); - ActionTester.runTest( 128, new ActionCombinationFactory() { @@ -172,7 +158,7 @@ public class CppLinkActionTest extends BuildViewTestCase { builder.setWholeArchive((i & 16) == 0); builder.setFake((i & 32) == 0); builder.setRuntimeSolibDir((i & 64) == 0 ? null : new PathFragment("so1")); - builder.setFeatureConfiguration(featureConfiguration); + builder.setFeatureConfiguration(new FeatureConfiguration()); return builder.build(); } @@ -191,8 +177,6 @@ public class CppLinkActionTest extends BuildViewTestCase { final Artifact oFile = getSourceArtifact("cc/a.o"); final Artifact oFile2 = getSourceArtifact("cc/a2.o"); final Artifact interfaceSoBuilder = getBinArtifactWithNoOwner("foo/build_interface_so"); - final FeatureConfiguration featureConfiguration = getMockFeatureConfiguration(); - ActionTester.runTest( 4, new ActionCombinationFactory() { @@ -210,7 +194,6 @@ public class CppLinkActionTest extends BuildViewTestCase { (i & 1) == 0 ? ImmutableList.of(oFile) : ImmutableList.of(oFile2)); builder.setLinkType( (i & 2) == 0 ? LinkTargetType.STATIC_LIBRARY : LinkTargetType.DYNAMIC_LIBRARY); - builder.setFeatureConfiguration(featureConfiguration); return builder.build(); } }); @@ -271,8 +254,7 @@ public class CppLinkActionTest extends BuildViewTestCase { "binary2", objects.build(), ImmutableList.<LibraryToLink>of(), - new FeatureConfiguration(), - false) + new FeatureConfiguration()) .setFake(true) .build(); @@ -303,8 +285,7 @@ public class CppLinkActionTest extends BuildViewTestCase { String outputPath, Iterable<Artifact> nonLibraryInputs, ImmutableList<LibraryToLink> libraryInputs, - FeatureConfiguration featureConfiguration, - boolean shouldIncludeToolchain) + FeatureConfiguration featureConfiguration) throws Exception { RuleContext ruleContext = createDummyRuleContext(); CppLinkActionBuilder builder = @@ -313,7 +294,7 @@ public class CppLinkActionTest extends BuildViewTestCase { new Artifact( new PathFragment(outputPath), getTargetConfiguration().getBinDirectory()), ruleContext.getConfiguration(), - shouldIncludeToolchain ? CppHelper.getToolchain(ruleContext) : null) + null) .addNonLibraryInputs(nonLibraryInputs) .addLibraries(NestedSetBuilder.wrap(Order.LINK_ORDER, libraryInputs)) .setLinkType(type) @@ -325,17 +306,6 @@ public class CppLinkActionTest extends BuildViewTestCase { .setFeatureConfiguration(featureConfiguration); return builder; } - - private CppLinkActionBuilder createLinkBuilder(Link.LinkTargetType type) throws Exception { - PathFragment output = new PathFragment("dummyRuleContext/output/path.xyz"); - return createLinkBuilder( - type, - output.getPathString(), - ImmutableList.<Artifact>of(), - ImmutableList.<LibraryToLink>of(), - new FeatureConfiguration(), - true); - } public Artifact getOutputArtifact(String relpath) { return new Artifact( @@ -343,70 +313,4 @@ public class CppLinkActionTest extends BuildViewTestCase { getTargetConfiguration().getBinDirectory(), getTargetConfiguration().getBinFragment().getRelative(relpath)); } - - private Artifact scratchArtifact(String s) { - try { - return new Artifact( - scratch.overwriteFile(outputBase.getRelative("WORKSPACE").getRelative(s).toString()), - Root.asDerivedRoot(scratch.dir(outputBase.getRelative("WORKSPACE").toString()))); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - - private void assertError(String expectedSubstring, CppLinkActionBuilder builder) { - try { - builder.build(); - fail(); - } catch (RuntimeException e) { - assertThat(e.getMessage()).contains(expectedSubstring); - } - } - - @Test - public void testInterfaceOutputWithoutBuildingDynamicLibraryIsError() throws Exception { - CppLinkActionBuilder builder = - createLinkBuilder(LinkTargetType.EXECUTABLE) - .setInterfaceOutput(scratchArtifact("FakeInterfaceOutput")); - - assertError("Interface output can only be used with non-fake DYNAMIC_LIBRARY targets", builder); - } - - @Test - public void testStaticLinkWithDynamicIsError() throws Exception { - CppLinkActionBuilder builder = - createLinkBuilder(LinkTargetType.STATIC_LIBRARY).setLinkStaticness(LinkStaticness.DYNAMIC); - - assertError("static library link must be static", builder); - } - - @Test - public void testStaticLinkWithSymbolsCountOutputIsError() throws Exception { - CppLinkActionBuilder builder = - createLinkBuilder(LinkTargetType.STATIC_LIBRARY) - .setLinkStaticness(LinkStaticness.FULLY_STATIC) - .setSymbolCountsOutput(scratchArtifact("dummySymbolCounts")); - - assertError("the symbol counts output must be null for static links", builder); - } - - @Test - public void testStaticLinkWithNativeDepsIsError() throws Exception { - CppLinkActionBuilder builder = - createLinkBuilder(LinkTargetType.STATIC_LIBRARY) - .setLinkStaticness(LinkStaticness.FULLY_STATIC) - .setNativeDeps(true); - - assertError("the native deps flag must be false for static links", builder); - } - - @Test - public void testStaticLinkWithWholeArchiveIsError() throws Exception { - CppLinkActionBuilder builder = - createLinkBuilder(LinkTargetType.STATIC_LIBRARY) - .setLinkStaticness(LinkStaticness.FULLY_STATIC) - .setWholeArchive(true); - - assertError("the need whole archive flag must be false for static links", builder); - } } 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 deleted file mode 100644 index 50fdebad0f..0000000000 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java +++ /dev/null @@ -1,134 +0,0 @@ -// Copyright 2016 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.rules.cpp; - -import static com.google.common.truth.Truth.assertThat; - -import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; -import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; -import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; -import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; -import java.util.List; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests that {@code CppLinkAction} is populated with the correct build variables. */ -@RunWith(JUnit4.class) -public class LinkBuildVariablesTest extends BuildViewTestCase { - - private CppLinkAction getCppLinkAction(ConfiguredTarget target, Link.LinkTargetType type) { - Artifact linkerOutput = null; - switch (type) { - case STATIC_LIBRARY: - case ALWAYS_LINK_STATIC_LIBRARY: - linkerOutput = getBinArtifact("lib" + target.getLabel().getName() + ".a", target); - break; - case PIC_STATIC_LIBRARY: - case ALWAYS_LINK_PIC_STATIC_LIBRARY: - linkerOutput = getBinArtifact("lib" + target.getLabel().getName() + "pic.a", target); - break; - case DYNAMIC_LIBRARY: - linkerOutput = getBinArtifact("lib" + target.getLabel().getName() + ".so", target); - break; - case EXECUTABLE: - linkerOutput = getExecutable(target); - break; - default: - throw new IllegalArgumentException( - String.format("Cannot get CppLinkAction for link type %s", type)); - } - return (CppLinkAction) getGeneratingAction(linkerOutput); - } - - private Variables getLinkBuildVariables(ConfiguredTarget target, Link.LinkTargetType type) { - return getCppLinkAction(target, type).getLinkCommandLine().getBuildVariables(); - } - - private List<String> getVariableValue(Variables variables, String variable) throws Exception { - FeatureConfiguration mockFeatureConfiguration = - CcToolchainFeaturesTest.buildFeatures( - "feature {", - " name: 'a'", - " flag_set {", - " action: 'foo'", - " flag_group {", - " flag: '%{" + variable + "}'", - " }", - " }", - "}") - .getFeatureConfiguration("a"); - return mockFeatureConfiguration.getCommandLine("foo", variables); - } - - @Test - public void testLinkstampBuildVariable() throws Exception { - scratch.file( - "x/BUILD", - "cc_binary(", - " name = 'bin',", - " srcs = ['a.cc'],", - " deps = [':lib'],", - ")", - "cc_library(", - " name = 'lib',", - " srcs = ['b.cc'],", - " linkstamp = 'c.cc',", - ")"); - scratch.file("x/a.cc"); - scratch.file("x/b.cc"); - scratch.file("x/c.cc"); - - ConfiguredTarget target = getConfiguredTarget("//x:bin"); - Variables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE); - List<String> variableValue = - getVariableValue(variables, CppLinkActionBuilder.LINKSTAMP_PATHS_VARIABLE); - assertThat(Iterables.getOnlyElement(variableValue)).contains("c.o"); - } - - @Test - public void testForcePicBuildVariable() throws Exception { - useConfiguration("--force_pic"); - scratch.file("x/BUILD", "cc_binary(", " name = 'bin',", " srcs = ['a.cc'],", ")"); - scratch.file("x/a.cc"); - - ConfiguredTarget target = getConfiguredTarget("//x:bin"); - Variables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE); - List<String> variableValue = - getVariableValue(variables, CppLinkActionBuilder.FORCE_PIC_VARIABLE); - assertThat(variableValue).contains(""); - } - - @Test - public void testWholeArchiveBuildVariables() throws Exception { - scratch.file( - "x/BUILD", - "cc_binary(", - " name = 'bin.so',", - " srcs = ['a.cc'],", - " linkopts = ['-shared'],", - " linkstatic = 1", - ")"); - scratch.file("x/a.cc"); - - ConfiguredTarget target = getConfiguredTarget("//x:bin.so"); - Variables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE); - List<String> variableValue = - getVariableValue(variables, CppLinkActionBuilder.GLOBAL_WHOLE_ARCHIVE_VARIABLE); - assertThat(variableValue).contains(""); - } -} |