From 2cfda71699e9e8d2d17ebd71e348bec45bfee8b4 Mon Sep 17 00:00:00 2001 From: Zach Yu Date: Thu, 15 Dec 2022 14:43:22 -0800 Subject: [PATCH] Refactor logics to find runtime library search directories * Extracts logic to find execroots as a separate method and moves it into the `collectLibrariesToLink()` method from the constructor. * Extracts logic to create rpaths for toolchain libraries as a separate method. * Reorganizes the logic for better readability. This debloats the constructor and makes further changes easier. --- .../rules/cpp/LibrariesToLinkCollector.java | 278 +++++++++--------- 1 file changed, 140 insertions(+), 138 deletions(-) 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 a0052121121e84..3e919ad22cb29f 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 @@ -50,11 +50,10 @@ public class LibrariesToLinkCollector { private final Artifact thinltoParamFile; private final FeatureConfiguration featureConfiguration; private final boolean needWholeArchive; - private final ImmutableList potentialExecRoots; - private final ImmutableList rpathRoots; private final boolean needToolchainLibrariesRpath; - private final Map ltoMap; private final RuleErrorConsumer ruleErrorConsumer; + private final Artifact output; + private final String workspaceName; public LibrariesToLinkCollector( boolean isNativeDeps, @@ -87,114 +86,13 @@ public LibrariesToLinkCollector( this.linkerInputs = linkerInputs; this.needWholeArchive = needWholeArchive; this.ruleErrorConsumer = ruleErrorConsumer; + this.output = output; + this.workspaceName = workspaceName; needToolchainLibrariesRpath = toolchainLibrariesSolibDir != null && (linkType.isDynamicLibrary() || (linkType == LinkTargetType.EXECUTABLE && linkingMode == LinkingMode.DYNAMIC)); - - // Calculate the correct relative value for the "-rpath" link option (which sets - // the search path for finding shared libraries). - if (isNativeDeps && cppConfiguration.shareNativeDeps()) { - // For shared native libraries, special symlinking is applied to ensure C++ - // toolchain libraries are available under $ORIGIN/_solib_[arch]. So we set the RPATH to find - // them. - // - // Note that we have to do this because $ORIGIN points to different paths for - // different targets. In other words, blaze-bin/d1/d2/d3/a_shareddeps.so and - // blaze-bin/d4/b_shareddeps.so have different path depths. The first could - // reference a standard blaze-bin/_solib_[arch] via $ORIGIN/../../../_solib[arch], - // 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. - potentialExecRoots = ImmutableList.of(); - rpathRoots = ImmutableList.of(ccToolchainProvider.getSolibDirectory() + "/"); - } else { - // The runtime location of the solib directory relative to the binary depends on four factors: - // - // * whether the binary is contained in the main repository or an external repository; - // * whether the binary is executed directly or from a runfiles tree; - // * whether the binary is staged as a symlink (sandboxed execution; local execution if the - // binary is in the runfiles of another target) or a regular file (remote execution) - the - // dynamic linker follows sandbox and runfiles symlinks into its location under the - // unsandboxed execroot, which thus becomes the effective $ORIGIN; - // * whether --experimental_sibling_repository_layout is enabled or not. - // - // The rpaths emitted into the binary thus have to cover the following cases (assuming that - // the binary target is located in the pkg `pkg` and has name `file`) for the directory used - // as $ORIGIN by the dynamic linker and the directory containing the solib directories: - // - // 1. main, direct, symlink: - // $ORIGIN: $EXECROOT/pkg - // solib root: $EXECROOT - // 2. main, direct, regular file: - // $ORIGIN: $EXECROOT/pkg - // solib root: $EXECROOT/pkg/file.runfiles/main_repo - // 3. main, runfiles, symlink: - // $ORIGIN: $EXECROOT/pkg - // solib root: $EXECROOT - // 4. main, runfiles, regular file: - // $ORIGIN: other_target.runfiles/main_repo/pkg - // solib root: other_target.runfiles/main_repo - // 5a. external, direct, symlink: - // $ORIGIN: $EXECROOT/external/other_repo/pkg - // solib root: $EXECROOT - // 5b. external, direct, symlink, with --experimental_sibling_repository_layout: - // $ORIGIN: $EXECROOT/../other_repo/pkg - // solib root: $EXECROOT/../other_repo - // 6a. external, direct, regular file: - // $ORIGIN: $EXECROOT/external/other_repo/pkg - // solib root: $EXECROOT/external/other_repo/pkg/file.runfiles/main_repo - // 6b. external, direct, regular file, with --experimental_sibling_repository_layout: - // $ORIGIN: $EXECROOT/../other_repo/pkg - // solib root: $EXECROOT/../other_repo/pkg/file.runfiles/other_repo - // 7a. external, runfiles, symlink: - // $ORIGIN: $EXECROOT/external/other_repo/pkg - // solib root: $EXECROOT - // 7b. external, runfiles, symlink, with --experimental_sibling_repository_layout: - // $ORIGIN: $EXECROOT/../other_repo/pkg - // solib root: $EXECROOT/../other_repo - // 8a. external, runfiles, regular file: - // $ORIGIN: other_target.runfiles/some_repo/pkg - // solib root: other_target.runfiles/main_repo - // 8b. external, runfiles, regular file, with --experimental_sibling_repository_layout: - // $ORIGIN: other_target.runfiles/some_repo/pkg - // solib root: other_target.runfiles/some_repo - // - // Cases 1, 3, 4, 5, 7, and 8b are covered by an rpath that walks up the root relative path. - // Cases 2 and 6 covered by walking into file.runfiles/main_repo. - // Case 8a is covered by walking up some_repo/pkg and then into main_repo. - boolean isExternal = - output.getRunfilesPath().startsWith(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX); - boolean usesLegacyRepositoryLayout = output.getRoot().isLegacy(); - ImmutableList.Builder execRoots = ImmutableList.builder(); - // Handles cases 1, 3, 4, 5, and 7. - execRoots.add("../".repeat(output.getRootRelativePath().segmentCount() - 1)); - // Handle cases 2 and 6. - String solibRepositoryName; - if (isExternal && !usesLegacyRepositoryLayout) { - // Case 6b - solibRepositoryName = output.getRunfilesPath().getSegment(1); - } else { - // Cases 2 and 6a - solibRepositoryName = workspaceName; - } - execRoots.add(output.getFilename() + ".runfiles/" + solibRepositoryName + "/"); - if (isExternal && usesLegacyRepositoryLayout) { - // Handles case 8a. The runfiles path is of the form ../some_repo/pkg/file and we need to - // walk up some_repo/pkg and then down into main_repo. - execRoots.add( - "../".repeat(output.getRunfilesPath().segmentCount() - 2) + workspaceName + "/"); - } - - potentialExecRoots = execRoots.build(); - rpathRoots = - potentialExecRoots.stream() - .map((execRoot) -> execRoot + ccToolchainProvider.getSolibDirectory() + "/") - .collect(toImmutableList()); - } - - ltoMap = generateLtoMap(); } /** @@ -236,6 +134,109 @@ public NestedSet getRuntimeLibrarySearchDirectories() { } } + private NestedSet collectToolchainRuntimeLibrarySearchDirectories( + ImmutableList potentialSolibParents) { + NestedSetBuilder runtimeLibrarySearchDirectories = NestedSetBuilder.linkOrder(); + if (!needToolchainLibrariesRpath) { + return runtimeLibrarySearchDirectories.build(); + } + + String toolchainLibrariesSolibName = toolchainLibrariesSolibDir.getBaseName(); + if (!(isNativeDeps && cppConfiguration.shareNativeDeps())) { + for (String potentialExecRoot : potentialSolibParents) { + runtimeLibrarySearchDirectories.add(potentialExecRoot + toolchainLibrariesSolibName + "/"); + } + } + if (isNativeDeps) { + runtimeLibrarySearchDirectories.add("../" + toolchainLibrariesSolibName + "/"); + runtimeLibrarySearchDirectories.add("."); + } + runtimeLibrarySearchDirectories.add(toolchainLibrariesSolibName + "/"); + + return runtimeLibrarySearchDirectories.build(); + } + + private ImmutableList findPotentialSolibParents() { + // The runtime location of the solib directory relative to the binary depends on four factors: + // + // * whether the binary is contained in the main repository or an external repository; + // * whether the binary is executed directly or from a runfiles tree; + // * whether the binary is staged as a symlink (sandboxed execution; local execution if the + // binary is in the runfiles of another target) or a regular file (remote execution) - the + // dynamic linker follows sandbox and runfiles symlinks into its location under the + // unsandboxed execroot, which thus becomes the effective $ORIGIN; + // * whether --experimental_sibling_repository_layout is enabled or not. + // + // The rpaths emitted into the binary thus have to cover the following cases (assuming that + // the binary target is located in the pkg `pkg` and has name `file`) for the directory used + // as $ORIGIN by the dynamic linker and the directory containing the solib directories: + // + // 1. main, direct, symlink: + // $ORIGIN: $EXECROOT/pkg + // solib root: $EXECROOT + // 2. main, direct, regular file: + // $ORIGIN: $EXECROOT/pkg + // solib root: $EXECROOT/pkg/file.runfiles/main_repo + // 3. main, runfiles, symlink: + // $ORIGIN: $EXECROOT/pkg + // solib root: $EXECROOT + // 4. main, runfiles, regular file: + // $ORIGIN: other_target.runfiles/main_repo/pkg + // solib root: other_target.runfiles/main_repo + // 5a. external, direct, symlink: + // $ORIGIN: $EXECROOT/external/other_repo/pkg + // solib root: $EXECROOT + // 5b. external, direct, symlink, with --experimental_sibling_repository_layout: + // $ORIGIN: $EXECROOT/../other_repo/pkg + // solib root: $EXECROOT/../other_repo + // 6a. external, direct, regular file: + // $ORIGIN: $EXECROOT/external/other_repo/pkg + // solib root: $EXECROOT/external/other_repo/pkg/file.runfiles/main_repo + // 6b. external, direct, regular file, with --experimental_sibling_repository_layout: + // $ORIGIN: $EXECROOT/../other_repo/pkg + // solib root: $EXECROOT/../other_repo/pkg/file.runfiles/other_repo + // 7a. external, runfiles, symlink: + // $ORIGIN: $EXECROOT/external/other_repo/pkg + // solib root: $EXECROOT + // 7b. external, runfiles, symlink, with --experimental_sibling_repository_layout: + // $ORIGIN: $EXECROOT/../other_repo/pkg + // solib root: $EXECROOT/../other_repo + // 8a. external, runfiles, regular file: + // $ORIGIN: other_target.runfiles/some_repo/pkg + // solib root: other_target.runfiles/main_repo + // 8b. external, runfiles, regular file, with --experimental_sibling_repository_layout: + // $ORIGIN: other_target.runfiles/some_repo/pkg + // solib root: other_target.runfiles/some_repo + // + // Cases 1, 3, 4, 5, 7, and 8b are covered by an rpath that walks up the root relative path. + // Cases 2 and 6 covered by walking into file.runfiles/main_repo. + // Case 8a is covered by walking up some_repo/pkg and then into main_repo. + boolean isExternal = + output.getRunfilesPath().startsWith(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX); + boolean usesLegacyRepositoryLayout = output.getRoot().isLegacy(); + ImmutableList.Builder solibParents = ImmutableList.builder(); + // Handles cases 1, 3, 4, 5, and 7. + solibParents.add("../".repeat(output.getRootRelativePath().segmentCount() - 1)); + // Handle cases 2 and 6. + String solibRepositoryName; + if (isExternal && !usesLegacyRepositoryLayout) { + // Case 6b + solibRepositoryName = output.getRunfilesPath().getSegment(1); + } else { + // Cases 2 and 6a + solibRepositoryName = workspaceName; + } + solibParents.add(output.getFilename() + ".runfiles/" + solibRepositoryName + "/"); + if (isExternal && usesLegacyRepositoryLayout) { + // Handles case 8a. The runfiles path is of the form ../some_repo/pkg/file and we need to + // walk up some_repo/pkg and then down into main_repo. + solibParents.add( + "../".repeat(output.getRunfilesPath().segmentCount() - 2) + workspaceName + "/"); + } + + return solibParents.build(); + } + /** * When linking a shared library fully or mostly static then we need to link in *all* dependent * files, not just what the shared library needs for its own code. This is done by wrapping all @@ -247,48 +248,43 @@ public NestedSet getRuntimeLibrarySearchDirectories() { */ public CollectedLibrariesToLink collectLibrariesToLink() { NestedSetBuilder librarySearchDirectories = NestedSetBuilder.linkOrder(); - NestedSetBuilder runtimeLibrarySearchDirectories = NestedSetBuilder.linkOrder(); ImmutableSet.Builder rpathRootsForExplicitSoDeps = ImmutableSet.builder(); NestedSetBuilder expandedLinkerInputsBuilder = NestedSetBuilder.linkOrder(); // List of command line parameters that need to be placed *outside* of // --whole-archive ... --no-whole-archive. SequenceBuilder librariesToLink = new SequenceBuilder(); - String toolchainLibrariesSolibName = - toolchainLibrariesSolibDir != null ? toolchainLibrariesSolibDir.getBaseName() : null; + ImmutableList potentialSolibParents; + ImmutableList rpathRoots; + // Calculate the correct relative value for the "-rpath" link option (which sets + // the search path for finding shared libraries). if (isNativeDeps && cppConfiguration.shareNativeDeps()) { - if (needToolchainLibrariesRpath) { - runtimeLibrarySearchDirectories.add("../" + toolchainLibrariesSolibName + "/"); - } + // For shared native libraries, special symlinking is applied to ensure C++ + // toolchain libraries are available under $ORIGIN/_solib_[arch]. So we set the RPATH to find + // them. + // + // Note that we have to do this because $ORIGIN points to different paths for + // different targets. In other words, blaze-bin/d1/d2/d3/a_shareddeps.so and + // blaze-bin/d4/b_shareddeps.so have different path depths. The first could + // reference a standard blaze-bin/_solib_[arch] via $ORIGIN/../../../_solib[arch], + // 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. + potentialSolibParents = ImmutableList.of(); + rpathRoots = ImmutableList.of(ccToolchainProvider.getSolibDirectory() + "/"); } else { - // For all other links, calculate the relative path from the output file to _solib_[arch] - // (the directory where all shared libraries are stored, which resides under the blaze-bin - // directory. In other words, given blaze-bin/my/package/binary, rpathRoot would be - // "../../_solib_[arch]". - if (needToolchainLibrariesRpath) { - for (String potentialExecRoot : potentialExecRoots) { - runtimeLibrarySearchDirectories.add( - potentialExecRoot + toolchainLibrariesSolibName + "/"); - } - } - if (isNativeDeps) { - // We also retain the $ORIGIN/ path to solibs that are in _solib_, as opposed to - // the package directory) - if (needToolchainLibrariesRpath) { - runtimeLibrarySearchDirectories.add("../" + toolchainLibrariesSolibName + "/"); - } - } - } - - if (needToolchainLibrariesRpath) { - if (isNativeDeps) { - runtimeLibrarySearchDirectories.add("."); - } - runtimeLibrarySearchDirectories.add(toolchainLibrariesSolibName + "/"); + potentialSolibParents = findPotentialSolibParents(); + rpathRoots = + potentialSolibParents.stream() + .map((execRoot) -> execRoot + ccToolchainProvider.getSolibDirectory() + "/") + .collect(toImmutableList()); } + Map ltoMap = generateLtoMap(); Pair includeSolibsPair = addLinkerInputs( + rpathRoots, + ltoMap, librarySearchDirectories, rpathRootsForExplicitSoDeps, librariesToLink, @@ -307,7 +303,8 @@ public CollectedLibrariesToLink collectLibrariesToLink() { } allRuntimeLibrarySearchDirectories.addAll(rpathRootsForExplicitSoDeps.build()); if (includeToolchainLibrariesSolibDir) { - allRuntimeLibrarySearchDirectories.addTransitive(runtimeLibrarySearchDirectories.build()); + allRuntimeLibrarySearchDirectories.addTransitive( + collectToolchainRuntimeLibrarySearchDirectories(potentialSolibParents)); } return new CollectedLibrariesToLink( @@ -318,6 +315,8 @@ public CollectedLibrariesToLink collectLibrariesToLink() { } private Pair addLinkerInputs( + ImmutableList rpathRoots, + Map ltoMap, NestedSetBuilder librarySearchDirectories, ImmutableSet.Builder rpathEntries, SequenceBuilder librariesToLink, @@ -366,9 +365,10 @@ private Pair addLinkerInputs( librariesToLink, expandedLinkerInputsBuilder, librarySearchDirectories, + rpathRoots, rpathEntries); } else { - addStaticInputLinkOptions(input, librariesToLink, expandedLinkerInputsBuilder); + addStaticInputLinkOptions(input, ltoMap, librariesToLink, expandedLinkerInputsBuilder); } } return Pair.of(includeSolibDir, includeToolchainLibrariesSolibDir); @@ -384,6 +384,7 @@ private void addDynamicInputLinkOptions( SequenceBuilder librariesToLink, NestedSetBuilder expandedLinkerInputsBuilder, NestedSetBuilder librarySearchDirectories, + ImmutableList rpathRoots, ImmutableSet.Builder rpathRootsForExplicitSoDeps) { Preconditions.checkState( input.getArtifactCategory() == ArtifactCategory.DYNAMIC_LIBRARY @@ -470,6 +471,7 @@ private void addDynamicInputLinkOptions( */ private void addStaticInputLinkOptions( LinkerInput input, + Map ltoMap, SequenceBuilder librariesToLink, NestedSetBuilder expandedLinkerInputsBuilder) { ArtifactCategory artifactCategory = input.getArtifactCategory();