From 187365d005d5b84fab8efa86c3663b39ae293fe1 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 1 Sep 2026 15:47:00 +0530 Subject: [PATCH] fix(cli): restore implementation hunks dropped by merge resolutions `import ebuild.cli.commands` raises NameError on master, so the `ebuild` command does not start and pytest cannot collect nine test modules -- the suite reports "9 errors during collection" and runs zero tests. Four PRs landed their tests while their implementation was discarded during conflict resolution, and the merges that did it were not gated by a run: f5209ac, current master, has no CI runs at all. #64 (6a22e36) tests/unit/test_golden_path_commands.py landed; _workspace_repo_paths, _cached_repo_libraries, _record_board_selection, _EOS_PROJECT_CONFIG and the `configure --board` option did not. #89 (32348f0) commands.py gained five `re.` uses, no `import re`. #90 (d3958f7) the initramfs test kept its assertions but lost its `_newc_members` helper and its `os` / `stat` imports. #92 registry.version_key was renamed version_sort_key without updating its caller in the tests. Restoring them, taken verbatim from the branches that were merged: * commands.py: `import re`; `_workspace_repo_paths`, `_cached_repo_libraries`, `_record_board_selection`, `_EOS_PROJECT_CONFIG`; the `--board` option on `configure` and the call that persists it; and the `{**_workspace_repo_paths(), **...}` merge at both `_install_packages` call sites. Without that last hunk a scaffolded project that `use`s eos or eboot gets no include or link paths from ~/.ebuild/repos, which is the "fatal error: eos/hal.h: No such file or directory" that #64 fixed. Without the option, the documented golden path -- `configure --board` then a bare `build` -- fails with "no such option: --board". * the initramfs test: `os` / `stat` imports and the `_newc_members` newc parser the assertions call. * test_package_registry.py: import the name the module now exports. Collection goes from 9 errors / 0 tests to 446 passing. The 12 that still fail are pre-existing and out of scope here: `doctor` (#72) and `package` (#77) are two more features whose implementations the same merges dropped and which should be restored by their authors, and the rest are separate bugs -- see the PR description. Co-Authored-By: Claude Opus 5 (1M context) --- ebuild/cli/commands.py | 148 +++++++++++++++++- .../test_integration_initramfs_security.py | 40 +++++ tests/unit/test_package_registry.py | 12 +- 3 files changed, 191 insertions(+), 9 deletions(-) diff --git a/ebuild/cli/commands.py b/ebuild/cli/commands.py index 442adc71..82d877e1 100644 --- a/ebuild/cli/commands.py +++ b/ebuild/cli/commands.py @@ -10,6 +10,7 @@ from __future__ import annotations import os +import re import shutil import subprocess import threading @@ -187,6 +188,95 @@ def note_skipped(name: str, _cause: BaseException) -> None: return package_paths +def _workspace_repo_paths() -> Dict[str, PackagePaths]: + """Include paths for the eos and eboot repos that `ebuild setup` cloned. + + A scaffolded project includes , but the generated build.yaml + carried no path to the headers, so every template failed with + "fatal error: eos/hal.h: No such file or directory" on the first build. + + These are resolved at build time from the cache rather than written into + build.yaml as absolute paths: the path is a fact about this machine, and + build.yaml is a file the developer commits. + + Returns an empty mapping when the cache is absent, so the error a developer + sees stays the missing header rather than a stack trace, and `ebuild setup` + remains the fix. + """ + from ebuild.deps import EBUILD_REPOS_DIR + + paths: Dict[str, PackagePaths] = {} + for name in ("eos", "eboot"): + root = Path(EBUILD_REPOS_DIR) / name + if not root.is_dir(): + continue + # Headers sit at two depths: kernel/include, hal/include ... and + # services/crypto/include, services/ota/include. Both are needed -- + # and live only in the deeper set. + include_dirs = sorted( + {p for pattern in ("include", "*/include", "*/*/include") + for p in root.glob(pattern) if p.is_dir()} + ) + if include_dirs: + lib_dirs, libraries = _cached_repo_libraries(root) + paths[name] = PackagePaths( + include_dirs=include_dirs, + lib_dirs=lib_dirs, + libraries=libraries, + ) + return paths + + +# Where `ebuild` puts the CMake build tree for a cached repo. Kept inside the +# clone so `ebuild setup` remains the only thing that owns ~/.ebuild/repos. +_REPO_BUILD_DIRNAME = "_ebuild" + + +def _cached_repo_libraries(root: Path) -> Tuple[List[Path], List[str]]: + """Static libraries a cached repo offers to projects that `use` it. + + Headers alone are not enough: a scaffolded project compiles against + and then fails at the link step with undefined references. + The repo is a CMake project with no install() rules, so there is nothing + to point a -L at until it has been built once. Build it on demand and + cache the result; subsequent builds reuse the tree. + + Returns ([], []) when the repo cannot be built here — a missing cmake, a + repo that is not a CMake project — so the developer still gets a link + error naming the symbol rather than a stack trace from ebuild. + """ + if not (root / "CMakeLists.txt").is_file(): + return [], [] + + build_dir = root / _REPO_BUILD_DIRNAME + archives = sorted(build_dir.rglob("*.a")) if build_dir.is_dir() else [] + + if not archives: + if shutil.which("cmake") is None: + return [], [] + try: + subprocess.run( + ["cmake", "-S", str(root), "-B", str(build_dir)], + check=True, capture_output=True, timeout=600, + ) + subprocess.run( + ["cmake", "--build", str(build_dir), "-j", str(os.cpu_count() or 1)], + check=True, capture_output=True, timeout=1800, + ) + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError): + return [], [] + archives = sorted(build_dir.rglob("*.a")) + + if not archives: + return [], [] + + # -L one directory per archive location; -l the archive basenames with + # the lib prefix and .a suffix stripped, which is what the linker wants. + lib_dirs = sorted({a.parent for a in archives}) + libraries = [a.stem[3:] for a in archives if a.stem.startswith("lib")] + return lib_dirs, libraries + + def _detect_libraries(lib_dir: Path, pkg_name: str) -> List[str]: """Detect installed library names from a lib/ directory.""" if not lib_dir.exists(): @@ -226,6 +316,46 @@ def _resolve_backend_request( return resolved_backend, backend_config +# The project-local file that records which board this checkout targets. +_EOS_PROJECT_CONFIG = "eos.yaml" + + +def _record_board_selection(board: str, log: Logger) -> None: + """Persist ``--board`` into eos.yaml under ``system.board``. + + The golden path is `configure --board` then a bare `build`, so the choice + has to outlive the configure process. It is written to eos.yaml rather + than build.yaml because the board is a property of the system being + targeted, which is what eos.yaml already describes. + """ + path = Path(_EOS_PROJECT_CONFIG) + if not path.is_file(): + log.error( + f"No {_EOS_PROJECT_CONFIG} here, so there is nothing to record the " + f"board against. Run this from a project directory created by " + f"'ebuild new'." + ) + raise SystemExit(1) + + import yaml + + try: + data = yaml.safe_load(path.read_text(encoding="utf-8")) or {} + except yaml.YAMLError as e: + log.error(f"{_EOS_PROJECT_CONFIG} is not valid YAML: {e}") + raise SystemExit(1) + + system = data.setdefault("system", {}) + previous = system.get("board") + system["board"] = board + path.write_text(yaml.safe_dump(data, sort_keys=False), encoding="utf-8") + + if previous and previous != board: + log.info(f"Board: {previous} -> {board}") + else: + log.info(f"Board: {board}") + + def _selected_board(default: str = "generic") -> str: """The board this project targets, from its eos.yaml. @@ -366,7 +496,8 @@ def _configure_ninja_backend( log.step("Resolving toolchain...") compiler = resolve_toolchain(cfg.toolchain) - package_paths = _install_packages(cfg, build_path, log, verbose=log.verbose) + package_paths = {**_workspace_repo_paths(), + **_install_packages(cfg, build_path, log, verbose=log.verbose)} log.step(f"Generating build.ninja in {build_path}/...") ninja_backend = NinjaBackend(cfg, build_path, compiler, package_paths=package_paths) @@ -809,7 +940,8 @@ def build(log: Logger, config_path: str, build_dir: str, backend: Optional[str], log.debug(f"Compiler: {compiler.cc}") # Install packages if any are declared - package_paths = _install_packages(cfg, build_path, log, verbose=log.verbose, jobs=jobs) + package_paths = {**_workspace_repo_paths(), + **_install_packages(cfg, build_path, log, verbose=log.verbose, jobs=jobs)} log.step(f"Generating build.ninja in {build_path}/...") ninja_backend = NinjaBackend(cfg, build_path, compiler, package_paths=package_paths) @@ -998,12 +1130,22 @@ def clean(log: Logger, build_dir: str) -> None: type=click.Choice(["auto", "cmake", "make", "meson", "cargo", "ninja", "kbuild"]), help="Force a specific build backend.", ) +@click.option( + "--board", + default=None, + help="Target board name (e.g., stm32f4, nrf52). Recorded in the project " + "config so later `ebuild build` / `flash` / `monitor` use it.", +) @click.pass_obj -def configure(log: Logger, config_path: str, build_dir: str, backend: Optional[str]) -> None: +def configure(log: Logger, config_path: str, build_dir: str, backend: Optional[str], + board: Optional[str]) -> None: """Generate build files without building.""" log.header("ebuild — Configure") try: + if board: + _record_board_selection(board, log) + log.step("Loading configuration...") cfg = load_config(config_path) log.info(f"Project: {cfg.name} v{cfg.version}") diff --git a/tests/ebuild/test_integration_initramfs_security.py b/tests/ebuild/test_integration_initramfs_security.py index 78cd916b..3dff3a7e 100644 --- a/tests/ebuild/test_integration_initramfs_security.py +++ b/tests/ebuild/test_integration_initramfs_security.py @@ -21,7 +21,9 @@ """ import gzip +import os import shutil +import stat import subprocess import pytest @@ -40,6 +42,44 @@ @requires_cpio +def _newc_members(data): + """Parse enough of ``newc`` to validate names, metadata, and contents.""" + members = {} + offset = 0 + while True: + header = data[offset:offset + 110] + assert len(header) == 110 + assert header[:6] == b"070701" + fields = [int(header[i:i + 8], 16) for i in range(6, 110, 8)] + inode = fields[0] + mode = fields[1] + link_count = fields[4] + file_size = fields[6] + name_size = fields[11] + + offset += 110 + encoded_name = data[offset:offset + name_size] + assert encoded_name.endswith(b"\0") + name = os.fsdecode(encoded_name[:-1]) + offset += name_size + offset += -offset % 4 + + contents = data[offset:offset + file_size] + assert len(contents) == file_size + offset += file_size + offset += -offset % 4 + members[name] = { + "inode": inode, + "mode": mode, + "link_count": link_count, + "contents": contents, + } + + if name == "TRAILER!!!": + return members + + + def test_create_initramfs_produces_valid_gzip_with_expected_content(tmp_path): """Functional regression: the pipeline must still work correctly.""" rootfs = tmp_path / "rootfs" diff --git a/tests/unit/test_package_registry.py b/tests/unit/test_package_registry.py index 96a715bf..cba5fc50 100644 --- a/tests/unit/test_package_registry.py +++ b/tests/unit/test_package_registry.py @@ -12,7 +12,7 @@ import pytest -from ebuild.packages.registry import PackageRegistry, version_key +from ebuild.packages.registry import PackageRegistry, version_sort_key def _recipe(name: str, version: str): @@ -31,7 +31,7 @@ class TestVersionKey: def test_numeric_versions_compare_numerically(self): """Plain string ordering would put 11.1.0 below 2.9.3.""" versions = ["2.9.3", "11.1.0", "1.3.1"] - assert sorted(versions, key=version_key) == ["1.3.1", "2.9.3", "11.1.0"] + assert sorted(versions, key=version_sort_key) == ["1.3.1", "2.9.3", "11.1.0"] @pytest.mark.parametrize( "version", @@ -40,18 +40,18 @@ def test_numeric_versions_compare_numerically(self): ) def test_non_numeric_versions_do_not_raise(self, version): """Any of these previously raised ValueError from int().""" - assert version_key(version) + assert version_sort_key(version) def test_pre_release_sorts_below_its_release(self): """Semver precedence: 3.6.0-rc1 comes before 3.6.0.""" - assert version_key("3.6.0-rc1") < version_key("3.6.0") + assert version_sort_key("3.6.0-rc1") < version_sort_key("3.6.0") def test_letter_suffix_sorts_above_its_base(self): """A patch respin such as 1.2.11b supersedes 1.2.11.""" - assert version_key("1.2.11") < version_key("1.2.11b") + assert version_sort_key("1.2.11") < version_sort_key("1.2.11b") def test_shorter_version_sorts_below_longer(self): - assert version_key("1.2") < version_key("1.2.1") + assert version_sort_key("1.2") < version_sort_key("1.2.1") class TestRegistryVersionSelection: