Skip to content

Commit 730749e

Browse files
committed
Reland go_shim, replacing go runner by SDK's raw go
### What does this PR do? Relands #54887, which #54925 reverted after it broke E2E jobs on `main`. Recalling that PR: `go_shim` wraps a tool so that `PATH` lookups of `go` reach Bazel's hermetic SDK, and so that the tool runs from the current working directory rather than the runfiles tree, superseding `rules_multitool`'s `cwd()`. It wraps `gotestsum` and `go_mod_tidy_all`, the latter dropping the `$(rlocationpath @rules_go//go)` resolution it had grown for itself and moving to its own package, where the shim taking over `//:go_mod_tidy_all` leaves its `py_binary` the plain name. Changed on top of it: `PATH` now holds the Go SDK's own `go`, resolved through `@rules_go//go:toolchain`, in place of `@rules_go//go`. The wrapper exports `GOROOT` and sets `GOTOOLCHAIN=local` itself rather than leaning on `.bazelrc`, and `RUNFILES_DIR` goes away with the launcher that needed it. ### Motivation Unbreak `dda inv test` on hosts whose ambient `go` differs from the one `go.work` requires, [reported](https://dd.slack.com/archives/C06PBHLD4DQ/p1786618886362859) by @vitkyrka and followed up by @pgimalac and @hush-hush. #54107 routed it through `bazel run //internal/tools:gotestsum`, bringing `gotestsum` under `.bazelrc`'s `--run_env=GOTOOLCHAIN=local` while it still resolved `go` through `PATH`, so every invocation failed with `go.work requires go >= 1.26.5` and exporting `GOTOOLCHAIN=auto` did not help. That fix then broke `main`, because `@rules_go//go` is a launcher rather than a `go`: it assigns `cmd.Dir` from `$BUILD_WORKING_DIRECTORY` on every exec, discarding the directory its caller chose. `gotest-custom` chooses one per test package, so each prebuilt E2E test binary ran at the repository root and no longer found its fixtures, failing on `compose/data`, `usmtest/test_tags.ps1`, `testdataprovision` and `checks/shared-library/files`. The SDK's `go` honors its caller, which is the whole of the difference. ### Describe how you validated your changes On Linux, a child `go` invoked from `test/new-e2e` under the shim reports that module's `go.mod`, where #54887 reported the repository root's. Building a program importing `net/http` through the shim links, confirming the SDK's sources and tools are reachable. The reported reproducer still holds: with a stub announcing itself as go 1.24.0 first in `PATH`, tools reach go1.26.5 and never the stub, and `bazel run //:go_mod_tidy_all -- -diff` exits 0 across every module. ### Additional Notes Prepending to `PATH` has no declarative spelling: `RunEnvironmentInfo` carries static strings only, a key given in both `environment` and `inherited_environment` resolves to the inherited value, and replacing `PATH` outright would hide `sh`, `cc`, `git` and whatever else the toolchain reaches for. Hence a (thin) wrapper reading the environment at runtime. The runfiles mirror `go_bin_for_host`, rules_go's own definition of a runnable `go`: the SDK binary plus its headers, libs, sources and tools. `install_gotestsum` and its per-platform variants keep extracting the raw binary, the shim being runfiles-bound and those tools driving pre-built test binaries through `--raw-command`, never resolving `go`.
1 parent a0a423d commit 730749e

10 files changed

Lines changed: 95 additions & 24 deletions

File tree

.gitlab-ci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -992,6 +992,7 @@ workflow:
992992
- changes:
993993
paths:
994994
- .gitlab/test/e2e/e2e.yml
995+
- internal/tools/**/* # Go tools: gotestsum, etc. (incident-59251/59255/59256/59257/59258)
995996
- test/e2e-framework/**/*
996997
- test/new-e2e/go.mod
997998
- flakes.yaml

BUILD.bazel

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ load("@dd_release_json//:release_json.bzl", "release_json")
1212
load("@gazelle//:def.bzl", "DEFAULT_LANGUAGES", "gazelle", "gazelle_binary")
1313
load("@package_metadata//rules:package_metadata.bzl", "package_metadata")
1414
load("@rules_license//rules:license.bzl", "license")
15+
load("//bazel/rules/go_shim:defs.bzl", "go_shim")
1516
load("//compliance/rules:purl.bzl", "purl_for_generic")
1617
load("//tasks:build_tags.bzl", "GAZELLE_BUILD_TAGS")
1718

@@ -123,9 +124,9 @@ alias(
123124
)
124125

125126
# bazel run //:go_mod_tidy_all -- -x
126-
alias(
127+
go_shim(
127128
name = "go_mod_tidy_all",
128-
actual = "//bazel/tools:go_mod_tidy_all",
129+
tool = "//bazel/rules/go_mod_tidy_all",
129130
)
130131

131132
run_binary(
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
load("@rules_python//python:py_binary.bzl", "py_binary")
2+
3+
py_binary(
4+
name = "go_mod_tidy_all",
5+
srcs = ["go_mod_tidy_all.py"],
6+
visibility = ["//:__pkg__"],
7+
)

bazel/tools/go_mod_tidy_all.py renamed to bazel/rules/go_mod_tidy_all/go_mod_tidy_all.py

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,9 @@
1212
from subprocess import PIPE, CalledProcessError
1313
from traceback import format_exception_only
1414

15-
from python.runfiles import runfiles
1615

17-
18-
async def _exec(go, *args, **kwargs):
19-
proc = await asyncio.create_subprocess_exec(go, *args, **kwargs)
16+
async def _exec(*args, **kwargs):
17+
proc = await asyncio.create_subprocess_exec(*args, **kwargs)
2018
try:
2119
stdout, _ = await proc.communicate()
2220
except BaseException: # reap the process on any CancelledError, KeyboardInterrupt, SystemExit, TimeoutError, etc.
@@ -30,26 +28,26 @@ async def _exec(go, *args, **kwargs):
3028
raise
3129
if proc.returncode == 0:
3230
return stdout
33-
raise CalledProcessError(proc.returncode, " ".join((os.path.basename(go), *args)), output=stdout)
31+
raise CalledProcessError(proc.returncode, " ".join(args), output=stdout)
3432

3533

36-
async def _tidy(max_workers, go, mod_path, args):
34+
async def _tidy(max_workers, mod_path, args):
3735
async with max_workers:
38-
await _exec(go, "mod", "tidy", "-C", mod_path, *args)
36+
await _exec("go", "mod", "tidy", "-C", mod_path, *args)
3937

4038

41-
async def main(go, args):
42-
mod_paths = await _exec(go, "list", "-f", "{{.Dir}}", "-m", stdout=PIPE)
39+
async def main(args):
40+
mod_paths = await _exec("go", "list", "-f", "{{.Dir}}", "-m", stdout=PIPE)
4341
max_workers = asyncio.Semaphore((os.cpu_count() or 1) + 4) # TODO(regis): cpu_count -> Py 3.13's process_cpu_count
4442
# global timeout: on cold cache, per-task timeouts were unfairly hit because early tasks download most modules
4543
async with asyncio.timeout(timedelta(minutes=15).total_seconds()), asyncio.TaskGroup() as tg:
4644
for mod_path in mod_paths.decode().splitlines():
47-
tg.create_task(_tidy(max_workers, go, mod_path, args))
45+
tg.create_task(_tidy(max_workers, mod_path, args))
4846

4947

5048
if __name__ == "__main__":
5149
logging.getLogger("asyncio").setLevel(logging.ERROR) # no `Unknown child process pid N, will report returncode 255`
5250
try:
53-
asyncio.run(main(runfiles.Create().Rlocation(sys.argv[1]), sys.argv[2:]))
51+
asyncio.run(main(sys.argv[1:]))
5452
except* BaseException as eg:
5553
sys.exit("\n".join(line.rstrip() for e in eg.exceptions for line in format_exception_only(e)))

bazel/rules/go_shim/BUILD.bazel

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
exports_files(
2+
[
3+
"template.bat",
4+
"template.sh",
5+
],
6+
visibility = ["//visibility:private"],
7+
)

bazel/rules/go_shim/defs.bzl

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
"""Shim to run a tool with the hermetic Go SDK's `go` first in $PATH, from the current working directory.
2+
3+
A (thin) wrapper is unavoidable because prepending to the inherited PATH cannot be expressed declaratively.
4+
The SDK's `go` is used over @rules_go//go, because the latter changes the current directory again, breaking subcommands.
5+
6+
References:
7+
- https://github.com/bazel-contrib/bazel-lib/blob/main/lib/private/bats.bzl (is_windows)
8+
- https://github.com/bazel-contrib/rules_multitool/blob/main/multitool/private/run_in.bzl (template)
9+
"""
10+
11+
load("@bazel_skylib//lib:paths.bzl", "paths")
12+
13+
_GO_TOOLCHAIN = "@rules_go//go:toolchain"
14+
15+
def _impl(ctx):
16+
sdk = ctx.toolchains[_GO_TOOLCHAIN].sdk
17+
go_dir = paths.dirname(sdk.go.short_path)
18+
goroot = paths.dirname(sdk.root_file.short_path)
19+
is_windows = ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo])
20+
template = ctx.file._template_bat if is_windows else ctx.file._template_sh
21+
tool = ctx.executable.tool.short_path
22+
wrapper = ctx.actions.declare_file("{}.{}".format(ctx.label.name, template.extension))
23+
ctx.actions.expand_template(
24+
is_executable = True,
25+
output = wrapper,
26+
substitutions = {
27+
"{{go_dir}}": go_dir.replace("/", "\\") if is_windows else go_dir,
28+
"{{goroot}}": goroot.replace("/", "\\") if is_windows else goroot,
29+
"{{tool}}": tool.replace("/", "\\") if is_windows else tool,
30+
},
31+
template = template,
32+
)
33+
return [DefaultInfo(
34+
executable = wrapper,
35+
runfiles = ctx.runfiles(
36+
[sdk.go],
37+
transitive_files = depset(transitive = [sdk.headers, sdk.libs, sdk.srcs, sdk.tools]),
38+
).merge(ctx.attr.tool[DefaultInfo].default_runfiles),
39+
)]
40+
41+
go_shim = rule(
42+
implementation = _impl,
43+
attrs = {
44+
"_template_bat": attr.label(allow_single_file = True, default = ":template.bat"),
45+
"_template_sh": attr.label(allow_single_file = True, default = ":template.sh"),
46+
"_windows_constraint": attr.label(default = "@platforms//os:windows"),
47+
"tool": attr.label(cfg = "exec", executable = True, mandatory = True),
48+
},
49+
executable = True,
50+
toolchains = [_GO_TOOLCHAIN],
51+
)

bazel/rules/go_shim/template.bat

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
@echo off
2+
3+
set "GOROOT=%cd%\{{goroot}}"
4+
set "GOTOOLCHAIN=local"
5+
set "PATH=%cd%\{{go_dir}};%PATH%"
6+
set "tool=%cd%\{{tool}}"
7+
8+
cd /d "%BUILD_WORKING_DIRECTORY%" || exit /b
9+
"%tool%" %*

bazel/rules/go_shim/template.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#!/usr/bin/env bash
2+
3+
set -euo pipefail
4+
5+
cd "$BUILD_WORKING_DIRECTORY"
6+
GOROOT="$OLDPWD/{{goroot}}" GOTOOLCHAIN=local PATH="$OLDPWD/{{go_dir}}:$PATH" exec "$OLDPWD/{{tool}}" "$@"

bazel/tools/BUILD.bazel

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,6 @@
11
load("@bazel_skylib//rules:write_file.bzl", "write_file")
22
load("@rules_python//python:py_binary.bzl", "py_binary")
33

4-
py_binary(
5-
name = "go_mod_tidy_all",
6-
srcs = ["go_mod_tidy_all.py"],
7-
args = ["$(rlocationpath @rules_go//go)"],
8-
data = ["@rules_go//go"],
9-
visibility = ["//:__pkg__"],
10-
deps = ["@rules_python//python/runfiles"],
11-
)
12-
134
py_binary(
145
name = "generate_module_bazel",
156
srcs = ["generate_module_bazel.py"],

internal/tools/BUILD.bazel

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
load("@bazel_lib//lib:transitions.bzl", "platform_transition_filegroup")
2-
load("@rules_multitool//multitool:cwd.bzl", "cwd")
32
load("@rules_pkg//pkg:install.bzl", "pkg_install")
43
load("@rules_pkg//pkg:mappings.bzl", "pkg_attributes", "pkg_files")
4+
load("//bazel/rules/go_shim:defs.bzl", "go_shim")
55

6-
cwd(
6+
go_shim(
77
name = "gotestsum",
88
tool = "@tools_gotest_gotestsum//:gotestsum",
99
)

0 commit comments

Comments
 (0)