diff --git a/CHANGELOG.md b/CHANGELOG.md index 536e83a..7f013f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,30 @@ ## [Unreleased] ### Fixed +- **`ebuild test` now finds Windows test binaries.** Native `type: test` + targets are linked as `.exe` on Windows (gcc appends the suffix), + but `ebuild test` asked ninja to build `` and then looked for that + unsuffixed path. Ninja reported an unknown target and the runner reported + "built, but no binary". Both steps now use `executable_output_path()` + (`ebuild/build/ninja_backend.py`, `ebuild/cli/commands.py`). +- **`ebuild package` now finds the Windows binary too.** It looked up + `/` directly instead of through `executable_output_path()`, + so on Windows it reported "No built artifact" after a build that had + succeeded. Now uses the same helper as `ebuild test` + (`ebuild/cli/commands.py`). +- **`ebuild build`'s flash/RAM report went silent on Windows.** + `_report_footprint` also looked up `/` directly, so on + Windows the artifact was never found and the function returned with no + diagnostic — the report just never appeared, with no indication it was + skipped rather than not applicable. Now uses `executable_output_path()` + and logs at debug level when it has nothing to measure + (`ebuild/cli/commands.py`). +- **`pytest` collection no longer aborts on Windows without gcc.** + `tests/ebuild/test_build_dir_resolution.py` evaluated + `subprocess.run(["gcc", "--version"])` in a `skipif`. When gcc is not + installed, Windows raises `FileNotFoundError` instead of a non-zero + returncode, which pytest treats as a collection ERROR and stops the + suite. The probe now uses `shutil.which` and catches `OSError`. - **A path containing a space produced a silently wrong `build.ninja`.** Paths were written into build statements unescaped, but Ninja ends the output list at the first unescaped `:` and splits on unescaped spaces. A build directory diff --git a/README.md b/README.md index 421a3ae..a903467 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,11 @@ Additional commands: `configure`, `install`, `add`, `list-packages`, when it printed nothing recognisable — a number inferred from a zero exit status is a guess presented as a measurement. +Projects that declare `type: test` targets in `build.yaml` are built and run +directly (no ctest/cargo/meson). On Windows those binaries are named +`.exe`, matching the Ninja edge; `ebuild test` now asks ninja for that +path rather than the unsuffixed name. + It also treats a run that executed **no tests** as a failure. `ctest` exits `0` when it finds nothing to run, and a `CMakeLists.txt` with `enable_testing()` and no `add_test()` still produces a `CTestTestfile.cmake` — so the runner is found, diff --git a/TASKS.md b/TASKS.md index b7702ab..2f49d9f 100644 --- a/TASKS.md +++ b/TASKS.md @@ -11,13 +11,15 @@ Status is one of: `todo`, `in-progress`, `blocked`, `review`, `done`. | ID | Task | Owner | Mode | Status | Depends on | |----|------|-------|------|--------|------------| -| T-002 | Fix Windows Ninja test-target path parsing | backend | Maintenance | todo | none | +| T-002 | Fix Windows Ninja test-target path parsing | backend | Maintenance | review | none | ## Completed | ID | Task | Owner | Verified by | Evidence | |----|------|-------|-------------|----------| | T-001 | Make initramfs creation portable and self-contained | backend | independent reviewer | Focused archive tests: **5 passed, 1 skipped** (symlink creation unavailable on this Windows host). Independent `bsdtar` extraction validated hard-link identity and payload. Full Python suite: **288 passed, 2 skipped, 1 unrelated failure** in the pre-existing Windows Ninja path assertion, recorded as T-002. QEMU boot was not run on Windows. | +| T-003 | `ebuild package` looks for the unsuffixed binary on Windows (`_build/app` rather than `_build/app.exe`) | backend | self (see PR #110 review, finding 2) | Was deferred out of T-002 for reviewability, then folded back in once `executable_output_path()` existed: `ebuild/cli/commands.py` now calls it at the `package` artifact lookup instead of `Path(build_dir) / name`. Covered by `tests/unit/test_package_efw.py::TestCommandPacks::test_it_finds_the_windows_suffixed_artifact`, which forces `_exe_suffix()` to `.exe` so it exercises the Windows path on any host, and also caught the fix's ripple effect on the suite's own real-Windows host: existing `test_package_efw.py` fixtures wrote an unsuffixed stand-in binary, which the fixed lookup could no longer find natively (`_exe_suffix()` returns `.exe` there unforced), so those fixtures now build the artifact through `executable_output_path()` too. Full suite run on this Windows host: **559 passed, 6 skipped, 0 failed**. | +| T-004 | `_report_footprint` (the flash/RAM report `ebuild build` prints) looks for the unsuffixed binary on Windows, and fails silently rather than logging why | backend | self (see PR #110 review, finding 1) | Third of three `build_dir / name` call sites, and the only one with no diagnostic on the early-return path. `ebuild/cli/commands.py:516` now uses `executable_output_path()`, and the bare `return` on a missing artifact now logs at debug level, matching the function's other two early exits. Covered by `tests/unit/test_footprint.py::TestCLIFootprintReport::test_looks_up_the_windows_suffixed_artifact`, which forces `_exe_suffix()` to `.exe`; confirmed to fail against the pre-fix lookup (no report emitted) and pass against the fix. Full suite on this Windows host: **560 passed, 6 skipped, 0 failed**. | --- diff --git a/ebuild/build/ninja_backend.py b/ebuild/build/ninja_backend.py index 6557c5d..abe67c1 100644 --- a/ebuild/build/ninja_backend.py +++ b/ebuild/build/ninja_backend.py @@ -41,6 +41,27 @@ def _exe_suffix() -> str: return ".exe" if sys.platform == "win32" else "" +def executable_output_path(build_dir: Path, target_name: str) -> Path: + """Return the linked binary path NinjaBackend emits for *target_name*. + + Args: + build_dir: Directory that contains ``build.ninja`` and the linked + outputs. + target_name: The ``name`` of an ``executable`` or ``test`` target. + + Returns: + ``build_dir / target_name`` on POSIX, or that path with ``.exe`` + appended on Windows, matching the compiler driver's output. + + Example: + >>> from pathlib import Path + >>> executable_output_path(Path("_build"), "hello").name in ( + ... "hello", "hello.exe") + True + """ + return Path(build_dir) / (target_name + _exe_suffix()) + + def _shared_flag() -> str: """The flag that makes the compiler driver emit a shared object. @@ -230,7 +251,7 @@ def _write_ninja(self) -> None: link_inputs = obj_files + dep_archives out = escape_ninja_path( - self.build_dir / (target.name + _exe_suffix())) + executable_output_path(self.build_dir, target.name)) lines.append( f"build {out}: link " f"{' '.join(link_inputs)}" ) diff --git a/ebuild/cli/commands.py b/ebuild/cli/commands.py index d3166e3..98201e0 100644 --- a/ebuild/cli/commands.py +++ b/ebuild/cli/commands.py @@ -26,7 +26,11 @@ import yaml from ebuild import __version__ -from ebuild.build.ninja_backend import NinjaBackend, PackagePaths +from ebuild.build.ninja_backend import ( + NinjaBackend, + PackagePaths, + executable_output_path, +) from ebuild.build.toolchain import resolve_toolchain from ebuild.cli.integration import register_commands as _register_integration_commands from ebuild.cli.logger import Logger @@ -509,8 +513,9 @@ def _report_footprint(cfg: "ProjectConfig", build_path: Path, log: Logger) -> No if not binaries: return - artifact = build_path / binaries[0].name + artifact = executable_output_path(build_path, binaries[0].name) if not artifact.is_file(): + log.debug(f"no artifact at {artifact}; skipping footprint") return prefix = getattr(cfg.toolchain, "target", None) or "host" @@ -2526,7 +2531,7 @@ def package(log: Logger, config_path: str, build_dir: str, log.error("No executable target in build.yaml — nothing to package.") raise SystemExit(1) - artifact = Path(build_dir) / binaries[0].name + artifact = executable_output_path(Path(build_dir), binaries[0].name) if not artifact.is_file(): log.error(f"No built artifact at {artifact}. Run 'ebuild build' first.") raise SystemExit(1) @@ -2609,11 +2614,14 @@ def _run_native_tests( from ebuild.build.dispatch import ninja_command # Ninja addresses targets by their output path, and `ebuild build` drives - # it with -f from the project root, so the same form is used here. + # it with -f from the project root, so the same form is used here. The + # path must include the platform suffix: on Windows the edge is + # ``.exe``, and asking ninja to build ```` is an unknown + # target. argv = ( ninja_command() + ["-f", str(build_path / "build.ninja")] - + [str(build_path / t.name) for t in selected] + + [str(executable_output_path(build_path, t.name)) for t in selected] ) result = subprocess.run(argv) if result.returncode != 0: @@ -2622,7 +2630,7 @@ def _run_native_tests( failures: List[str] = [] for target in selected: - binary = build_path / target.name + binary = executable_output_path(build_path, target.name) if not binary.is_file(): log.error(f"{target.name}: built, but no binary at {binary}") failures.append(target.name) diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..4f605cc --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,28 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""Shared pytest helpers for the ebuild test suite.""" + +import shutil +import subprocess + + +def gcc_is_missing() -> bool: + """True when this host cannot run gcc. + + ``subprocess.run(['gcc', ...])`` raises ``FileNotFoundError`` on Windows + when gcc is not installed. Evaluating that directly in ``skipif`` is not + a skip: it aborts collection of the file and, with default pytest, the + suite. Used by both ``tests/ebuild/test_build_dir_resolution.py`` and + ``tests/unit/test_footprint.py`` -- previously each carried its own copy + of this probe, one of them the version this existed to fix. + """ + gcc = shutil.which("gcc") + if gcc is None: + return True + try: + return subprocess.run( + [gcc, "--version"], capture_output=True + ).returncode != 0 + except OSError: + return True diff --git a/tests/ebuild/test_build_dir_resolution.py b/tests/ebuild/test_build_dir_resolution.py index 28d9eb3..428ff33 100644 --- a/tests/ebuild/test_build_dir_resolution.py +++ b/tests/ebuild/test_build_dir_resolution.py @@ -26,6 +26,7 @@ from __future__ import annotations import os +import shutil import subprocess import textwrap from pathlib import Path @@ -35,6 +36,7 @@ from click.testing import CliRunner from ebuild.cli import commands +from tests.conftest import gcc_is_missing pytestmark = pytest.mark.needs_yaml @@ -241,9 +243,32 @@ def test_configure_and_build_from_outside_agree_on_the_build_dir( # ── end to end, with a real compiler ──────────────────────── +# `gcc_is_missing()` lives in tests/conftest.py, shared with +# tests/unit/test_footprint.py -- both need to know whether the host can +# link a real binary before running a skipif against it. + + +def test_gcc_probe_does_not_raise_when_gcc_cannot_start(monkeypatch): + """Collection must stay a skip, not an ERROR, if gcc is absent.""" + monkeypatch.setattr( + shutil, "which", lambda name: r"C:\missing\gcc.exe" + ) + + def boom(*args, **kwargs): + raise FileNotFoundError(2, "The system cannot find the file specified") + + monkeypatch.setattr(subprocess, "run", boom) + assert gcc_is_missing() is True + + +def test_gcc_probe_reports_missing_when_which_finds_nothing(monkeypatch): + """The common case on a bare Windows host: no gcc on PATH at all.""" + monkeypatch.setattr(shutil, "which", lambda name: None) + assert gcc_is_missing() is True + @pytest.mark.skipif( - subprocess.run(["gcc", "--version"], capture_output=True).returncode != 0, + gcc_is_missing(), reason="needs a working gcc to link the executable", ) def test_end_to_end_build_from_outside_produces_the_binary(tmp_path, monkeypatch): diff --git a/tests/unit/test_footprint.py b/tests/unit/test_footprint.py index 5450853..d9146f6 100644 --- a/tests/unit/test_footprint.py +++ b/tests/unit/test_footprint.py @@ -32,6 +32,7 @@ measure, over_budget, ) +from tests.conftest import gcc_is_missing class TestAccounting: @@ -57,6 +58,10 @@ def test_a_pure_bss_buffer_costs_ram_but_not_flash(self): class TestMeasure: + @pytest.mark.skipif( + gcc_is_missing(), + reason="needs a working gcc to link the executable", + ) def test_measures_a_real_binary(self, tmp_path): src = tmp_path / "m.c" src.write_text("static char buf[4096];\nint main(void){return buf[0];}\n") @@ -203,6 +208,55 @@ def test_units(self, n, expected): assert format_size(n) == expected +class TestCLIFootprintReport: + """`_report_footprint` (`ebuild/cli/commands.py`) is the CLI-level + consumer wired into `ebuild build`. Like `ebuild test` and `ebuild + package`, it has to look up the same suffixed binary NinjaBackend + linked.""" + + def test_looks_up_the_windows_suffixed_artifact(self, tmp_path, monkeypatch): + """On Windows the linked binary is `.exe`; the report must + look there, not at the unsuffixed name NinjaBackend never produces + on that platform -- the third of three call sites this rule applies + to, and the only one that failed silently. + + `_exe_suffix()` is forced to `.exe` rather than switching on + `sys.platform`, so this exercises the Windows path -- and fails + against the pre-fix code -- on any host the suite runs on. + """ + from types import SimpleNamespace + + from ebuild.build import ninja_backend + from ebuild.cli import commands + from ebuild.core.config import ProjectConfig, TargetConfig + + monkeypatch.setattr(ninja_backend, "_exe_suffix", lambda: ".exe") + monkeypatch.setattr( + "ebuild.build.footprint.find_size_tool", + lambda prefix: "/usr/bin/size") + monkeypatch.setattr( + "ebuild.build.footprint.measure", + lambda artifact, tool=None: Footprint(text=1000, data=200, bss=500)) + + cfg = ProjectConfig( + name="app", version="1", source_dir=tmp_path, + targets=[TargetConfig(name="app", target_type="executable", + sources=["a.c"])], + ) + build = tmp_path / "b" + build.mkdir() + (build / "app.exe").write_bytes(b"\x7fELF") + + logs = [] + log = SimpleNamespace( + info=logs.append, debug=lambda *a, **k: None, + warning=lambda *a, **k: None) + + commands._report_footprint(cfg, build, log) + + assert any("Flash" in line for line in logs), logs + + def _completed(returncode=0, stdout="", stderr=""): return subprocess.CompletedProcess( args=["size"], returncode=returncode, stdout=stdout, stderr=stderr) diff --git a/tests/unit/test_golden_path_commands.py b/tests/unit/test_golden_path_commands.py index a6a0ee2..ddad86d 100644 --- a/tests/unit/test_golden_path_commands.py +++ b/tests/unit/test_golden_path_commands.py @@ -168,6 +168,55 @@ def outputs(line): assert ": link " in edge assert ": ar_rule" not in edge + def test_native_runner_asks_ninja_for_the_linked_binary(self, tmp_path, monkeypatch): + """`ebuild test` must name the same output NinjaBackend linked. + + On Windows the edge is ``t_smoke.exe`` because gcc appends ``.exe``. + Asking ninja to build ``t_smoke`` is an unknown target, and looking + for the unsuffixed path then reports "built, but no binary". + + ``_exe_suffix()`` is forced to ``.exe`` rather than switching on + ``sys.platform``, so this exercises the Windows path — and fails + against the pre-fix code — on any host the suite runs on. + """ + from types import SimpleNamespace + + from ebuild.build import ninja_backend + from ebuild.build.ninja_backend import NinjaBackend + from ebuild.cli import commands + from ebuild.core.config import ProjectConfig + + monkeypatch.setattr(ninja_backend, "_exe_suffix", lambda: ".exe") + + cfg = ProjectConfig( + name="p", version="1", source_dir=tmp_path, + targets=[TargetConfig(name="t_smoke", target_type="test", + sources=["t.c"])], + ) + build = tmp_path / "b" + NinjaBackend(cfg, build, + SimpleNamespace(cc="cc", cxx="c++", ar="ar")).generate() + + expected = build / "t_smoke.exe" + expected.write_bytes(b"") + + monkeypatch.setattr(commands, "_configure_ninja_backend", + lambda *a, **k: None) + + runs = [] + + def fake_run(argv, *a, **k): + runs.append([str(x) for x in argv]) + return SimpleNamespace(returncode=0) + + monkeypatch.setattr(commands.subprocess, "run", fake_run) + + commands._run_native_tests( + cfg, list(cfg.targets), build, _SilentLog(), None) + + assert runs, "expected ninja to be invoked" + assert str(expected) in runs[0], runs[0] + @pytest.mark.ebuild class TestExternalTestRunners: diff --git a/tests/unit/test_package_efw.py b/tests/unit/test_package_efw.py index 91d2be2..479d0c6 100644 --- a/tests/unit/test_package_efw.py +++ b/tests/unit/test_package_efw.py @@ -31,6 +31,7 @@ pack, verify, ) +from ebuild.build.ninja_backend import executable_output_path from ebuild.cli.commands import cli @@ -164,7 +165,8 @@ def _project(self, tmp_path, built=True): })) if built: (tmp_path / "_build").mkdir() - (tmp_path / "_build" / "node").write_bytes(b"\x7fELF" + b"\x00" * 64) + executable_output_path(tmp_path / "_build", "node").write_bytes( + b"\x7fELF" + b"\x00" * 64) return tmp_path def test_it_refuses_before_a_build(self, tmp_path, monkeypatch): @@ -244,7 +246,8 @@ def _project(self, tmp_path): "sources": ["src/main.c"]}], })) (tmp_path / "_build").mkdir() - (tmp_path / "_build" / "node").write_bytes(b"\x7fELF" + b"\x00" * 64) + executable_output_path(tmp_path / "_build", "node").write_bytes( + b"\x7fELF" + b"\x00" * 64) return tmp_path def _packaged(self, tmp_path, monkeypatch, argv=(), **tool_kw): @@ -261,6 +264,28 @@ def test_it_writes_the_image_and_reports_its_size(self, tmp_path, monkeypatch): assert image.is_file() assert str(image.stat().st_size) in result.output + def test_it_finds_the_windows_suffixed_artifact(self, tmp_path, monkeypatch): + """On Windows the linked binary is `.exe`; `package` must look + for that path rather than the unsuffixed one NinjaBackend never + produces there. + + `_exe_suffix()` is forced to `.exe` rather than switching on + `sys.platform`, so this exercises the Windows path -- and fails + against the pre-fix code, which looked for the unsuffixed name -- + on any host the suite runs on. + """ + from ebuild.build import ninja_backend + monkeypatch.setattr(ninja_backend, "_exe_suffix", lambda: ".exe") + + tool = _efwtool_that_packs(tmp_path) + monkeypatch.chdir(self._project(tmp_path)) + assert (tmp_path / "_build" / "node.exe").is_file() + monkeypatch.setattr("ebuild.build.firmware_image.shutil.which", + lambda n: str(tool) if n == "efwtool" else None) + result = CliRunner().invoke(cli, ["package"]) + assert result.exit_code == 0, result.output + assert (tmp_path / "node.efw").is_file() + def test_the_default_name_comes_from_the_project(self, tmp_path, monkeypatch): """Not from the target or the build directory: `--output` is optional, so the fallback is the name a developer will look for."""