diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..b828a4a3 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,8 @@ +# YAML is linted with yamllint's default new-lines rule (LF). The Windows +# runners check out with core.autocrlf=true, which turned every LF file into +# CRLF and failed the rule on line 1 of the first file -- except a file that +# already carried a stray CR, which autocrlf leaves alone. Pin the encoding +# instead of guessing at the checkout's, so the same bytes are linted on +# every OS. +*.yml text eol=lf +*.yaml text eol=lf diff --git a/.github/workflows/auto-assign.yml b/.github/workflows/auto-assign.yml index 60639c32..5f438f5a 100644 --- a/.github/workflows/auto-assign.yml +++ b/.github/workflows/auto-assign.yml @@ -30,4 +30,4 @@ jobs: repo: context.repo.repo, issue_number: context.payload.pull_request.number, assignees: [context.payload.pull_request.user.login] - }); + }); diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index e8507e51..a201167d 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -21,14 +21,14 @@ jobs: security-events: write id-token: write steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: persist-credentials: false - - uses: ossf/scorecard-action@v2.4.0 + - uses: ossf/scorecard-action@4eaacf0543bb3f2c246792bd56e8cdeffafb205a # v2.4.3 with: results_file: results.sarif results_format: sarif publish_results: true - - uses: github/codeql-action/upload-sarif@v3 + - uses: github/codeql-action/upload-sarif@faaca9a8f6edddba5725ffe5adefdab6669a2eca # v3 with: sarif_file: results.sarif diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ba01756..c8106c3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,27 @@ `cjson` (v1.7.18), `nanopb` (v0.4.9.1), `lvgl` (v9.2.2), `tinyusb` (v0.18.0), and `unity` (v2.6.1). ### Fixed +- **`ebuild.lock` is read, not only written.** `_install_packages` created the `Lockfile` after resolution and only ever saved it, and nothing called `Lockfile.load()`, so the lock pinned nothing: an unpinned package resolved to the newest recipe on every machine and every run, and a stale `ebuild.lock` sat beside it claiming otherwise. The lock is now loaded before resolving and handed to `PackageResolver.resolve(requested, lockfile=...)`, which uses it as a pin for every package the request leaves open. Precedence is request, then lock, then newest. A locked version no recipe provides is an error (`ebuild.lock pins 'zlib' at v9.9.9 ...`), not a silent fallback, and a locked entry whose recorded URL or checksum no longer matches the recipe of that version is refused -- the version reproduces the name, the lock exists to reproduce the bytes. An explicit request outranks the lock and the lock is rewritten from the result, as before. `Lockfile.load()` drops malformed entries instead of failing later. A lock that is not valid YAML, or cannot be opened, is reported as a resolution error (`[error] ... ebuild.lock is not valid YAML: ...`) instead of a `yaml.scanner.ScannerError` traceback; the recorded `build` system is compared on read-back like `url` and `checksum` (`Lockfile.CHECKED_FIELDS` is the one list both sides use); and `docs/dependency-management.md` documents the file, its precedence, and how to upgrade. (`ebuild/packages/resolver.py`, `ebuild/packages/lockfile.py`, `ebuild/cli/commands.py`, `docs/dependency-management.md`) +- **CI on master runs to completion again.** ruff stopped the pipeline at its + first step on findings the merges had introduced (an F811 duplicate import, + W292, E402). `PackageRecipe.to_dict()` -- defined by #111, deleted by #112's + replay of the same file -- is restored; nine `test_index_sync` cases and mypy + had failed without it. It now emits `install_args`, which the original never + did, and hands back copies of its list fields rather than the live lists; + the index sync mapping carries `install_args` through to the cached recipe. + The vendored `core/eos/docs/three-way-alignment.md` is reverted to its pin + (the correction #109 made there is filed upstream as embeddedos-org/eos#149). + The OSSF Scorecard action moved to the ghcr.io-hosted release and is pinned + by commit. yamllint on the Windows legs: YAML is pinned to LF in + `.gitattributes`, so an existing Windows clone needs its files checked out + again once (`git rm --cached -r . && git reset --hard HEAD`, or a re-clone; + see CONTRIBUTING.md). On Python 3.10 and 3.11 `ebuild/plugins/__init__.py` + now type-checks: the `entry_points()` fallback is spelled out with a cast + instead of a `# type: ignore` naming the wrong error code. + (`ebuild/packages/recipe.py`, `ebuild/packages/index_sync.py`, + `ebuild/plugins/__init__.py`, `core/eos/docs/three-way-alignment.md`, + `.github/workflows/scorecard.yml`, `.gitattributes`, `.yamllint.yml`, and + the three lint-fixed test files.) - **`ebuild test` now finds Windows test binaries.** The Ninja edge for a native `type: test` target already carried the platform suffix (`_exe_suffix()` names it `.exe` on Windows), but `ebuild test` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ecda1e24..29dfb670 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,6 +20,14 @@ This certifies you have the right to submit the code under the MIT license. 6. Commit with DCO sign-off: git commit -s 7. Push and create Pull Request +**Windows contributors:** `.gitattributes` pins `*.yml` and `*.yaml` to LF so +yamllint sees the same bytes on every platform. The attribute governs future +checkouts, not files already sitting in a working tree, and `git add +--renormalize .` rewrites only the index, never the files. After pulling that +change, from a clean tree run `git rm --cached -r . && git reset --hard HEAD` +once (or re-clone) so the YAML files are checked out again as LF; otherwise +yamllint will still see CRLF locally. + ## Coding Standards ### C (ISO C11) diff --git a/core/eos/docs/three-way-alignment.md b/core/eos/docs/three-way-alignment.md index 767c720f..7f9c8c15 100644 --- a/core/eos/docs/three-way-alignment.md +++ b/core/eos/docs/three-way-alignment.md @@ -8,7 +8,7 @@ This document tracks alignment between all three EoS components to ensure they r | Dimension | eos | eboot | ebuild | Status | |-----------|-----|-------|--------|--------| -| Board definitions | 84 board YAMLs in `eos/boards/` (upstream) | 83 board dirs / 138 `eboot_add_board()` calls in `eboot/boards/` (upstream, pinned rev) | `TARGET_ARCH` 14 + `MCU_TO_EBOOT_BOARD` 138 in `ebuild/sdk_generator.py` | ⚠️ Unverified: 84 vs 83 vs 14/138 — three inventories describe the same set and nothing cross-checks them; see the resolver drift note in PR #109 | +| Board definitions | 25 YAML files in `eos/boards/` | 25 board ports in `eboot/boards/` | `MCU_TO_EBOOT_BOARD` + `EOS_BOARD_MAP` in project generator | ✅ Aligned | | Product profiles | 41 profiles in `eos/products/*.h` | — | `PRODUCT_MAP` (41 entries) in project generator | ✅ Aligned | | Platform enum | — | 24 `eos_platform_t` entries in `eos_hal.h` | MCU_DATABASE (100+ MCUs) in hw analyzer | ✅ Aligned | | Peripheral keywords | 33 HAL APIs in `hal.h` + `hal_extended.h` | — | `PERIPHERAL_KEYWORDS` (24 types) + `ComponentDB` (200+ parts) | ✅ Aligned | @@ -118,7 +118,7 @@ Customer Input │ LLMClient (optional) ──► deep analysis │ │ │ │ 2. EosProjectGenerator │ -│ MCU_TO_EBOOT_BOARD (alias of ebuild/sdk_generator.py) ► eboot board dir │ +│ MCU_TO_EBOOT_BOARD ──────────────────► eboot board dir │ │ EOS_BOARD_MAP ──────────────────────► eos board YAML │ │ PRODUCT_MAP (41 entries) ──────────► eos product .h │ │ MULTICORE_MCUS ─────────────────────► multicore config │ diff --git a/docs/dependency-management.md b/docs/dependency-management.md index a4588900..b4de84b0 100644 --- a/docs/dependency-management.md +++ b/docs/dependency-management.md @@ -360,6 +360,31 @@ ebuild includes a curated suite of pre-packaged recipes under `recipes/`: | **`unity`** | 2.6.1 | CMake | MIT | Standard embedded C unit testing framework | | **`zlib`** | 1.3.1 | CMake | Zlib | General-purpose lossless data compression | +### `ebuild.lock` — Reproducible Package Resolution + +Every resolution (`ebuild configure`, `ebuild build`, `ebuild package`) writes +`ebuild.lock` next to `build.yaml`, recording for each resolved package the +exact `version`, `url`, `checksum` and `build` system it used — and reads it +back first, so the next resolution lands on the same artifacts: + +- **Precedence:** a version named in `build.yaml` wins; otherwise the version + in `ebuild.lock` wins; only a package in neither is resolved to the newest + recipe. A lock never overrides an explicit request, and an unpinned package + therefore stays put until you move it. +- **Drift is refused, not silently accepted.** If the lock pins a version whose + recipe now has a different `url`, `checksum` or `build`, resolution stops with + an error naming the field, because the version alone would reproduce the + package's name and not its bytes. Delete `ebuild.lock` or pin the version + explicitly if the change is intended. +- **Commit it** for a reproducible build; CI and colleagues then resolve to the + same artifacts you did. +- **To upgrade** a locked package, name the new version in `build.yaml`, or + delete `ebuild.lock` to resolve everything afresh; either way the next + resolution rewrites the lock. +- A lock that is not valid YAML, or cannot be opened, is reported as a + resolution error and stops the build. A lock of the wrong shape (an entry + that is not a mapping) is ignored entry by entry and rewritten. + ### Offline & Air-Gapped Operation For isolated CI/CD pipelines and field deployments: diff --git a/ebuild/cli/commands.py b/ebuild/cli/commands.py index de8928c8..3fe60a00 100644 --- a/ebuild/cli/commands.py +++ b/ebuild/cli/commands.py @@ -40,7 +40,7 @@ from ebuild.packages.builder import BuildError, PackageBuilder from ebuild.packages.cache import PackageCache from ebuild.packages.fetcher import FetchError, PackageFetcher -from ebuild.packages.lockfile import Lockfile +from ebuild.packages.lockfile import Lockfile, LockfileError from ebuild.packages.recipe import RecipeError from ebuild.packages.registry import create_registry, find_recipe_dirs from ebuild.packages.resolver import PackageResolver, ResolveError @@ -81,16 +81,26 @@ def _install_packages( registry = create_registry(*recipe_dirs) log.debug(f"Registry: {registry.package_count} recipes from {[str(p) for p in registry.search_paths]}") + # The lockfile is read before resolving, so an unpinned package lands on + # the version the last resolution recorded rather than on whatever is + # newest now. It was only ever written before, which pinned nothing. + lock_path = cfg.source_dir / Lockfile.FILENAME + lockfile = Lockfile(lock_path) + try: + lockfile.load() + except LockfileError as e: + # A lock that cannot be read is a resolution the user has to settle, + # reported through the handler every caller of this function has. + raise ResolveError(str(e)) from e + if lockfile.package_names: + log.debug(f"Lockfile read: {lock_path} ({len(lockfile.package_names)} packages)") + resolver = PackageResolver(registry) requested = [{"name": p.name, "version": p.version} for p in cfg.packages] - resolved = resolver.resolve(requested) + resolved = resolver.resolve(requested, lockfile=lockfile) log.info(f"Packages to install: {', '.join(r.name + ' v' + r.version for r in resolved)}") - # Lockfile - lock_path = cfg.source_dir / Lockfile.FILENAME - lockfile = Lockfile(lock_path) - # Cache and fetcher pkg_cache_dir = build_dir / "packages" cache = PackageCache(pkg_cache_dir) diff --git a/ebuild/packages/index_sync.py b/ebuild/packages/index_sync.py index 42f417f5..f273935e 100644 --- a/ebuild/packages/index_sync.py +++ b/ebuild/packages/index_sync.py @@ -343,6 +343,7 @@ def sync( "dependencies": entry.get("dependencies", []), "configure_args": entry.get("configure_args", []), "build_args": entry.get("build_args", []), + "install_args": entry.get("install_args", []), "patches": entry.get("patches", []), } diff --git a/ebuild/packages/lockfile.py b/ebuild/packages/lockfile.py index e6778ee0..a16f6b69 100644 --- a/ebuild/packages/lockfile.py +++ b/ebuild/packages/lockfile.py @@ -17,6 +17,16 @@ from ebuild.packages.recipe import PackageRecipe +class LockfileError(Exception): + """``ebuild.lock`` exists but could not be read. + + Distinct from a lock of the wrong *shape*, which :meth:`Lockfile.load` + sanitises to an empty lock: a file that is not YAML, or cannot be opened, + is a condition the user has to resolve, and the CLI reports it as a + resolution error rather than a traceback. + """ + + class Lockfile: """Manages the ebuild.lock file for reproducible package resolution. @@ -26,6 +36,15 @@ class Lockfile: FILENAME = "ebuild.lock" + # Lock field -> PackageRecipe attribute, for every field lock() records + # that identifies the artifact. The resolver compares each of these; a + # field recorded here and compared nowhere is what this file used to be. + CHECKED_FIELDS = ( + ("url", "url"), + ("checksum", "checksum"), + ("build", "build_system"), + ) + def __init__(self, lock_path: str | Path) -> None: self.lock_path = Path(lock_path) self._entries: Dict[str, Dict[str, str]] = {} @@ -39,15 +58,37 @@ def load(self) -> None: if not self.lock_path.exists(): return - with open(self.lock_path, "r", encoding="utf-8") as f: - raw = yaml.safe_load(f) + try: + with open(self.lock_path, "r", encoding="utf-8") as f: + raw = yaml.safe_load(f) + except (yaml.YAMLError, UnicodeDecodeError) as e: + # A lock that is not UTF-8 (a foreign editor, a bad binary merge) + # fails in the codec, not the parser, and UnicodeDecodeError is a + # ValueError -- neither a YAMLError nor an OSError. Same message: + # the file is not something this tool wrote, and the remedy is + # the same. + raise LockfileError( + f"{self.lock_path} is not valid YAML: {e}. " + f"Fix it, or delete it to resolve afresh." + ) from e + except OSError as e: + raise LockfileError( + f"{self.lock_path} could not be read: {e}. " + f"Fix it, or delete it to resolve afresh." + ) from e if not isinstance(raw, dict): return packages = raw.get("packages", {}) if isinstance(packages, dict): - self._entries = packages + # Keep only well-formed entries; a hand-edited or truncated file + # must not turn into a KeyError deep inside the resolver. + self._entries = { + str(name): {str(k): str(v) for k, v in entry.items() if v is not None} + for name, entry in packages.items() + if isinstance(entry, dict) + } def save(self) -> None: """Write the lockfile to disk.""" diff --git a/ebuild/packages/recipe.py b/ebuild/packages/recipe.py index 6cbdb382..87cd4585 100644 --- a/ebuild/packages/recipe.py +++ b/ebuild/packages/recipe.py @@ -89,6 +89,37 @@ def validate(self) -> None: f"Must be one of {self.VALID_BUILD_SYSTEMS}." ) + def to_dict(self) -> Dict[str, Any]: + """Convert recipe to dictionary for YAML serialization.""" + data: Dict[str, Any] = { + "package": self.name, + "version": self.version, + } + if self.description: + data["description"] = self.description + if self.license: + data["license"] = self.license + data["url"] = self.url + if self.checksum: + data["checksum"] = self.checksum + data["build"] = self.build_system + # Copies, not the live lists: a caller that appends to what it got + # back must not edit the recipe behind its back. The key order is the + # one index_sync's recipe_dict uses, with install_args after + # build_args; parse_recipe() reads every key by name, so a dump and a + # reload agree field for field regardless of order. + if self.dependencies: + data["dependencies"] = list(self.dependencies) + if self.configure_args: + data["configure_args"] = list(self.configure_args) + if self.build_args: + data["build_args"] = list(self.build_args) + if self.install_args: + data["install_args"] = list(self.install_args) + if self.patches: + data["patches"] = list(self.patches) + return data + def _parse_string_list( raw: Dict[str, Any], diff --git a/ebuild/packages/resolver.py b/ebuild/packages/resolver.py index ccf795bc..08beaad1 100644 --- a/ebuild/packages/resolver.py +++ b/ebuild/packages/resolver.py @@ -12,6 +12,7 @@ from typing import Dict, List, Optional from ebuild.core.graph import CycleError, DependencyGraph +from ebuild.packages.lockfile import Lockfile from ebuild.packages.recipe import PackageRecipe from ebuild.packages.registry import PackageRegistry @@ -33,27 +34,40 @@ def __init__(self, registry: PackageRegistry) -> None: def resolve( self, requested: List[Dict[str, str]], + lockfile: Optional[Lockfile] = None, ) -> List[PackageRecipe]: """Resolve a list of requested packages into a full build order. - Version selection: - An explicitly requested version pins the package for the whole - resolution, no matter where in the dependency graph the package is - first reached. Packages requested without a version resolve to the - newest one in the registry. Requesting two different versions of - the same package is a conflict and raises rather than silently - selecting one of them. + Version selection, in order of precedence: + 1. An explicitly requested version pins the package for the whole + resolution, no matter where in the dependency graph the package + is first reached. Requesting two different versions of the same + package is a conflict and raises rather than silently selecting + one of them. + 2. A version recorded in ``lockfile`` pins the package the same + way. That is what the lockfile is for: the next resolution on + this or any other machine lands on the same recipe, not on + whatever is newest by then. A locked version that no recipe + provides is an error, not a fallback -- silently resolving to + something else is exactly what the lock exists to prevent. A + locked entry that names a URL or checksum is also checked + against the recipe it resolves to: the same version behind + different bytes is refused. + 3. Otherwise the newest version in the registry. Args: requested: List of dicts with 'name' and optional 'version' keys. + lockfile: An already-loaded Lockfile, or None to resolve without + one. The caller decides whether to rewrite it afterwards. Returns: List of PackageRecipe in correct build order (dependencies first). Raises: ResolveError: If a package or dependency cannot be found, if two - incompatible versions of the same package are requested, or if - the dependency graph contains a cycle. + incompatible versions of the same package are requested, if + the lockfile pins a version or bytes the registry does not + provide, or if the dependency graph contains a cycle. """ resolved: Dict[str, PackageRecipe] = {} graph = DependencyGraph() @@ -64,9 +78,10 @@ def resolve( # appearing later in the list is swallowed by the memoization in # _collect(). pins = self._collect_pins(requested) + locked = self._locked_entries(lockfile, pins) for pkg in requested: - self._collect(pkg.get("name", ""), pins, resolved, graph) + self._collect(pkg.get("name", ""), pins, locked, resolved, graph) try: order = graph.topological_sort() @@ -102,10 +117,30 @@ def _collect_pins(requested: List[Dict[str, str]]) -> Dict[str, str]: return pins + @staticmethod + def _locked_entries( + lockfile: Optional[Lockfile], + pins: Dict[str, str], + ) -> Dict[str, Dict[str, str]]: + """Lockfile entries that take part in this resolution. + + An explicit pin outranks the lock for that package: the request is + the statement of intent, the lock is the record of the last + resolution, and the caller rewrites the record afterwards. + """ + if lockfile is None: + return {} + return { + name: entry + for name, entry in lockfile.locked_packages.items() + if name not in pins and isinstance(entry, dict) and entry.get("version") + } + def _collect( self, name: str, pins: Dict[str, str], + locked: Dict[str, Dict[str, str]], resolved: Dict[str, PackageRecipe], graph: DependencyGraph, ) -> None: @@ -114,8 +149,19 @@ def _collect( return version = pins.get(name) + entry = locked.get(name) + if version is None and entry is not None: + version = str(entry["version"]) + recipe = self.registry.get(name, version) if recipe is None: + if entry is not None: + raise ResolveError( + f"{Lockfile.FILENAME} pins '{name}' at v{version}, and no " + f"recipe provides that version. Restore the recipe, or " + f"request the version you want explicitly, or delete " + f"{Lockfile.FILENAME} to resolve afresh." + ) raise ResolveError( f"Package '{name}'" + (f" v{version}" if version else "") @@ -123,13 +169,39 @@ def _collect( f"Available: {[r.name for r in self.registry.list_packages()]}" ) + if entry is not None: + self._check_locked_bytes(name, entry, recipe) + resolved[name] = recipe graph.add_node(name) for dep_name in recipe.dependencies: - self._collect(dep_name, pins, resolved, graph) + self._collect(dep_name, pins, locked, resolved, graph) graph.add_edge(name, dep_name) + @staticmethod + def _check_locked_bytes( + name: str, entry: Dict[str, str], recipe: PackageRecipe + ) -> None: + """The locked version must still mean the same artifact. + + A recipe can be edited to keep its version and change its URL or + checksum; the version alone would then reproduce the name, not the + bytes. The build system is part of what turns those bytes into the + installed artifact, so it is held to the same rule. Only fields the + lock recorded are compared. + """ + for field, attr in Lockfile.CHECKED_FIELDS: + locked_value = entry.get(field) + if locked_value and locked_value != getattr(recipe, attr): + raise ResolveError( + f"{Lockfile.FILENAME} pins '{name}' v{recipe.version} with " + f"{field} '{locked_value}', but the recipe for that version " + f"now has '{getattr(recipe, attr)}'. The lock exists to " + f"notice this; if the change is intended, delete " + f"{Lockfile.FILENAME} or pin the version explicitly." + ) + def resolve_single(self, name: str, version: Optional[str] = None) -> PackageRecipe: """Resolve a single package recipe from the registry.""" recipe = self.registry.get(name, version) diff --git a/tests/ebuild/test_package_recipe.py b/tests/ebuild/test_package_recipe.py index 3c25922d..962de8f2 100644 --- a/tests/ebuild/test_package_recipe.py +++ b/tests/ebuild/test_package_recipe.py @@ -4,8 +4,10 @@ """Tests for ebuild.packages.recipe.""" import pytest +import yaml from ebuild.packages.recipe import ( + PackageRecipe, RecipeError, _parse_recipe, load_recipe_from_string, @@ -115,3 +117,66 @@ def test_depends_alias_must_be_a_list(): with pytest.raises(RecipeError, match="dependencies"): load_recipe_from_string(content) + + +def _fully_populated_recipe() -> PackageRecipe: + """A recipe with every field set, so a round trip has to carry them all.""" + return PackageRecipe( + name="demo", + version="1.2.3", + url="https://example.com/demo-1.2.3.tar.gz", + checksum="sha256:" + "ab" * 32, + build_system="autoconf", + dependencies=["zlib", "openssl"], + patches=["fix-build.patch"], + configure_args=["--enable-static"], + build_args=["VERBOSE=1"], + install_args=["DESTDIR=/tmp/stage"], + description="A demo package", + license="MIT", + ) + + +def test_to_dict_round_trips_every_field(): + """Dumping to YAML and parsing it back must reproduce the recipe exactly. + + to_dict() predates install_args and never emitted it, so a recipe cached + by index_sync came back with install_args == [] while every other field + survived. A field-for-field comparison catches the next one too. + """ + recipe = _fully_populated_recipe() + + reloaded = parse_recipe(yaml.safe_load(yaml.safe_dump(recipe.to_dict()))) + + assert reloaded == recipe + assert reloaded.install_args == ["DESTDIR=/tmp/stage"] + + # Key order is not a correctness property -- parse_recipe() reads every + # key by name, as recipe.py says -- but it is a stability property: the + # cached recipe YAML that index_sync writes is diffed by humans, and this + # keeps install_args next to build_args, where index_sync's recipe_dict + # puts it. If this fails after a deliberate reordering, update both + # emitters together and then this line; it is not a bug in to_dict(). + keys = list(recipe.to_dict()) + assert keys.index("install_args") == keys.index("build_args") + 1 + + +def test_to_dict_returns_copies_not_live_lists(): + """Mutating a list from to_dict() must not reach into the recipe.""" + recipe = _fully_populated_recipe() + + data = recipe.to_dict() + for field_name in ( + "dependencies", + "patches", + "configure_args", + "build_args", + "install_args", + ): + data[field_name].append("injected") + + assert recipe.dependencies == ["zlib", "openssl"] + assert recipe.patches == ["fix-build.patch"] + assert recipe.configure_args == ["--enable-static"] + assert recipe.build_args == ["VERBOSE=1"] + assert recipe.install_args == ["DESTDIR=/tmp/stage"] diff --git a/tests/unit/test_index_sync.py b/tests/unit/test_index_sync.py index 7265fd2d..fa3d2962 100644 --- a/tests/unit/test_index_sync.py +++ b/tests/unit/test_index_sync.py @@ -10,6 +10,7 @@ from unittest.mock import MagicMock, patch import pytest +import yaml from click.testing import CliRunner from ebuild.cli.commands import cli @@ -110,6 +111,42 @@ def test_index_sync_success(tmp_path): assert "1.0.0" in content +def test_index_sync_caches_install_args(tmp_path): + """An index entry's install_args must reach the cached recipe YAML. + + The entry-to-recipe mapping listed every list field except this one, and + to_dict() never emitted it, so the cached copy of a package silently lost + the arguments its install step needs. + """ + mgr = IndexSyncManager(index_dir=tmp_path) + + sample_index = [ + { + "name": "staged-pkg", + "version": "2.0.0", + "url": "https://example.com/staged-pkg-2.0.0.tar.gz", + "checksum": "sha256:" + "ab" * 32, + "build_system": "make", + "install_args": ["DESTDIR=/tmp/stage", "PREFIX=/usr"], + } + ] + raw_json = json.dumps(sample_index).encode("utf-8") + + mock_resp = MagicMock() + mock_resp.read.return_value = raw_json + mock_resp.headers = {"Content-Length": str(len(raw_json))} + mock_resp.__enter__.return_value = mock_resp + + with patch("urllib.request.urlopen", return_value=mock_resp): + res = mgr.sync(url="https://example.com/index.json", force=True) + + assert res.package_count == 1 + cached = yaml.safe_load( + (mgr.recipes_dir / "staged-pkg.yaml").read_text(encoding="utf-8") + ) + assert cached["install_args"] == ["DESTDIR=/tmp/stage", "PREFIX=/usr"] + + def test_index_sync_corrupted_json(tmp_path): mgr = IndexSyncManager(index_dir=tmp_path) diff --git a/tests/unit/test_lockfile_cli.py b/tests/unit/test_lockfile_cli.py new file mode 100644 index 00000000..489ed062 --- /dev/null +++ b/tests/unit/test_lockfile_cli.py @@ -0,0 +1,121 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""The CLI reads ebuild.lock before resolving, and rewrites it after. + +:mod:`tests.unit.test_resolver` proves what the resolver does with a lock; +this proves the CLI hands it one. Before this, ``_install_packages`` built a +``Lockfile`` only after ``resolve()`` had run and only ever wrote it, so the +file on disk pinned nothing. +""" + +from types import SimpleNamespace + +import pytest + +from ebuild.cli import commands +from ebuild.cli.logger import Logger + +pytestmark = pytest.mark.ebuild + + +def test_install_packages_resolves_with_the_lock_it_finds(tmp_path, monkeypatch): + src = tmp_path / "project" + (src / "recipes").mkdir(parents=True) + (src / "ebuild.lock").write_text( + "lockfile_version: 1\n" + "packages:\n" + " zlib: {version: '1.2.13', url: 'https://example.invalid/z.tgz', checksum: '', build: cmake}\n", + encoding="utf-8", + ) + + seen = {} + + class RecordingResolver: + def __init__(self, registry): + seen["registry"] = registry + + def resolve(self, requested, lockfile=None): + seen["requested"] = requested + seen["locked"] = None if lockfile is None else lockfile.locked_packages + return [] # nothing to fetch or build; the lock is then rewritten empty + + monkeypatch.setattr(commands, "PackageResolver", RecordingResolver) + monkeypatch.setattr(commands, "_find_recipe_dirs", lambda root: [src / "recipes"]) + + cfg = SimpleNamespace( + packages=[SimpleNamespace(name="zlib", version=None)], + source_dir=src, + ) + result = commands._install_packages(cfg, tmp_path / "build", Logger()) + + assert result == {} + assert seen["requested"] == [{"name": "zlib", "version": None}] + assert seen["locked"] == { + "zlib": {"version": "1.2.13", "url": "https://example.invalid/z.tgz", + "checksum": "", "build": "cmake"}, + }, "the resolver must be given the lock that was on disk" + + # Rewritten from this resolution: nothing resolved, so nothing locked. + rewritten = (src / "ebuild.lock").read_text(encoding="utf-8") + assert "zlib" not in rewritten + assert "lockfile_version: 1" in rewritten + + +def test_install_packages_reports_a_corrupt_lock_as_a_resolve_error(tmp_path, monkeypatch): + """Every caller of _install_packages already catches ResolveError and + prints it as ``[error] ...``; a lock that is not YAML has to arrive + there, not as a yaml.scanner.ScannerError traceback.""" + from ebuild.packages.resolver import ResolveError + + src = tmp_path / "project" + (src / "recipes").mkdir(parents=True) + (src / "ebuild.lock").write_text( + "lockfile_version: 1\npackages:\n zlib: {version: '1.2.13\n", # unterminated quote + encoding="utf-8", + ) + monkeypatch.setattr(commands, "_find_recipe_dirs", lambda root: [src / "recipes"]) + cfg = SimpleNamespace(packages=[SimpleNamespace(name="zlib", version=None)], source_dir=src) + + with pytest.raises(ResolveError, match="ebuild.lock is not valid YAML"): + commands._install_packages(cfg, tmp_path / "build", Logger()) + + +def test_build_command_prints_a_corrupt_lock_without_a_traceback(tmp_path, monkeypatch): + """End to end through the CLI: exit 1, the message, and no traceback.""" + import textwrap + + from click.testing import CliRunner + + from ebuild.cli.commands import cli + + (tmp_path / "src").mkdir() + (tmp_path / "src" / "main.c").write_text("int main(void) { return 0; }\n") + (tmp_path / "recipes").mkdir() + (tmp_path / "build.yaml").write_text(textwrap.dedent("""\ + project: + name: demo + version: "1.0.0" + packages: + - name: zlib + targets: + - name: demo + type: executable + sources: ["src/main.c"] + toolchain: + arch: x86_64 + """)) + (tmp_path / "ebuild.lock").write_text("packages:\n zlib: {version: '1.2.13\n", encoding="utf-8") + monkeypatch.chdir(tmp_path) + + result = CliRunner().invoke(cli, ["build"]) + + output = result.output or "" + try: + output += result.stderr or "" + except ValueError: + pass + assert result.exit_code == 1, output + assert "ebuild.lock is not valid YAML" in output, output + assert "Traceback" not in output, output + assert result.exception is None or isinstance(result.exception, SystemExit), repr(result.exception) diff --git a/tests/unit/test_resolver.py b/tests/unit/test_resolver.py index 30503e45..d13ceef9 100644 --- a/tests/unit/test_resolver.py +++ b/tests/unit/test_resolver.py @@ -157,3 +157,196 @@ def test_resolve_single_is_unaffected(registry): recipe = PackageResolver(registry).resolve_single("zlib", "1.2.13") assert recipe.version == "1.2.13" + + +# ── The lockfile takes part in resolution ───────────────────── + +from ebuild.packages.lockfile import Lockfile # noqa: E402 + + +def make_registry_with_checksums(tmp_path, recipes): + """Like make_registry, with an optional per-recipe checksum.""" + for filename, fields in recipes.items(): + lines = [ + f"package: {fields['package']}", + f"version: '{fields['version']}'", + f"url: {fields.get('url', 'https://example.invalid/' + fields['package'] + '.tar.gz')}", + ] + if fields.get("checksum"): + lines.append(f"checksum: {fields['checksum']}") + if fields.get("dependencies"): + lines.append("dependencies: [%s]" % ", ".join(fields["dependencies"])) + (tmp_path / filename).write_text("\n".join(lines) + "\n", encoding="utf-8") + registry = PackageRegistry() + registry.add_search_path(tmp_path) + registry.scan() + return registry + + +def lock_with(tmp_path, entries): + """A Lockfile holding ``{name: {version, url, checksum, build}}``.""" + lockfile = Lockfile(tmp_path / "ebuild.lock") + lockfile._entries = entries # what load() would have produced + return lockfile + + +def test_lockfile_pins_a_package_the_request_leaves_open(registry): + """The lock is the record of the last resolution; without it an + unpinned zlib resolves to 1.3.0, the newest. With it, 1.2.13. + + Regression: the lockfile was written after every resolution and never + read, so it pinned nothing. + """ + lock = lock_with(registry.search_paths[0], { + "zlib": {"version": "1.2.13", "url": "https://example.invalid/zlib.tar.gz", + "checksum": "", "build": "cmake"}, + }) + + assert versions_of(PackageResolver(registry).resolve([{"name": "app"}]))["zlib"] == "1.3.0" + locked = PackageResolver(registry).resolve([{"name": "app"}], lockfile=lock) + assert versions_of(locked)["zlib"] == "1.2.13" + + +def test_an_explicit_request_outranks_the_lock(registry): + """The request is the statement of intent; the lock is only a record.""" + lock = lock_with(registry.search_paths[0], { + "zlib": {"version": "1.2.13", "url": "", "checksum": "", "build": "cmake"}, + }) + order = PackageResolver(registry).resolve( + [{"name": "app"}, {"name": "zlib", "version": "1.3.0"}], lockfile=lock) + assert versions_of(order)["zlib"] == "1.3.0" + + +def test_a_locked_version_no_recipe_provides_is_an_error_not_a_fallback(registry): + lock = lock_with(registry.search_paths[0], { + "zlib": {"version": "9.9.9", "url": "", "checksum": "", "build": "cmake"}, + }) + with pytest.raises(ResolveError, match=r"ebuild\.lock pins 'zlib' at v9\.9\.9"): + PackageResolver(registry).resolve([{"name": "app"}], lockfile=lock) + + +def test_the_lock_notices_a_recipe_whose_bytes_changed(tmp_path): + """Same version, different checksum: the name reproduces, the bytes do + not, and that is what the lock exists to catch.""" + registry = make_registry_with_checksums(tmp_path, { + "zlib.yaml": {"package": "zlib", "version": "1.3.0", + "checksum": "sha256:" + "b" * 64}, + }) + lock = lock_with(tmp_path, { + "zlib": {"version": "1.3.0", "url": "https://example.invalid/zlib.tar.gz", + "checksum": "sha256:" + "a" * 64, "build": "cmake"}, + }) + with pytest.raises(ResolveError, match="checksum"): + PackageResolver(registry).resolve([{"name": "zlib"}], lockfile=lock) + + # The control: the same lock with the recipe's real checksum resolves. + lock._entries["zlib"]["checksum"] = "sha256:" + "b" * 64 + assert versions_of(PackageResolver(registry).resolve([{"name": "zlib"}], lockfile=lock))["zlib"] == "1.3.0" + + +def test_a_lock_entry_the_request_does_not_reach_is_ignored(registry): + """Locked packages that nothing depends on any more are not resolved + into the build; the caller rewrites the lock from what was.""" + lock = lock_with(registry.search_paths[0], { + "orphan": {"version": "1.0.0", "url": "", "checksum": "", "build": "cmake"}, + }) + names = [r.name for r in PackageResolver(registry).resolve([{"name": "app"}], lockfile=lock)] + assert "orphan" not in names + + +def test_load_drops_malformed_entries(tmp_path): + """A hand-edited lock must not become a KeyError inside the resolver.""" + path = tmp_path / "ebuild.lock" + path.write_text( + "lockfile_version: 1\n" + "packages:\n" + " zlib: {version: '1.2.13', url: null, checksum: '', build: cmake}\n" + " broken: just-a-string\n", + encoding="utf-8", + ) + lock = Lockfile(path) + lock.load() + assert lock.package_names == ["zlib"] + assert lock.get_locked_entry("zlib") == {"version": "1.2.13", "checksum": "", "build": "cmake"} + + +def test_the_lock_notices_a_recipe_whose_build_system_changed(tmp_path): + """Same version, URL and checksum, different build system: the source + bytes reproduce, the installed artifact does not. The lock records + ``build``; a field recorded and never compared is the defect the lock + itself was.""" + # Only for the zlib.yaml it writes; the registry is built below, after + # the recipe has its build system appended. + make_registry_with_checksums(tmp_path, { + "zlib.yaml": {"package": "zlib", "version": "1.3.0", + "checksum": "sha256:" + "b" * 64}, + }) + (tmp_path / "zlib.yaml").write_text( + (tmp_path / "zlib.yaml").read_text(encoding="utf-8") + "build: make\n", + encoding="utf-8", + ) + registry = PackageRegistry() + registry.add_search_path(tmp_path) + registry.scan() + + lock = lock_with(tmp_path, { + "zlib": {"version": "1.3.0", "url": "https://example.invalid/zlib.tar.gz", + "checksum": "sha256:" + "b" * 64, "build": "cmake"}, + }) + with pytest.raises(ResolveError, match="build 'cmake'"): + PackageResolver(registry).resolve([{"name": "zlib"}], lockfile=lock) + + # The control: the lock that records the recipe's real build system resolves. + lock._entries["zlib"]["build"] = "make" + assert versions_of(PackageResolver(registry).resolve([{"name": "zlib"}], lockfile=lock))["zlib"] == "1.3.0" + + +def test_every_field_the_lock_records_is_one_the_resolver_compares(tmp_path): + """lock() and the resolver's check are two lists of the same thing; this + keeps them from drifting apart again. ``version`` is the lookup key, not + a compared field.""" + from ebuild.packages.recipe import PackageRecipe + + recipe = PackageRecipe(name="zlib", version="1.3.0", url="https://example.invalid/z.tgz", + checksum="sha256:" + "a" * 64, build_system="cmake") + lock = Lockfile(tmp_path / "ebuild.lock") + lock.lock([recipe]) + recorded = set(lock.get_locked_entry("zlib")) - {"version"} + checked = {field for field, _attr in Lockfile.CHECKED_FIELDS} + assert recorded == checked, (recorded, checked) + for _field, attr in Lockfile.CHECKED_FIELDS: + assert hasattr(recipe, attr), attr + + +def test_load_refuses_a_lock_that_is_not_yaml(tmp_path): + """A corrupt lock is a condition to report, not a traceback to print.""" + from ebuild.packages.lockfile import LockfileError + + path = tmp_path / "ebuild.lock" + path.write_text("lockfile_version: 1\npackages:\n zlib: {version: '1.2.13\n", encoding="utf-8") + with pytest.raises(LockfileError, match="not valid YAML"): + Lockfile(path).load() + + +def test_load_refuses_a_lock_that_is_not_utf8(tmp_path): + """A 0xff byte fails in the codec, not the parser; the caller must see + the same LockfileError, not a UnicodeDecodeError traceback.""" + from ebuild.packages.lockfile import LockfileError + + path = tmp_path / "ebuild.lock" + path.write_bytes(b"packages:\n zlib: {version: '\xff'}\n") + with pytest.raises(LockfileError, match="not valid YAML"): + Lockfile(path).load() + + path.write_bytes("packages: {}\n".encode("utf-16")) # the BOM alone does it + with pytest.raises(LockfileError, match="not valid YAML"): + Lockfile(path).load() + + +def test_load_refuses_a_lock_it_cannot_open(tmp_path): + from ebuild.packages.lockfile import LockfileError + + path = tmp_path / "ebuild.lock" + path.mkdir() # exists, so load() does not return early; open() fails + with pytest.raises(LockfileError, match="could not be read"): + Lockfile(path).load()