Fix rpath for binaries in external repositories (bazelbuild#16079)

fmeum · web-flow · commit e745468461f9 · 2022-08-10T08:26:40.000-07:00
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
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -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;
@@ -798,7 +799,8 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
             allowLtoIndexing,
             nonExpandedLinkerInputs,
             needWholeArchive,
-            ruleErrorConsumer);
+            ruleErrorConsumer,
+            ((RuleContext) actionConstructionContext).getWorkspaceName());
     CollectedLibrariesToLink collectedLibrariesToLink =
         librariesToLinkCollector.collectLibrariesToLink();
 
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
@@ -20,6 +20,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;
@@ -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;
@@ -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 =
-          Strings.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 = Strings.repeat("../", runfilesPath.segmentCount() - 2) + workspaceName + "/";
+      } else {
+        // runfilesPath is of the form pkg/file, walk back pkg to reach the main workspace.
+        runfilesExecRoot = Strings.repeat("../", runfilesPath.segmentCount() - 1);
+      }
+      rpathRoot = runfilesExecRoot + ccToolchainProvider.getSolibDirectory() + "/";
     }
 
     ltoMap = generateLtoMap();
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -3811,4 +3811,62 @@ EOF
   expect_log "Executing genrule .* failed: (Exit 1):"
 }
 
+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"
