From d2ace0fd9560ca18c7b81655f3d255d2aa47f3ed Mon Sep 17 00:00:00 2001 From: agayushh Date: Mon, 21 Sep 2026 23:52:27 +0530 Subject: [PATCH] fix(ebuild): use ninja_command() in ebuild build ebuild test already preferred a PATH ninja binary and fell back to python -m ninja. ebuild build still hardcoded the module form, so a system ninja install was not enough for the main command. Fixes #159 Signed-off-by: agayushh --- CHANGELOG.md | 4 ++ README.md | 5 +- demo.md | 4 +- ebuild/cli/commands.py | 3 +- tests/unit/test_ninja_command.py | 91 ++++++++++++++++++++++++++++++++ 5 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_ninja_command.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ba01756..33c3935a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,10 @@ relative `--build-dir` now resolves against the directory containing `build.yaml`, as an absolute path, so both sides agree regardless of the working directory (`ebuild/cli/commands.py`). +- **`ebuild build` now uses `ninja_command()`.** `ebuild test` already preferred a + `ninja` binary on PATH and fell back to `python -m ninja`. `ebuild build` still + hardcoded the module form, so a system ninja install was not enough for the + main command (`ebuild/cli/commands.py`). ### Added - `ebuild.build.dispatch.UnknownBackendError`, raised for a backend a dispatch diff --git a/README.md b/README.md index 8b1f5dcb..3477c7ab 100644 --- a/README.md +++ b/README.md @@ -68,8 +68,9 @@ pip install -e . # from the repo root ``` Runtime dependencies (`click`, `pyyaml`, `ninja`) are installed automatically. -Note the `ninja` **pip package** is required — a system `ninja` binary alone is -not enough, because ebuild invokes `python -m ninja`. +ebuild prefers a `ninja` binary on PATH and falls back to `python -m ninja` +if none is present, so a system ninja install is enough. The pip `ninja` +package is the fallback when no binary is on PATH. ## Usage diff --git a/demo.md b/demo.md index 28ef189d..3e35dd3c 100644 --- a/demo.md +++ b/demo.md @@ -7,8 +7,8 @@ C program end-to-end via `ebuild build`. - Python 3.8+ - A system C compiler (`gcc` or `clang`) -- The `ninja` **pip package** — a system `ninja` binary alone is not enough, since - `ebuild` invokes `python -m ninja` internally. +- A `ninja` binary on PATH, or the `ninja` pip package as fallback — + `ebuild` prefers the binary and uses `python -m ninja` only if none is present. ## 1. Set up a virtual environment diff --git a/ebuild/cli/commands.py b/ebuild/cli/commands.py index de8928c8..1713af48 100644 --- a/ebuild/cli/commands.py +++ b/ebuild/cli/commands.py @@ -1029,7 +1029,8 @@ def build(log: Logger, config_path: str, build_dir: str, backend: Optional[str], log.success(f"Generated {_shown(build_path / 'compile_commands.json')}") log.step("Invoking ninja...") - ninja_cmd = [sys.executable, "-m", "ninja", "-f", str(build_path / "build.ninja")] + from ebuild.build.dispatch import ninja_command + ninja_cmd = ninja_command() + ["-f", str(build_path / "build.ninja")] if log.verbose: ninja_cmd.append("-v") diff --git a/tests/unit/test_ninja_command.py b/tests/unit/test_ninja_command.py new file mode 100644 index 00000000..d1994cdb --- /dev/null +++ b/tests/unit/test_ninja_command.py @@ -0,0 +1,91 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""`ebuild build` must use ninja_command(), not a hardcoded python -m ninja. + +ninja_command() prefers a ninja binary on PATH and falls back to the PyPI +module. ebuild test already uses it. ebuild build used to skip it, so a +system ninja install was not enough for the main command. +""" + +import subprocess +import sys +import textwrap + +import pytest +from click.testing import CliRunner + +from ebuild.build.dispatch import ninja_command +from ebuild.cli.commands import cli + + +@pytest.fixture +def project(tmp_path, monkeypatch): + """Minimal target project so `ebuild build` takes the Ninja backend path.""" + (tmp_path / "src").mkdir() + (tmp_path / "src" / "main.c").write_text("int main(void) { return 0; }\n") + (tmp_path / "build.yaml").write_text(textwrap.dedent("""\ + project: + name: demo + version: "1.0.0" + + targets: + - name: demo + type: executable + sources: ["src/main.c"] + + toolchain: + compiler: gcc + arch: x86_64 + """)) + monkeypatch.chdir(tmp_path) + return tmp_path + + +class TestNinjaCommand: + def test_prefers_ninja_on_path(self, monkeypatch): + monkeypatch.setattr("shutil.which", lambda name: "/usr/bin/ninja" if name == "ninja" else None) + assert ninja_command() == ["/usr/bin/ninja"] + + def test_falls_back_to_the_python_module(self, monkeypatch): + monkeypatch.setattr("shutil.which", lambda name: None) + assert ninja_command() == [sys.executable, "-m", "ninja"] + + +class TestBuildUsesNinjaCommand: + def test_build_uses_the_path_binary_when_present(self, project, monkeypatch): + """If this still started with sys.executable, build is on the old argv.""" + captured = {} + + def fake_run(cmd, *args, **kwargs): + captured["cmd"] = list(cmd) + return subprocess.CompletedProcess(cmd, returncode=0, stdout=b"", stderr=b"") + + monkeypatch.setattr("ebuild.cli.commands.subprocess.run", fake_run) + monkeypatch.setattr( + "shutil.which", + lambda name: "/opt/ninja" if name == "ninja" else None, + ) + + result = CliRunner().invoke(cli, ["build"], catch_exceptions=False) + assert result.exit_code == 0, result.output + cmd = captured["cmd"] + assert cmd[0] == "/opt/ninja" + assert "-f" in cmd + assert str(project / "_build" / "build.ninja") in cmd + + def test_build_falls_back_to_the_module_when_path_is_empty(self, project, monkeypatch): + captured = {} + + def fake_run(cmd, *args, **kwargs): + captured["cmd"] = list(cmd) + return subprocess.CompletedProcess(cmd, returncode=0, stdout=b"", stderr=b"") + + monkeypatch.setattr("ebuild.cli.commands.subprocess.run", fake_run) + monkeypatch.setattr("shutil.which", lambda name: None) + + result = CliRunner().invoke(cli, ["build"], catch_exceptions=False) + assert result.exit_code == 0, result.output + cmd = captured["cmd"] + assert cmd[:3] == [sys.executable, "-m", "ninja"] + assert "-f" in cmd