From b4b9bdd95f09cf77687807824824acfcbced36b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjam=C3=ADn=20Guzm=C3=A1n?= Date: Wed, 2 Sep 2026 11:38:34 -0600 Subject: [PATCH] feat(ebuild): add runner args support for flash tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allows developers to pass extra arguments to underlying flash tools (like OpenOCD, esptool, pyOCD) without modifying the ebuild source. Runner arguments are resolved using the following precedence chain: 1. CLI passthrough 2. Environment variable (EBUILD_FLASH_RUNNER_ARGS) 3. Project configuration Signed-off-by: Benjamín Guzmán --- CHANGELOG.md | 7 + ebuild/cli/commands.py | 122 ++++++++++++- ebuild/core/config.py | 9 + ebuild/firmware/flash.py | 16 +- tests/unit/cli/__init__.py | 0 tests/unit/cli/test_commands_utils.py | 77 ++++++++ tests/unit/cli/test_flash_args.py | 243 ++++++++++++++++++++++++++ 7 files changed, 462 insertions(+), 12 deletions(-) create mode 100644 tests/unit/cli/__init__.py create mode 100644 tests/unit/cli/test_commands_utils.py create mode 100644 tests/unit/cli/test_flash_args.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ba01756..79011088 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,6 +122,13 @@ notably the CLI's `except RuntimeError`, which turns this into a clean `exit 1` rather than a traceback. New code should catch `UnknownBackendError`. +- **Runner arguments can now override defaults via CLI, Environment, or Config.** + The `flash` command now resolves extra tool arguments following a strict + precedence chain. CLI passthrough (`--`) overrides the + `EBUILD_FLASH_RUNNER_ARGS` environment variable, which in turn overrides the + `runner_args` list in the `flash:` section of `build.yaml`. This enables + developers to instantly customize underlying tools (like OpenOCD or ESPTool) + without requiring new native `ebuild` flags (`ebuild/cli/commands.py`). ## [3.0.1] - 2026-05-16 diff --git a/ebuild/cli/commands.py b/ebuild/cli/commands.py index de8928c8..b754f75a 100644 --- a/ebuild/cli/commands.py +++ b/ebuild/cli/commands.py @@ -12,12 +12,13 @@ import glob import os import re +import shlex import shutil import subprocess import threading import sys from pathlib import Path -from typing import Any, Dict, List, Optional, Tuple, TYPE_CHECKING +from typing import Any, Callable, Dict, List, Optional, Tuple, TYPE_CHECKING if TYPE_CHECKING: from ebuild.eos_ai.eos_hw_analyzer import HardwareProfile @@ -51,6 +52,80 @@ # Canonical recipe search path discovery _find_recipe_dirs = find_recipe_dirs +# yaml key for the runner args config +_RUNNER_ARGS_CONFIG_KEY = "runner_args" + + +def _resolve_runner_args( + cli_args: tuple, + env_key: str, + cfg_loader: "Callable[[], ProjectConfig]", + config_section: str, + config_key: str, + log: "Logger", + clear_args: bool = False, +) -> list[str]: + """Resolve tool arguments following a precedence chain: CLI > Environment > Configuration File. + + This function short-circuits to avoid unnecessary config file reads if CLI or environment + arguments are present. + + Args: + cli_args: Tuple of unparsed CLI arguments captured by Click. + env_key: The environment variable name to check for arguments. + cfg_loader: A callable that returns the parsed ProjectConfig. + config_section: The section name within the parsed ProjectConfig (e.g. 'flash_config'). + config_key: The key within the section mapping that holds the arguments list. + log: Logger instance for recording debug or warning messages. + clear_args: If true, explicitly clear the arguments, ignoring config and environment. + + Returns: + A list of string arguments to pass to the underlying tool. Returns empty list if none found. + """ + if clear_args: + log.debug("Runner args explicitly cleared via flag") + return [] + + # CLI args (highest precedence) + if cli_args: + args_list = list(cli_args) + log.debug(f"Loaded runner args from CLI: {args_list}") + return args_list + + # Environment variable + env_args = os.environ.get(env_key) + if env_args and env_args.strip(): + args_list = shlex.split(env_args.strip()) + log.debug(f"Loaded runner args from environment ({env_key}): {args_list}") + return args_list + + # Config file + try: + cfg = cfg_loader() + section = getattr(cfg, config_section, {}) + if not isinstance(section, dict): + section = {} + + config_args = section.get(config_key, []) + if isinstance(config_args, list): + args_list = [str(a) for a in config_args] + if args_list: + log.debug(f"Loaded runner args from config: {args_list}") + return args_list + elif isinstance(config_args, str): + args_list = shlex.split(config_args) + if args_list: + log.debug(f"Loaded runner args from config: {args_list}") + return args_list + else: + log.warning(f"Invalid format for {config_key} in config file: must be a list or string") + return [] + except FileNotFoundError: + pass # config is optional + except Exception as e: + log.warning(f"Failed to load config: {e}") + + return [] def _install_packages( @@ -1572,21 +1647,42 @@ def firmware(log: Logger, config_path: str, build_dir: str, rtos: str, board: st raise SystemExit(1) -@cli.command() +@cli.command(context_settings=dict(ignore_unknown_options=True)) @click.argument("image", type=click.Path(exists=True)) @click.option("--tool", default="openocd", type=click.Choice(["openocd", "pyocd", "nrfjprog", "esptool", "stflash"]), help="Flash tool to use.") @click.option("--target", default="stm32f4", help="Target MCU/board.") @click.option("--address", default="0x08000000", help="Flash base address (hex).") -@click.option("--reset-after", is_flag=True, default=False, help="Reset target after flashing.") +@click.option( + "--reset-after", is_flag=True, default=False, help="Reset target after flashing." +) +@click.option( + "--config", + "config_path", + default="build.yaml", + type=click.Path(), + help="Path to build config.", +) +@click.option( + "--no-runner-args", + is_flag=True, + default=False, + help="Clear runner args (overrides config and environment).", +) +@click.argument("cli_args", nargs=-1, type=click.UNPROCESSED) @click.pass_obj def flash(log: Logger, image: str, tool: str, target: str, address: str, - reset_after: bool) -> None: + reset_after: bool, config_path: str, no_runner_args: bool, cli_args: tuple) -> None: """Flash a firmware image to the target device. Supports OpenOCD, pyOCD, nrfjprog, esptool, and st-flash. + Underlying tool arguments (runner args) can be set by, in order of precedence, + extra args on the CLI (unrecognized options will be passed as runner args), + by setting the EBUILD_FLASH_RUNNER_ARGS environment variable, + or by defining a `runner_args` list in the `flash` section of build.yaml. + Examples: ebuild flash firmware.bin --tool openocd --target stm32f4 @@ -1596,6 +1692,8 @@ def flash(log: Logger, image: str, tool: str, target: str, address: str, ebuild flash firmware.bin --tool esptool --address 0x10000 ebuild flash firmware.bin --tool pyocd --target nrf52840 --reset-after + + ebuild flash firmware.bin --tool esptool -- --port /dev/ttyUSB0 """ log.header("ebuild — Flash") @@ -1605,10 +1703,24 @@ def flash(log: Logger, image: str, tool: str, target: str, address: str, image_path = Path(image) addr = int(address, 0) + runner_args = _resolve_runner_args( + cli_args, + "EBUILD_FLASH_RUNNER_ARGS", + lambda: load_config(config_path), + "flash_config", + _RUNNER_ARGS_CONFIG_KEY, + log, + clear_args=no_runner_args, + ) + log.step(f"Flashing {image_path.name} to {target} via {tool}...") log.info(f" Address: {hex(addr)}") + if runner_args: + log.info(f" Runner args: {' '.join(runner_args)}") - do_flash(image_path, tool=tool, target=target, address=addr) + do_flash( + image_path, tool=tool, target=target, address=addr, extra_args=runner_args + ) log.success(f"Flash complete: {image_path.name}") if reset_after: diff --git a/ebuild/core/config.py b/ebuild/core/config.py index e28ff15a..e2e6aa64 100644 --- a/ebuild/core/config.py +++ b/ebuild/core/config.py @@ -103,6 +103,7 @@ class ProjectConfig: backend: str = "auto" backend_config: Dict[str, Any] = field(default_factory=dict) system_config: Dict[str, Any] = field(default_factory=dict) + flash_config: Dict[str, Any] = field(default_factory=dict) def get_target(self, name: str) -> Optional[TargetConfig]: for t in self.targets: @@ -260,6 +261,13 @@ def load_config(config_path: str | Path) -> ProjectConfig: raise ConfigError("'system' must be a mapping.") system_config = dict(system_config) + flash_config = raw.get("flash", {}) + if flash_config is None: + flash_config = {} + if not isinstance(flash_config, dict): + raise ConfigError("'flash' must be a mapping.") + flash_config = dict(flash_config) + # For cmake/make/meson builds, pull defines from config if raw.get("cmake") and isinstance(raw["cmake"], dict): backend_config.update(raw["cmake"]) @@ -345,4 +353,5 @@ def load_config(config_path: str | Path) -> ProjectConfig: # every consumer saw an empty mapping and the whole [system] # section was silently inert. system_config=system_config, + flash_config=flash_config, ) diff --git a/ebuild/firmware/flash.py b/ebuild/firmware/flash.py index b2a26ce8..f1f11dd0 100644 --- a/ebuild/firmware/flash.py +++ b/ebuild/firmware/flash.py @@ -22,8 +22,8 @@ class FlashError(Exception): "openocd": ["openocd", "-f", "interface/stlink.cfg"], "pyocd": ["pyocd", "flash"], "nrfjprog": ["nrfjprog", "--program"], - "esptool": ["esptool.py", "--chip", "esp32", "write_flash"], - "stflash": ["st-flash", "write"], + "esptool": ["esptool.py", "--chip", "esp32"], + "stflash": ["st-flash"], } @@ -53,6 +53,7 @@ def flash( if tool == "openocd": cmd.extend(["-f", f"target/{target}.cfg"]) + cmd.extend(extra_args or []) image_str = str(image_path) if image_str.count("{") != image_str.count("}"): raise FlashError( @@ -68,15 +69,16 @@ def flash( cmd.extend(["-c", f"program {{{image_path}}} {hex(address)} verify reset exit"]) elif tool == "pyocd": cmd.extend([str(image_path), "--target", target, "--base-address", hex(address)]) + cmd.extend(extra_args or []) elif tool == "nrfjprog": cmd.extend([str(image_path), "--sectorerase", "--verify"]) + cmd.extend(extra_args or []) elif tool == "esptool": - cmd.extend([hex(address), str(image_path)]) + cmd.extend(extra_args or []) + cmd.extend(["write_flash", hex(address), str(image_path)]) elif tool == "stflash": - cmd.extend([str(image_path), hex(address)]) - - if extra_args: - cmd.extend(extra_args) + cmd.extend(extra_args or []) + cmd.extend(["write", str(image_path), hex(address)]) result = subprocess.run(cmd, capture_output=True) if result.returncode != 0: diff --git a/tests/unit/cli/__init__.py b/tests/unit/cli/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/cli/test_commands_utils.py b/tests/unit/cli/test_commands_utils.py new file mode 100644 index 00000000..9ce8dabb --- /dev/null +++ b/tests/unit/cli/test_commands_utils.py @@ -0,0 +1,77 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +import pytest +from ebuild.cli.commands import _resolve_runner_args +from ebuild.core.config import load_config + +class DummyLogger: + def debug(self, msg): pass + def warning(self, msg): pass + def info(self, msg): pass + def error(self, msg): pass + +def create_loader(config_path): + return lambda: load_config(config_path) + +@pytest.mark.parametrize( + "config_val, env_val, cli_val, clear_args_val, expected", + [ + # CLI priority short-circuits config + (None, "--env-val", ("--cli-arg",), False, ["--cli-arg"]), + # Env priority + (None, "--env-val", (), False, ["--env-val"]), + # Config priority + (["--config-arg"], None, (), False, ["--config-arg"]), + # None matches + (None, None, (), False, []), + # Valid config string parsing + ("--adapter jlink", None, (), False, ["--adapter", "jlink"]), + # Invalid config warns and falls back to empty + (12345, None, (), False, []), + # Explicit clear overrides config and env + (["--config-arg"], "--env-val", (), True, []), + ] +) +def test_resolve_runner_args_unit(tmp_path, monkeypatch, config_val, env_val, cli_val, clear_args_val, expected): + """Directly unit test the _resolve_runner_args function combinations.""" + env_key = "TEST_ENV_KEY" + if env_val is not None: + monkeypatch.setenv(env_key, env_val) + else: + monkeypatch.delenv(env_key, raising=False) + + config_file = tmp_path / "build.yaml" + if config_val is not None: + if isinstance(config_val, list): + args_str = ", ".join(f'"{a}"' for a in config_val) + config_file.write_text(f'project:\n name: test\nsystem:\n test_args: [{args_str}]\n', encoding="utf-8") + else: + config_file.write_text(f'project:\n name: test\nsystem:\n test_args: {config_val}\n', encoding="utf-8") + + res = _resolve_runner_args( + cli_args=cli_val, + env_key=env_key, + cfg_loader=create_loader(str(config_file)), + config_section="system_config", + config_key="test_args", + log=DummyLogger(), + clear_args=clear_args_val, + ) + assert res == expected + +def test_resolve_runner_args_null_safety(): + """Verify null safety with empty or non-dict sections.""" + class FakeConfig: + system_config = None # Explicitly None to test null safety + + res = _resolve_runner_args( + cli_args=(), + env_key="TEST_ENV_KEY", + cfg_loader=lambda: FakeConfig(), + config_section="system_config", + config_key="test_args", + log=DummyLogger() + ) + assert res == [] + diff --git a/tests/unit/cli/test_flash_args.py b/tests/unit/cli/test_flash_args.py new file mode 100644 index 00000000..0457c667 --- /dev/null +++ b/tests/unit/cli/test_flash_args.py @@ -0,0 +1,243 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +from click.testing import CliRunner +import pytest +import subprocess + +from ebuild.cli.commands import flash +from ebuild.core.config import load_config +import ebuild.firmware.flash + + +class DummyLogger: + def header(self, msg): + pass + + def step(self, msg): + pass + + def info(self, msg): + pass + + def success(self, msg): + pass + + def error(self, msg): + pass + + def warning(self, msg): + pass + + def debug(self, msg): + pass + + +def test_flash_config_parsing(tmp_path): + """Test that load_config correctly parses the flash section.""" + config_file = tmp_path / "build.yaml" + config_file.write_text( + """ +project: + name: test +flash: + runner_args: ["--adapter", "jlink"] +""", + encoding="utf-8", + ) + + cfg = load_config(config_file) + assert cfg.flash_config.get("runner_args") == ["--adapter", "jlink"] + + +@pytest.mark.parametrize( + "config_args, env_args, cli_args, expected", + [ + # runner args set only by config file + (["--config-file-arg"], None, [], ["--config-file-arg"]), + # runner args set only by env variable + ([], "--env-arg", [], ["--env-arg"]), + # runner args set only by CLI (with --) + ([], None, ["--", "--cli-arg"], ["--cli-arg"]), + # runner args set only by CLI (without --) + ([], None, ["--cli-arg"], ["--cli-arg"]), + # runner args set from env variable override config file + (["--config-arg"], "--env-arg", [], ["--env-arg"]), + # runner args set from CLI override env var + ([], "--env-arg", ["--cli-arg"], ["--cli-arg"]), + # runner args set from CLI override config file + (["--config-arg"], None, ["--cli-arg"], ["--cli-arg"]), + # runner args set from CLI override both env var and config file + (["--config-arg"], "--env-arg", ["--cli-arg"], ["--cli-arg"]), + # runner args set from CLI (multiple args) + ([], None, ["--speed", "4000", "--reset"], + ["--speed", "4000", "--reset"]), + # explicitly clear args via CLI overrides both env var and config file + (["--config-arg"], "--env-arg", ["--no-runner-args"], []), + # no args at all + ([], None, [], None), + ], +) +def test_flash_args_combinations( + tmp_path, monkeypatch, config_args, env_args, cli_args, expected +): + """Test all combinations of config, env, and cli args precedence.""" + called_args = {} + + def mock_flash(*args, **kwargs): + called_args.update(kwargs) + + monkeypatch.setattr(ebuild.firmware.flash, "flash", mock_flash) + + image = tmp_path / "dummy.bin" + image.touch() + + # write the config file for testing + config_file = tmp_path / "build.yaml" + if config_args: + args_str = ", ".join(f'"{a}"' for a in config_args) + config_file.write_text( + f""" +project: + name: test +flash: + runner_args: [{args_str}] +""", + encoding="utf-8", + ) + else: + config_file.write_text("project:\n name: test\n", encoding="utf-8") + + # set the env variable + env = {} + if env_args is not None: + env["EBUILD_FLASH_RUNNER_ARGS"] = env_args + + # run flash command with env, config and cli args defined in the test case + runner = CliRunner() + invoke_args = [str(image), "--config", str(config_file)] + cli_args + + result = runner.invoke(flash, invoke_args, env=env, obj=DummyLogger()) + assert result.exit_code == 0, result.output + + if expected is None: + assert not called_args.get("extra_args") + else: + assert called_args.get("extra_args") == expected + + +@pytest.mark.parametrize( + "tool, extra_args, expected_cmd", + [ + ( + "openocd", + ["-c", "adapter speed 4000"], + [ + "openocd", + "-f", "interface/stlink.cfg", + "-f", "target/stm32f4.cfg", + "-c", "adapter speed 4000", + "-c", "program {{{image_path}}} 0x8000000 verify reset exit", + ], + ), + ( + "esptool", + ["--port", "/dev/ttyUSB0", "--baud", "921600"], + [ + "esptool.py", + "--chip", "esp32", + "--port", "/dev/ttyUSB0", + "--baud", "921600", + "write_flash", "0x8000000", "{image_path}", + ], + ), + ( + "pyocd", + ["--erase", "chip"], + [ + "pyocd", + "flash", "{image_path}", + "--target", "stm32f4", + "--base-address", "0x8000000", + "--erase", "chip", + ], + ), + ( + "nrfjprog", + ["--reset", "--log"], + [ + "nrfjprog", + "--program", "{image_path}", + "--sectorerase", + "--verify", + "--reset", + "--log", + ], + ), + ( + "stflash", + ["--reset", "--serial", "1234"], + [ + "st-flash", + "--reset", + "--serial", "1234", + "write", "{image_path}", "0x8000000", + ], + ), + # Test empty extra_args + ( + "openocd", + [], + [ + "openocd", + "-f", "interface/stlink.cfg", + "-f", "target/stm32f4.cfg", + "-c", "program {{{image_path}}} 0x8000000 verify reset exit", + ], + ), + ], +) +def test_flash_extra_args_positioning_via_cli( + tmp_path, monkeypatch, tool, extra_args, expected_cmd +): + """ + Test that extra_args are correctly positioned + when invoking the full CLI flash command. + """ + image_path = tmp_path / "dummy.bin" + image_path.touch() + + # we need a dummy build.yaml to prevent the load_config step from failing + config_file = tmp_path / "build.yaml" + config_file.write_text("project:\n name: test\n", encoding="utf-8") + + captured_cmd = [] + + def mock_subprocess_run(cmd, *args, **kwargs): + captured_cmd.extend(cmd) + return subprocess.CompletedProcess( + args=cmd, returncode=0, stdout=b"", stderr=b"" + ) + + monkeypatch.setattr(subprocess, "run", mock_subprocess_run) + + runner = CliRunner() + invoke_args = [ + str(image_path), + "--config", str(config_file), + "--tool", tool, + "--target", "stm32f4", + "--address", "0x08000000", + ] + if extra_args: + invoke_args.append("--") + invoke_args.extend(extra_args) + + result = runner.invoke(flash, invoke_args, obj=DummyLogger()) + assert result.exit_code == 0, result.output + + expected_cmd = [ + arg.format(image_path=str(image_path)) for arg in expected_cmd + ] + + assert captured_cmd == expected_cmd