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

Fix rpath for binaries in external repositories #16008

Closed
wants to merge 2 commits into from
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 @@ -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 @@ -2883,4 +2883,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"