Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,8 @@ workflow:
- changes:
paths:
- .gitlab/test/e2e/e2e.yml
- bazel/rules/go_shim/**/* # Runs bazelified Go tools
- internal/tools/**/* # Go tools: gotestsum, etc. (incident-59251/59255/59256/59257/59258)

This comment was marked as outdated.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include the shim implementation in E2E change rules

When a follow-up changes bazel/rules/go_shim/defs.bzl or either wrapper template without also touching internal/tools, this rule will not schedule the E2E jobs, even though tasks/gotest.py runs E2E tests through the shimmed //internal/tools:gotestsum target. Since the newly introduced shim code controls the working-directory behavior that previously broke E2E, add bazel/rules/go_shim/**/* to this change set so those runtime-only regressions are exercised before reaching main.

Useful? React with 👍 / 👎.

I suppose this remark is valuable

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chouetz addressed in 2nd commit, please take another look

- test/e2e-framework/**/*
- test/new-e2e/go.mod
- flakes.yaml
Expand Down
5 changes: 3 additions & 2 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ load("@dd_release_json//:release_json.bzl", "release_json")
load("@gazelle//:def.bzl", "DEFAULT_LANGUAGES", "gazelle", "gazelle_binary")
load("@package_metadata//rules:package_metadata.bzl", "package_metadata")
load("@rules_license//rules:license.bzl", "license")
load("//bazel/rules/go_shim:defs.bzl", "go_shim")
load("//compliance/rules:purl.bzl", "purl_for_generic")
load("//tasks:build_tags.bzl", "GAZELLE_BUILD_TAGS")

Expand Down Expand Up @@ -123,9 +124,9 @@ alias(
)

# bazel run //:go_mod_tidy_all -- -x
alias(
go_shim(
name = "go_mod_tidy_all",
actual = "//bazel/tools:go_mod_tidy_all",
tool = "//bazel/rules/go_mod_tidy_all",
)

run_binary(
Expand Down
7 changes: 7 additions & 0 deletions bazel/rules/go_mod_tidy_all/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
load("@rules_python//python:py_binary.bzl", "py_binary")

py_binary(
name = "go_mod_tidy_all",
srcs = ["go_mod_tidy_all.py"],
visibility = ["//:__pkg__"],
)
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@
from subprocess import PIPE, CalledProcessError
from traceback import format_exception_only

from python.runfiles import runfiles


async def _exec(go, *args, **kwargs):
proc = await asyncio.create_subprocess_exec(go, *args, **kwargs)
async def _exec(*args, **kwargs):
proc = await asyncio.create_subprocess_exec(*args, **kwargs)
try:
stdout, _ = await proc.communicate()
except BaseException: # reap the process on any CancelledError, KeyboardInterrupt, SystemExit, TimeoutError, etc.
Expand All @@ -30,26 +28,26 @@ async def _exec(go, *args, **kwargs):
raise
if proc.returncode == 0:
return stdout
raise CalledProcessError(proc.returncode, " ".join((os.path.basename(go), *args)), output=stdout)
raise CalledProcessError(proc.returncode, " ".join(args), output=stdout)


async def _tidy(max_workers, go, mod_path, args):
async def _tidy(max_workers, mod_path, args):
async with max_workers:
await _exec(go, "mod", "tidy", "-C", mod_path, *args)
await _exec("go", "mod", "tidy", "-C", mod_path, *args)


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


if __name__ == "__main__":
logging.getLogger("asyncio").setLevel(logging.ERROR) # no `Unknown child process pid N, will report returncode 255`
try:
asyncio.run(main(runfiles.Create().Rlocation(sys.argv[1]), sys.argv[2:]))
asyncio.run(main(sys.argv[1:]))
except* BaseException as eg:
sys.exit("\n".join(line.rstrip() for e in eg.exceptions for line in format_exception_only(e)))
7 changes: 7 additions & 0 deletions bazel/rules/go_shim/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
exports_files(
[
"template.bat",
"template.sh",
],
visibility = ["//visibility:private"],
)
51 changes: 51 additions & 0 deletions bazel/rules/go_shim/defs.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
"""Shim to run a tool with the hermetic Go SDK's `go` first in $PATH, from the current working directory.

A (thin) wrapper is unavoidable because prepending to the inherited PATH cannot be expressed declaratively.
The SDK's `go` is used over @rules_go//go, because the latter changes the current directory again, breaking subcommands.

References:
- https://github.com/bazel-contrib/bazel-lib/blob/main/lib/private/bats.bzl (is_windows)
- https://github.com/bazel-contrib/rules_multitool/blob/main/multitool/private/run_in.bzl (template)
"""

load("@bazel_skylib//lib:paths.bzl", "paths")

_GO_TOOLCHAIN = "@rules_go//go:toolchain"

def _impl(ctx):
sdk = ctx.toolchains[_GO_TOOLCHAIN].sdk
go_dir = paths.dirname(sdk.go.short_path)
goroot = paths.dirname(sdk.root_file.short_path)
is_windows = ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo])
template = ctx.file._template_bat if is_windows else ctx.file._template_sh
tool = ctx.executable.tool.short_path
wrapper = ctx.actions.declare_file("{}.{}".format(ctx.label.name, template.extension))
ctx.actions.expand_template(
is_executable = True,
output = wrapper,
substitutions = {
"{{go_dir}}": go_dir.replace("/", "\\") if is_windows else go_dir,
"{{goroot}}": goroot.replace("/", "\\") if is_windows else goroot,
"{{tool}}": tool.replace("/", "\\") if is_windows else tool,
},
template = template,
)
return [DefaultInfo(
executable = wrapper,
runfiles = ctx.runfiles(
[sdk.go],
transitive_files = depset(transitive = [sdk.headers, sdk.libs, sdk.srcs, sdk.tools]),
).merge(ctx.attr.tool[DefaultInfo].default_runfiles),
)]

go_shim = rule(
implementation = _impl,
attrs = {
"_template_bat": attr.label(allow_single_file = True, default = ":template.bat"),
"_template_sh": attr.label(allow_single_file = True, default = ":template.sh"),
"_windows_constraint": attr.label(default = "@platforms//os:windows"),
"tool": attr.label(cfg = "exec", executable = True, mandatory = True),
},
executable = True,
toolchains = [_GO_TOOLCHAIN],
)
9 changes: 9 additions & 0 deletions bazel/rules/go_shim/template.bat
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
@echo off

set "GOROOT=%cd%\{{goroot}}"
set "GOTOOLCHAIN=local"
set "PATH=%cd%\{{go_dir}};%PATH%"
set "tool=%cd%\{{tool}}"

cd /d "%BUILD_WORKING_DIRECTORY%" || exit /b
"%tool%" %*
6 changes: 6 additions & 0 deletions bazel/rules/go_shim/template.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/usr/bin/env bash

set -euo pipefail

cd "$BUILD_WORKING_DIRECTORY"
GOROOT="$OLDPWD/{{goroot}}" GOTOOLCHAIN=local PATH="$OLDPWD/{{go_dir}}:$PATH" exec "$OLDPWD/{{tool}}" "$@"
9 changes: 0 additions & 9 deletions bazel/tools/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
load("@bazel_skylib//rules:write_file.bzl", "write_file")
load("@rules_python//python:py_binary.bzl", "py_binary")

py_binary(
name = "go_mod_tidy_all",
srcs = ["go_mod_tidy_all.py"],
args = ["$(rlocationpath @rules_go//go)"],
data = ["@rules_go//go"],
visibility = ["//:__pkg__"],
deps = ["@rules_python//python/runfiles"],
)

py_binary(
name = "generate_module_bazel",
srcs = ["generate_module_bazel.py"],
Expand Down
4 changes: 2 additions & 2 deletions internal/tools/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
load("@bazel_lib//lib:transitions.bzl", "platform_transition_filegroup")
load("@rules_multitool//multitool:cwd.bzl", "cwd")
load("@rules_pkg//pkg:install.bzl", "pkg_install")
load("@rules_pkg//pkg:mappings.bzl", "pkg_attributes", "pkg_files")
load("//bazel/rules/go_shim:defs.bzl", "go_shim")

cwd(
go_shim(
name = "gotestsum",
tool = "@tools_gotest_gotestsum//:gotestsum",
)
Expand Down
Loading