Cover all cases for rpath generation (#4471)

**What type of PR is this?**

Bug fix

**What does this PR do? Why is it needed?**

This is based on the corresponding logic in rules_cc.

**Which issues(s) does this PR fix?**

Fixes #4451

**Other notes for review**

Author: fmeum

go/private/rpath.bzl

@@ -17,39 +17,41 @@ load(
     "paths",
 )
 
-# TODO: Replace with a Bazel-provided function when it's available.
-# https://github.com/bazelbuild/bazel/issues/14307
-def _rlocation_path(ctx, file):
-    """Returns the path relative to the runfiles directory."""
-    if file.short_path.startswith("../"):
-        return file.short_path[3:]
-    else:
-        return ctx.workspace_name + "/" + file.short_path
-
 def _rpath(go, library, executable = None):
     """Returns the potential rpaths of a library, possibly relative to another file."""
     if not executable:
         return [paths.dirname(library.short_path)]
 
-    origin = go.mode.goos == "darwin" and "@loader_path" or "$ORIGIN"
+    origin = "@loader_path" if go.mode.goos == "darwin" else "$ORIGIN"
 
-    # Accomodate for two kinds of executable paths.
+    # Accommodate for three kinds of executable paths.
     rpaths = []
+    library_dir = paths.dirname(library.short_path)
+
+    # Based on the logic for Bazel's own C++ rules:
+    # https://github.com/bazelbuild/bazel/blob/51a4b8e5de225ba163d19ddcc330aff8860a1520/src/main/starlark/builtins_bzl/common/cc/link/collect_solib_dirs.bzl#L301
+    # with the bug fix https://github.com/bazelbuild/bazel/pull/27154.
+    # We ignore the cases for --experimental_sibling_repository_layout.
 
     # 1. Where the executable is inside its own .runfiles directory.
-    #  This is the case for generated libraries as well as remote builds.
-    #   a) go back to the runfiles root from the executable file in .runfiles
-    depth = _rlocation_path(go._ctx, executable).count("/")
+    # This covers the cases 1, 3, 4, 5, and 7 in the linked code above.
+    #   a) go back to the workspace root from the executable file in .runfiles
+    depth = executable.short_path.count("/")
     back_to_root = paths.join(*([".."] * depth))
 
-    #   b) then walk back to the library's dir within runfiles dir.
-    rpaths.append(paths.join(origin, back_to_root, paths.dirname(_rlocation_path(go._ctx, library))))
+    #   b) then walk back to the library's short path
+    rpaths.append(paths.join(origin, back_to_root, library_dir))
 
     # 2. Where the executable is outside the .runfiles directory:
-    #  This is the case for local pre-built libraries, as well as local
-    #  generated libraries.
+    # This covers the cases 2 and 6 in the linked code above.
     runfiles_dir = paths.basename(executable.short_path) + ".runfiles"
-    rpaths.append(paths.join(origin, runfiles_dir, go._ctx.workspace_name, paths.dirname(library.short_path)))
+    rpaths.append(paths.join(origin, runfiles_dir, go._ctx.workspace_name, library_dir))
+
+    # 3. Where the executable is from a different repo
+    # This covers the case 8 in the linked code above.
+    if executable.short_path.startswith("../"):
+        back_to_repo_root = paths.join(*([".."] * (depth - 1)))
+        rpaths.append(paths.join(origin, back_to_repo_root, go._ctx.workspace_name, library_dir))
 
     return rpaths
 

tests/core/cgo/BUILD.bazel

@@ -541,3 +541,15 @@ go_test(
     srcs = ["cgo_required_test.go"],
     pure = "on",
 )
+
+go_bazel_test(
+    name = "wrapped_cgo_test",
+    srcs = ["wrapped_cgo_test.go"],
+    target_compatible_with = select({
+        # TODO: Try to enable this test on macOS when the minimum version
+        # version of Bazel sets the correct install_name.
+        # "@platforms//os:macos": [],
+        "@platforms//os:linux": [],
+        "//conditions:default": ["@platforms//:incompatible"],
+    }),
+)

tests/core/cgo/wrapped_cgo_test.go

@@ -0,0 +1,129 @@
+package wrapped_cgo_test
+
+import (
+	"testing"
+
+	"github.com/bazelbuild/rules_go/go/tools/bazel_testing"
+)
+
+func TestMain(m *testing.M) {
+	bazel_testing.TestMain(m, bazel_testing.Args{
+		Main: `
+-- wrapped_test.bzl --
+CODE = """#!/usr/bin/env bash
+echo "Running wrapped test {0}"
+{0} $@
+"""
+def _wrapped_test_impl(ctx):
+    out = ctx.actions.declare_file(ctx.label.name + ".sh")
+    ctx.actions.write(out, CODE.format(ctx.executable.test.short_path))
+    return [DefaultInfo(
+        executable = out,
+        default_runfiles = ctx.attr.test[DefaultInfo].default_runfiles.merge(ctx.runfiles(files=[out])),
+    )]
+
+wrapped_test = rule(
+    implementation = _wrapped_test_impl,
+    attrs = {
+        "test": attr.label(mandatory = True, executable = True, cfg = "target"),
+        # Required for Bazel to collect coverage of instrumented C/C++ binaries
+        # executed by go_test.
+        # This is just a shell script and thus cheap enough to depend on
+        # unconditionally.
+        "_collect_cc_coverage": attr.label(
+            default = "@bazel_tools//tools/test:collect_cc_coverage",
+            cfg = "exec",
+        ),
+        # Required for Bazel to merge coverage reports for Go and other
+        # languages into a single report per test.
+        # Using configuration_field ensures that the tool is only built when
+        # run with bazel coverage, not with bazel test.
+        "_lcov_merger": attr.label(
+            default = configuration_field(fragment = "coverage", name = "output_generator"),
+            cfg = "exec",
+        ),
+    },
+    test = True,
+)
+-- BUILD.bazel --
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+load("//:wrapped_test.bzl", "wrapped_test")
+
+cc_library(
+    name = "hello",
+    srcs = ["hello.c"],
+    hdrs = ["hello.h"],
+    visibility = ["//visibility:public"],
+)
+
+cc_shared_library(
+    name = "hello_shared_lib",
+    deps = [":hello"],
+)
+
+# This weird dance is necessary to force rules_go to link against a shared library.
+cc_library(
+    name = "hello_shared",
+    srcs = [":hello_shared_lib"],
+    hdrs = ["hello.h"],
+)
+
+go_library(
+    name = "go_hello",
+    srcs = ["hello.go"],
+    cgo = True,
+    cdeps = [":hello_shared"],
+    importpath = "example.com/hello",
+    visibility = ["//visibility:public"],
+)
+
+go_test(
+    name = "go_hello_test",
+    srcs = ["hello_test.go"],
+    deps = [":go_hello"],
+    pure = "off",
+    static = "off",
+)
+
+wrapped_test(
+    name = "wrapped_go_hello_test",
+    test = ":go_hello_test",
+)
+-- hello.h --
+void hello();
+-- hello.c --
+#include "hello.h"
+#include <stdio.h>
+void hello() {
+    printf("Hello, World!\n");
+}
+-- hello_test.go --
+package hello_test
+import (
+    "testing"
+
+    "example.com/hello"
+)
+func TestHello(t *testing.T) {
+    hello.Hello()
+}
+
+-- hello.go --
+package hello
+
+// #include "hello.h"
+import "C"
+func Hello() {
+    C.hello()
+}
+`,
+	})
+}
+
+func TestWrappedCgo(t *testing.T) {
+	if o, err := bazel_testing.BazelOutput("test", "//:wrapped_go_hello_test", "--test_env=GO_TEST_WRAP_TESTV=1", "--test_output=all", "--experimental_cc_shared_library"); err != nil {
+		t.Fatalf("bazel test //:wrapped_go_hello_test: %v\n%s", err, string(o))
+	} else {
+		t.Logf("bazel test //:wrapped_go_hello_test: %s", string(o))
+	}
+}