From b4536e7a166504e8cf9dc16339d3741cda6f4260 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 9 Mar 2023 23:01:34 +0100 Subject: [PATCH] Make //go usable in scripts run with `bazel run` 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` select a Go SDK suitable for the host, not the exec platform, we forbid its use in the exec 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. --- go/BUILD.bazel | 8 ++- go/private/rules/go_bin_for_host.bzl | 10 ++++ go/tools/go_bin_runner/BUILD.bazel | 8 +-- go/tools/go_bin_runner/main.go | 5 +- .../go_bin_runner/go_bin_runner_test.go | 60 ++++++++++++++++++- 5 files changed, 82 insertions(+), 9 deletions(-) diff --git a/go/BUILD.bazel b/go/BUILD.bazel index 0900ad5d09..f88616c257 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 a6fd97130c..ed2ab91d91 100644 --- a/go/private/rules/go_bin_for_host.bzl +++ b/go/private/rules/go_bin_for_host.bzl @@ -15,8 +15,18 @@ load("@local_config_platform//:constraints.bzl", "HOST_CONSTRAINTS") load("//go/private:go_toolchain.bzl", "GO_TOOLCHAIN") +def _ensure_target_cfg(ctx): + if "-exec-" 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 0e34820876..a0e26d6546 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 5fe4fd940e..bca4f7fca5 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 cb238f7b53..2cd42660f0 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) + } +}