From c84510beb7553ba5d036f75ad27ce32ca26998d9 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                 | 15 ++++++--
 tests/core/go_binary/non_executable_test.go | 38 ++++++++++++++++++++-
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/go/private/rules/binary.bzl b/go/private/rules/binary.bzl
index 8d95020ef9..fbd59f5f3e 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,6 +125,15 @@ def _go_binary_impl(ctx):
         executable = executable,
     )
 
+    # 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_runfiles = runfiles
+    data_runfiles = default_runfiles.merge(ctx.runfiles([executable]))
+
     providers = [
         library,
         source,
@@ -134,8 +144,9 @@ def _go_binary_impl(ctx):
         ),
         DefaultInfo(
             files = depset([executable]),
-            runfiles = runfiles,
-            executable = executable,
+            data_runfiles = data_runfiles,
+            default_runfiles = default_runfiles,
+            executable = executable if go.mode.link in LINKMODES_EXECUTABLE else None,
         ),
     ]
 
diff --git a/tests/core/go_binary/non_executable_test.go b/tests/core/go_binary/non_executable_test.go
index e1a352426a..97d16405df 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 != "src/main":
+            fail("Unexpected runfile: %s" % runfile)
+
+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)
+	}
+}