Make `//go` usable in scripts run with `bazel run` (#3474)

Allows developers to build their own local convenience scripts around
`go`, e.g. to run `go mod tidy` backed by a hermetic Go toolchain.

This requires getting rid of the `env` attribute as it is not evaluated
if the binary is run as a dependency of another target.

Since `//go` selects a Go SDK suitable for the host, not the exec
platform, we forbid its use in the exec or host configuration. As remarked in
https://github.com/bazelbuild/rules_go/issues/2255, using raw `go` in
a genrule is not a good idea to start with.

5 files changed, 85 insertions, 9 deletions

diff --git a/go/BUILD.bazel b/go/BUILD.bazel
index 0900ad5d..f88616c2 100644
--- a/go/BUILD.bazel
+++ b/go/BUILD.bazel
@@ -1,10 +1,14 @@
 load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
 
+# The 'go' binary of the current Go toolchain compatible with the host.
+# Use this with `bazel run` to perform utility actions such as `go mod tidy` in
+# a hermetic fashion.
+# Note: This is not meant to and cannot be used as a tool in e.g. a genrule. If
+# you need this functionality, please file an issue describing your use case.
 alias(
     name = "go",
     actual = "//go/tools/go_bin_runner",
-    # Meant to be run with `bazel run`, but should not be depended on.
-    visibility = ["//visibility:private"],
+    visibility = ["//visibility:public"],
 )
 
 filegroup(
diff --git a/go/private/rules/go_bin_for_host.bzl b/go/private/rules/go_bin_for_host.bzl
index a6fd9713..3f2dca99 100644
--- a/go/private/rules/go_bin_for_host.bzl
+++ b/go/private/rules/go_bin_for_host.bzl
@@ -15,8 +15,21 @@
 load("@local_config_platform//:constraints.bzl", "HOST_CONSTRAINTS")
 load("//go/private:go_toolchain.bzl", "GO_TOOLCHAIN")
 
+def _ensure_target_cfg(ctx):
+    # A target is assumed to be built in the target configuration if it is neither in the exec nor
+    # the host configuration (the latter has been removed in Bazel 6). Since there is no API for
+    # this, use the output directory to determine the configuration, which is a common pattern.
+    if "-exec-" in ctx.bin_dir.path or "/host/" in ctx.bin_dir.path:
+        fail("//go is only meant to be used with 'bazel run', not as a tool. " +
+             "If you need to use it as a tool (e.g. in a genrule), please " +
+             "open an issue at " +
+             "https://github.com/bazelbuild/rules_go/issues/new explaining " +
+             "your use case.")
+
 def _go_bin_for_host_impl(ctx):
     """Exposes the go binary of the current Go toolchain for the host."""
+    _ensure_target_cfg(ctx)
+
     sdk = ctx.toolchains[GO_TOOLCHAIN].sdk
     sdk_files = ctx.runfiles([sdk.go] + sdk.headers + sdk.libs + sdk.srcs + sdk.tools)
 
diff --git a/go/tools/go_bin_runner/BUILD.bazel b/go/tools/go_bin_runner/BUILD.bazel
index 0e348208..a0e26d65 100644
--- a/go/tools/go_bin_runner/BUILD.bazel
+++ b/go/tools/go_bin_runner/BUILD.bazel
@@ -5,7 +5,7 @@ load("//go/private/rules:go_bin_for_host.bzl", "go_bin_for_host")
 
 go_bin_for_host(
     name = "go_bin_for_host",
-    visibility = ["//go:__pkg__"],
+    visibility = ["//visibility:private"],
 )
 
 go_library(
@@ -30,10 +30,10 @@ go_binary(
     name = "go_bin_runner",
     data = [":go_bin_for_host"],
     embed = [":go_bin_runner_lib"],
-    env = {
-        "GO_BIN_RLOCATIONPATH": "$(rlocationpath :go_bin_for_host)",
+    visibility = ["//go:__pkg__"],
+    x_defs = {
+        "GoBinRlocationPath": "$(rlocationpath :go_bin_for_host)",
     },
-    visibility = ["//visibility:public"],
 )
 
 filegroup(
diff --git a/go/tools/go_bin_runner/main.go b/go/tools/go_bin_runner/main.go
index 5fe4fd94..bca4f7fc 100644
--- a/go/tools/go_bin_runner/main.go
+++ b/go/tools/go_bin_runner/main.go
@@ -9,9 +9,10 @@ import (
 	"github.com/bazelbuild/rules_go/go/runfiles"
 )
 
+var GoBinRlocationPath = "not set"
+
 func main() {
-	goBinRlocation := os.Getenv("GO_BIN_RLOCATIONPATH")
-	goBin, err := runfiles.Rlocation(goBinRlocation)
+	goBin, err := runfiles.Rlocation(GoBinRlocationPath)
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/tests/integration/go_bin_runner/go_bin_runner_test.go b/tests/integration/go_bin_runner/go_bin_runner_test.go
index cb238f7b..2cd42660 100644
--- a/tests/integration/go_bin_runner/go_bin_runner_test.go
+++ b/tests/integration/go_bin_runner/go_bin_runner_test.go
@@ -24,7 +24,38 @@ import (
 )
 
 func TestMain(m *testing.M) {
-	bazel_testing.TestMain(m, bazel_testing.Args{})
+	bazel_testing.TestMain(m, bazel_testing.Args{
+		Main: `
+-- BUILD.bazel --
+sh_binary(
+    name = "go_version",
+    srcs = ["go_version.sh"],
+    env = {"GO": "$(rlocationpath @io_bazel_rules_go//go)"},
+    data = ["@io_bazel_rules_go//go"],
+    deps = ["@bazel_tools//tools/bash/runfiles"],
+)
+
+genrule(
+    name = "foo",
+    outs = ["bar"],
+    tools = ["@io_bazel_rules_go//go"],
+    cmd = "$(location @io_bazel_rules_go//go) > $@",
+)
+
+-- go_version.sh --
+# --- begin runfiles.bash initialization v2 ---
+# Copy-pasted from the Bazel Bash runfiles library v2.
+set -uo pipefail; f=bazel_tools/tools/bash/runfiles/runfiles.bash
+source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \
+  source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \
+  source "$0.runfiles/$f" 2>/dev/null || \
+  source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
+  source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
+  { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e
+# --- end runfiles.bash initialization v2 ---
+
+$(rlocation "$GO") version
+`})
 }
 
 func TestGoEnv(t *testing.T) {
@@ -47,3 +78,30 @@ func TestGoEnv(t *testing.T) {
 		t.Fatalf("GOROOT was not equal to %s", filepath.Join(outputBase, "external", "go_sdk"))
 	}
 }
+
+func TestGoVersionFromScript(t *testing.T) {
+	err := os.Chmod("go_version.sh", 0755)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	goVersionOut, err := bazel_testing.BazelOutput("run", "//:go_version")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if !strings.HasPrefix(string(goVersionOut), "go version go1.") {
+		t.Fatalf("go version output did not start with \"go version go1.\": %s", string(goVersionOut))
+	}
+}
+
+func TestNoGoInExec(t *testing.T) {
+	_, err := bazel_testing.BazelOutput("build", "//:foo")
+	if err == nil {
+		t.Fatal("expected build to fail")
+	}
+	stderr := string(err.(*bazel_testing.StderrExitError).Err.Stderr)
+	if !strings.Contains(stderr, "//go is only meant to be used with 'bazel run'") {
+		t.Fatalf("expected \"//go is only meant to be used with 'bazel run'\" in stderr, got %s", stderr)
+	}
+}