From d17a769965f12363f339c7b93524f49dbcdd1b1e Mon Sep 17 00:00:00 2001 From: Brentley Jones Date: Wed, 9 Feb 2022 07:27:42 -0600 Subject: [PATCH] Add the default solib dir to the rpath for cc_imports with transitions (#14757) PR #14011 took care of cc_libraries. This fixes the same issue for cc_imports. Work towards #13819. Closes #14272. PiperOrigin-RevId: 427137675 (cherry picked from commit 8734ccf9847eafb7193388cd9c6fa78faa78283f) Co-authored-by: Yuval K --- .../rules/cpp/LibrariesToLinkCollector.java | 8 ++ .../cpp/CcImportBaseConfiguredTargetTest.java | 122 ++++++++++++++++++ 2 files changed, 130 insertions(+) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java index 9a9136a6646743..9e37b9737b9c15 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java @@ -333,6 +333,14 @@ private void addDynamicInputLinkOptions( rpathRootsForExplicitSoDeps.add( rpathRoot + dotdots + libDir.relativeTo(commonParent).getPathString()); + + // Unless running locally, libraries will be available under the root relative path, so we + // should add that to the rpath as well. + if (inputArtifact.getRootRelativePathString().startsWith("_solib_")) { + PathFragment artifactPathUnderSolib = inputArtifact.getRootRelativePath().subFragment(1); + rpathRootsForExplicitSoDeps.add( + rpathRoot + artifactPathUnderSolib.getParentDirectory().getPathString()); + } } librarySearchDirectories.add(inputArtifact.getExecPath().getParentDirectory().getPathString()); diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcImportBaseConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcImportBaseConfiguredTargetTest.java index 1ac1fa220baf15..d06b9a80fdddeb 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcImportBaseConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcImportBaseConfiguredTargetTest.java @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.packages.util.Crosstool.CcToolchainConfig; +import java.util.List; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -405,4 +406,125 @@ private void setupTestCcImportLoadedThroughMacro(boolean loadMacro) throws Excep getAnalysisMock().ccSupport().getMacroLoadStatement(loadMacro, "cc_import"), "cc_import(name='a', static_library='a.a')"); } + + @Test + public void testCcImportWithSharedLibraryAddsRpathEntry() throws Exception { + AnalysisMock.get() + .ccSupport() + .setupCcToolchainConfig( + mockToolsConfig, + CcToolchainConfig.builder().withFeatures(CppRuleClasses.SUPPORTS_DYNAMIC_LINKER)); + useConfiguration("--cpu=k8"); + ConfiguredTarget target = + scratchConfiguredTarget( + "a", + "foo", + starlarkImplementationLoadStatement, + "cc_import(name = 'foo', shared_library = 'libfoo.so')"); + scratch.file("bin/BUILD", "cc_binary(name='bin', deps=['//a:foo'])"); + + Artifact dynamicLibrary = + target + .get(CcInfo.PROVIDER) + .getCcLinkingContext() + .getLibraries() + .getSingleton() + .getResolvedSymlinkDynamicLibrary(); + Iterable dynamicLibrariesForRuntime = + target + .get(CcInfo.PROVIDER) + .getCcLinkingContext() + .getDynamicLibrariesForRuntime(/* linkingStatically= */ false); + assertThat(artifactsToStrings(ImmutableList.of(dynamicLibrary))) + .containsExactly("src a/libfoo.so"); + assertThat(artifactsToStrings(dynamicLibrariesForRuntime)) + .containsExactly("bin _solib_k8/_U_S_Sa_Cfoo___Ua/libfoo.so"); + + ConfiguredTarget main = getConfiguredTarget("//bin:bin"); + Artifact mainBin = getBinArtifact("bin", main); + CppLinkAction action = (CppLinkAction) getGeneratingAction(mainBin); + List linkArgv = action.getLinkCommandLine().arguments(); + assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/../_solib_k8/_U_S_Sa_Cfoo___Ua"); + } + + @Test + public void testCcImportWithSharedLibraryWithTransitionAddsRpathEntry() throws Exception { + AnalysisMock.get() + .ccSupport() + .setupCcToolchainConfig( + mockToolsConfig, + CcToolchainConfig.builder().withFeatures(CppRuleClasses.SUPPORTS_DYNAMIC_LINKER)); + useConfiguration("--cpu=k8"); + ConfiguredTarget target = + scratchConfiguredTarget( + "a", + "foo", + starlarkImplementationLoadStatement, + "cc_import(name = 'foo', shared_library = 'libfoo.so')"); + + scratch.file( + "bin/custom_transition.bzl", + "def _custom_transition_impl(settings, attr):", + " _ignore = settings, attr", + "", + " return {'//command_line_option:copt': ['-DFLAG']}", + "", + "custom_transition = transition(", + " implementation = _custom_transition_impl,", + " inputs = [],", + " outputs = ['//command_line_option:copt'],", + ")", + "", + "def _apply_custom_transition_impl(ctx):", + " cc_infos = []", + " for dep in ctx.attr.deps:", + " cc_infos.append(dep[CcInfo])", + " merged_cc_info = cc_common.merge_cc_infos(cc_infos = cc_infos)", + " return merged_cc_info", + "", + "apply_custom_transition = rule(", + " implementation = _apply_custom_transition_impl,", + " attrs = {", + " '_whitelist_function_transition': attr.label(", + " default = '//tools/allowlists/function_transition_allowlist',", + " ),", + " 'deps': attr.label_list(cfg = custom_transition),", + " },", + ")"); + scratch.overwriteFile( + "tools/allowlists/function_transition_allowlist/BUILD", + "package_group(", + " name = 'function_transition_allowlist',", + " packages = ['//...'],", + ")"); + scratch.file( + "bin/BUILD", + "load(':custom_transition.bzl', 'apply_custom_transition')", + "cc_library(name='lib', deps=['//a:foo'])", + "apply_custom_transition(name='transitioned_lib', deps=[':lib'])", + "cc_binary(name='bin', deps=[':transitioned_lib'])"); + + Artifact dynamicLibrary = + target + .get(CcInfo.PROVIDER) + .getCcLinkingContext() + .getLibraries() + .getSingleton() + .getResolvedSymlinkDynamicLibrary(); + Iterable dynamicLibrariesForRuntime = + target + .get(CcInfo.PROVIDER) + .getCcLinkingContext() + .getDynamicLibrariesForRuntime(/* linkingStatically= */ false); + assertThat(artifactsToStrings(ImmutableList.of(dynamicLibrary))) + .containsExactly("src a/libfoo.so"); + assertThat(artifactsToStrings(dynamicLibrariesForRuntime)) + .containsExactly("bin _solib_k8/_U_S_Sa_Cfoo___Ua/libfoo.so"); + + ConfiguredTarget main = getConfiguredTarget("//bin:bin"); + Artifact mainBin = getBinArtifact("bin", main); + CppLinkAction action = (CppLinkAction) getGeneratingAction(mainBin); + List linkArgv = action.getLinkCommandLine().arguments(); + assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/../_solib_k8/_U_S_Sa_Cfoo___Ua"); + } }