Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the runfiles directory to the runtime library search paths #14600

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.RuleErrorConsumer;
import com.google.devtools.build.lib.cmdline.LabelConstants;
Expand All @@ -40,7 +41,6 @@ public class LibrariesToLinkCollector {
private final PathFragment toolchainLibrariesSolibDir;
private final CppConfiguration cppConfiguration;
private final CcToolchainProvider ccToolchainProvider;
private final Artifact outputArtifact;
private final boolean isLtoIndexing;
private final PathFragment solibDir;
private final Iterable<? extends LinkerInput> linkerInputs;
Expand All @@ -49,7 +49,8 @@ public class LibrariesToLinkCollector {
private final Artifact thinltoParamFile;
private final FeatureConfiguration featureConfiguration;
private final boolean needWholeArchive;
private final String rpathRoot;
private final ImmutableList<String> potentialExecRoots;
private final ImmutableList<String> rpathRoots;
private final boolean needToolchainLibrariesRpath;
private final Map<Artifact, Artifact> ltoMap;
private final RuleErrorConsumer ruleErrorConsumer;
Expand All @@ -76,7 +77,6 @@ public LibrariesToLinkCollector(
this.cppConfiguration = cppConfiguration;
this.ccToolchainProvider = toolchain;
this.toolchainLibrariesSolibDir = toolchainLibrariesSolibDir;
this.outputArtifact = output;
this.solibDir = solibDir;
this.isLtoIndexing = isLtoIndexing;
this.allLtoArtifacts = allLtoArtifacts;
Expand Down Expand Up @@ -106,12 +106,13 @@ public LibrariesToLinkCollector(
// and the second could use $ORIGIN/../_solib_[arch]. But since this is a shared
// artifact, both are symlinks to the same place, so
// there's no *one* RPATH setting that fits all targets involved in the sharing.
rpathRoot = ccToolchainProvider.getSolibDirectory() + "/";
potentialExecRoots = ImmutableList.of();
rpathRoots = ImmutableList.of(ccToolchainProvider.getSolibDirectory() + "/");
} else {
// When executed from within a runfiles directory, the binary lies under a path such as
// target.runfiles/some_repo/pkg/file, whereas the solib directory is located under
// target.runfiles/main_repo.
PathFragment runfilesPath = outputArtifact.getRunfilesPath();
PathFragment runfilesPath = output.getRunfilesPath();
String runfilesExecRoot;
if (runfilesPath.startsWith(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX)) {
// runfilesPath is of the form ../some_repo/pkg/file, walk back some_repo/pkg and then
Expand All @@ -121,7 +122,22 @@ public LibrariesToLinkCollector(
// runfilesPath is of the form pkg/file, walk back pkg to reach the main workspace.
runfilesExecRoot = "../".repeat(runfilesPath.segmentCount() - 1);
}
rpathRoot = runfilesExecRoot + ccToolchainProvider.getSolibDirectory() + "/";

// When the binary gets run through remote execution as the main executable (i.e., not as a
// dependency of another target), it will not be able to find its libraries at
// $ORIGIN/../../../_solib_[arch], as execution is not taking place inside the runfiles
// directory. Solve this by adding ${executable}.runfiles/${workspace}/_solib_[arch] as an
// alternative rpath root.
PathFragment rootRelativePath = output.getRootRelativePath();
potentialExecRoots = ImmutableList.of(
rootRelativePath.getBaseName()
+ ".runfiles/"
+ workspaceName
+ "/",
runfilesExecRoot);
rpathRoots = ImmutableList.copyOf(
Lists.transform(potentialExecRoots,
(execRoot) -> execRoot + ccToolchainProvider.getSolibDirectory() + "/"));
}

ltoMap = generateLtoMap();
Expand Down Expand Up @@ -196,10 +212,12 @@ public CollectedLibrariesToLink collectLibrariesToLink() {
// directory. In other words, given blaze-bin/my/package/binary, rpathRoot would be
// "../../_solib_[arch]".
if (needToolchainLibrariesRpath) {
runtimeLibrarySearchDirectories.add(
"../".repeat(outputArtifact.getRootRelativePath().segmentCount() - 1)
+ toolchainLibrariesSolibName
+ "/");
for (String potentialExecRoot : potentialExecRoots) {
runtimeLibrarySearchDirectories.add(
potentialExecRoot
+ toolchainLibrariesSolibName
+ "/");
}
}
if (isNativeDeps) {
// We also retain the $ORIGIN/ path to solibs that are in _solib_<arch>, as opposed to
Expand Down Expand Up @@ -231,7 +249,9 @@ public CollectedLibrariesToLink collectLibrariesToLink() {
NestedSetBuilder<String> allRuntimeLibrarySearchDirectories = NestedSetBuilder.linkOrder();
// rpath ordering matters for performance; first add the one where most libraries are found.
if (includeSolibDir) {
allRuntimeLibrarySearchDirectories.add(rpathRoot);
for (String rpathRoot : rpathRoots) {
allRuntimeLibrarySearchDirectories.add(rpathRoot);
}
}
allRuntimeLibrarySearchDirectories.addAll(rpathRootsForExplicitSoDeps.build());
if (includeToolchainLibrariesSolibDir) {
Expand Down Expand Up @@ -346,17 +366,21 @@ private void addDynamicInputLinkOptions(
// When all dynamic deps are built in transitioned configurations, the default solib dir is
// not created. While resolving paths, the dynamic linker stops at the first directory that
// does not exist, even when followed by "../". We thus have to normalize the relative path.
String relativePathToRoot =
rpathRoot + dotdots + libDir.relativeTo(commonParent).getPathString();
String normalizedPathToRoot = PathFragment.create(relativePathToRoot).getPathString();
rpathRootsForExplicitSoDeps.add(normalizedPathToRoot);
for (String rpathRoot : rpathRoots) {
String relativePathToRoot =
rpathRoot + dotdots + libDir.relativeTo(commonParent).getPathString();
String normalizedPathToRoot = PathFragment.create(relativePathToRoot).getPathString();
rpathRootsForExplicitSoDeps.add(normalizedPathToRoot);
}

// 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());
for (String rpathRoot : rpathRoots) {
rpathRootsForExplicitSoDeps.add(
rpathRoot + artifactPathUnderSolib.getParentDirectory().getPathString());
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2155,6 +2155,7 @@ public void testRpathAndLinkPathsWithoutTransitions() throws Exception {
Artifact mainBin = getBinArtifact("main", main);
CppLinkAction action = (CppLinkAction) getGeneratingAction(mainBin);
List<String> linkArgv = action.getLinkCommandLine().arguments();
assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/main.runfiles/__main__/_solib_k8/");
assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/../_solib_k8/");
assertThat(linkArgv)
.contains("-L" + TestConstants.PRODUCT_NAME + "-out/k8-fastbuild/bin/_solib_k8");
Expand Down Expand Up @@ -2215,6 +2216,7 @@ public void testRpathRootIsAddedEvenWithTransitionedDepsOnly() throws Exception
Artifact mainBin = getBinArtifact("main", main);
CppLinkAction action = (CppLinkAction) getGeneratingAction(mainBin);
List<String> linkArgv = action.getLinkCommandLine().arguments();
assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/main.runfiles/__main__/_solib_k8/");
assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/../_solib_k8/");
assertThat(Joiner.on(" ").join(linkArgv))
.contains("-Wl,-rpath,$ORIGIN/../../../k8-fastbuild-ST-");
Expand Down