Skip to content

Commit c00e3f6

Browse files
authored
Fix: shim tools requiring go to use rules_go's hermetic SDK (#54887)
### What does this PR do? Unbreak `dda inv test` on hosts whose ambient `go` differs from the one `go.work` requires. Add `go_shim`, wrapping a tool so that `PATH` lookups of `go` reach Bazel's hermetic SDK, and so that it runs from the current working directory rather than the runfiles tree, superseding `rules_multitool`'s `cwd()`. Wrap `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. ### Motivation Credits to @vitkyrka for [reporting](https://dd.slack.com/archives/C06PBHLD4DQ/p1786618886362859), as well as to @pgimalac and @hush-hush for following up. #54107 routed `dda inv test` through `bazel run //internal/tools:gotestsum`, bringing `gotestsum` under `.bazelrc`'s `--run_env=GOTOOLCHAIN=local`. That flag exists for `bazel run //:go`, where `PATH` already holds the hermetic SDK and pinning the toolchain is exactly right, but `gotestsum` instead execs the literal `go` resolved through `PATH`, so it inherited the pin while still reaching for whatever `go` the host had, and Go's own version auto-upgrade could no longer paper over the difference. Every invocation then failed outright with `go.work requires go >= 1.26.5`, and exporting `GOTOOLCHAIN=auto` did not help, `--run_env` taking precedence over the caller's environment. `go_mod_tidy_all` had already met the same need and solved it **alone**, being our own code and able to take a path. `gotestsum` has no such flag, leaving `PATH` as the only lever, hence one rule for both and for whatever comes next. ### Describe how you validated your changes On Linux, the reported reproducer, `dda inv test --targets=./pkg/discovery/tracermetadata/...`, passes while a stub announcing itself as go 1.24.0 shadows `go` in `PATH`, and `bazel run //:go_mod_tidy_all -- -diff` exits 0 across every module without ever reaching that stub. On my Windows VM, where no `go` is installed at all, `where go` through the shim reports the hermetic `go.exe` and `go version` prints go1.26.5, against nothing found on merged `main`. On both, a missing or unreachable `$BUILD_WORKING_DIRECTORY` aborts before the tool runs rather than running it somewhere unintended, and the tool's own exit code surfaces unchanged. ### Additional Notes Prepending to `PATH` has no declarative spelling: `RunEnvironmentInfo` carries static strings only, and a key given in both `environment` and `inherited_environment` resolves to the inherited value, while replacing `PATH` outright would hide `sh`, `cc`, `git` and whatever else the toolchain reaches for. Hence a (thin) wrapper reading the environment at runtime, inspired by: - https://github.com/bazel-contrib/bazel-lib/blob/main/lib/private/bats.bzl - https://github.com/bazel-contrib/rules_multitool/blob/main/multitool/private/run_in.bzl `RUNFILES_DIR` is exported because `@rules_go//go` resolves its own runfiles through it and `bazel run` does not export it everywhere: without it the hermetic `go` exits with `runfiles: no runfiles found` on Windows. Co-authored-by: regis.desgroppes <regis.desgroppes@datadoghq.com>
1 parent afcc03c commit c00e3f6

9 files changed

Lines changed: 88 additions & 24 deletions

File tree

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: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
"""Shim to run a tool with @rules_go//go first in $PATH, from the current working directory like rules_go//go does.
2+
3+
A (thin) wrapper is unavoidable because prepending to the inherited PATH cannot be expressed declaratively.
4+
5+
References:
6+
- https://github.com/bazel-contrib/bazel-lib/blob/main/lib/private/bats.bzl (is_windows)
7+
- https://github.com/bazel-contrib/rules_go/blob/master/go/tools/go_bin_runner/main.go (BUILD_WORKING_DIRECTORY)
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+
def _go_shim_impl(ctx):
14+
go_dir = paths.dirname(ctx.executable._go.short_path)
15+
is_windows = ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo])
16+
template = ctx.file._template_bat if is_windows else ctx.file._template_sh
17+
tool = ctx.executable.tool.short_path
18+
wrapper = ctx.actions.declare_file("{}.{}".format(ctx.label.name, template.extension))
19+
ctx.actions.expand_template(
20+
is_executable = True,
21+
output = wrapper,
22+
substitutions = {
23+
"{{go_dir}}": go_dir.replace("/", "\\") if is_windows else go_dir,
24+
"{{tool}}": tool.replace("/", "\\") if is_windows else tool,
25+
},
26+
template = template,
27+
)
28+
return [DefaultInfo(
29+
executable = wrapper,
30+
runfiles = ctx.runfiles([ctx.executable._go, ctx.executable.tool]).merge_all([
31+
ctx.attr._go[DefaultInfo].default_runfiles,
32+
ctx.attr.tool[DefaultInfo].default_runfiles,
33+
]),
34+
)]
35+
36+
go_shim = rule(
37+
implementation = _go_shim_impl,
38+
executable = True,
39+
attrs = {
40+
"_go": attr.label(cfg = "target", default = "@rules_go//go", executable = True),
41+
"_template_bat": attr.label(allow_single_file = True, default = ":template.bat"),
42+
"_template_sh": attr.label(allow_single_file = True, default = ":template.sh"),
43+
"_windows_constraint": attr.label(default = "@platforms//os:windows"),
44+
"tool": attr.label(cfg = "exec", executable = True, mandatory = True),
45+
},
46+
)

bazel/rules/go_shim/template.bat

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
@echo off
2+
3+
set "PATH=%cd%\{{go_dir}};%PATH%"
4+
set "RUNFILES_DIR=%cd%\.."
5+
set "tool=%cd%\{{tool}}"
6+
7+
cd /d "%BUILD_WORKING_DIRECTORY%" || exit /b
8+
"%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+
PATH="$OLDPWD/{{go_dir}}:$PATH" RUNFILES_DIR="$OLDPWD/.." 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)