Avoid arg limits for internal uses of Bazel (#190)

* Avoid arg limits for internal uses of Bazel

* address review

Author: ofek

CHANGELOG.md

@@ -8,19 +8,27 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ## Unreleased
 
+***Changed:***
+
+- Rename `build bazel` command to `bzl`
+
+***Added:***
+
+- Methods on the `app.tools.bazel` tool now prevent exceeding platform-specific command line length limits
+
 ## 0.27.0 - 2025-09-22
 
 ***Changed:***
 
-- Rename the `[git.user]` section to `[tools.git.author]`
+- Rename the `[git.user]` config section to `[tools.git.author]`
 - The `Tool` interface now uses a single execution context instead of the dedicated methods `format_command` and `env_vars`
-- Updated Atlassian API to 4.0.7
 
 ***Added:***
 
 - Add `git` tool
 - Add `[user]` section to the configuration
 - Add abstract context manager `execution_context` method to the `Tool` interface
+- Updated Atlassian API to 4.0.7
 
 ## 0.26.0 - 2025-09-09
 

docs/reference/api/tools.md

@@ -5,23 +5,28 @@
 ::: dda.tools.Tools
     options:
       members:
+      - bazel
       - docker
+      - git
       - go
       - uv
-      - git
+
+::: dda.tools.bazel.Bazel
+    options:
+      members: []
 
 ::: dda.tools.docker.Docker
     options:
       members: []
 
-::: dda.tools.go.Go
+::: dda.tools.git.Git
     options:
       members: []
 
-::: dda.tools.uv.UV
+::: dda.tools.go.Go
     options:
       members: []
 
-::: dda.tools.git.Git
+::: dda.tools.uv.UV
     options:
       members: []

src/dda/cli/base.py

@@ -502,6 +502,16 @@ def ensure_features_installed(
         app.tools.uv.wait(command, message="Synchronizing dependencies", cwd=str(temp_dir), env=env_vars)
 
 
+def get_raw_args(ctx: click.Context) -> list[str]:
+    level = len(ctx.command_path.split())
+    return _get_argv()[level:]
+
+
+def _get_argv() -> list[str]:
+    # This is used for test assertions only
+    return sys.argv
+
+
 def _get_root_ctx(ctx: click.Context) -> DynamicContext:
     while ctx.parent is not None:
         ctx = ctx.parent

src/dda/cli/build/bazel/__init__.py renamed to src/dda/cli/bzl/__init__.py

@@ -7,18 +7,20 @@
 
 import click
 
-from dda.cli.base import dynamic_command, pass_app
+from dda.cli.base import dynamic_command, get_raw_args
 
 if TYPE_CHECKING:
     from dda.cli.application import Application
 
 
 @dynamic_command(
     short_help="Run Bazel commands",
-    context_settings={"help_option_names": [], "ignore_unknown_options": True},
+    context_settings={"help_option_names": [], "ignore_unknown_options": True, "allow_extra_args": True},
 )
-@click.argument("args", nargs=-1)
-@pass_app
-def cmd(app: Application, *, args: tuple[str, ...]) -> None:
-    process = app.tools.bazel.attach(list(args), check=False)
+@click.pass_context
+def cmd(ctx: click.Context) -> None:
+    app: Application = ctx.obj
+    with app.tools.bazel.ignore_arg_limits():
+        process = app.tools.bazel.attach(get_raw_args(ctx), check=False)
+
     app.abort(code=process.returncode)

src/dda/tools/bazel.py

@@ -4,8 +4,8 @@
 from __future__ import annotations
 
 from contextlib import contextmanager
-from functools import cached_property
-from typing import TYPE_CHECKING
+from functools import cache, cached_property
+from typing import TYPE_CHECKING, Any
 
 from dda.tools.base import ExecutionContext, Tool
 from dda.utils.platform import PLATFORM_ID, which
@@ -28,9 +28,52 @@ class Bazel(Tool):
     to an internal location if `bazel` nor `bazelisk` are already on PATH.
     """
 
+    def __init__(self, *args: Any, **kwargs: Any) -> None:
+        super().__init__(*args, **kwargs)
+
+        # Avoid platform-specific command line length limits by default
+        self.__ignore_arg_limits = False
+
     @contextmanager
     def execution_context(self, command: list[str]) -> Generator[ExecutionContext, None, None]:
-        yield ExecutionContext(command=[self.path, *command], env_vars={})
+        first_arg: str | None = None
+        for arg in command:
+            if not arg.startswith("-"):
+                first_arg = arg
+                break
+
+        if first_arg is None:
+            yield ExecutionContext(command=[self.path, *command], env_vars={})
+            return
+
+        try:
+            sep_index = command.index("--")
+        except ValueError:
+            if self.__ignore_arg_limits:
+                yield ExecutionContext(command=[self.path, *command], env_vars={})
+                return
+
+            msg = "Bazel arguments must come after the `--` separator"
+            raise ValueError(msg) from None
+
+        if first_arg in self.target_accepting_commands:
+            arg_file_flag = "--target_pattern_file"
+        elif first_arg in self.query_accepting_commands:
+            arg_file_flag = "--query_file"
+        else:
+            yield ExecutionContext(command=[self.path, *command], env_vars={})
+            return
+
+        from tempfile import NamedTemporaryFile
+
+        with NamedTemporaryFile(mode="w", encoding="utf-8") as f:
+            f.write("\n".join(command[sep_index + 1 :]))
+            f.flush()
+
+            yield ExecutionContext(
+                command=[self.path, *command[:sep_index], arg_file_flag, f.name],
+                env_vars={},
+            )
 
     @property
     def managed(self) -> bool:
@@ -67,6 +110,22 @@ def __external_path(self) -> str | None:
 
         return None
 
+    @property
+    def target_accepting_commands(self) -> frozenset[str]:
+        return target_accepting_commands()
+
+    @property
+    def query_accepting_commands(self) -> frozenset[str]:
+        return query_accepting_commands()
+
+    @contextmanager
+    def ignore_arg_limits(self) -> Generator[None, None, None]:
+        self.__ignore_arg_limits = True
+        try:
+            yield
+        finally:
+            self.__ignore_arg_limits = False
+
 
 def get_download_url() -> str:
     import platform
@@ -80,3 +139,13 @@ def get_download_url() -> str:
 
     url = f"https://github.com/bazelbuild/bazelisk/releases/latest/download/bazelisk-{system}-{arch}"
     return f"{url}.exe" if PLATFORM_ID == "windows" else url
+
+
+@cache
+def target_accepting_commands() -> frozenset[str]:
+    return frozenset({"build", "coverage", "fetch", "run", "test"})
+
+
+@cache
+def query_accepting_commands() -> frozenset[str]:
+    return frozenset({"aquery", "cquery", "query"})

tests/cli/build/__init__.py renamed to tests/cli/bzl/__init__.py

@@ -10,11 +10,13 @@
 
 
 def test_default_download(dda, helpers, isolation, mocker):
+    args = ["build", "//..."]
+    mocker.patch("dda.cli.base._get_argv", return_value=["dda", "bzl", *args])
     downloader = mocker.patch("dda.utils.network.http.manager.HTTPClientManager.download")
     subprocess_runner = mocker.patch("subprocess.run", return_value=subprocess.CompletedProcess(args=[], returncode=0))
 
     with EnvVars(exclude=["PATH"]):
-        result = dda("build", "bazel", "build", "//...")
+        result = dda("bzl", *args)
 
     assert result.exit_code == 0, result.output
     assert result.output == helpers.dedent(
@@ -28,24 +30,26 @@ def test_default_download(dda, helpers, isolation, mocker):
         get_download_url(),
         path=internal_bazel_path,
     )
-    subprocess_runner.assert_called_once_with([str(internal_bazel_path), "build", "//..."])
+    subprocess_runner.assert_called_once_with([str(internal_bazel_path), *args])
 
 
 def test_default_exists(dda, helpers, temp_dir, mocker):
     external_bazel_path = temp_dir.joinpath("bazel").as_exe()
     helpers.create_binary(external_bazel_path)
 
+    args = ["build", "//..."]
+    mocker.patch("dda.cli.base._get_argv", return_value=["dda", "bzl", *args])
     downloader = mocker.patch("dda.utils.network.http.manager.HTTPClientManager.download")
     subprocess_runner = mocker.patch("subprocess.run", return_value=subprocess.CompletedProcess(args=[], returncode=0))
 
     with EnvVars({"PATH": str(temp_dir)}):
-        result = dda("build", "bazel", "build", "//...")
+        result = dda("bzl", *args)
 
     assert result.exit_code == 0, result.output
     assert not result.output
 
     downloader.assert_not_called()
-    subprocess_runner.assert_called_once_with([str(external_bazel_path), "build", "//..."])
+    subprocess_runner.assert_called_once_with([str(external_bazel_path), *args])
 
 
 def test_config_force_managed(dda, helpers, isolation, config_file, temp_dir, mocker):
@@ -55,11 +59,13 @@ def test_config_force_managed(dda, helpers, isolation, config_file, temp_dir, mo
     external_bazel_path = temp_dir.joinpath("bazel").as_exe()
     helpers.create_binary(external_bazel_path)
 
+    args = ["build", "//..."]
+    mocker.patch("dda.cli.base._get_argv", return_value=["dda", "bzl", *args])
     downloader = mocker.patch("dda.utils.network.http.manager.HTTPClientManager.download")
     subprocess_runner = mocker.patch("subprocess.run", return_value=subprocess.CompletedProcess(args=[], returncode=0))
 
     with EnvVars({"PATH": str(temp_dir)}):
-        result = dda("build", "bazel", "build", "//...")
+        result = dda("bzl", *args)
 
     assert result.exit_code == 0, result.output
     assert result.output == helpers.dedent(
@@ -73,17 +79,19 @@ def test_config_force_managed(dda, helpers, isolation, config_file, temp_dir, mo
         get_download_url(),
         path=internal_bazel_path,
     )
-    subprocess_runner.assert_called_once_with([str(internal_bazel_path), "build", "//..."])
+    subprocess_runner.assert_called_once_with([str(internal_bazel_path), *args])
 
 
 def test_config_force_unmanaged(dda, helpers, config_file, mocker):
     config_file.data["tools"]["bazel"]["managed"] = False
     config_file.save()
 
+    args = ["build", "//..."]
+    mocker.patch("dda.cli.base._get_argv", return_value=["dda", "bzl", *args])
     downloader = mocker.patch("dda.utils.network.http.manager.HTTPClientManager.download")
 
     with EnvVars(exclude=["PATH"]):
-        result = dda("build", "bazel", "build", "//...")
+        result = dda("bzl", *args)
 
     assert result.exit_code == 1, result.output
     assert result.output == helpers.dedent(
@@ -93,3 +101,20 @@ def test_config_force_unmanaged(dda, helpers, config_file, mocker):
     )
 
     downloader.assert_not_called()
+
+
+def test_arg_interception(dda, config_file, mocker):
+    config_file.data["tools"]["bazel"]["managed"] = False
+    config_file.save()
+
+    args = ["build", "--", "//..."]
+    mocker.patch("dda.cli.base._get_argv", return_value=["dda", "bzl", *args])
+    downloader = mocker.patch("dda.utils.network.http.manager.HTTPClientManager.download")
+
+    with EnvVars(exclude=["PATH"]):
+        result = dda("bzl", *args)
+
+    assert result.exit_code == 1, result.output
+    assert result.output.startswith("Executable `bazel` not found: ['bazel', 'build', '--target_pattern_file', '")
+
+    downloader.assert_not_called()

tests/cli/build/test_bazel.py renamed to tests/cli/bzl/test_bzl.py

@@ -20,7 +20,7 @@
 from dda.utils.ci import running_in_ci
 from dda.utils.fs import Path, temp_directory
 from dda.utils.git.constants import GitEnvVars
-from dda.utils.platform import PLATFORM_ID
+from dda.utils.platform import PLATFORM_ID, which
 from dda.utils.process import EnvVars
 
 if TYPE_CHECKING:
@@ -160,6 +160,11 @@ def uv_on_path() -> Path:
     return Path(shutil.which("uv"))
 
 
+@pytest.fixture(scope="session")
+def bazel_on_path() -> Path:
+    return Path(which("bazel"))
+
+
 def pytest_runtest_setup(item):
     for marker in item.iter_markers():
         if marker.name == "requires_ci" and not running_in_ci():  # no cov

tests/conftest.py

@@ -0,0 +1,56 @@
+# SPDX-FileCopyrightText: 2025-present Datadog, Inc. <dev@datadoghq.com>
+#
+# SPDX-License-Identifier: MIT
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+import pytest
+
+from dda.tools.bazel import query_accepting_commands, target_accepting_commands
+
+if TYPE_CHECKING:
+    from pytest_mock import MockerFixture
+
+    from dda.cli.application import Application
+    from dda.utils.fs import Path
+
+
+class TestArgLengthLimits:
+    @pytest.mark.parametrize("command", sorted(target_accepting_commands()))
+    def test_ambiguous_targets(self, app: Application, command: str) -> None:
+        with (
+            pytest.raises(ValueError, match="Bazel arguments must come after the `--` separator"),
+            app.tools.bazel.execution_context([command, "//..."]),
+        ):
+            pass
+
+    @pytest.mark.parametrize("command", sorted(query_accepting_commands()))
+    def test_ambiguous_query(self, app: Application, command: str) -> None:
+        with (
+            pytest.raises(ValueError, match="Bazel arguments must come after the `--` separator"),
+            app.tools.bazel.execution_context([command, "deps(//foo)"]),
+        ):
+            pass
+
+    def test_targets_arg_file(self, app: Application, bazel_on_path: Path, mocker: MockerFixture) -> None:
+        writer = mocker.MagicMock()
+        writer.name = "targets.txt"
+        mocker.patch(
+            "tempfile.NamedTemporaryFile",
+            return_value=mocker.MagicMock(__enter__=lambda *_, **__: writer, __exit__=lambda *_, **__: None),
+        )
+        with app.tools.bazel.execution_context(["build", "--foo", "--", "//foo", "//bar"]) as context:
+            assert context.command == [str(bazel_on_path), "build", "--foo", "--target_pattern_file", "targets.txt"]
+            writer.write.assert_called_once_with("//foo\n//bar")
+
+    def test_query_arg_file(self, app: Application, bazel_on_path: Path, mocker: MockerFixture) -> None:
+        writer = mocker.MagicMock()
+        writer.name = "query.txt"
+        mocker.patch(
+            "tempfile.NamedTemporaryFile",
+            return_value=mocker.MagicMock(__enter__=lambda *_, **__: writer, __exit__=lambda *_, **__: None),
+        )
+        with app.tools.bazel.execution_context(["query", "--foo", "--", "deps(//foo)"]) as context:
+            assert context.command == [str(bazel_on_path), "query", "--foo", "--query_file", "query.txt"]
+            writer.write.assert_called_once_with("deps(//foo)")