From a65206b40b703cd14b91122c3b19a9461397ceab Mon Sep 17 00:00:00 2001 From: Thompson Opeyemi <57995305+thompsondev@users.noreply.github.com> Date: Thu, 3 Sep 2026 09:52:40 +0100 Subject: [PATCH 1/4] fix(ebuild): ask ninja for the Windows test binary Native type: test targets link as name.exe on Windows, but ebuild test asked ninja to build the unsuffixed path. Pytest collection also aborted when gcc was missing. Signed-off-by: Thompson Opeyemi <57995305+thompsondev@users.noreply.github.com> Co-authored-by: Cursor --- CHANGELOG.md | 12 +++++++ README.md | 5 +++ TASKS.md | 3 +- docs/compatibility.md | 1 + ebuild/build/ninja_backend.py | 23 +++++++++++- ebuild/cli/commands.py | 15 +++++--- tests/ebuild/test_build_dir_resolution.py | 34 +++++++++++++++++- tests/unit/test_golden_path_commands.py | 44 +++++++++++++++++++++++ 8 files changed, 130 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 536e83a..46b650f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,18 @@ ## [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`). +- **`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..48fe869 100644 --- a/TASKS.md +++ b/TASKS.md @@ -11,7 +11,8 @@ 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 | +| T-003 | `ebuild package` looks for the unsuffixed binary on Windows (`_build/app` rather than `_build/app.exe`) | backend | Maintenance | todo | none | ## Completed diff --git a/docs/compatibility.md b/docs/compatibility.md index b989658..f485130 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -103,6 +103,7 @@ Download from [ARM Developer](https://developer.arm.com/downloads/-/gnu-rm) and | Limitation | Affected | Workaround | |-----------|----------|------------| | eOSuite excluded on Windows | `--with eosuite` | Use WSL2 or Linux VM | +| `ebuild package` looks for `_build/` rather than `_build/.exe` | Windows | Tracked as T-003 | | EIPC Go build separate from CMake | `layers/eipc/` | Build EIPC separately with `make` | | ESP32 requires ESP-IDF | `--target esp32` | Install ESP-IDF v5.x first | | MIPS toolchain not in most distros | `--target malta` | Build from source or use Docker | 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..c14b892 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 @@ -2609,11 +2613,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 +2629,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/ebuild/test_build_dir_resolution.py b/tests/ebuild/test_build_dir_resolution.py index 28d9eb3..fa2ab21 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 @@ -242,8 +243,39 @@ def test_configure_and_build_from_outside_agree_on_the_build_dir( # ── end to end, with a real compiler ──────────────────────── +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 in ``skipif`` is not a skip: + it aborts collection of this file and, with default pytest, the suite. + """ + 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 + + +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 + + @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_golden_path_commands.py b/tests/unit/test_golden_path_commands.py index a6a0ee2..4b2ba8f 100644 --- a/tests/unit/test_golden_path_commands.py +++ b/tests/unit/test_golden_path_commands.py @@ -168,6 +168,50 @@ 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". + """ + import sys + from types import SimpleNamespace + + from ebuild.build.ninja_backend import NinjaBackend + from ebuild.cli import commands + from ebuild.core.config import ProjectConfig + + 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() + + suffix = ".exe" if sys.platform == "win32" else "" + expected = build / f"t_smoke{suffix}" + 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: From 64aa812bd78cbc84ee352c1bfaf13d5af4afd1e7 Mon Sep 17 00:00:00 2001 From: Thompson Opeyemi <57995305+thompsondev@users.noreply.github.com> Date: Thu, 3 Sep 2026 15:44:33 +0100 Subject: [PATCH 2/4] fix(test_footprint): skip test if gcc is not available Added a conditional skip for the test measuring a real binary, ensuring it only runs when a working gcc is present. This prevents unnecessary test failures on systems without gcc installed. Co-authored-by: Cursor --- tests/unit/test_footprint.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_footprint.py b/tests/unit/test_footprint.py index 5450853..a73d9f2 100644 --- a/tests/unit/test_footprint.py +++ b/tests/unit/test_footprint.py @@ -15,6 +15,7 @@ """ import os +import shutil import subprocess from pathlib import Path @@ -57,13 +58,20 @@ def test_a_pure_bss_buffer_costs_ram_but_not_flash(self): class TestMeasure: + @pytest.mark.skipif( + shutil.which("gcc") is None, + 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") # gcc on Windows appends .exe when -o names no extension, so the # path measured here has to carry it or there is nothing to measure. exe = tmp_path / ("m.exe" if os.name == "nt" else "m") - subprocess.run(["gcc", str(src), "-o", str(exe)], check=True) + try: + subprocess.run(["gcc", str(src), "-o", str(exe)], check=True) + except FileNotFoundError: + pytest.skip("needs a working gcc to link the executable") fp = measure(exe) assert fp.text > 0 From c7d83d86ef995cc5bbae481b197671a76e5cdf13 Mon Sep 17 00:00:00 2001 From: Thompson Opeyemi <57995305+thompsondev@users.noreply.github.com> Date: Thu, 3 Sep 2026 16:01:20 +0100 Subject: [PATCH 3/4] fix: address PR #110 review findings 1, 2, and 4 Finding 1 (medium): test_native_runner_asks_ninja_for_the_linked_binary read sys.platform to pick the expected suffix, so on any POSIX runner it asserted the unsuffixed path -- exactly what the unfixed code produced -- and passed against the bug. Force _exe_suffix() to ".exe" instead, so the test exercises the Windows path on any host. Verified it now fails against the pre-fix argv construction and passes against the fix. Finding 2 (low): `ebuild package` still read `Path(build_dir) / binaries[0].name` instead of the executable_output_path() helper the ninja edge and `ebuild test` were just converted to, so it reported "No built artifact" after a successful Windows build. Took the one-line fix now that the helper exists, closed out T-003 in TASKS.md and docs/compatibility.md, and added a suffix-forcing regression test mirroring finding 1's. Doing so surfaced that the pre-existing package-command test fixtures wrote an unsuffixed stand-in binary; on this suite's real Windows host the fixed lookup could no longer find it, so those fixtures now build their artifact through executable_output_path() too. Finding 4 (low): _gcc_is_missing() only had a test for the OSError branch (gcc present but unable to start); the shutil.which() -> None branch, the common case on a bare Windows host, was untested. Added that case. Verified on this Windows host: full suite 559 passed, 6 skipped, 0 failed (previously 1 known pre-existing gcc-dependent failure). Finding 3 (no CI has run on this head) is a maintainer action (approve the queued workflow runs) and isn't addressed here. --- CHANGELOG.md | 5 ++++ TASKS.md | 2 +- docs/compatibility.md | 1 - ebuild/cli/commands.py | 2 +- tests/ebuild/test_build_dir_resolution.py | 6 +++++ tests/unit/test_golden_path_commands.py | 11 ++++++--- tests/unit/test_package_efw.py | 29 +++++++++++++++++++++-- 7 files changed, 48 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46b650f..1bc37f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ 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`). - **`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 diff --git a/TASKS.md b/TASKS.md index 48fe869..256b291 100644 --- a/TASKS.md +++ b/TASKS.md @@ -12,13 +12,13 @@ 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 | review | none | -| T-003 | `ebuild package` looks for the unsuffixed binary on Windows (`_build/app` rather than `_build/app.exe`) | backend | Maintenance | todo | 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**. | --- diff --git a/docs/compatibility.md b/docs/compatibility.md index f485130..b989658 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -103,7 +103,6 @@ Download from [ARM Developer](https://developer.arm.com/downloads/-/gnu-rm) and | Limitation | Affected | Workaround | |-----------|----------|------------| | eOSuite excluded on Windows | `--with eosuite` | Use WSL2 or Linux VM | -| `ebuild package` looks for `_build/` rather than `_build/.exe` | Windows | Tracked as T-003 | | EIPC Go build separate from CMake | `layers/eipc/` | Build EIPC separately with `make` | | ESP32 requires ESP-IDF | `--target esp32` | Install ESP-IDF v5.x first | | MIPS toolchain not in most distros | `--target malta` | Build from source or use Docker | diff --git a/ebuild/cli/commands.py b/ebuild/cli/commands.py index c14b892..29d6b0b 100644 --- a/ebuild/cli/commands.py +++ b/ebuild/cli/commands.py @@ -2530,7 +2530,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) diff --git a/tests/ebuild/test_build_dir_resolution.py b/tests/ebuild/test_build_dir_resolution.py index fa2ab21..162c021 100644 --- a/tests/ebuild/test_build_dir_resolution.py +++ b/tests/ebuild/test_build_dir_resolution.py @@ -274,6 +274,12 @@ def boom(*args, **kwargs): 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( _gcc_is_missing(), reason="needs a working gcc to link the executable", diff --git a/tests/unit/test_golden_path_commands.py b/tests/unit/test_golden_path_commands.py index 4b2ba8f..ddad86d 100644 --- a/tests/unit/test_golden_path_commands.py +++ b/tests/unit/test_golden_path_commands.py @@ -174,14 +174,20 @@ def test_native_runner_asks_ninja_for_the_linked_binary(self, tmp_path, monkeypa 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. """ - import sys 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", @@ -191,8 +197,7 @@ def test_native_runner_asks_ninja_for_the_linked_binary(self, tmp_path, monkeypa NinjaBackend(cfg, build, SimpleNamespace(cc="cc", cxx="c++", ar="ar")).generate() - suffix = ".exe" if sys.platform == "win32" else "" - expected = build / f"t_smoke{suffix}" + expected = build / "t_smoke.exe" expected.write_bytes(b"") monkeypatch.setattr(commands, "_configure_ninja_backend", 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.""" From 26a767f4043bb114f40d2e085145d7e5f42d055f Mon Sep 17 00:00:00 2001 From: Thompson Opeyemi <57995305+thompsondev@users.noreply.github.com> Date: Thu, 3 Sep 2026 16:42:58 +0100 Subject: [PATCH 4/4] fix: address PR #110 review findings 1 and 4 (round two) Finding 1 (high): _report_footprint -- the flash/RAM summary `ebuild build` prints -- was the third of three call sites building an executable path from a target name, and the only one left unsuffixed. On Windows it silently returned with no diagnostic (the other two early exits in the function already log at debug level), so a developer had no way to tell the report was missing rather than not applicable. Uses executable_output_path() now, and the missing-artifact return logs at debug level to match the function's other guards. Added a suffix-forcing regression test mirroring the two this PR already has; confirmed it fails against the pre-fix lookup and passes against the fix. Finding 4 (low): _gcc_is_missing() existed twice -- the hardened version in tests/ebuild/test_build_dir_resolution.py and a bare shutil.which() check in tests/unit/test_footprint.py that the inner try/except only patched over. Hoisted the probe into tests/conftest.py as gcc_is_missing(), used from both files, and dropped the now-dead inner try/except in test_footprint.py. Findings 2 and 3 (PR body vs diff, and contradictory test-result numbers) are description-only issues on the open PR, not code -- not addressed in this commit. Full suite on this Windows host: 560 passed, 6 skipped, 0 failed. --- CHANGELOG.md | 7 +++ TASKS.md | 1 + ebuild/cli/commands.py | 3 +- tests/conftest.py | 28 +++++++++++ tests/ebuild/test_build_dir_resolution.py | 27 +++-------- tests/unit/test_footprint.py | 58 ++++++++++++++++++++--- 6 files changed, 97 insertions(+), 27 deletions(-) create mode 100644 tests/conftest.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1bc37f2..7f013f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,13 @@ 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 diff --git a/TASKS.md b/TASKS.md index 256b291..2f49d9f 100644 --- a/TASKS.md +++ b/TASKS.md @@ -19,6 +19,7 @@ Status is one of: `todo`, `in-progress`, `blocked`, `review`, `done`. |----|------|-------|-------------|----------| | 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/cli/commands.py b/ebuild/cli/commands.py index 29d6b0b..98201e0 100644 --- a/ebuild/cli/commands.py +++ b/ebuild/cli/commands.py @@ -513,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" 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 162c021..428ff33 100644 --- a/tests/ebuild/test_build_dir_resolution.py +++ b/tests/ebuild/test_build_dir_resolution.py @@ -36,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 @@ -242,23 +243,9 @@ def test_configure_and_build_from_outside_agree_on_the_build_dir( # ── end to end, with a real compiler ──────────────────────── - -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 in ``skipif`` is not a skip: - it aborts collection of this file and, with default pytest, the suite. - """ - 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 +# `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): @@ -271,17 +258,17 @@ 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 + 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 + assert gcc_is_missing() is True @pytest.mark.skipif( - _gcc_is_missing(), + 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 a73d9f2..d9146f6 100644 --- a/tests/unit/test_footprint.py +++ b/tests/unit/test_footprint.py @@ -15,7 +15,6 @@ """ import os -import shutil import subprocess from pathlib import Path @@ -33,6 +32,7 @@ measure, over_budget, ) +from tests.conftest import gcc_is_missing class TestAccounting: @@ -59,7 +59,7 @@ def test_a_pure_bss_buffer_costs_ram_but_not_flash(self): class TestMeasure: @pytest.mark.skipif( - shutil.which("gcc") is None, + gcc_is_missing(), reason="needs a working gcc to link the executable", ) def test_measures_a_real_binary(self, tmp_path): @@ -68,10 +68,7 @@ def test_measures_a_real_binary(self, tmp_path): # gcc on Windows appends .exe when -o names no extension, so the # path measured here has to carry it or there is nothing to measure. exe = tmp_path / ("m.exe" if os.name == "nt" else "m") - try: - subprocess.run(["gcc", str(src), "-o", str(exe)], check=True) - except FileNotFoundError: - pytest.skip("needs a working gcc to link the executable") + subprocess.run(["gcc", str(src), "-o", str(exe)], check=True) fp = measure(exe) assert fp.text > 0 @@ -211,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)