From 1e8c6fb21068cb5772875ff56c3b36424c469e86 Mon Sep 17 00:00:00 2001 From: Prakhar Maheshwari Date: Sun, 30 Aug 2026 17:28:33 +0530 Subject: [PATCH] fix(ebuild): repair SyntaxError in dispatch.py and unify backend errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem `ebuild/build/dispatch.py` does not parse. `BackendDispatcher.configure()` carries two `else:` clauses attached to the same `if` chain: File "ebuild/build/dispatch.py", line 133 else: ^^^^ SyntaxError: invalid syntax Every `ebuild` command that reaches the dispatcher imports this module, so on master `ebuild build`, `ebuild configure` and `ebuild clean` all fail at import, and pytest aborts collection with 2 errors before running anything. The cause is a merge, not a typo. Two branches independently added an unhandled-backend guard to `configure()` — one raising `ValueError` listing `ALL_BACKENDS`, one raising `RuntimeError` with a ninja-specific hint. The merge kept both bodies. Their two test suites survived as well and disagree: `tests/ebuild/test_dispatch.py` expects `ValueError`, `tests/unit/test_dispatch.py` expects `RuntimeError`. Two further defects were preserved in the same chain: - `configure()` listed `ninja` alongside make/kbuild as a silent no-op. `ebuild build` routes `backend: ninja` with no `targets` into the dispatcher (`commands.py`: `if resolved_backend != "ninja" or not cfg.targets`), which has no ninja configure step. Returning quietly there is the exact false-success the `RuntimeError` guard was written to stop. - The `ValueError` message advertised `ALL_BACKENDS`, which contains `ninja`, so rejecting `ninja` produced a message naming `ninja` as supported. ## Fix - Remove the duplicated `else:` and consolidate the two guards into one. - Add `UnknownBackendError(ValueError, RuntimeError)`. Deriving from both keeps each pre-existing caller contract intact rather than silently dropping one, and it is load-bearing beyond the tests: `ebuild build` handles this through `except RuntimeError`, so a plain `ValueError` would reach the user as a traceback instead of a clean `exit 1`. - Name the backends each step actually handles (`CONFIGURE_BACKENDS`, `BUILD_BACKENDS`, `CLEAN_BACKENDS`) and report those in the message, so it can no longer contradict the rejection. `clean` keeps `ninja`; it only removes a build directory and needs no toolchain. - Drop `ninja` from the `configure()` no-op branch so it raises, carrying the actionable "requires 'targets' in build.yaml" hint. Behaviour change: `BackendDispatcher.configure("ninja")` previously returned None and now raises. Nothing in-tree relies on the old behaviour — the CLI `configure` command routes ninja to `_configure_ninja_backend` before the dispatcher is constructed. ## Testing - `pytest tests/ebuild/test_dispatch.py tests/unit/test_dispatch.py` — 31 passed. Both previously-conflicting suites pass unmodified. - Full suite: 196 passed, 1 skipped, 11 failed. All 11 failures are pre-existing and unrelated (`_object_path` missing from `NinjaBackend`, initramfs integration, build-failure output); they were present before this change and are unaffected by it. - End-to-end, a `build.yaml` with `backend: ninja` and no `targets`: [error] Unknown build backend 'ninja'. BackendDispatcher can configure: cargo, cmake, kbuild, make, meson. ebuild's own ninja backend is invoked directly rather than through BackendDispatcher, and requires 'targets' in build.yaml -- add targets or choose another backend. EXIT=1 Clean message, correct exit code, no traceback. - Six regression tests added covering the dual-inheritance contract, the ninja rejection, the actionable hint, the non-contradictory backend list, `clean("ninja")` still working, and cargo/make/kbuild remaining no-ops. Signed-off-by: Prakhar Maheshwari Signed-off-by: Prakhar Maheshwari --- CHANGELOG.md | 23 ++++++++++ ebuild/build/dispatch.py | 79 ++++++++++++++++++++++------------- tests/ebuild/test_dispatch.py | 66 +++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e52b6a89..59f6dde3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,21 @@ ## [Unreleased] ### Fixed +- **`ebuild.build.dispatch` was unimportable.** Two branches independently added + an unhandled-backend `else` clause to `BackendDispatcher.configure()`; the + merge kept both, leaving a second `else` after the first and a `SyntaxError` + that broke every command importing the module. The duplicate is removed and + the two clauses are consolidated into one + (`ebuild/build/dispatch.py`). +- **`configure(backend="ninja")` no longer silently succeeds.** `ebuild build` + routes `backend: ninja` with no `targets` into the dispatcher, which has no + ninja configure step; the no-op let the CLI report success having built + nothing. It now raises with a message naming the missing `targets` + (`ebuild/build/dispatch.py`). +- **Unhandled-backend errors no longer contradict themselves.** The message + listed `ALL_BACKENDS` as supported, which includes `ninja` — the very backend + being rejected. Each step now reports only the backends it handles + (`ebuild/build/dispatch.py`). - **Ninja backend: header changes now trigger a rebuild.** The generated `cc` rule declared no depfile, so Ninja only knew about the sources listed in `build.yaml`. Editing a header left stale object files in place and the build @@ -10,6 +25,14 @@ `depfile`/`deps`, so Ninja tracks the real include graph (`ebuild/build/ninja_backend.py`). +### Added +- `ebuild.build.dispatch.UnknownBackendError`, raised for a backend a dispatch + step does not handle. It derives from both `ValueError` and `RuntimeError` + because the clauses it replaces raised one each and callers depend on both — + notably the CLI's `except RuntimeError`, which turns this into a clean + `exit 1` rather than a traceback. New code should catch + `UnknownBackendError`. + ## [3.0.1] - 2026-05-16 ### Production Release — Unified EmbeddedOS-org v3.0.1 diff --git a/ebuild/build/dispatch.py b/ebuild/build/dispatch.py index 0d708dfa..ddbe7699 100644 --- a/ebuild/build/dispatch.py +++ b/ebuild/build/dispatch.py @@ -25,6 +25,48 @@ ALL_BACKENDS = {"cmake", "make", "meson", "cargo", "kbuild", "ninja"} +#: Backends ``BackendDispatcher`` accepts, per step. ``ninja`` is absent from +#: configure/build on purpose: ebuild's own Ninja backend is driven directly by +#: the CLI (see ``NinjaBackend``), never through this dispatcher. ``clean`` +#: still accepts it because removing a stale ``_build/`` needs no toolchain. +CONFIGURE_BACKENDS = {"cmake", "meson", "cargo", "make", "kbuild"} + +BUILD_BACKENDS = {"cmake", "make", "meson", "cargo", "kbuild"} + +CLEAN_BACKENDS = {"cmake", "make", "meson", "cargo", "kbuild", "ninja"} + + +class UnknownBackendError(ValueError, RuntimeError): + """Raised when a backend name is not handled by ``BackendDispatcher``. + + Inherits from both :class:`ValueError` and :class:`RuntimeError` because + the two behaviours this consolidates were introduced independently and + both are depended on: callers (and the CLI's ``except RuntimeError`` + handler, which turns this into a clean ``exit 1`` instead of a traceback) + may catch either. New code should catch ``UnknownBackendError``. + """ + + +def _unknown_backend(backend: str, action: str, supported: set) -> UnknownBackendError: + """Build the error raised for a backend a step cannot handle. + + Args: + backend: The rejected backend name. + action: Verb phrase naming the step, e.g. ``"configure"``. + supported: Backend names the step does accept. + """ + message = ( + f"Unknown build backend '{backend}'. " + f"BackendDispatcher can {action}: {', '.join(sorted(supported))}." + ) + if backend == "ninja": + message += ( + " ebuild's own ninja backend is invoked directly rather than " + "through BackendDispatcher, and requires 'targets' in build.yaml " + "-- add targets or choose another backend." + ) + return UnknownBackendError(message) + def detect_backend(source_dir: Path) -> str: """Auto-detect the build system from project files. @@ -100,7 +142,7 @@ def configure( dry_run: If True, log commands instead of executing them. Raises: - ValueError: If the backend is not recognized. + UnknownBackendError: If this step does not handle the backend. """ config = config or {} self.build_dir.mkdir(parents=True, exist_ok=True) @@ -118,26 +160,11 @@ def configure( cmd = ["meson", "setup", str(self.build_dir), str(self.source_dir)] _run_or_log(cmd, dry_run) - elif backend == "cargo": - pass # Cargo does not have a separate configure step - - elif backend in ("make", "kbuild", "ninja"): - pass # No separate configure step - - else: - raise ValueError( - f"Unknown build backend '{backend}'. " - f"Supported backends: {', '.join(sorted(ALL_BACKENDS))}" - ) + elif backend in ("cargo", "make", "kbuild"): + pass # These backends have no separate configure step. else: - raise RuntimeError( - f"BackendDispatcher cannot configure backend '{backend}'. " - "This dispatcher only handles cmake, meson, and cargo " - "(make/kbuild need no configure step). ebuild's own ninja " - "backend is invoked directly and requires 'targets' in " - "build.yaml -- add targets or choose another backend." - ) + raise _unknown_backend(backend, "configure", CONFIGURE_BACKENDS) def build( self, @@ -154,7 +181,7 @@ def build( dry_run: If True, log commands instead of executing them. Raises: - ValueError: If the backend is not recognized. + UnknownBackendError: If this step does not handle the backend. """ config = config or {} @@ -185,10 +212,7 @@ def build( _run_or_log(cmd, dry_run) else: - raise ValueError( - f"Unknown build backend '{backend}'. " - f"Supported backends: {', '.join(sorted(ALL_BACKENDS))}" - ) + raise _unknown_backend(backend, "build", BUILD_BACKENDS) def clean( self, @@ -203,7 +227,7 @@ def clean( dry_run: If True, log commands instead of executing them. Raises: - ValueError: If the backend is not recognized. + UnknownBackendError: If this step does not handle the backend. """ if backend == "cmake": _run_or_log( @@ -237,7 +261,4 @@ def clean( check=False, ) else: - raise ValueError( - f"Unknown build backend '{backend}'. " - f"Supported backends: {', '.join(sorted(ALL_BACKENDS))}" - ) + raise _unknown_backend(backend, "clean", CLEAN_BACKENDS) diff --git a/tests/ebuild/test_dispatch.py b/tests/ebuild/test_dispatch.py index aa71c394..7c564dc3 100644 --- a/tests/ebuild/test_dispatch.py +++ b/tests/ebuild/test_dispatch.py @@ -12,6 +12,7 @@ from ebuild.build.dispatch import ( ALL_BACKENDS, BackendDispatcher, + UnknownBackendError, detect_backend, ) @@ -165,3 +166,68 @@ def test_clean_kbuild(self, mock_sub, tmp_path): mock_sub.run.assert_called_once() cmd = mock_sub.run.call_args[0][0] assert "clean" in cmd + + +# ── BackendDispatcher — unknown-backend error contract ────── + + +class TestUnknownBackendError: + """The error raised for an unhandled backend. + + ``configure()`` and ``build()`` grew two independent unhandled-backend + branches on separate branches; merging them left duplicated ``else`` + clauses (a SyntaxError that made the module unimportable) raising two + different types. ``UnknownBackendError`` is the single type, derived + from both so neither caller contract broke. + """ + + def test_error_is_both_value_and_runtime_error(self, tmp_path): + """Callers catching either legacy type must keep working. + + ``ebuild build`` funnels this through ``except RuntimeError``; the + older dispatcher tests catch ``ValueError``. + """ + assert issubclass(UnknownBackendError, ValueError) + assert issubclass(UnknownBackendError, RuntimeError) + + d = BackendDispatcher(tmp_path, tmp_path / "build") + with pytest.raises(UnknownBackendError): + d.build("gradle") + + def test_configure_ninja_raises_instead_of_silently_passing(self, tmp_path): + """``backend: ninja`` with no targets reaches the dispatcher. + + A silent no-op here let the CLI report "Build completed + successfully" with exit code 0 having built nothing. + """ + d = BackendDispatcher(tmp_path, tmp_path / "build") + with pytest.raises(UnknownBackendError, match="ninja"): + d.configure("ninja") + + def test_ninja_error_explains_the_targets_requirement(self, tmp_path): + """The message must be actionable, not just a rejection.""" + d = BackendDispatcher(tmp_path, tmp_path / "build") + with pytest.raises(UnknownBackendError, match="targets"): + d.build("ninja") + + def test_error_does_not_list_the_rejected_backend_as_supported(self, tmp_path): + """ALL_BACKENDS contains 'ninja', so listing it here contradicted + the rejection. Each step reports only what it actually handles.""" + d = BackendDispatcher(tmp_path, tmp_path / "build") + with pytest.raises(UnknownBackendError) as excinfo: + d.configure("ninja") + + supported = str(excinfo.value).split("BackendDispatcher can configure:")[1] + supported = supported.split(".")[0] + assert "ninja" not in supported + + def test_clean_still_accepts_ninja(self, tmp_path): + """clean() genuinely handles ninja -- only configure/build do not.""" + d = BackendDispatcher(tmp_path, tmp_path / "build") + d.clean("ninja", dry_run=True) # must not raise + + def test_no_configure_step_backends_stay_noops(self, tmp_path): + """cargo/make/kbuild are accepted-and-skipped, not errors.""" + d = BackendDispatcher(tmp_path, tmp_path / "build") + for backend in ("cargo", "make", "kbuild"): + d.configure(backend) # must not raise