From c3f1349858f4dd639e490060b38444c1decc2b85 Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <fabian@meumertzhe.im>
Date: Thu, 12 May 2022 08:47:00 +0200
Subject: [PATCH] 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/rules/binary.bzl                 | 27 ++++++++++++---
 tests/core/c_linkmodes/BUILD.bazel          |  2 +-
 tests/core/go_binary/non_executable_test.go | 38 ++++++++++++++++++++-
 3 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/go/private/rules/binary.bzl b/go/private/rules/binary.bzl
index 8d95020ef9..2015f84d0f 100644
--- a/go/private/rules/binary.bzl
+++ b/go/private/rules/binary.bzl
@@ -34,6 +34,7 @@ load(
 )
 load(
     "//go/private:mode.bzl",
+    "LINKMODES_EXECUTABLE",
     "LINKMODE_C_ARCHIVE",
     "LINKMODE_C_SHARED",
     "LINKMODE_NORMAL",
@@ -124,19 +125,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
diff --git a/tests/core/c_linkmodes/BUILD.bazel b/tests/core/c_linkmodes/BUILD.bazel
index ee7f68e663..d9cbd1da04 100644
--- a/tests/core/c_linkmodes/BUILD.bazel
+++ b/tests/core/c_linkmodes/BUILD.bazel
@@ -54,7 +54,7 @@ cc_test(
         "@io_bazel_rules_go//go/platform:windows": ["skip.c"],
         "//conditions:default": ["add_test_shared.c"],
     }),
-    deps = select({
+    data = select({
         "@io_bazel_rules_go//go/platform:windows": [],
         "//conditions:default": [":adder_shared"],
     }),
diff --git a/tests/core/go_binary/non_executable_test.go b/tests/core/go_binary/non_executable_test.go
index e1a352426a..b2eb987c44 100644
--- a/tests/core/go_binary/non_executable_test.go
+++ b/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)
+	}
+}