From 18a7701810a5685952d5827c69bdf59601521c6f Mon Sep 17 00:00:00 2001 From: Luca Di Grazia Date: Sun, 4 Sep 2022 20:27:41 +0200 Subject: [PATCH] Automated rollback of commit 64a966dd8cd5dc564d179d2fe9ecb1c3c7b56b14. *** Reason for rollback *** Breaks a target in the canary's nightly, thus preventing us from releasing tomorrow: [] b/124656723 *** Original change description *** Introduce --incompatible_remove_legacy_whole_archive This flag disables legacy whole archive behavior, as well as makes the --legacy_whole_archive option a noop. See details in https://github.com/bazelbuild/bazel/issues/7362. It also introduces a possibility to enable legacy behavior per target by `--features=legacy_whole_archive` option, or `features = [ "legacy_whole_archive" ]` attribute. RELNOTES: None. PiperOrigin-RevId: 234481950 --- .../build/lib/rules/cpp/CppConfiguration.java | 8 - .../lib/rules/cpp/CppLinkActionBuilder.java | 40 +--- .../build/lib/rules/cpp/CppOptions.java | 52 +---- .../build/lib/rules/cpp/CppRuleClasses.java | 3 - .../rules/nativedeps/NativeDepsHelper.java | 10 +- .../lib/rules/cpp/CppLinkActionTest.java | 193 ++++++++---------- 6 files changed, 101 insertions(+), 205 deletions(-) diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index 5a614b43b33..7dd60c96768 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -396,10 +396,6 @@ public boolean legacyWholeArchive() { return cppOptions.legacyWholeArchive; } - public boolean removeLegacyWholeArchive() { - return cppOptions.removeLegacyWholeArchive; - } - public boolean getInmemoryDotdFiles() { return cppOptions.inmemoryDotdFiles; } @@ -548,10 +544,6 @@ public boolean disableLegacyCrosstoolFields() { return cppOptions.disableLegacyCrosstoolFields; } - public boolean disableExpandIfAllAvailableInFlagSet() { - return cppOptions.disableExpandIfAllAvailableInFlagSet; - } - public static String getLegacyCrosstoolFieldErrorMessage(String field) { Preconditions.checkNotNull(field); return field diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java index cf1e82c6381..574504828b4 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java @@ -138,7 +138,7 @@ public Artifact create( private boolean isLtoIndexing = false; private boolean usePicForLtoBackendActions = false; private Iterable allLtoArtifacts = null; - + private final List variablesExtensions = new ArrayList<>(); private final NestedSetBuilder linkActionInputs = NestedSetBuilder.stableOrder(); private final ImmutableList.Builder linkActionOutputs = ImmutableList.builder(); @@ -646,8 +646,7 @@ public CppLinkAction build() throws InterruptedException { boolean needWholeArchive = wholeArchive - || needWholeArchive( - featureConfiguration, linkingMode, linkType, linkopts, cppConfiguration); + || needWholeArchive(linkingMode, linkType, linkopts, isNativeDeps, cppConfiguration); // Disallow LTO indexing for test targets that link statically, and optionally for any // linkstatic target (which can be used to disable LTO indexing for non-testonly cc_binary // built due to data dependences for a blaze test invocation). Otherwise this will provoke @@ -1118,42 +1117,15 @@ private boolean shouldUseLinkDynamicLibraryTool() { /** The default heuristic on whether we need to use whole-archive for the link. */ private static boolean needWholeArchive( - FeatureConfiguration featureConfiguration, - LinkingMode linkingMode, + Link.LinkingMode staticness, LinkTargetType type, Collection linkopts, + boolean isNativeDeps, CppConfiguration cppConfig) { + boolean mostlyStatic = (staticness == Link.LinkingMode.STATIC); boolean sharedLinkopts = type.isDynamicLibrary() || linkopts.contains("-shared") || cppConfig.hasSharedLinkOption(); - // Fasten your seat belt, the logic below doesn't make perfect sense and it's full of obviously - // missed corner cases. The world still stands and depends on this behavior, so ¯\_(ツ)_/¯. - if (!sharedLinkopts) { - // We are not producing shared library, there is no reason to use --whole-archive with - // executable (if the executable doesn't use the symbols, nobody else will, so --whole-archive - // is not needed). - return false; - } - if (cppConfig.removeLegacyWholeArchive()) { - // --incompatible_remove_legacy_whole_archive has been flipped, no --whole-archive for the - // entire build. - return false; - } - if (linkingMode != LinkingMode.STATIC) { - // legacy whole archive only applies to static linking mode. - return false; - } - if (featureConfiguration.getRequestedFeatures().contains(CppRuleClasses.LEGACY_WHOLE_ARCHIVE)) { - // --incompatible_remove_legacy_whole_archive has not been flipped, and this target requested - // --whole-archive using features. - return true; - } - if (cppConfig.legacyWholeArchive()) { - // --incompatible_remove_legacy_whole_archive has not been flipped, so whether to - // use --whole-archive depends on --legacy_whole_archive. - return true; - } - // Hopefully future default. - return false; + return (isNativeDeps || cppConfig.legacyWholeArchive()) && mostlyStatic && sharedLinkopts; } private static ImmutableSet constructOutputs( diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java index 6e6415c767e..526e9b0ec7d 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java @@ -360,18 +360,16 @@ public String getTypeDescription() { public Label customMalloc; @Option( - name = "legacy_whole_archive", - defaultValue = "true", - documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS, - effectTags = {OptionEffectTag.ACTION_COMMAND_LINES, OptionEffectTag.AFFECTS_OUTPUTS}, - metadataTags = {OptionMetadataTag.DEPRECATED}, - help = - "Deprecated, superseded by --incompatible_remove_legacy_whole_archive " - + "(see https://github.com/bazelbuild/bazel/issues/7362 for details). " - + "When on, use --whole-archive for cc_binary rules that have " - + "linkshared=1 and either linkstatic=1 or '-static' in linkopts. " - + "This is for backwards compatibility only. " - + "A better alternative is to use alwayslink=1 where required.") + name = "legacy_whole_archive", + defaultValue = "true", + documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS, + effectTags = {OptionEffectTag.ACTION_COMMAND_LINES, OptionEffectTag.AFFECTS_OUTPUTS}, + help = + "When on, use --whole-archive for cc_binary rules that have " + + "linkshared=1 and either linkstatic=1 or '-static' in linkopts. " + + "This is for backwards compatibility only. " + + "A better alternative is to use alwayslink=1 where required." + ) public boolean legacyWholeArchive; @Option( @@ -714,20 +712,6 @@ public Label getFdoPrefetchHintsLabel() { + "(see https://github.com/bazelbuild/bazel/issues/6861 for migration instructions).") public boolean disableLegacyCrosstoolFields; - @Option( - name = "incompatible_remove_legacy_whole_archive", - defaultValue = "false", - documentationCategory = OptionDocumentationCategory.TOOLCHAIN, - effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, - metadataTags = { - OptionMetadataTag.INCOMPATIBLE_CHANGE, - OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES - }, - help = - "If true, Bazel will not link library dependencies as whole archive by default " - + "(see https://github.com/bazelbuild/bazel/issues/7362 for migration instructions).") - public boolean removeLegacyWholeArchive; - @Option( name = "incompatible_remove_cpu_and_compiler_attributes_from_cc_toolchain", defaultValue = "false", @@ -743,20 +727,6 @@ public Label getFdoPrefetchHintsLabel() { + "(see https://github.com/bazelbuild/bazel/issues/7075 for migration instructions).") public boolean removeCpuCompilerCcToolchainAttributes; - @Option( - name = "incompatible_disable_expand_if_all_available_in_flag_set", - defaultValue = "false", - documentationCategory = OptionDocumentationCategory.TOOLCHAIN, - effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, - metadataTags = { - OptionMetadataTag.INCOMPATIBLE_CHANGE, - OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES - }, - help = - "If true, Bazel will not allow specifying expand_if_all_available in flag_sets" - + "(see https://github.com/bazelbuild/bazel/issues/7008 for migration instructions).") - public boolean disableExpandIfAllAvailableInFlagSet; - @Option( name = "incompatible_disable_crosstool_file", defaultValue = "false", @@ -893,13 +863,11 @@ public FragmentOptions getHost() { host.doNotUseCpuTransformer = doNotUseCpuTransformer; host.disableGenruleCcToolchainDependency = disableGenruleCcToolchainDependency; host.disableDepsetInUserFlags = disableDepsetInUserFlags; - host.disableExpandIfAllAvailableInFlagSet = disableExpandIfAllAvailableInFlagSet; host.disableLegacyCcProvider = disableLegacyCcProvider; host.removeCpuCompilerCcToolchainAttributes = removeCpuCompilerCcToolchainAttributes; host.disableLegacyCrosstoolFields = disableLegacyCrosstoolFields; host.disableCrosstool = disableCrosstool; host.enableCcToolchainResolution = enableCcToolchainResolution; - host.removeLegacyWholeArchive = removeLegacyWholeArchive; host.dontEnableHostNonhost = dontEnableHostNonhost; return host; } diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java index 03999104b63..ef836e76054 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java @@ -412,9 +412,6 @@ public static Label ccToolchainTypeAttribute(RuleDefinitionEnvironment env) { */ public static final String SUPPORTS_DYNAMIC_LINKER = "supports_dynamic_linker"; - /** A feature marking that the target needs to link its deps in --whole-archive block. */ - public static final String LEGACY_WHOLE_ARCHIVE = "legacy_whole_archive"; - /** Ancestor for all rules that do include scanning. */ public static final class CcIncludeScanningRule implements RuleDefinition { @Override diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java index ce5d87d879d..c40af6c7d4e 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java @@ -230,15 +230,13 @@ public static NativeDepsRunfiles createNativeDepsAction( sharedLibrary = nativeDeps; } FdoContext fdoContext = toolchain.getFdoContext(); - ImmutableSet.Builder requestedFeatures = - ImmutableSet.builder().addAll(ruleContext.getFeatures()).add(STATIC_LINKING_MODE); - if (!ruleContext.getDisabledFeatures().contains(CppRuleClasses.LEGACY_WHOLE_ARCHIVE)) { - requestedFeatures.add(CppRuleClasses.LEGACY_WHOLE_ARCHIVE); - } FeatureConfiguration featureConfiguration = CcCommon.configureFeaturesOrReportRuleError( ruleContext, - /* requestedFeatures= */ requestedFeatures.build(), + /* requestedFeatures= */ ImmutableSet.builder() + .addAll(ruleContext.getFeatures()) + .add(STATIC_LINKING_MODE) + .build(), /* unsupportedFeatures= */ ruleContext.getDisabledFeatures(), toolchain); CppLinkActionBuilder builder = diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java index c5746450573..3392bfba4b2 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java @@ -41,7 +41,6 @@ import com.google.devtools.build.lib.analysis.util.ActionTester.ActionCombinationFactory; import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; -import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; @@ -96,19 +95,6 @@ public Artifact getDerivedArtifact(PathFragment rootRelativePath, ArtifactRoot r private final FeatureConfiguration getMockFeatureConfiguration(RuleContext ruleContext) throws Exception { - CToolchain.FlagGroup flagGroup = - CToolchain.FlagGroup.newBuilder().addFlag("-lcpp_standard_library").build(); - CToolchain.FlagSet flagSet = - CToolchain.FlagSet.newBuilder() - .addAction("c++-link-executable") - .addFlagGroup(flagGroup) - .build(); - CToolchain.Feature linkCppStandardLibrary = - CToolchain.Feature.newBuilder() - .setName("link_cpp_standard_library") - .setEnabled(true) - .addFlagSet(flagSet) - .build(); ImmutableList features = new ImmutableList.Builder() .addAll( @@ -119,7 +105,6 @@ private final FeatureConfiguration getMockFeatureConfiguration(RuleContext ruleC /* supportsEmbeddedRuntimes= */ true, /* supportsInterfaceSharedLibraries= */ false)) .addAll(CppActionConfigs.getFeaturesToAppearLastInFeaturesList(ImmutableSet.of())) - .add(linkCppStandardLibrary) .build(); ImmutableList actionConfigs = @@ -133,7 +118,6 @@ private final FeatureConfiguration getMockFeatureConfiguration(RuleContext ruleC return CcToolchainFeaturesTest.buildFeatures(ruleContext, features, actionConfigs) .getFeatureConfiguration( ImmutableSet.of( - "link_cpp_standard_library", Link.LinkTargetType.EXECUTABLE.getActionName(), Link.LinkTargetType.NODEPS_DYNAMIC_LIBRARY.getActionName(), Link.LinkTargetType.DYNAMIC_LIBRARY.getActionName(), @@ -227,66 +211,6 @@ public void testLibOptsAndLibSrcsAreInCorrectOrder() throws Exception { .matches(".* -Wl,-rpath[^ ]*some-dir(?= ).* -Wl,-rpath[^ ]*some-other-dir .*"); } - @Test - public void testLegacyWholeArchiveHasNoEffectOnDynamicModeDynamicLibraries() throws Exception { - getAnalysisMock() - .ccSupport() - .setupCrosstool(mockToolsConfig, MockCcSupport.SUPPORTS_DYNAMIC_LINKER_FEATURE); - scratch.file( - "x/BUILD", - "cc_binary(", - " name = 'libfoo.so',", - " srcs = ['foo.cc'],", - " linkshared = 1,", - " linkstatic = 0,", - ")"); - useConfiguration("--legacy_whole_archive"); - assertThat(getLibfooArguments()).doesNotContain("-Wl,-whole-archive"); - } - - @Test - public void testLegacyWholeArchive() throws Exception { - getAnalysisMock() - .ccSupport() - .setupCrosstool(mockToolsConfig, MockCcSupport.SUPPORTS_DYNAMIC_LINKER_FEATURE); - scratch.file( - "x/BUILD", - "cc_binary(", - " name = 'libfoo.so',", - " srcs = ['foo.cc'],", - " linkshared = 1,", - ")"); - // --incompatible_remove_legacy_whole_archive not flipped, --legacy_whole_archive wins. - useConfiguration("--legacy_whole_archive", "--noincompatible_remove_legacy_whole_archive"); - assertThat(getLibfooArguments()).contains("-Wl,-whole-archive"); - useConfiguration("--nolegacy_whole_archive", "--noincompatible_remove_legacy_whole_archive"); - assertThat(getLibfooArguments()).doesNotContain("-Wl,-whole-archive"); - - // --incompatible_remove_legacy_whole_archive flipped, --legacy_whole_archive ignored. - useConfiguration("--legacy_whole_archive", "--incompatible_remove_legacy_whole_archive"); - assertThat(getLibfooArguments()).doesNotContain("-Wl,-whole-archive"); - useConfiguration("--nolegacy_whole_archive", "--incompatible_remove_legacy_whole_archive"); - assertThat(getLibfooArguments()).doesNotContain("-Wl,-whole-archive"); - - // Even when --nolegacy_whole_archive, features can still add the behavior back. - useConfiguration( - "--nolegacy_whole_archive", - "--noincompatible_remove_legacy_whole_archive", - "--features=legacy_whole_archive"); - assertThat(getLibfooArguments()).contains("-Wl,-whole-archive"); - // Even when --nolegacy_whole_archive, features can still add the behavior, but not when - // --incompatible_remove_legacy_whole_archive is flipped. - useConfiguration( - "--incompatible_remove_legacy_whole_archive", "--features=legacy_whole_archive"); - assertThat(getLibfooArguments()).doesNotContain("-Wl,-whole-archive"); - } - - private List getLibfooArguments() throws LabelSyntaxException { - ConfiguredTarget configuredTarget = getConfiguredTarget("//x:libfoo.so"); - CppLinkAction linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/libfoo.so"); - return linkAction.getArguments(); - } - @Test public void testExposesRuntimeLibrarySearchDirectoriesVariable() throws Exception { scratch.file( @@ -320,17 +244,10 @@ public void testExposesRuntimeLibrarySearchDirectoriesVariable() throws Exceptio public void testCompilesTestSourcesIntoDynamicLibrary() throws Exception { if (OS.getCurrent() == OS.WINDOWS) { // Skip the test on Windows. - // TODO(#7524): This test should work on Windows just fine, investigate and fix. + // TODO(bazel-team): maybe we should move that test that doesn't work with MSVC toolchain to + // its own suite with a TestSpec? return; } - getAnalysisMock() - .ccSupport() - .setupCrosstool( - mockToolsConfig, - MockCcSupport.SUPPORTS_PIC_FEATURE, - MockCcSupport.SUPPORTS_DYNAMIC_LINKER_FEATURE, - MockCcSupport.SUPPORTS_INTERFACE_SHARED_LIBRARIES_FEATURE); - scratch.file( "x/BUILD", "cc_test(name = 'a', srcs = ['a.cc'])", @@ -339,15 +256,17 @@ public void testCompilesTestSourcesIntoDynamicLibrary() throws Exception { useConfiguration("--experimental_link_compile_output_separately", "--force_pic"); ConfiguredTarget configuredTarget = getConfiguredTarget("//x:a"); + String cpu = CrosstoolConfigurationHelper.defaultCpu(); CppLinkAction linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/a"); assertThat(artifactsToStrings(linkAction.getInputs())) - .contains("bin _solib_k8/libx_Sliba.ifso"); + .contains("bin _solib_" + cpu + "/libx_Sliba.ifso"); assertThat(linkAction.getArguments()) - .contains(getBinArtifactWithNoOwner("_solib_k8/libx_Sliba.ifso").getExecPathString()); + .contains( + getBinArtifactWithNoOwner("_solib_" + cpu + "/libx_Sliba.ifso").getExecPathString()); RunfilesProvider runfilesProvider = configuredTarget.getProvider(RunfilesProvider.class); assertThat(artifactsToStrings(runfilesProvider.getDefaultRunfiles().getArtifacts())) - .contains("bin _solib_k8/libx_Sliba.so"); + .contains("bin _solib_" + cpu + "/libx_Sliba.so"); configuredTarget = getConfiguredTarget("//x:b"); linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/b"); @@ -361,16 +280,10 @@ public void testCompilesTestSourcesIntoDynamicLibrary() throws Exception { public void testCompilesDynamicModeTestSourcesWithFeatureIntoDynamicLibrary() throws Exception { if (OS.getCurrent() == OS.WINDOWS) { // Skip the test on Windows. - // TODO(#7524): This test should work on Windows just fine, investigate and fix. + // TODO(bazel-team): maybe we should move that test that doesn't work with MSVC toolchain to + // its own suite with a TestSpec? return; } - getAnalysisMock() - .ccSupport() - .setupCrosstool( - mockToolsConfig, - MockCcSupport.SUPPORTS_PIC_FEATURE, - MockCcSupport.SUPPORTS_DYNAMIC_LINKER_FEATURE, - MockCcSupport.SUPPORTS_INTERFACE_SHARED_LIBRARIES_FEATURE); scratch.file( "x/BUILD", "cc_test(name='a', srcs=['a.cc'], features=['dynamic_link_test_srcs'])", @@ -380,14 +293,16 @@ public void testCompilesDynamicModeTestSourcesWithFeatureIntoDynamicLibrary() th useConfiguration("--force_pic"); ConfiguredTarget configuredTarget = getConfiguredTarget("//x:a"); + String cpu = CrosstoolConfigurationHelper.defaultCpu(); CppLinkAction linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/a"); assertThat(artifactsToStrings(linkAction.getInputs())) - .contains("bin _solib_k8/libx_Sliba.ifso"); + .contains("bin _solib_" + cpu + "/libx_Sliba.ifso"); assertThat(linkAction.getArguments()) - .contains(getBinArtifactWithNoOwner("_solib_k8/libx_Sliba.ifso").getExecPathString()); + .contains( + getBinArtifactWithNoOwner("_solib_" + cpu + "/libx_Sliba.ifso").getExecPathString()); RunfilesProvider runfilesProvider = configuredTarget.getProvider(RunfilesProvider.class); assertThat(artifactsToStrings(runfilesProvider.getDefaultRunfiles().getArtifacts())) - .contains("bin _solib_k8/libx_Sliba.so"); + .contains("bin _solib_" + cpu + "/libx_Sliba.so"); configuredTarget = getConfiguredTarget("//x:b"); linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/b"); @@ -409,24 +324,20 @@ public void testCompilesDynamicModeBinarySourcesWithoutFeatureIntoDynamicLibrary throws Exception { if (OS.getCurrent() == OS.WINDOWS) { // Skip the test on Windows. - // TODO(#7524): This test should work on Windows just fine, investigate and fix. + // TODO(bazel-team): maybe we should move that test that doesn't work with MSVC toolchain to + // its own suite with a TestSpec? return; } - getAnalysisMock() - .ccSupport() - .setupCrosstool( - mockToolsConfig, - MockCcSupport.SUPPORTS_DYNAMIC_LINKER_FEATURE, - MockCcSupport.SUPPORTS_PIC_FEATURE); scratch.file( "x/BUILD", "cc_binary(name = 'a', srcs = ['a.cc'], features = ['-static_link_test_srcs'])"); scratch.file("x/a.cc", "int main() {}"); useConfiguration("--force_pic", "--dynamic_mode=default"); ConfiguredTarget configuredTarget = getConfiguredTarget("//x:a"); + String cpu = CrosstoolConfigurationHelper.defaultCpu(); CppLinkAction linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/a"); assertThat(artifactsToStrings(linkAction.getInputs())) - .doesNotContain("bin _solib_k8/libx_Sliba.ifso"); + .doesNotContain("bin _solib_" + cpu + "/libx_Sliba.ifso"); assertThat(artifactsToStrings(linkAction.getInputs())).contains("bin x/_objs/a/a.pic.o"); RunfilesProvider runfilesProvider = configuredTarget.getProvider(RunfilesProvider.class); assertThat(artifactsToStrings(runfilesProvider.getDefaultRunfiles().getArtifacts())) @@ -751,8 +662,8 @@ public void testInterfaceOutputForDynamicLibrary() throws Exception { .ccSupport() .setupCrosstool( mockToolsConfig, - MockCcSupport.SUPPORTS_INTERFACE_SHARED_LIBRARIES_FEATURE, - MockCcSupport.SUPPORTS_DYNAMIC_LINKER_FEATURE); + MockCcSupport.SUPPORTS_INTERFACE_SHARED_LIBRARIES, + "supports_interface_shared_objects: false"); useConfiguration(); scratch.file("foo/BUILD", "cc_library(name = 'foo', srcs = ['foo.cc'])"); @@ -774,7 +685,7 @@ public void testInterfaceOutputForDynamicLibraryLegacy() throws Exception { FeatureConfiguration featureConfiguration = CcToolchainFeaturesTest.buildFeatures( ruleContext, - MockCcSupport.SUPPORTS_INTERFACE_SHARED_LIBRARIES_FEATURE, + "supports_interface_shared_objects: true ", "feature {", " name: 'build_interface_libraries'", " flag_set {", @@ -936,6 +847,64 @@ public void expand(Artifact artifact, Collection output) { treeArtifactsPaths); } + @Test + public void testLinksTreeArtifactLibraryForDeps() throws Exception { + // This test only makes sense if start/end lib archives are supported. + analysisMock.ccSupport().setupCrosstool(mockToolsConfig, "supports_start_end_lib: true"); + useConfiguration("--start_end_lib"); + RuleContext ruleContext = createDummyRuleContext(); + + SpecialArtifact testTreeArtifact = createTreeArtifact("library_directory"); + + TreeFileArtifact library0 = ActionInputHelper.treeFileArtifact(testTreeArtifact, "library0.o"); + TreeFileArtifact library1 = ActionInputHelper.treeFileArtifact(testTreeArtifact, "library1.o"); + + ArtifactExpander expander = + new ArtifactExpander() { + @Override + public void expand(Artifact artifact, Collection output) { + if (artifact.equals(testTreeArtifact)) { + output.add(library0); + output.add(library1); + } + }; + }; + + Artifact archiveFile = scratchArtifact("library.a"); + + CppLinkActionBuilder builder = + createLinkBuilder(ruleContext, LinkTargetType.STATIC_LIBRARY) + .setLibraryIdentifier("foo") + .addLibrary( + LinkerInputs.newInputLibrary( + archiveFile, + ArtifactCategory.STATIC_LIBRARY, + null, + ImmutableList.of(testTreeArtifact), + LtoCompilationContext.EMPTY, + null, + /* mustKeepDebug= */ false)); + + CppLinkAction linkAction = builder.build(); + + Iterable treeArtifactsPaths = ImmutableList.of(testTreeArtifact.getExecPathString()); + Iterable treeFileArtifactsPaths = + ImmutableList.of(library0.getExecPathString(), library1.getExecPathString()); + + // Should only reference the tree artifact. + verifyArguments( + linkAction.getLinkCommandLine().getRawLinkArgv(), + treeArtifactsPaths, + treeFileArtifactsPaths); + verifyArguments(linkAction.getArguments(), treeArtifactsPaths, treeFileArtifactsPaths); + + // Should only reference tree file artifacts. + verifyArguments( + linkAction.getLinkCommandLine().getRawLinkArgv(expander), + treeFileArtifactsPaths, + treeArtifactsPaths); + } + @Test public void testStaticLinking() throws Exception { RuleContext ruleContext = createDummyRuleContext(); @@ -1040,7 +1009,7 @@ public void testPieOptionKeptForExecutables() throws Exception { public void testLinkoptsComeAfterLinkerInputs() throws Exception { RuleContext ruleContext = createDummyRuleContext(); - String solibPrefix = "_solib_k8"; + String solibPrefix = "_solib_" + CrosstoolConfigurationHelper.defaultCpu(); Iterable linkerInputs = LinkerInputs.opaqueLibrariesToLink( ArtifactCategory.DYNAMIC_LIBRARY, @@ -1096,6 +1065,6 @@ public void testSplitExecutableLinkCommand() throws Exception { assertThat(linkCommandLine).contains("output/path.a"); assertThat(linkCommandLine).contains("path.a-2.params"); - assertThat(result.second).contains("-lcpp_standard_library"); + assertThat(result.second).contains("-lstdc++"); } }