Skip to content

Commit f88aba5

Browse files
authored
Consolidate bazel invocations in the Python code (#47952)
### What does this PR do? Unify calls to `bazel` from Python with _tailored_ `ctx.run`s. This includes the transitional `bazel` helper and callers predating the helper. ### Motivation Favor a single point of maintenance (tested) that: 1. provides an actionable error message should `bazel` be missing (`inv install-tools`), 2. preserves `bazel`'s colored output (pseudo-tty, unless captured), 3. doesn't bury actual errors under frames of Python internals (incurred by `subprocess.check_call`/`check_output`), 4. takes over the burden of `invoke` lacking shell-agnostic command lines (pyinvoke/invoke#2, pyinvoke/invoke#312, pyinvoke/invoke#341, pyinvoke/invoke#514, pyinvoke/invoke#698 and counting). ### Describe how you validated your changes Ran the existing unit tests: ```sh python -m pytest tasks/unit_tests/libs/build/bazel_tests.py ``` Co-authored-by: regis.desgroppes <regis.desgroppes@datadoghq.com>
1 parent f440a21 commit f88aba5

File tree

9 files changed

+45
-38
lines changed

9 files changed

+45
-38
lines changed

tasks/go.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -379,15 +379,15 @@ def _go_only_tidy(ctx, verbose: bool):
379379

380380
def _bazel_tidy(ctx, verbose: bool):
381381
# 1. deps/go.MODULE.bazel ↺ (prune stale use_repo declarations to not hinder next `bazel` commands)
382-
bazel("mod", "tidy")
382+
bazel(ctx, "mod", "tidy")
383383
# 2. go.work + **/go.mod -> **/go.mod (sync each workspace module's deps to the workspace build list)
384-
bazel("run", "//:go", "work", "sync")
384+
bazel(ctx, "run", "//:go", "work", "sync")
385385
# 3. **/*.go + **/go.mod -> **/go.mod, **/go.sum (reconcile each module's requirements with its actual imports)
386-
bazel("run", "//:go_mod_tidy_all", *(("--", "-x") if verbose else ()))
386+
bazel(ctx, "run", "//:go_mod_tidy_all", *(("--", "-x") if verbose else ()))
387387
# 4. go.work + **/go.mod -> deps/go.MODULE.bazel (update use_repo declarations)
388-
bazel("mod", "tidy")
388+
bazel(ctx, "mod", "tidy")
389389
# 5. deps/go.MODULE.bazel + /BUILD.bazel + **/*.go + **/go.mod -> **/BUILD.bazel (infer build rules from Go source)
390-
bazel("run", "//:gazelle")
390+
bazel(ctx, "run", "//:gazelle")
391391

392392

393393
@task(autoprint=True)

tasks/libs/build/bazel.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,25 @@
77
import subprocess
88
import sys
99

10+
from invoke import Exit
11+
from invoke.context import Context
12+
1013
from tasks.libs.common.color import color_message
1114

1215

1316
def bazel_not_found_message(color: str) -> str:
1417
return color_message("Please run `inv install-tools` for `bazel` support!", color)
1518

1619

17-
def bazel(*args: str, capture_output: bool = False) -> None | str:
20+
def bazel(ctx: Context, *args: str, capture_output: bool = False, sudo: bool = False) -> None | str:
1821
"""Execute a bazel command. Returns the captured standard output as string if capture_output=True."""
1922

20-
cmd = shutil.which("bazel") or sys.exit(bazel_not_found_message("red"))
21-
print(color_message(shlex.join(("bazel", *args)), "bold"), file=sys.stderr)
22-
return (subprocess.check_output if capture_output else subprocess.check_call)((cmd, *args), text=True)
23+
if not shutil.which("bazel"):
24+
raise Exit(bazel_not_found_message("red"))
25+
result = (ctx.sudo if sudo else ctx.run)(
26+
(subprocess.list2cmdline if sys.platform == "win32" else shlex.join)(("bazel", *args)),
27+
echo=True,
28+
in_stream=False,
29+
**({"hide": "out"} if capture_output else {"pty": sys.stdout.isatty()}), # type: ignore[dict-item]
30+
)
31+
return result.stdout if capture_output else None

tasks/libs/common/gomodules.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from typing import ClassVar
1111

1212
import yaml
13+
from invoke.context import Context
1314

1415
import tasks
1516
from tasks.libs.build.bazel import bazel
@@ -230,11 +231,12 @@ def __version(self, agent_version):
230231

231232
return "v0" + agent_version[1:]
232233

233-
def __compute_dependencies(self):
234+
def __compute_dependencies(self, ctx: Context):
234235
"""
235236
Computes the list of github.com/DataDog/datadog-agent/ dependencies of the module.
236237
"""
237238
output = bazel(
239+
ctx,
238240
"run",
239241
"//internal/tools/modparser",
240242
"--",
@@ -272,10 +274,9 @@ def go_mod_path(self):
272274
"""Return the absolute path of the Go module go.mod file."""
273275
return self.full_path() + "/go.mod"
274276

275-
@property
276-
def dependencies(self):
277+
def dependencies(self, ctx: Context):
277278
if not self._dependencies:
278-
self._dependencies = self.__compute_dependencies()
279+
self._dependencies = self.__compute_dependencies(ctx)
279280
return self._dependencies
280281

281282
@property

tasks/modules.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ def go_work(ctx: Context):
8585
"""
8686
Update the go work to use all the modules defined in modules.yml
8787
"""
88-
bazel("run", "//:write_go_work")
88+
bazel(ctx, "run", "//:write_go_work")
8989

9090

9191
@task

tasks/pkg_template.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55

66

77
@task
8-
def generate(_: Context):
8+
def generate(ctx: Context):
99
"""
1010
Generate the code for the template package.
1111
Takes the code from the Go standard library and applies the patches.
1212
"""
13-
bazel("run", "//pkg/template:generate")
13+
bazel(ctx, "run", "//pkg/template:generate")

tasks/release.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ def update_modules(ctx, release_branch=None, version=None, trust=False):
124124
with agent_context(ctx, release_branch, skip_checkout=release_branch is None):
125125
modules = get_default_modules()
126126
for module in modules.values():
127-
for dependency in module.dependencies:
127+
for dependency in module.dependencies(ctx):
128128
dependency_mod = modules[dependency]
129129
if (
130130
agent_version.startswith('6')

tasks/system_probe.py

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
from tasks.build_tags import UNIT_TEST_TAGS, get_default_build_tags
2424
from tasks.flavor import AgentFlavor
25+
from tasks.libs.build.bazel import bazel
2526
from tasks.libs.build.ninja import NinjaWriter
2627
from tasks.libs.ciproviders.gitlab_api import ReferenceTag
2728
from tasks.libs.common.color import color_message
@@ -410,7 +411,7 @@ def build_libpcap(ctx, env: dict, arch: Arch | None = None):
410411
ctx.run(f"echo 'libpcap version {version} already exists at {target_file}'")
411412
return
412413

413-
ctx.run(f"bazelisk run -- @libpcap//:install --destdir='{embedded_path}'")
414+
bazel(ctx, "run", "--", "@libpcap//:install", f"--destdir={embedded_path}")
414415
ctx.run(f"strip -g {target_file}")
415416
return
416417

@@ -1268,13 +1269,9 @@ def bazel_build_ebpf(ctx: Context, arch: Arch, build_dir: str, strip: bool = Tru
12681269
inplace_targets = _BAZEL_EBPF_INPLACE_TARGETS
12691270

12701271
all_targets = _BAZEL_EBPF_PREBUILT_TARGETS + _BAZEL_EBPF_CORE_TARGETS + list(inplace_targets.keys())
1271-
targets_str = " ".join(all_targets)
1272-
12731272
print(f"Building {len(all_targets)} eBPF targets via Bazel...")
1274-
ctx.run(f"bazelisk build {targets_str}")
1275-
1276-
result = ctx.run("bazelisk info bazel-bin", hide=True)
1277-
bazel_bin = result.stdout.strip()
1273+
bazel(ctx, "build", *all_targets)
1274+
bazel_bin = bazel(ctx, "info", "bazel-bin", capture_output=True).strip()
12781275

12791276
co_re_dir = os.path.join(build_dir, "co-re")
12801277
os.makedirs(build_dir, exist_ok=True)
@@ -1349,7 +1346,7 @@ def build_object_files(
13491346
# Install Bazel-managed LLVM BPF tools (needed for stripping and runtime compilation).
13501347
sudo = "" if is_root() else "sudo"
13511348
ctx.run(f"{sudo} mkdir -p /opt/datadog-agent/embedded/bin")
1352-
ctx.run(f"{sudo} bazelisk run -- @llvm_bpf//:install --destdir=/opt/datadog-agent")
1349+
bazel(ctx, "run", "--", "@llvm_bpf//:install", "--destdir=/opt/datadog-agent", sudo=not is_root())
13531350

13541351
# Build eBPF .o files via Bazel
13551352
bazel_build_ebpf(ctx, arch_obj, build_dir)
@@ -1403,16 +1400,16 @@ def build_rust_binaries(ctx: Context, arch: Arch, output_dir: Path | None = None
14031400
"arm64": "//bazel/platforms:linux_arm64",
14041401
}
14051402

1406-
platform_flag = ""
1403+
platform_flags = []
14071404
if arch.kmt_arch in platform_map:
1408-
platform_flag = f"--platforms={platform_map[arch.kmt_arch]}"
1405+
platform_flags.append(f"--platforms={platform_map[arch.kmt_arch]}")
14091406

14101407
for source_path in RUST_BINARIES:
14111408
if packages and not any(source_path.startswith(package) for package in packages):
14121409
continue
14131410

14141411
install_dest = output_dir / source_path if output_dir else Path(source_path)
1415-
ctx.run(f"bazelisk run {platform_flag} -- @//{source_path}:install --destdir={install_dest}")
1412+
bazel(ctx, "run", *platform_flags, "--", f"@//{source_path}:install", f"--destdir={install_dest}")
14161413

14171414

14181415
_BAZEL_CWS_BALOUM_TARGETS = {
@@ -1439,10 +1436,8 @@ def build_cws_object_files(
14391436

14401437
if with_unit_test:
14411438
targets = list(_BAZEL_CWS_BALOUM_TARGETS.keys())
1442-
ctx.run(f"bazelisk build {' '.join(targets)}")
1443-
1444-
result = ctx.run("bazelisk info bazel-bin", hide=True)
1445-
bazel_bin = result.stdout.strip()
1439+
bazel(ctx, "build", *targets)
1440+
bazel_bin = bazel(ctx, "info", "bazel-bin", capture_output=True).strip()
14461441

14471442
for target, dest_name in _BAZEL_CWS_BALOUM_TARGETS.items():
14481443
label_path, name = target.lstrip("/").rsplit(":", 1)

tasks/unit_tests/libs/build/bazel_tests.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,26 @@
22
import unittest
33
from unittest.mock import patch
44

5+
from invoke import Context, Exit, MockContext
6+
57
from tasks.libs.build.bazel import bazel
68
from tasks.libs.common.utils import get_repo_root
79

810

911
class TestBazel(unittest.TestCase):
1012
def test_bazel_call(self):
11-
self.assertEqual(bazel("info", "release"), 0)
13+
self.assertIsNone(bazel(Context(), "info", "release"))
1214

1315
def test_bazel_output(self):
1416
expected_version = (get_repo_root() / ".bazelversion").read_text().strip()
15-
actual_output = bazel("info", "release", capture_output=True).strip()
17+
actual_output = bazel(Context(), "info", "release", capture_output=True).strip()
1618
self.assertEqual(actual_output, f"release {expected_version}")
1719

1820
@patch.dict(os.environ, {"PATH": os.devnull})
1921
def test_bazel_not_found(self):
20-
with self.assertRaises(SystemExit) as cm:
21-
bazel("info")
22-
self.assertIn("Please run `inv install-tools` for `bazel` support!", cm.exception.code)
22+
with self.assertRaises(Exit) as cm:
23+
bazel(MockContext(), "info")
24+
self.assertIn("Please run `inv install-tools` for `bazel` support!", cm.exception.message)
2325

2426

2527
if __name__ == "__main__":

tasks/update_go.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ def update_go(
9595
res = ctx.run("go version")
9696
if res and res.stdout.startswith(f"go version go{version} "):
9797
print("Updating the code in pkg/template...")
98-
bazel("run", "//pkg/template:generate")
98+
bazel(ctx, "run", "//pkg/template:generate")
9999
print("Running the tidy task...")
100100
tidy(ctx)
101101
else:

0 commit comments

Comments
 (0)