diff --git a/ebuild/build/dispatch.py b/ebuild/build/dispatch.py index 7b41376d..d27faa00 100644 --- a/ebuild/build/dispatch.py +++ b/ebuild/build/dispatch.py @@ -27,48 +27,6 @@ SUPPORTED_BACKENDS = TIER_1 | TIER_2 | TIER_3 -class BackendError(RuntimeError): - """Raised when the external dispatcher cannot handle a backend.""" - - -def _validate_backend(backend: str, supported: Set[str]) -> None: - """Reject values that the requested dispatcher operation cannot handle.""" - if backend in supported: - return - - if backend in ALL_BACKENDS: - message = f"BackendDispatcher cannot handle backend '{backend}'." - else: - message = f"Unknown build backend '{backend}'." - - supported_names = ", ".join(sorted(supported)) - raise BackendError(f"{message} Supported backends: {supported_names}.") - -#: Backends this dispatcher actually drives. "ninja" is ebuild's own backend -- -#: the CLI invokes NinjaBackend directly and never routes it through here. -DISPATCHED_BACKENDS = {"cmake", "make", "meson", "cargo", "kbuild"} - - -class UnknownBackendError(ValueError, RuntimeError): - """Raised when a backend reaches the dispatcher that it cannot drive. - - Subclasses both ValueError and RuntimeError: callers treat an unrecognized - backend name as a bad argument, while the CLI treats a backend it failed to - route (notably "ninja") as a routing failure. Silently doing nothing here is - what made `ebuild build` report "Build completed successfully" without ever - running a compiler. - """ - - -def _unknown_backend(backend: str, step: str) -> UnknownBackendError: - return UnknownBackendError( - f"Unknown build backend '{backend}'. " - f"Supported backends: {', '.join(sorted(DISPATCHED_BACKENDS))}. " - "ebuild's own 'ninja' backend is invoked directly by the CLI and is " - f"not dispatched here, so it cannot be {step} through BackendDispatcher." - ) - - def ninja_command(): """Return the argv prefix that runs ninja on this machine. @@ -107,6 +65,10 @@ class UnknownBackendError(ValueError, RuntimeError): """ +#: The name this error carried before the two mechanisms were consolidated. +#: Kept so callers and tests that import it keep working. +BackendError = UnknownBackendError + def _unknown_backend(backend: str, action: str, supported: set) -> UnknownBackendError: """Build the error raised for a backend a step cannot handle. @@ -204,7 +166,13 @@ def configure( Raises: UnknownBackendError: If this step does not handle the backend. """ - _validate_backend(backend, SUPPORTED_BACKENDS) + # Reject before any side effect. The removed duplicate validation + # ran ahead of the mkdir below; raising only from the else branch + # left a stray build directory behind for a backend this step + # never handles. + if backend not in CONFIGURE_BACKENDS: + raise _unknown_backend(backend, "configure", CONFIGURE_BACKENDS) + config = config or {} self.build_dir.mkdir(parents=True, exist_ok=True) @@ -244,7 +212,6 @@ def build( Raises: UnknownBackendError: If this step does not handle the backend. """ - _validate_backend(backend, SUPPORTED_BACKENDS) config = config or {} if backend == "cmake": @@ -291,7 +258,6 @@ def clean( Raises: UnknownBackendError: If this step does not handle the backend. """ - _validate_backend(backend, ALL_BACKENDS) if backend == "cmake": _run_or_log( ["cmake", "--build", str(self.build_dir), "--target", "clean"], diff --git a/ebuild/build/footprint.py b/ebuild/build/footprint.py index 0414f81f..afd477e4 100644 --- a/ebuild/build/footprint.py +++ b/ebuild/build/footprint.py @@ -61,6 +61,16 @@ r"^\s*(?P\d+)\s+(?P\d+)\s+(?P\d+)\s+\d+\s+[0-9a-fA-F]+\s" ) +#: Apple's size(1) prints "__TEXT __DATA __OBJC others" instead of the +#: Berkeley "text data bss" columns. _SIZE_LINE matches that row too and +#: read __OBJC -- always 0 -- as bss, so every footprint measured on macOS +#: silently reported RAM without any zero-initialised data in it. +_MACHO_COLUMNS = re.compile(r"^\s*__TEXT\s+__DATA\s+__OBJC\s+others\b") +_MACHO_SEGMENT = re.compile(r"^Segment\s+(?P\S+?):\s+(?P\d+)") +_MACHO_SECTION = re.compile( + r"^\s+Section\s+(?P\S+?):\s+(?P\d+)(?P\s*\(zerofill\))?" +) + class FootprintError(RuntimeError): """Raised when a footprint cannot be measured.""" @@ -105,6 +115,55 @@ def find_size_tool(toolchain_prefix: Optional[str] = None) -> Optional[str]: return shutil.which("size") +def _run_size(tool: str, args: list, artifact: Path): + """Run the size tool, turning every failure mode into FootprintError.""" + try: + proc = subprocess.run([tool] + args, capture_output=True, + text=True, timeout=60) + except (OSError, subprocess.TimeoutExpired) as exc: + raise FootprintError(f"{tool} failed on {artifact}: {exc}") from exc + if proc.returncode != 0: + raise FootprintError( + f"{tool} exited {proc.returncode} on {artifact}: " + f"{(proc.stderr or proc.stdout).strip()[:200]}" + ) + return proc + + +def _measure_macho(tool: str, artifact: Path) -> "Footprint": + """Section sizes from Apple size(1), which needs -m to report them. + + Sections rather than segments: a segment is page-aligned, so __TEXT + reads as 16 KB for 112 bytes of code. The Berkeley numbers this + mirrors are section sizes, and the two tools must not disagree about + what a number means. + """ + proc = _run_size(tool, ["-m", str(artifact)], artifact) + text = data = bss = 0 + segment = "" + for line in proc.stdout.splitlines(): + seg = _MACHO_SEGMENT.match(line) + if seg: + segment = seg.group("name") + continue + sec = _MACHO_SECTION.match(line) + if not sec: + continue + size = int(sec.group("size")) + if sec.group("zero"): + bss += size # zerofill: RAM only, never stored + elif segment == "__TEXT": + text += size + elif segment not in ("__PAGEZERO", "__LINKEDIT"): + data += size + if not (text or data or bss): + raise FootprintError( + f"could not read any section size from {tool} -m output " + f"for {artifact}" + ) + return Footprint(text=text, data=data, bss=bss) + + def measure(artifact: Path, size_tool: Optional[str] = None) -> Footprint: """Section sizes of ``artifact``. @@ -122,19 +181,14 @@ def measure(artifact: Path, size_tool: Optional[str] = None) -> Footprint: "(install binutils, or the toolchain's binutils for a cross build)" ) - try: - proc = subprocess.run([tool, str(artifact)], capture_output=True, - text=True, timeout=60) - except (OSError, subprocess.TimeoutExpired) as exc: - raise FootprintError(f"{tool} failed on {artifact}: {exc}") from exc - - if proc.returncode != 0: - raise FootprintError( - f"{tool} exited {proc.returncode} on {artifact}: " - f"{(proc.stderr or proc.stdout).strip()[:200]}" - ) + proc = _run_size(tool, [str(artifact)], artifact) for line in proc.stdout.splitlines(): + # Apple size(1) leads with a column header naming Mach-O segments. + # Its data row matches _SIZE_LINE, so without this the __OBJC + # column is silently taken for bss. + if _MACHO_COLUMNS.match(line): + return _measure_macho(tool, artifact) m = _SIZE_LINE.match(line) if m: return Footprint(text=int(m.group("text")), diff --git a/ebuild/build/ninja_backend.py b/ebuild/build/ninja_backend.py index f3cfc76d..6557c5d0 100644 --- a/ebuild/build/ninja_backend.py +++ b/ebuild/build/ninja_backend.py @@ -31,6 +31,26 @@ class PackagePaths: "-fno-pie", "-fno-PIE"} +def _exe_suffix() -> str: + """The extension the compiler driver gives an executable. + + gcc on Windows appends .exe when -o names no extension, so an edge + declaring "app" produced "app.exe" on disk: ninja never saw its own + output, treated the target as dirty and relinked on every build. + """ + return ".exe" if sys.platform == "win32" else "" + + +def _shared_flag() -> str: + """The flag that makes the compiler driver emit a shared object. + + macOS links dynamic libraries with -dynamiclib; ELF platforms use + -shared. It lives on the rule rather than on each edge so a + shared_library target cannot be emitted without it. + """ + return "-dynamiclib" if sys.platform == "darwin" else "-shared" + + def escape_ninja_path(path) -> str: """Escape a path for use inside a Ninja build statement. @@ -78,22 +98,6 @@ def generate(self) -> None: self._write_ninja() self._write_compile_commands() - def _object_path(self, target, src: str) -> Path: - """Object file path for *src* as compiled by *target*. - - Object paths are namespaced by target name. Two targets may legitimately - list the same source: a library and a test binary sharing a helper, or - one source built twice with different defines. Each needs its own - object, because each compiles with its own cflags. Keying only on the - source made both targets claim one output, which ninja rejects with - "multiple rules generate ...". - - Example: - >>> backend._object_path(target, "src/main.c") # target.name == "app" - PosixPath('_build/obj/app/src/main.o') - """ - return (self.build_dir / "obj" / target.name / src).with_suffix(".o") - def _get_toolchain_cflags(self) -> List[str]: """Return toolchain-level cflags including sysroot. @@ -179,6 +183,13 @@ def _write_ninja(self) -> None: " command = $ar rcs $out $in", " description = AR $out", "", + # A shared_library edge names this rule. Without the rule the + # generated build.ninja referenced an undefined rule and ninja + # refused the whole file. + "rule link_shared", + f" command = $cc {_shared_flag()} $ldflags $in -o $out $libs", + " description = LINK_SHARED $out", + "", ] toolchain_ldflags = self._get_toolchain_ldflags() @@ -218,9 +229,10 @@ def _write_ninja(self) -> None: ) link_inputs = obj_files + dep_archives - out = escape_ninja_path(self.build_dir / target.name) + out = escape_ninja_path( + self.build_dir / (target.name + _exe_suffix())) lines.append( - f"build {_ninja_path(out)}: link " f"{' '.join(_ninja_path(x) for x in link_inputs)}" + f"build {out}: link " f"{' '.join(link_inputs)}" ) if ldflags: lines.append(f" ldflags = {' '.join(ldflags)}") @@ -238,7 +250,7 @@ def _write_ninja(self) -> None: out = escape_ninja_path(self.build_dir / f"lib{target.name}{ext}") if target.target_type == "static_library": - lines.append(f"build {_ninja_path(out)}: ar_rule " f"{' '.join(_ninja_path(x) for x in obj_files)}") + lines.append(f"build {out}: ar_rule " f"{' '.join(obj_files)}") else: # Shared libraries need the same -L/-l wiring executables # get, which the rule preamble alone does not supply. The @@ -254,7 +266,7 @@ def _write_ninja(self) -> None: for lib in pkg.libraries: libs.append(f"-l{lib}") - lines.append(f"build {_ninja_path(out)}: link_shared " f"{' '.join(_ninja_path(x) for x in obj_files)}") + lines.append(f"build {out}: link_shared " f"{' '.join(obj_files)}") if ldflags: lines.append(f" ldflags = {' '.join(ldflags)}") if libs: diff --git a/ebuild/cli/commands.py b/ebuild/cli/commands.py index 6d7ecbea..d3166e3e 100644 --- a/ebuild/cli/commands.py +++ b/ebuild/cli/commands.py @@ -9,6 +9,7 @@ from __future__ import annotations +import glob import os import re import shutil @@ -361,6 +362,28 @@ def _resolve_backend_request( resolved_backend = detect_backend(source_dir) log.info(f"Auto-detected backend: {resolved_backend}") + # A build.yaml that declares its own targets is a statement that + # ebuild builds this project. detect_backend() only inspects the + # filesystem, so a Makefile kept for `make flash` -- or a + # CMakeLists.txt belonging to one subcomponent -- used to outrank + # that statement: the dispatcher ran the external tool, the + # declared targets were never built, and the build still reported + # success. + # + # Only auto-detection is overridden. An explicit `backend:` in + # build.yaml or --backend on the command line still wins, which + # is how a project keeps both a target list and an external + # build. + if resolved_backend != "ninja" and cfg.targets: + log.info( + f"build.yaml declares {len(cfg.targets)} target(s), so " + f"the ninja backend is used instead of the detected " + f"{resolved_backend}. To build with {resolved_backend}, " + f"set 'backend: {resolved_backend}' in build.yaml or " + f"pass --backend {resolved_backend}." + ) + resolved_backend = "ninja" + return resolved_backend, backend_config @@ -521,7 +544,18 @@ def _report_footprint(cfg: "ProjectConfig", build_path: Path, log: Logger) -> No def _board_config() -> Optional[Dict[str, Any]]: """The project's own board description, if it ships one. - return resolved_backend, backend_config + A project that states its part's real capacity should not be measured + against the reference part for its family. + """ + path = Path("board.yaml") + if not path.is_file(): + return None + try: + import yaml + data = yaml.safe_load(path.read_text(encoding="utf-8")) + except Exception: + return None + return data if isinstance(data, dict) else None def _configure_ninja_backend( @@ -2411,7 +2445,10 @@ def test(log: Logger, config_path: str, build_dir: str, log.info(" ".join(argv)) try: - result = subprocess.run(argv, cwd=str(cwd)) + # Captured rather than inherited, because the exit status alone cannot + # distinguish "every test passed" from "there were no tests". The + # output is echoed below so the terminal reads as it did before. + result = subprocess.run(argv, cwd=str(cwd), capture_output=True, text=True) except FileNotFoundError: log.error( f"{name} is not installed or not on PATH, so the tests cannot be " @@ -2419,11 +2456,132 @@ def test(log: Logger, config_path: str, build_dir: str, ) raise SystemExit(1) + output = (result.stdout or "") + (result.stderr or "") + if output: + click.echo(output.rstrip()) + if result.returncode != 0: log.error(f"Tests failed ({name} exited {result.returncode}).") raise SystemExit(result.returncode) - log.success("All tests passed.") + # ctest exits 0 when it finds nothing to run. A CMakeLists with + # enable_testing() and no add_test() produces a CTestTestfile.cmake, so the + # runner is found, ctest prints "No tests were found!!!", exits 0, and the + # only honest reading of that is not "All tests passed". + if _ran_no_tests(name, output): + log.error(f"{name} completed without running a single test.") + log.info(" A pass here would mean nothing; treating it as a failure.") + raise SystemExit(1) + + counts = _parse_test_counts(name, output) + if counts is not None: + passed, failed = counts + log.success(f"All tests passed ({passed} passed, {failed} failed).") + else: + # No recognised summary. Report the verdict without inventing a number + # the runner did not print. + log.success("All tests passed.") + + + +@cli.command() +@click.option("--config", "config_path", default="build.yaml", + type=click.Path(), help="Path to the build configuration file.") +@click.option("--build-dir", default="_build", type=click.Path(), + help="Build output directory.") +@click.option("--output", "output_path", default=None, type=click.Path(), + help="Destination .efw path. Defaults to .efw.") +@click.option("--load", "load_addr", default=None, + help="Load address, e.g. 0x08000000.") +@click.option("--entry", "entry_addr", default=None, + help="Entry address, e.g. 0x08000100.") +@click.pass_obj +def package(log: Logger, config_path: str, build_dir: str, + output_path: Optional[str], load_addr: Optional[str], + entry_addr: Optional[str]) -> None: + """Assemble the built artifact into an eFirmware `.efw` image. + + The step ยง29's development-to-device flow puts between eBuild and the + device. eFirmware implements the format and ships `efwtool`; this drives + it, so a developer does not have to know the tool exists. + """ + from ebuild.build.firmware_image import ( + FirmwareImageError, find_efwtool, missing_tool_message, pack, verify, + ) + from ebuild.deps import EBUILD_REPOS_DIR + + log.header("ebuild โ€” Package") + + try: + cfg = load_config(config_path) + except FileNotFoundError: + log.error(f"No {config_path} here. Run this from a project directory.") + raise SystemExit(1) + except (ConfigError, RecipeError) as e: + log.error(f"Configuration error: {e}") + raise SystemExit(1) + + binaries = [t for t in cfg.targets if t.target_type == "executable"] + if not binaries: + log.error("No executable target in build.yaml โ€” nothing to package.") + raise SystemExit(1) + + artifact = 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) + + efwtool = find_efwtool(Path(EBUILD_REPOS_DIR)) + if efwtool is None: + log.error(missing_tool_message(Path(EBUILD_REPOS_DIR))) + raise SystemExit(1) + + output = Path(output_path or f"{cfg.name}.efw") + log.step(f"Packing {artifact.name} -> {output}") + try: + pack(efwtool, artifact, output, version=cfg.version or "0.0.0", + load_addr=load_addr, entry_addr=entry_addr) + verdict = verify(efwtool, output) + except FirmwareImageError as exc: + log.error(str(exc)) + raise SystemExit(1) + + log.success(f"{output} ({output.stat().st_size} bytes)") + for line in verdict.splitlines(): + log.info(f" {line}") + log.info("") + log.info(f"Inspect it with: {efwtool} inspect {output}") + +@cli.command() +@click.option("--json", "as_json", is_flag=True, + help="Emit the checks as JSON, for CI.") +@click.pass_obj +def doctor(log: Logger, as_json: bool) -> None: + """Diagnose the build environment in one command. + + Reports what is installed, what is missing, and what each missing piece + would cost. Read-only: it names the fix rather than applying it. + + Exits non-zero only for problems that actually stop a build, so a + host-only machine with no cross toolchain still passes. + """ + from ebuild.system.doctor import exit_code, format_report, run_all + + checks = run_all() + + if as_json: + import json as _json + click.echo(_json.dumps( + [{"name": c.name, "status": c.status, + "detail": c.detail, "fix": c.fix} for c in checks], + indent=2, + )) + raise SystemExit(exit_code(checks)) + + log.header("ebuild โ€” Environment") + for line in format_report(checks).splitlines(): + click.echo(line) + raise SystemExit(exit_code(checks)) def _run_native_tests( @@ -2486,6 +2644,60 @@ def _run_native_tests( log.success(f"All {len(selected)} test targets passed.") +#: What each runner prints when it completed having executed nothing. ctest's is +#: the one that matters: it pairs the message with a zero exit status. +_NO_TESTS_MARKERS = { + "ctest": ("No tests were found",), + "meson test": ("No tests defined",), + "cargo test": ("running 0 tests",), +} + +#: Each runner's own summary line, anchored to the phrasing it prints so that a +#: format change shows up as "no counts" rather than as a wrong number. +_TEST_COUNT_PATTERNS = { + "ctest": re.compile( + r"tests passed,\s*(?P\d+)\s+tests? failed out of\s*(?P\d+)"), + "meson test": re.compile( + r"^Ok:\s*(?P\d+).*?^Fail:\s*(?P\d+)", re.S | re.M), + "cargo test": re.compile( + r"test result:.*?(?P\d+) passed;\s*(?P\d+) failed"), +} + + +def _ran_no_tests(name: str, output: str) -> bool: + """True when the runner finished having executed nothing. + + Checked two ways because neither is reliable alone: the marker phrase + catches ctest, which prints no summary at all in this case, and the counts + catch a runner that prints a well-formed summary totalling zero. + """ + for marker in _NO_TESTS_MARKERS.get(name, ()): + if marker in output: + return True + counts = _parse_test_counts(name, output) + return counts is not None and counts[0] + counts[1] == 0 + + +def _parse_test_counts(name: str, output: str): + """(passed, failed) from the runner's own summary, or None. + + `make test` has no standard summary format. Rather than invent one, its + counts stay unknown and the exit status carries the verdict. + """ + pattern = _TEST_COUNT_PATTERNS.get(name) + if pattern is None: + return None + match = pattern.search(output) + if not match: + return None + groups = match.groupdict() + failed = int(groups["failed"]) + if groups.get("passed") is not None: + return int(groups["passed"]), failed + # ctest reports failures out of a total; passed is the remainder. + return int(groups["total"]) - failed, failed + + def _resolve_test_runner( source_dir: Path, build_dir: Path, diff --git a/ebuild/cli/integration.py b/ebuild/cli/integration.py index 48c5ba3d..9fd1eb1d 100644 --- a/ebuild/cli/integration.py +++ b/ebuild/cli/integration.py @@ -623,7 +623,13 @@ def sdk(log: Logger, target: str, output: str, list_targets: bool) -> None: sdk_dir = generate_sdk(target, output) log.success("SDK generated: " + str(sdk_dir)) - @cli_group.command() + # Named "package-deliverable", not "package": #77 added a `package` + # command that writes an eFirmware .efw image, and register_commands() + # runs last, so this one silently replaced it on the group and + # tests/unit/test_package_efw.py got this signature instead. Two + # different deliverables cannot share one name; the .efw image keeps + # "package" because it is the step in the device flow. + @cli_group.command("package-deliverable") @click.option("--target", required=True, help="Target hardware (e.g., raspi4)") @click.option("--version", default=__version__, help="Release version") @click.option("--build-dir", default="build", help="Build directory with compiled artifacts") diff --git a/ebuild/core/config.py b/ebuild/core/config.py index 8b2609b6..e28ff15a 100644 --- a/ebuild/core/config.py +++ b/ebuild/core/config.py @@ -341,4 +341,8 @@ def load_config(config_path: str | Path) -> ProjectConfig: source_dir=config_path.parent, backend=backend, backend_config=backend_config, + # Parsed and validated above, but never handed to the config: + # every consumer saw an empty mapping and the whole [system] + # section was silently inert. + system_config=system_config, ) diff --git a/ebuild/packages/registry.py b/ebuild/packages/registry.py index c7679bd6..6d43f4c3 100644 --- a/ebuild/packages/registry.py +++ b/ebuild/packages/registry.py @@ -81,32 +81,6 @@ def version_sort_key(version: str) -> tuple: return (release, 0, suffix) -def _version_sort_key(version: str) -> tuple: - """Ordering key for a recipe version string. - - The recipe format documents no version grammar, and every bundled recipe - uses dot-separated integers (``1.3.1``, ``11.1.0``). That is the format - this registry orders: components are compared numerically, so ``1.10.0`` - correctly sorts above ``1.9.0``. - - Anything else -- a Debian revision (``1.2.13-1``), a prerelease - (``3.6.0-rc1``), a ``v``-prefixed upstream tag (``v2.9.3``) -- is outside - that format. Rather than guess at its ordering, such a version is ranked - below every numeric one and ordered lexicographically among its peers. - Ranking it below matters for ``get()``: with ``3.6.0`` and ``3.6.0-rc1`` - both registered, the plain numeric release is chosen as latest. - - Returning a key instead of raising keeps a single out-of-format recipe - from breaking commands that merely enumerate the registry -- including - the resolver's "package not found" message, which lists every known - package and so used to fail with ValueError instead of ResolveError. - """ - parts = version.split(".") - if parts and all(part.isdigit() for part in parts): - return (1, tuple(int(part) for part in parts), "") - return (0, (), version) - - class PackageRegistry: """Registry of available package recipes. @@ -166,7 +140,7 @@ def get(self, name: str, version: Optional[str] = None) -> Optional[PackageRecip if version: return versions.get(version) - latest_version = sorted(versions.keys(), key=_version_sort_key)[-1] + latest_version = sorted(versions.keys(), key=version_sort_key)[-1] return versions[latest_version] def has(self, name: str, version: Optional[str] = None) -> bool: @@ -178,7 +152,7 @@ def list_packages(self) -> List[PackageRecipe]: result = [] for name in sorted(self._recipes.keys()): versions = self._recipes[name] - latest = sorted(versions.keys(), key=_version_sort_key)[-1] + latest = sorted(versions.keys(), key=version_sort_key)[-1] result.append(versions[latest]) return result @@ -189,7 +163,7 @@ def list_all_versions(self, name: str) -> List[PackageRecipe]: versions[v] for v in sorted( versions.keys(), - key=_version_sort_key, + key=version_sort_key, ) ] diff --git a/tests/ebuild/test_build_cli.py b/tests/ebuild/test_build_cli.py index 0d46cea0..7ed1bbab 100644 --- a/tests/ebuild/test_build_cli.py +++ b/tests/ebuild/test_build_cli.py @@ -29,6 +29,11 @@ def test_system_only_config_does_not_report_build_success(tmp_path): assert result.exit_code == 1 assert "Auto-detected backend: ninja" in result.output - assert "BackendDispatcher cannot handle backend 'ninja'" in result.output + # The dispatcher raised this through two separate mechanisms that a + # merge left side by side; they are now one, and the surviving message + # also says what to do about it. The guarantee under test -- ninja is + # rejected here rather than silently no-op'd -- is unchanged. + assert "Unknown build backend 'ninja'" in result.output + assert "requires 'targets' in build.yaml" in result.output assert "Build completed successfully" not in result.output assert not build_dir.exists() diff --git a/tests/ebuild/test_build_dir_resolution.py b/tests/ebuild/test_build_dir_resolution.py index c17ecb11..28d9eb32 100644 --- a/tests/ebuild/test_build_dir_resolution.py +++ b/tests/ebuild/test_build_dir_resolution.py @@ -25,6 +25,7 @@ from __future__ import annotations +import os import subprocess import textwrap from pathlib import Path @@ -123,7 +124,9 @@ def test_build_from_inside_project_is_unchanged(tmp_path, monkeypatch, record_ni assert result.exit_code == 0, result.output assert (tmp_path / "_build" / "build.ninja").is_file() - assert "Generated _build/build.ninja" in result.output + # The path is printed with the platform separator, so asserting the + # POSIX spelling failed on Windows against "Generated _build\\build.ninja". + assert f"Generated {os.path.join('_build', 'build.ninja')}" in result.output def test_absolute_build_dir_is_honoured_verbatim(outside_project, record_ninja, tmp_path): @@ -255,4 +258,6 @@ def test_end_to_end_build_from_outside_produces_the_binary(tmp_path, monkeypatch ) assert result.exit_code == 0, result.output - assert (project_dir / "_build" / "app").is_file() + # gcc appends .exe on Windows; the backend now names the edge to match. + exe = "app.exe" if os.name == "nt" else "app" + assert (project_dir / "_build" / exe).is_file() diff --git a/tests/ebuild/test_dispatch.py b/tests/ebuild/test_dispatch.py index bec8f32c..d4d75f74 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, + BackendError, UnknownBackendError, detect_backend, ) diff --git a/tests/ebuild/test_package_registry.py b/tests/ebuild/test_package_registry.py index ae29836c..5b777f04 100644 --- a/tests/ebuild/test_package_registry.py +++ b/tests/ebuild/test_package_registry.py @@ -78,10 +78,14 @@ def test_out_of_format_versions_order_deterministically(): ordered = [r.version for r in registry.list_all_versions("demo")] - # Out-of-format versions rank below every numeric one; numeric versions - # keep the ordering guaranteed by - # test_list_all_versions_uses_numeric_version_order. - assert ordered == ["1.2.13-1", "v2.9.3", "1.9.0", "1.10.0"] + # The registry orders through version_sort_key, which reads these forms + # rather than ranking them all below the numeric ones: a leading "v" is + # ignored (v2.9.3 is littlefs's own tag format, so it has to order as + # 2.9.3, not below 1.10.0), and a suffix sorts just under the release it + # qualifies, so 1.2.13-1 precedes 1.9.0. The registry previously used a + # second, private key that ranked every non-numeric version below every + # numeric one; the two disagreed and only the private one was wired in. + assert ordered == ["1.2.13-1", "1.9.0", "1.10.0", "v2.9.3"] def test_one_out_of_format_recipe_does_not_hide_valid_packages(): diff --git a/tests/unit/test_empty_test_run_is_not_a_pass.py b/tests/unit/test_empty_test_run_is_not_a_pass.py index ca3037e0..6a597418 100644 --- a/tests/unit/test_empty_test_run_is_not_a_pass.py +++ b/tests/unit/test_empty_test_run_is_not_a_pass.py @@ -23,9 +23,13 @@ That is worse than having no test command, because it actively reassures. """ +import sys + import pytest +import yaml +from click.testing import CliRunner -from ebuild.cli.commands import _parse_test_counts, _ran_no_tests +from ebuild.cli.commands import _parse_test_counts, _ran_no_tests, cli class TestEmptyRunDetection: @@ -85,3 +89,112 @@ def test_make_has_no_standard_summary(self): def test_unrecognised_output_yields_no_counts(self): assert _parse_test_counts("ctest", "something else entirely") is None + + +class TestTheVerdictReachesTheDeveloper: + """The detection above is only worth having if `ebuild test` acts on it. + These drive the command itself, so a runner that ran nothing cannot be + reported as a pass by a code path that never consulted `_ran_no_tests`. + + The runner is stubbed and answered by `sys.executable`, not by ctest: + the point under test is what ebuild concludes from a runner's output, and + the three CI platforms do not agree on which runners are installed. + """ + + def _project(self, tmp_path): + (tmp_path / "build.yaml").write_text(yaml.safe_dump({ + "project": {"name": "node", "version": "1.0.0"}, + "workspace": {"backend": "ninja", "build_dir": "build"}, + "toolchain": {"target": "host"}, + "targets": [{"name": "node", "type": "executable", + "sources": ["src/main.c"]}], + }), encoding="utf-8") + return tmp_path + + def _run(self, tmp_path, monkeypatch, *, name, output, exit_code=0): + """Invoke `ebuild test` against a runner that prints `output`. + + The output is passed through a file rather than inlined into `-c`. + `ebuild test` echoes the argv it is about to run, so an inlined + string appears in the captured output whether or not the command + echoes what the runner printed -- which is the thing being asserted. + """ + monkeypatch.chdir(self._project(tmp_path)) + canned = tmp_path / "runner_stdout.txt" + canned.write_text(output, encoding="utf-8") + argv = [sys.executable, "-c", + "import sys; sys.stdout.write(" + "open(sys.argv[1], encoding='utf-8').read()); " + f"sys.exit({exit_code!r})", + str(canned)] + monkeypatch.setattr( + "ebuild.cli.commands._resolve_test_runner", + lambda *_a, **_kw: (name, argv, tmp_path)) + return CliRunner().invoke(cli, ["test"]) + + def test_a_run_that_found_nothing_fails_the_command(self, tmp_path, monkeypatch): + """ctest exits 0 when it finds nothing to run. Trusting that status + reports a green suite for a project containing no tests at all.""" + result = self._run(tmp_path, monkeypatch, + name="ctest", output="No tests were found!!!\n") + assert result.exit_code == 1 + assert "without running a single test" in result.output + assert "All tests passed" not in result.output + + def test_a_real_run_reports_the_runner_s_own_counts(self, tmp_path, monkeypatch): + result = self._run( + tmp_path, monkeypatch, name="ctest", + output="100% tests passed, 0 tests failed out of 17\n") + assert result.exit_code == 0 + assert "17 passed" in result.output + + def test_a_runner_with_no_summary_still_gets_a_verdict(self, tmp_path, monkeypatch): + """`make test` prints no standard summary. The exit status carries the + verdict; inventing a count would be worse than reporting none.""" + result = self._run(tmp_path, monkeypatch, + name="make test", output="cc -o t t.c\n./t\n") + assert result.exit_code == 0 + assert "All tests passed." in result.output + assert "passed," not in result.output # no invented numbers + + def test_the_runner_s_output_is_echoed(self, tmp_path, monkeypatch): + """It is captured so the empty-run check can read it; capturing it + without echoing it would take away the failure detail the developer + needs.""" + result = self._run( + tmp_path, monkeypatch, name="ctest", + output="1/2 Test #1: parses_a_header .... Passed\n" + "100% tests passed, 0 tests failed out of 2\n") + assert "parses_a_header" in result.output + + def test_a_failing_run_exits_with_the_runner_s_status(self, tmp_path, monkeypatch): + result = self._run( + tmp_path, monkeypatch, name="ctest", exit_code=8, + output="50% tests passed, 1 tests failed out of 2\n") + assert result.exit_code == 8 + assert "Tests failed" in result.output + + def test_a_silent_runner_does_not_echo_a_blank_line(self, tmp_path, monkeypatch): + """A runner can succeed without printing anything. Echoing an empty + capture would put a stray blank line between the step and the + verdict.""" + result = self._run(tmp_path, monkeypatch, name="make test", output="") + assert result.exit_code == 0 + assert "All tests passed." in result.output + assert "\n\n\n" not in result.output + + def test_a_runner_that_is_not_installed_says_so(self, tmp_path, monkeypatch): + monkeypatch.chdir(self._project(tmp_path)) + monkeypatch.setattr( + "ebuild.cli.commands._resolve_test_runner", + lambda *_a, **_kw: ("ctest", ["definitely-not-a-real-tool"], tmp_path)) + result = CliRunner().invoke(cli, ["test"]) + assert result.exit_code == 1 + assert "not installed" in result.output + + def test_a_project_with_no_runner_at_all_is_refused(self, tmp_path, monkeypatch): + """Not an error to have no tests -- an error to be told they passed.""" + monkeypatch.chdir(self._project(tmp_path)) + result = CliRunner().invoke(cli, ["test"]) + assert result.exit_code == 1 + assert "No test runner found" in result.output diff --git a/tests/unit/test_footprint.py b/tests/unit/test_footprint.py index 3ee44ed4..54508539 100644 --- a/tests/unit/test_footprint.py +++ b/tests/unit/test_footprint.py @@ -14,6 +14,7 @@ ram = data + bss """ +import os import subprocess from pathlib import Path @@ -22,6 +23,8 @@ from ebuild.build.footprint import ( Footprint, FootprintError, + _measure_macho, + _run_size, board_capacity, find_size_tool, format_report, @@ -57,7 +60,9 @@ class TestMeasure: 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") - exe = tmp_path / "m" + # 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) fp = measure(exe) @@ -196,3 +201,158 @@ class TestFormatSize: ]) def test_units(self, n, expected): assert format_size(n) == expected + + +def _completed(returncode=0, stdout="", stderr=""): + return subprocess.CompletedProcess( + args=["size"], returncode=returncode, stdout=stdout, stderr=stderr) + + +class TestSizeToolFailures: + """`_run_size` is the single place every `size` invocation goes through, so + a failure mode it does not convert into FootprintError escapes as a bare + OSError and takes down a build that otherwise succeeded.""" + + def test_a_tool_that_cannot_be_executed_becomes_a_footprint_error( + self, tmp_path, monkeypatch): + def boom(*_a, **_kw): + raise OSError("Exec format error") + + monkeypatch.setattr(subprocess, "run", boom) + with pytest.raises(FootprintError, match="Exec format error"): + _run_size("size", [], tmp_path / "app") + + def test_a_tool_that_hangs_becomes_a_footprint_error( + self, tmp_path, monkeypatch): + """The 60 s timeout exists so a wedged `size` cannot hang the build; + the expiry has to arrive as a FootprintError like every other + failure, or the timeout just changes which exception escapes.""" + def hang(*_a, **_kw): + raise subprocess.TimeoutExpired(cmd="size", timeout=60) + + monkeypatch.setattr(subprocess, "run", hang) + with pytest.raises(FootprintError, match="failed on"): + _run_size("size", [], tmp_path / "app") + + def test_a_nonzero_exit_reports_the_tool_s_own_message( + self, tmp_path, monkeypatch): + monkeypatch.setattr( + subprocess, "run", + lambda *_a, **_kw: _completed(1, stderr="not an object file")) + with pytest.raises(FootprintError, match="not an object file"): + _run_size("size", [], tmp_path / "app") + + +#: Real `size -m` output, from a host binary with 4 bytes of initialised data +#: and a 4 KB zero-initialised buffer. Reproduced verbatim rather than +#: measured, because the parsing has to be exercised on the Linux and Windows +#: legs too -- where no Mach-O binary can be produced and every line of this +#: parser would otherwise go unrun. +_MACHO_SIZE_M = """\ +Segment __PAGEZERO: 4294967296 (zero fill) +Segment __TEXT: 16384 +\tSection __text: 36 +\tSection __unwind_info: 88 +\ttotal 124 +Segment __DATA: 16384 +\tSection __data: 4 +\tSection __bss: 4096 (zerofill) +\ttotal 4100 +Segment __LINKEDIT: 16384 +total 4295016448 +""" + + +class TestMachOSections: + """Apple's size(1) reports segments, which are page-aligned: __TEXT reads + as 16 KB for the 124 bytes below. Only the section numbers mean what the + Berkeley columns mean, and the two tools must not disagree.""" + + def _measured(self, monkeypatch, stdout): + monkeypatch.setattr(subprocess, "run", + lambda *_a, **_kw: _completed(stdout=stdout)) + return _measure_macho("size", Path("app")) + + def test_sections_land_in_the_region_that_holds_them(self, monkeypatch): + fp = self._measured(monkeypatch, _MACHO_SIZE_M) + assert fp.text == 36 + 88 + assert fp.data == 4 # __data: stored in the image and copied to RAM + assert fp.bss == 4096 # zerofill: RAM only + + def test_the_page_aligned_segment_totals_are_not_the_answer(self, monkeypatch): + """Reading "Segment __TEXT: 16384" would report 16 KB of flash for + 124 bytes of code, and disagree with `size` on the same binary.""" + fp = self._measured(monkeypatch, _MACHO_SIZE_M) + assert fp.flash == 128 + assert 16384 not in (fp.text, fp.data, fp.bss) + + def test_linkedit_and_pagezero_are_not_charged_to_the_image(self, monkeypatch): + """__LINKEDIT holds symbol tables stripped at load and __PAGEZERO is + an unmapped guard region. Neither costs the device anything, and + __PAGEZERO alone would add 4 GB of "data".""" + fp = self._measured(monkeypatch, _MACHO_SIZE_M + ( + "Segment __LINKEDIT: 16384\n" + "\tSection __symtab: 900\n" + )) + assert fp.data == 4 + + def test_a_non_text_segment_is_charged_to_data(self, monkeypatch): + """__DATA_CONST is neither __TEXT nor zerofill: it is stored in the + image, so it is flash, and dropping it would understate the binary.""" + fp = self._measured(monkeypatch, ( + "Segment __DATA_CONST: 16384\n" + "\tSection __const: 256\n" + )) + assert fp.data == 256 + assert fp.text == 0 + + def test_output_with_no_sections_raises_rather_than_reporting_zero( + self, monkeypatch): + """A parser that stopped matching would otherwise report a firmware + image of 0 bytes, which reads as a successful measurement.""" + with pytest.raises(FootprintError, match="could not read any section"): + self._measured(monkeypatch, "Segment __TEXT: 16384\ntotal 16384\n") + + +class TestOutputFormatDispatch: + """Which parser runs is decided by the output, not by the host platform, + so both branches are reachable from either.""" + + def _measure_with(self, tmp_path, monkeypatch, stdout): + artifact = tmp_path / "app" + artifact.write_bytes(b"\x7fELF") + monkeypatch.setattr(subprocess, "run", + lambda *_a, **_kw: _completed(stdout=stdout)) + return measure(artifact, size_tool="size") + + def test_berkeley_columns_are_read_as_text_data_bss(self, tmp_path, monkeypatch): + fp = self._measure_with(tmp_path, monkeypatch, ( + " text\t data\t bss\t dec\t hex\tfilename\n" + " 1720\t 600\t 4096\t 6416\t 1910\tapp\n" + )) + assert (fp.text, fp.data, fp.bss) == (1720, 600, 4096) + + def test_the_apple_column_header_routes_to_the_section_parser( + self, tmp_path, monkeypatch): + """Apple's header row matches _SIZE_LINE too, and its __OBJC column -- + always 0 -- was being read as bss. Every footprint measured on macOS + reported RAM with no zero-initialised data in it.""" + columns = ("__TEXT\t__DATA\t__OBJC\tothers\tdec\thex\n" + "16384\t16384\t0\t4294983680\t4295016448\t10000c000\n") + + def size(argv, **_kw): + # The second call is the `-m` re-run the header should trigger. + return _completed(stdout=_MACHO_SIZE_M if "-m" in argv else columns) + + artifact = tmp_path / "app" + artifact.write_bytes(b"\x7fELF") + monkeypatch.setattr(subprocess, "run", size) + + fp = measure(artifact, size_tool="size") + assert fp.bss == 4096, "the __OBJC column was read as bss" + assert fp.flash == 128 + + def test_unparseable_output_raises_rather_than_reporting_zero( + self, tmp_path, monkeypatch): + with pytest.raises(FootprintError, match="could not read a size line"): + self._measure_with(tmp_path, monkeypatch, "size: bad file\n") diff --git a/tests/unit/test_golden_path_commands.py b/tests/unit/test_golden_path_commands.py index 7282d1ca..a6a0ee22 100644 --- a/tests/unit/test_golden_path_commands.py +++ b/tests/unit/test_golden_path_commands.py @@ -17,6 +17,7 @@ from click.testing import CliRunner from ebuild.cli.commands import ( + _board_config, _cached_repo_libraries, _record_board_selection, _resolve_test_runner, @@ -84,6 +85,48 @@ def test_board_without_a_project_is_an_error(self, tmp_path, monkeypatch): assert not Path("eos.yaml").exists() +@pytest.mark.ebuild +class TestProjectBoardConfig: + """A family spans several densities -- an STM32F407 has 1 MB of flash and + an STM32F401 has 256 KB -- so a project that states its own numbers must + not be measured against the reference part for its family.""" + + def test_no_board_yaml_is_not_an_error(self, tmp_path, monkeypatch): + """Most projects ship none, and the reference table covers them.""" + monkeypatch.chdir(tmp_path) + assert _board_config() is None + + def test_the_project_s_own_numbers_are_read_back(self, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + Path("board.yaml").write_text( + yaml.safe_dump({"memory": {"flash": "256K", "ram": "64K"}}), + encoding="utf-8") + assert _board_config() == {"memory": {"flash": "256K", "ram": "64K"}} + + def test_unparseable_yaml_falls_back_instead_of_failing_the_build( + self, tmp_path, monkeypatch): + """The footprint report is a convenience. Failing a build that linked + over a malformed board.yaml would be worse than the silence it + replaces, so this degrades to the reference table.""" + monkeypatch.chdir(tmp_path) + Path("board.yaml").write_text("memory: {flash: [unclosed\n", + encoding="utf-8") + assert _board_config() is None + + def test_a_yaml_document_that_is_not_a_mapping_is_ignored( + self, tmp_path, monkeypatch): + """`board_capacity` indexes into it, so a list or a bare string would + raise out of a build that had already succeeded.""" + monkeypatch.chdir(tmp_path) + Path("board.yaml").write_text("- stm32f4\n", encoding="utf-8") + assert _board_config() is None + + def test_an_empty_board_yaml_is_ignored(self, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + Path("board.yaml").write_text("", encoding="utf-8") + assert _board_config() is None + + @pytest.mark.ebuild class TestTestTargetType: """`test` is a distinct target type so `ebuild test` knows which binaries diff --git a/tests/unit/test_ninja_backend.py b/tests/unit/test_ninja_backend.py index e3c02b5d..c0dd0090 100644 --- a/tests/unit/test_ninja_backend.py +++ b/tests/unit/test_ninja_backend.py @@ -81,9 +81,13 @@ def test_static_library_unaffected(self): self.assertNotIn(": link ", edge) -class TestNinjaPathEscaping(unittest.TestCase): +class TestNinjaPathEscapingContract(unittest.TestCase): """Ninja splits build statements on unescaped spaces and colons. + Renamed from TestNinjaPathEscaping: a second class of that name is + defined further down this file, and the later definition replaced this + one, so none of these four ever ran. + A Windows absolute path puts a drive-letter colon into the output field, so Ninja read the statement as a rule separator and rejected every generated file with "expected build command name" -- the backend produced no usable @@ -92,15 +96,15 @@ class TestNinjaPathEscaping(unittest.TestCase): """ def test_colons_and_spaces_in_paths_are_escaped(self): - self.assertEqual(_ninja_path(r"C:\build\main.o"), r"C$:\build\main.o") - self.assertEqual(_ninja_path("/tmp/my project/main.o"), "/tmp/my$ project/main.o") + self.assertEqual(escape_ninja_path(r"C:\build\main.o"), r"C$:\build\main.o") + self.assertEqual(escape_ninja_path("/tmp/my project/main.o"), "/tmp/my$ project/main.o") def test_dollar_is_escaped_before_the_escapes_it_introduces(self): - self.assertEqual(_ninja_path("a$b"), "a$$b") - self.assertEqual(_ninja_path("a$b:c"), "a$$b$:c") + self.assertEqual(escape_ninja_path("a$b"), "a$$b") + self.assertEqual(escape_ninja_path("a$b:c"), "a$$b$:c") def test_ordinary_posix_paths_are_unchanged(self): - self.assertEqual(_ninja_path("/tmp/build/obj/app/src/main.o"), + self.assertEqual(escape_ninja_path("/tmp/build/obj/app/src/main.o"), "/tmp/build/obj/app/src/main.o") def test_every_build_statement_has_one_unescaped_colon(self): diff --git a/tests/unit/test_package_efw.py b/tests/unit/test_package_efw.py index 7d6bc0b1..91d2be2c 100644 --- a/tests/unit/test_package_efw.py +++ b/tests/unit/test_package_efw.py @@ -18,6 +18,7 @@ import os import stat +import sys import pytest import yaml @@ -33,11 +34,38 @@ from ebuild.cli.commands import cli -def _fake_efwtool(tmp_path, body): - """A stand-in for efwtool, so the wiring is testable without building C.""" - tool = tmp_path / "efwtool" - tool.write_text("#!/bin/sh\n" + body) - tool.chmod(tool.stat().st_mode | stat.S_IEXEC) +def _fake_efwtool(tmp_path, *, exit_code=0, stderr="", writes_output=False, + records_argv=False): + """A stand-in for efwtool, so the wiring is testable without building C. + + Driven through sys.executable rather than written as a /bin/sh script: + Windows cannot execute one, so every test here failed there with + "WinError 193: %1 is not a valid Win32 application" as soon as the + `package` command was registered and these tests began running. + """ + script = tmp_path / "_efwtool_impl.py" + script.write_text( + "import pathlib, sys\n" + f"if {records_argv!r}:\n" + " pathlib.Path(__file__).with_name('argv.txt').write_text(\n" + " ' '.join(sys.argv[1:]), encoding='utf-8')\n" + f"if {writes_output!r}:\n" + " pathlib.Path(sys.argv[3]).touch()\n" + f"sys.stderr.write({stderr!r})\n" + f"sys.exit({exit_code!r})\n", + encoding="utf-8", + ) + if os.name == "nt": + tool = tmp_path / "efwtool.bat" + tool.write_text( + f'@echo off\r\n"{sys.executable}" "{script}" %*\r\n', encoding="utf-8" + ) + else: + tool = tmp_path / "efwtool" + tool.write_text( + f'#!/bin/sh\nexec "{sys.executable}" "{script}" "$@"\n', encoding="utf-8" + ) + tool.chmod(tool.stat().st_mode | stat.S_IEXEC) return tool @@ -79,28 +107,28 @@ def test_it_gives_the_build_command_when_the_checkout_exists(self, tmp_path): class TestPack: def test_a_missing_artifact_is_refused_before_the_tool_runs(self, tmp_path): - tool = _fake_efwtool(tmp_path, "exit 0\n") + tool = _fake_efwtool(tmp_path) with pytest.raises(FirmwareImageError, match="no artifact"): pack(tool, tmp_path / "nope", tmp_path / "out.efw") def test_a_tool_that_writes_nothing_is_caught(self, tmp_path): """efwtool exiting 0 without producing a file would otherwise be reported as a successful package.""" - tool = _fake_efwtool(tmp_path, "exit 0\n") + tool = _fake_efwtool(tmp_path) payload = tmp_path / "app" payload.write_bytes(b"\x7fELF") with pytest.raises(FirmwareImageError, match="wrote no image"): pack(tool, payload, tmp_path / "out.efw") def test_a_failing_tool_surfaces_its_own_message(self, tmp_path): - tool = _fake_efwtool(tmp_path, 'echo "bad magic" >&2\nexit 3\n') + tool = _fake_efwtool(tmp_path, stderr="bad magic\n", exit_code=3) payload = tmp_path / "app" payload.write_bytes(b"x") with pytest.raises(FirmwareImageError, match="bad magic"): pack(tool, payload, tmp_path / "out.efw") def test_version_and_addresses_reach_the_tool(self, tmp_path): - tool = _fake_efwtool(tmp_path, 'echo "$@" > "$(dirname "$0")/argv.txt"\ntouch "$3"\n') + tool = _fake_efwtool(tmp_path, records_argv=True, writes_output=True) payload = tmp_path / "app" payload.write_bytes(b"x") pack(tool, payload, tmp_path / "out.efw", version="2.1.0", @@ -113,7 +141,7 @@ def test_version_and_addresses_reach_the_tool(self, tmp_path): def test_addresses_are_omitted_when_not_given(self, tmp_path): """A host build has no load address, and passing an empty one would make efwtool reject the call.""" - tool = _fake_efwtool(tmp_path, 'echo "$@" > "$(dirname "$0")/argv.txt"\ntouch "$3"\n') + tool = _fake_efwtool(tmp_path, records_argv=True, writes_output=True) payload = tmp_path / "app" payload.write_bytes(b"x") pack(tool, payload, tmp_path / "out.efw") @@ -171,3 +199,126 @@ def test_it_says_how_to_get_efwtool(self, tmp_path, monkeypatch): result = CliRunner().invoke(cli, ["package"]) assert result.exit_code == 1 assert "ebuild setup" in result.output + + +def _efwtool_that_packs(tmp_path, *, verdict="efw v1 node 4128 bytes OK"): + """A stand-in that answers both subcommands `package` drives. + + `_fake_efwtool` above writes argv[3] unconditionally, which `verify` -- + invoked as `efwtool verify ` -- does not have. The command runs + pack and then verify against the same binary, so a fake that serves only + one of them cannot exercise the path between them. + """ + script = tmp_path / "_efwtool_both.py" + script.write_text( + "import pathlib, sys\n" + "if sys.argv[1] == 'pack':\n" + " pathlib.Path(sys.argv[3]).write_bytes(b'EFW0' + b'\\x00' * 60)\n" + f"elif sys.argv[1] == 'verify':\n" + f" sys.stdout.write({verdict!r} + '\\n')\n" + "sys.exit(0)\n", + encoding="utf-8", + ) + if os.name == "nt": + tool = tmp_path / "efwtool_both.bat" + tool.write_text( + f'@echo off\r\n"{sys.executable}" "{script}" %*\r\n', encoding="utf-8") + else: + tool = tmp_path / "efwtool_both" + tool.write_text( + f'#!/bin/sh\nexec "{sys.executable}" "{script}" "$@"\n', encoding="utf-8") + tool.chmod(tool.stat().st_mode | stat.S_IEXEC) + return tool + + +class TestCommandPacks: + """The refusals above are the cheap half. These cover the path a developer + actually takes -- a built artifact, a tool on PATH, an image on disk.""" + + def _project(self, tmp_path): + (tmp_path / "build.yaml").write_text(yaml.safe_dump({ + "project": {"name": "node", "version": "2.1.0"}, + "workspace": {"backend": "ninja", "build_dir": "build"}, + "toolchain": {"target": "host"}, + "targets": [{"name": "node", "type": "executable", + "sources": ["src/main.c"]}], + })) + (tmp_path / "_build").mkdir() + (tmp_path / "_build" / "node").write_bytes(b"\x7fELF" + b"\x00" * 64) + return tmp_path + + def _packaged(self, tmp_path, monkeypatch, argv=(), **tool_kw): + tool = _efwtool_that_packs(tmp_path, **tool_kw) + monkeypatch.chdir(self._project(tmp_path)) + monkeypatch.setattr("ebuild.build.firmware_image.shutil.which", + lambda n: str(tool) if n == "efwtool" else None) + return CliRunner().invoke(cli, ["package", *argv]) + + def test_it_writes_the_image_and_reports_its_size(self, tmp_path, monkeypatch): + result = self._packaged(tmp_path, monkeypatch) + assert result.exit_code == 0, result.output + image = tmp_path / "node.efw" + assert image.is_file() + assert str(image.stat().st_size) in result.output + + 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.""" + result = self._packaged(tmp_path, monkeypatch) + assert result.exit_code == 0, result.output + assert (tmp_path / "node.efw").is_file() + + def test_output_overrides_the_default_path(self, tmp_path, monkeypatch): + result = self._packaged(tmp_path, monkeypatch, + argv=["--output", "dist.efw"]) + assert result.exit_code == 0, result.output + assert (tmp_path / "dist.efw").is_file() + assert not (tmp_path / "node.efw").exists() + + def test_the_verify_verdict_is_shown(self, tmp_path, monkeypatch): + """Packing and verifying with the same tool does not prove the format + is right, but the developer is entitled to see that the file on disk + parses at all.""" + result = self._packaged(tmp_path, monkeypatch, + verdict="header ok\npayload crc ok") + assert result.exit_code == 0, result.output + assert "header ok" in result.output + assert "payload crc ok" in result.output + + def test_it_says_how_to_inspect_the_result(self, tmp_path, monkeypatch): + result = self._packaged(tmp_path, monkeypatch) + assert "inspect" in result.output + + def test_a_tool_failure_is_reported_rather_than_raised(self, tmp_path, monkeypatch): + """FirmwareImageError has to become an error message and exit 1. An + uncaught one would print a stack trace over a build that succeeded.""" + tool = _fake_efwtool(tmp_path, stderr="bad magic\n", exit_code=3) + monkeypatch.chdir(self._project(tmp_path)) + 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 == 1 + assert "bad magic" in result.output + assert "Traceback" not in result.output + + +class TestCommandConfigErrors: + """`package` is reachable from any directory, so it has to say which one + it is standing in rather than raising FileNotFoundError at the developer.""" + + def test_no_build_yaml_names_the_file(self, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + result = CliRunner().invoke(cli, ["package"]) + assert result.exit_code == 1 + assert "build.yaml" in result.output + assert "Traceback" not in result.output + + def test_a_malformed_build_yaml_is_reported_as_a_config_error( + self, tmp_path, monkeypatch): + (tmp_path / "build.yaml").write_text("project: [not, a, mapping]\n", + encoding="utf-8") + monkeypatch.chdir(tmp_path) + result = CliRunner().invoke(cli, ["package"]) + assert result.exit_code == 1 + assert "Configuration error" in result.output + assert "Traceback" not in result.output