Skip to content

Commit

Permalink
Fix rpath for binaries in external repositories
Browse files Browse the repository at this point in the history
The solib directory is located within the subdirectory of the runfiles
directory corresponding to the workspace. Thus, if a binary is contained
in an external repository, its $ORIGIN relative rpath has to first
ascend to the runfiles directory and then descend into the workspace
directory.

Fixes bazelbuild#16003

Closes bazelbuild#16008.

PiperOrigin-RevId: 466634083
Change-Id: I4ada28b459f23f68a2091dbaad9147cfec2fbe43
  • Loading branch information
fmeum authored and Copybara-Service committed Aug 10, 2022
1 parent 04e8b63 commit 95a5bfd
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ParameterFile;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.RuleErrorConsumer;
import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction;
Expand Down Expand Up @@ -801,7 +802,8 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
allowLtoIndexing,
nonExpandedLinkerInputs,
needWholeArchive,
ruleErrorConsumer);
ruleErrorConsumer,
((RuleContext) actionConstructionContext).getWorkspaceName());
CollectedLibrariesToLink collectedLibrariesToLink =
librariesToLinkCollector.collectLibrariesToLink();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.RuleErrorConsumer;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
Expand Down Expand Up @@ -69,7 +70,8 @@ public LibrariesToLinkCollector(
boolean allowLtoIndexing,
Iterable<LinkerInput> linkerInputs,
boolean needWholeArchive,
RuleErrorConsumer ruleErrorConsumer) {
RuleErrorConsumer ruleErrorConsumer,
String workspaceName) {
this.isNativeDeps = isNativeDeps;
this.cppConfiguration = cppConfiguration;
this.ccToolchainProvider = toolchain;
Expand Down Expand Up @@ -106,10 +108,20 @@ public LibrariesToLinkCollector(
// there's no *one* RPATH setting that fits all targets involved in the sharing.
rpathRoot = ccToolchainProvider.getSolibDirectory() + "/";
} else {
rpathRoot =
"../".repeat(outputArtifact.getRootRelativePath().segmentCount() - 1)
+ ccToolchainProvider.getSolibDirectory()
+ "/";
// 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();
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
// descend into the main workspace.
runfilesExecRoot = "../".repeat(runfilesPath.segmentCount() - 2) + workspaceName + "/";
} else {
// runfilesPath is of the form pkg/file, walk back pkg to reach the main workspace.
runfilesExecRoot = "../".repeat(runfilesPath.segmentCount() - 1);
}
rpathRoot = runfilesExecRoot + ccToolchainProvider.getSolibDirectory() + "/";
}

ltoMap = generateLtoMap();
Expand Down
58 changes: 58 additions & 0 deletions src/test/shell/bazel/remote/remote_execution_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2906,4 +2906,62 @@ EOF
|| fail "Remote execution generated different result"
}

function test_external_cc_test() {
if [[ "$PLATFORM" == "darwin" ]]; then
# TODO(b/37355380): This test is disabled due to RemoteWorker not supporting
# setting SDKROOT and DEVELOPER_DIR appropriately, as is required of
# action executors in order to select the appropriate Xcode toolchain.
return 0
fi

cat >> WORKSPACE <<'EOF'
local_repository(
name = "other_repo",
path = "other_repo",
)
EOF

mkdir -p other_repo
touch other_repo/WORKSPACE

mkdir -p other_repo/lib
cat > other_repo/lib/BUILD <<'EOF'
cc_library(
name = "lib",
srcs = ["lib.cpp"],
hdrs = ["lib.h"],
visibility = ["//visibility:public"],
)
EOF
cat > other_repo/lib/lib.h <<'EOF'
void print_greeting();
EOF
cat > other_repo/lib/lib.cpp <<'EOF'
#include <cstdio>
void print_greeting() {
printf("Hello, world!\n");
}
EOF

mkdir -p other_repo/test
cat > other_repo/test/BUILD <<'EOF'
cc_test(
name = "test",
srcs = ["test.cpp"],
deps = ["//lib"],
)
EOF
cat > other_repo/test/test.cpp <<'EOF'
#include "lib/lib.h"
int main() {
print_greeting();
}
EOF

bazel test \
--test_output=errors \
--remote_executor=grpc://localhost:${worker_port} \
@other_repo//test >& $TEST_log || fail "Test should pass"
}

run_suite "Remote execution and remote cache tests"

0 comments on commit 95a5bfd

Please sign in to comment.