Don't include non-executable go_binary in dependent's runfiles (#3151)

* Remove unnecessary rpath special handling on macOS

Based on
bazelbuild/bazel#10254 (comment)
and bazelbuild/bazel#12304 being fixed, this
special handling of rpaths on macOS appears to be unnecessary.

This cleanup ensures that Bazel sets the correct metadata for the exec
location of Go libraries linked in c-shared mode, which in turn allows
to not include them in the runfiles of all dependents - cc_* targets
depending on them will now generate the correct rpath entries to find
the libraries at runtime at the usual position.

* Don't include non-executable go_binary in dependent's runfiles

If a go_binary is built with a non-executable link mode such as
`c-archive`, its dependents currently pick up a runfile dependency on it
since its DefaultInfo specifies the resulting archive as an executable.
This is unnecessary as the dependent should be able to decide whether to
include the file (e.g. dynamic linking) or not (e.g. static linking).

With this commit, the executable field of the DefaultInfo is only
populated if the go_binary is built with an executable link mode.

Follow-up to #3143

go/private/actions/link.bzl

@@ -20,7 +20,6 @@ load(
 )
 load(
     "//go/private:mode.bzl",
-    "LINKMODE_C_SHARED",
     "LINKMODE_NORMAL",
     "LINKMODE_PLUGIN",
     "extld_from_cc_toolchain",
@@ -108,13 +107,6 @@ def emit_link(
     if go.mode.link == LINKMODE_PLUGIN:
         tool_args.add("-pluginpath", archive.data.importpath)
 
-    # TODO: Rework when https://github.com/bazelbuild/bazel/pull/12304 is mainstream
-    if go.mode.link == LINKMODE_C_SHARED and (go.mode.goos in ["darwin", "ios"]):
-        extldflags.extend([
-            "-install_name",
-            rpath.install_name(executable),
-        ])
-
     arcs = _transitive_archives_without_test_archives(archive, test_archives)
     arcs.extend(test_archives)
     if (go.coverage_enabled and go.coverdata and

go/private/rules/binary.bzl

@@ -38,16 +38,13 @@ load(
 )
 load(
     "//go/private:mode.bzl",
+    "LINKMODES_EXECUTABLE",
     "LINKMODE_C_ARCHIVE",
     "LINKMODE_C_SHARED",
     "LINKMODE_NORMAL",
     "LINKMODE_PLUGIN",
     "LINKMODE_SHARED",
 )
-load(
-    "//go/private:rpath.bzl",
-    "rpath",
-)
 
 _EMPTY_DEPSET = depset([])
 
@@ -73,8 +70,6 @@ def new_cc_import(
         static_library = None,
         alwayslink = False,
         linkopts = []):
-    if dynamic_library:
-        linkopts = linkopts + rpath.flags(go, dynamic_library)
     return CcInfo(
         compilation_context = cc_common.create_compilation_context(
             defines = defines,
@@ -128,19 +123,35 @@ def _go_binary_impl(ctx):
         executable = executable,
     )
 
+    if go.mode.link in LINKMODES_EXECUTABLE:
+        # The executable is automatically added to the runfiles.
+        default_info = DefaultInfo(
+            files = depset([executable]),
+            runfiles = runfiles,
+            executable = executable,
+        )
+    else:
+        # Workaround for https://github.com/bazelbuild/bazel/issues/15043
+        # As of Bazel 5.1.1, native rules do not pick up the "files" of a data
+        # dependency's DefaultInfo, only the "data_runfiles". Since transitive
+        # non-data dependents should not pick up the executable as a runfile
+        # implicitly, the deprecated "default_runfiles" and "data_runfiles"
+        # constructor parameters have to be used.
+        default_info = DefaultInfo(
+            files = depset([executable]),
+            default_runfiles = runfiles,
+            data_runfiles = runfiles.merge(ctx.runfiles([executable])),
+        )
+
     providers = [
         library,
         source,
         archive,
+        default_info,
         OutputGroupInfo(
             cgo_exports = archive.cgo_exports,
             compilation_outputs = [archive.data.file],
         ),
-        DefaultInfo(
-            files = depset([executable]),
-            runfiles = runfiles,
-            executable = executable,
-        ),
     ]
 
     # If the binary's linkmode is c-archive or c-shared, expose CcInfo

tests/core/go_binary/non_executable_test.go

@@ -26,21 +26,51 @@ func TestMain(m *testing.M) {
 		Main: `
 -- src/BUILD.bazel --
 load("@io_bazel_rules_go//go:def.bzl", "go_binary")
+load(":rules.bzl", "no_runfiles_check")
 
 go_binary(
     name = "archive",
     srcs = ["archive.go"],
+    cgo = True,
     linkmode = "c-archive",
 )
+
+cc_binary(
+    name = "main",
+    srcs = ["main.c"],
+    deps = [":archive"],
+)
+
+no_runfiles_check(
+    name = "no_runfiles",
+    target = ":main",
+)
 -- src/archive.go --
 package main
 
+import "C"
+
 func main() {}
+-- src/main.c --
+int main() {}
+-- src/rules.bzl --
+def _no_runfiles_check_impl(ctx):
+    runfiles = ctx.attr.target[DefaultInfo].default_runfiles.files.to_list()
+    for runfile in runfiles:
+        if runfile.short_path not in ["src/main", "src/main.exe"]:
+            fail("Unexpected runfile: %s" % runfile.short_path)
+
+no_runfiles_check = rule(
+    implementation = _no_runfiles_check_impl,
+    attrs = {
+        "target": attr.label(),
+    }
+)
 `,
 	})
 }
 
-func TestNonExecutableGoBinary(t *testing.T) {
+func TestNonExecutableGoBinaryCantBeRun(t *testing.T) {
 	if err := bazel_testing.RunBazel("build", "//src:archive"); err != nil {
 		t.Fatal(err)
 	}
@@ -49,3 +79,9 @@ func TestNonExecutableGoBinary(t *testing.T) {
 		t.Errorf("Expected bazel run to fail due to //src:archive not being executable")
 	}
 }
+
+func TestNonExecutableGoBinaryNotInRunfiles(t *testing.T) {
+	if err := bazel_testing.RunBazel("build", "//src:no_runfiles"); err != nil {
+		t.Fatal(err)
+	}
+}