diff --git a/codecov.yml b/codecov.yml new file mode 100644 index 0000000..df894dd --- /dev/null +++ b/codecov.yml @@ -0,0 +1,8 @@ +coverage: + status: + project: + default: + target: 80% + patch: + default: + target: 80% diff --git a/collider/cache.py b/collider/cache.py index f2a8165..5e39d70 100644 --- a/collider/cache.py +++ b/collider/cache.py @@ -5,6 +5,7 @@ from __future__ import annotations +import errno import json import shutil import tempfile @@ -237,7 +238,7 @@ def _ensure_archive(self, url: str, filename: str, expected_hash: str, offline: f'Cached archive "{safe_name}" is corrupt: ' f'expected {expected_hash}, got {cached_hash}. Re-downloading.' ) - cached_path.unlink() + cached_path.unlink(missing_ok=True) parsed = urllib.parse.urlparse(url) if parsed.scheme == 'http': @@ -290,7 +291,7 @@ def _ensure_archive(self, url: str, filename: str, expected_hash: str, offline: try: tmp_path.replace(cached_path) except OSError as exc: - if exc.errno != 18: + if exc.errno != errno.EXDEV: raise # /tmp and cache may live on different filesystems, so fall back to a move. shutil.move(tmp_path, cached_path) diff --git a/collider/config.py b/collider/config.py index 882deb7..0b56c89 100644 --- a/collider/config.py +++ b/collider/config.py @@ -3,7 +3,6 @@ """Application config paths and loading.""" -import json import os from dataclasses import dataclass @@ -12,6 +11,7 @@ from collider.cache import WrapCache from collider.Context import Context +from collider.errors import ColliderUserError from collider.file_model.configfile import ConfigFile from collider.log import logger from collider.repository.implementation.RepositoryInterface import RepositoryInterface @@ -68,18 +68,9 @@ def load(*, offline: bool = False) -> Context: else: try: config_file = ConfigFile.from_path(config_path) - except (json.JSONDecodeError, UnicodeDecodeError) as exc: - logger.critical( - f'Invalid JSON in "{config_path.as_posix()}". ' - 'Fix or delete the file to regenerate defaults.' - ) - raise SystemExit(os.EX_DATAERR) from exc - except TypeError as exc: - logger.critical( - f'Invalid config file "{config_path.as_posix()}". ' - 'Fix or delete the file to regenerate defaults.' - ) - raise SystemExit(os.EX_DATAERR) from exc + except ColliderUserError as exc: + logger.critical('Fix or delete the config file to regenerate defaults.') + raise SystemExit(exc.exit_code) from exc logger.debug(f'Loaded config file from "{config_path.as_posix()}".') # Shared cache keeps wrap and archive data reusable across projects. diff --git a/collider/entrypoint.py b/collider/entrypoint.py index ae91b41..fb2c026 100644 --- a/collider/entrypoint.py +++ b/collider/entrypoint.py @@ -18,6 +18,7 @@ from collider import config from collider.Context import Context +from collider.errors import ColliderUserError from collider.log import Level, configure_logging, logger from collider.utils import core from collider.utils.meson.meson import MesonUnavailableError @@ -152,6 +153,10 @@ def main() -> int: # A missing or outdated Meson is a user environment problem, not a Collider bug, # so report a clean exit code instead of routing it through error_handler. return os.EX_UNAVAILABLE + except ColliderUserError as e: + # Usage and user-environment errors are logged at the raise site; exit cleanly + # with the carried code instead of routing them through error_handler. + return e.exit_code def entrypoint() -> None: diff --git a/collider/errors.py b/collider/errors.py new file mode 100644 index 0000000..c194bc2 --- /dev/null +++ b/collider/errors.py @@ -0,0 +1,23 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 MOG Robotics OÜ + +"""Shared exception types for user-facing error reporting.""" + + +class ColliderUserError(Exception): + """ + Raised for usage or user-environment problems, not Collider bugs. + The entry point reports these cleanly and exits with the carried code instead of + routing them through the internal-bug handler. + """ + + def __init__(self, message: str, exit_code: int) -> None: + """ + :param message: Human-readable description, already logged at the raise site. + :param exit_code: Process exit code (os.EX_*) the CLI should return. + """ + # A user error that exits successfully is a logic bug, not a recoverable state. + if exit_code == 0: + raise ValueError('ColliderUserError must use a non-zero exit code.') + super().__init__(message) + self.exit_code = exit_code diff --git a/collider/file_model/FileModelInterface.py b/collider/file_model/FileModelInterface.py index accbb8e..5aedeac 100644 --- a/collider/file_model/FileModelInterface.py +++ b/collider/file_model/FileModelInterface.py @@ -4,6 +4,7 @@ """Abstract file model interface.""" import json +import os import tempfile from abc import ABC @@ -14,6 +15,7 @@ import jsonschema +from collider.errors import ColliderUserError from collider.log import logger from collider.utils.dataclass import prepare_ctor_kwargs, to_json_dict from collider.utils.fs import atomic_write_text @@ -97,12 +99,18 @@ def from_path(cls: Type[T], filepath: Path) -> T: try: with open(filepath, 'r', encoding='UTF-8') as f: loaded_file = cls.from_stream(f) - except (json.JSONDecodeError, UnicodeDecodeError): - logger.error(f'Invalid JSON in "{filepath}".') - raise + except (ValueError, TypeError, KeyError) as exc: + # Malformed, schema-invalid, or undeserializable content (bad JSON, bad enum + # value, unknown registry key) is a user data problem, not a bug. + # json.JSONDecodeError and UnicodeDecodeError are ValueError subclasses. + logger.critical(msg := f'File "{filepath}" is invalid: {exc}') + raise ColliderUserError(msg, os.EX_DATAERR) from exc except FileNotFoundError: logger.warning(f'File not found: "{filepath}".') raise + except OSError as exc: + logger.critical(msg := f'Cannot read file "{filepath}": {exc}') + raise ColliderUserError(msg, os.EX_IOERR) from exc return loaded_file diff --git a/collider/repository/implementation/Wrap.py b/collider/repository/implementation/Wrap.py index cd8c618..e0f8646 100644 --- a/collider/repository/implementation/Wrap.py +++ b/collider/repository/implementation/Wrap.py @@ -5,6 +5,7 @@ import hashlib import json +import os import time import urllib.parse import urllib.request @@ -12,6 +13,7 @@ from pathlib import Path from typing import Optional +from collider.errors import ColliderUserError from collider.log import logger from collider.Package import WrapPackage from collider.repository.entries import RejectedEntry, RepoPackageEntry, packages_from_releases @@ -56,6 +58,20 @@ def _releases_cache_path(cache_path: Path, url: urllib.parse.ParseResult) -> Pat return Path(cache_path) / 'wrapdb' / f'{url.netloc}-{digest}' / _RELEASES_FILENAME +def _load_releases_cache(cache_file: Path) -> Optional[dict[str, WrapDbReleasesEntry]]: + """ + Load cached releases.json, or None when the file is missing or unreadable. + :param cache_file: Path to the cached releases.json. + :return: Parsed releases mapping, or None to signal a cache miss. + """ + try: + return json.loads(cache_file.read_text(encoding='utf-8')) + except (OSError, ValueError) as exc: + # ValueError covers both JSONDecodeError and UnicodeDecodeError. + logger.debug(f'Cached releases.json unusable: {exc}') + return None + + def _ensure_v2_url(url: urllib.parse.ParseResult) -> urllib.parse.ParseResult: path = url.path.rstrip('/') if not path.endswith('/v2'): @@ -107,32 +123,57 @@ def _from_url_impl( if cache_path is not None: cache_file = _releases_cache_path(cache_path, effective_url) - releases: dict[str, WrapDbReleasesEntry] + releases: Optional[dict[str, WrapDbReleasesEntry]] = None if offline: - if cache_file is None or not cache_file.exists(): - raise ValueError('Offline mode requires cached wrap releases.') - releases = json.loads(cache_file.read_text(encoding='utf-8')) + if cache_file is None or (releases := _load_releases_cache(cache_file)) is None: + raise ColliderUserError( + 'Offline mode requires cached wrap releases.', os.EX_DATAERR + ) elif ( cache_file is not None and cache_file.exists() and (time.time() - cache_file.stat().st_mtime) < _RELEASES_TTL_SECONDS + # A corrupt within-TTL cache falls through to a network refresh. + and (releases := _load_releases_cache(cache_file)) is not None ): logger.debug('Using cached releases.json (within TTL).') - releases = json.loads(cache_file.read_text(encoding='utf-8')) - else: + if releases is None and not offline: try: with urllib.request.urlopen( releases_url, timeout=DEFAULT_NETWORK_TIMEOUT ) as response: releases = json.load(response) + if not isinstance(releases, dict): + # A 200 body that is not a JSON object (e.g. `null`, a list, or + # an HTML error page parsed as a string) is a repository data + # problem, not a Collider bug. + logger.critical( + f'WrapDB at "{effective_url.geturl()}" returned non-object releases.json.' + ) + raise ColliderUserError( + 'WrapDB returned malformed releases.json.', os.EX_DATAERR + ) if cache_file is not None: atomic_write_text(cache_file, json.dumps(releases), encoding='utf-8') - except Exception as e: - if cache_file is None or not cache_file.exists(): - raise e + except ColliderUserError: # pylint: disable=try-except-raise + # Re-raise before the generic handler so a malformed-releases user + # error is not swallowed as a network failure and cache-fallback. + raise + except Exception: + cached = _load_releases_cache(cache_file) if cache_file is not None else None + if cached is None: + raise logger.warning('Failed to refresh wrap releases; using cached data.') - releases = json.loads(cache_file.read_text(encoding='utf-8')) - + releases = cached + + if not isinstance(releases, dict): + # Defensive: covers a non-object cached releases.json (e.g. a list or + # `null`) that slipped past the network-fetch validation above. + logger.critical( + f'Cached releases for "{effective_url.geturl()}" are not a JSON object' + f'{f"; delete {cache_file}" if cache_file is not None else ""}.' + ) + raise ColliderUserError('Cached wrap releases are malformed.', os.EX_DATAERR) packages, rejected = _wrap_releases_to_packages(releases) if rejected: logger.warning( diff --git a/collider/subcommand/Install.py b/collider/subcommand/Install.py index 943af0f..5796bd9 100644 --- a/collider/subcommand/Install.py +++ b/collider/subcommand/Install.py @@ -479,7 +479,7 @@ def _do_install(self, name: str, package: WrapPackage) -> bool: Path.cwd() / SUBPROJECTS_DIR, offline=self.offline, ) - except (FileNotFoundError, ValueError) as e: + except (OSError, ValueError) as e: logger.critical(str(e)) return False @@ -488,6 +488,9 @@ def _do_install(self, name: str, package: WrapPackage) -> bool: except FileExistsError as exc: logger.critical(str(exc)) return False + except OSError as exc: + logger.critical(f'Could not install wrap file: {exc}') + return False logger.info(f'Installed "{name}" version "{package.version}".') return True diff --git a/collider/subcommand/Repo.py b/collider/subcommand/Repo.py index b8e7f82..07d25f9 100644 --- a/collider/subcommand/Repo.py +++ b/collider/subcommand/Repo.py @@ -6,7 +6,6 @@ from __future__ import annotations import argparse -import json import os import urllib.parse @@ -225,11 +224,9 @@ def _load_or_create_config(config_path: Path) -> Optional[ConfigFile]: logger.critical(f'Failed to initialize config at "{config_path.as_posix()}": {exc}') return None - try: - return ConfigFile.from_path(config_path) - except (TypeError, json.JSONDecodeError, UnicodeDecodeError) as exc: - logger.critical(f'Invalid config file "{config_path.as_posix()}": {exc}') - return None + # ConfigFile.from_path raises ColliderUserError on a corrupt config; let the + # carried exit code propagate to the entrypoint instead of degrading to EX_DATAERR. + return ConfigFile.from_path(config_path) @staticmethod def _normalize_repo_url(url: str) -> str: diff --git a/collider/subcommand/Setup.py b/collider/subcommand/Setup.py index 0cb749a..4df4b1a 100644 --- a/collider/subcommand/Setup.py +++ b/collider/subcommand/Setup.py @@ -14,6 +14,7 @@ from typing import Optional from collider.Context import Context +from collider.errors import ColliderUserError from collider.file_model.colliderfile import Colliderfile from collider.log import logger from collider.subcommand.SubcommandInterface import SubcommandInterface @@ -102,7 +103,7 @@ def __init__(self, args: argparse.Namespace, context: Context): else: logger.critical(msg := 'Expected "--" separator before meson arguments.') logger.critical(f'E.g. "collider setup -- {" ".join(self.meson_setup_args)}"') - raise ValueError(msg) + raise ColliderUserError(msg, os.EX_USAGE) @override def execute(self) -> int: @@ -130,13 +131,10 @@ def execute(self) -> int: ) return os.EX_NOINPUT - try: - if not self._verify_no_lock_drift(): - return os.EX_DATAERR - fallback_args = self._force_fallback_args() - except ValueError as exc: - logger.critical(str(exc)) + # A malformed or unreadable lock raises ColliderUserError, handled at the entrypoint. + if not self._verify_no_lock_drift(): return os.EX_DATAERR + fallback_args = self._force_fallback_args() try: meson.setup( @@ -163,7 +161,7 @@ def _verify_no_lock_drift(self) -> bool: default is to fail fast; --allow-drift downgrades this to a loud warning for the legitimate local-patch workflow. :return: True when setup may proceed, False when it must abort. - :raises ValueError: When collider.lock exists but is malformed. + :raises ColliderUserError: When collider.lock exists but cannot be read or parsed. """ drifted = detect_locked_wrap_drift(self.sourcedir) if not drifted: @@ -206,7 +204,7 @@ def _force_fallback_args(self) -> list[str]: `--force-fallback-for`, so a user-supplied one is deferred to rather than overridden with a flag Meson would discard alongside a misleading message. :return: A single `--force-fallback-for` argument, or an empty list. - :raises ValueError: When collider.lock exists but is malformed. + :raises ColliderUserError: When collider.lock exists but cannot be read or parsed. """ managed = managed_package_names(self.sourcedir) diff --git a/collider/subcommand/Status.py b/collider/subcommand/Status.py index bf0ab18..f3a4c56 100644 --- a/collider/subcommand/Status.py +++ b/collider/subcommand/Status.py @@ -174,7 +174,13 @@ def _resolve_versions( exclude_optional=any(dep.exclude_optional for dep in tracked), ) return {name: candidate.version for name, candidate in resolution.mapping.items()} - except Exception: + except Exception as exc: + # Status stays best-effort on resolution failures, but says so instead of + # silently listing transitive wraps as untracked. + logger.warning( + f'Version resolution failed ({type(exc).__name__}: {exc}); ' + 'transitive wraps may be listed as untracked.' + ) return {} @staticmethod diff --git a/collider/subcommand/pkg/Add.py b/collider/subcommand/pkg/Add.py index c2f9852..d6b7fc1 100644 --- a/collider/subcommand/pkg/Add.py +++ b/collider/subcommand/pkg/Add.py @@ -468,7 +468,7 @@ def _install_package( offline=self.offline, ) logger.debug('Package cache updated.') - except (FileNotFoundError, ValueError) as e: + except (OSError, ValueError) as e: logger.critical(str(e)) return None @@ -481,6 +481,9 @@ def _install_package( f'{exc} Remove or rename the existing file/directory before installing this package.' ) return None + except OSError as exc: + logger.critical(f'Could not install wrap file: {exc}') + return None return package @staticmethod diff --git a/collider/subcommand/pkg/Info.py b/collider/subcommand/pkg/Info.py index 0da2c21..e5029c2 100644 --- a/collider/subcommand/pkg/Info.py +++ b/collider/subcommand/pkg/Info.py @@ -18,6 +18,7 @@ from packaging.version import InvalidVersion from collider.Context import Context +from collider.errors import ColliderUserError from collider.file_model.colliderfile import Colliderfile from collider.log import logger from collider.repository.entries import RepoPackageEntry @@ -169,7 +170,14 @@ def _read_installed_wrap_text(self) -> Optional[str]: wrap_path = Path.cwd() / 'subprojects' / f'{self.package_name}.wrap' if not wrap_path.exists(): return None - return wrap_path.read_text(encoding='utf-8') + try: + return wrap_path.read_text(encoding='utf-8') + except UnicodeDecodeError as exc: + logger.critical(msg := f'Cannot read wrap file "{wrap_path}": {exc}') + raise ColliderUserError(msg, os.EX_DATAERR) from exc + except OSError as exc: + logger.critical(msg := f'Cannot read wrap file "{wrap_path}": {exc}') + raise ColliderUserError(msg, os.EX_IOERR) from exc def _resolve_installed(self, wrap_text: Optional[str]) -> str: if wrap_text is None: diff --git a/collider/subcommand/pkg/Prune.py b/collider/subcommand/pkg/Prune.py index 0e9ca89..bd9e0d0 100644 --- a/collider/subcommand/pkg/Prune.py +++ b/collider/subcommand/pkg/Prune.py @@ -14,6 +14,7 @@ import resolvelib from collider.Context import Context +from collider.errors import ColliderUserError from collider.file_model.lockfile import Lockfile from collider.log import logger from collider.subcommand.SubcommandInterface import SubcommandInterface @@ -96,17 +97,24 @@ def run_prune(context: Context, dry_run: bool = False) -> int: lockfile_path = Path.cwd() / Lockfile.get_filename() if not lockfile_path.exists(): logger.warning( - 'No lockfile found. Collider cannot safely determine which transitive wraps ' - 'are orphaned without existing ownership metadata.\n' - 'Run "collider lock" to create ownership metadata for future operations; ' + 'Collider cannot safely determine which transitive wraps are orphaned ' + 'without existing ownership metadata; ' 'existing leftover wraps may still need to be removed manually.' ) + # Trailing one-liner so scripts can detect the skip without parsing mid-stream logs. + logger.warning('prune skipped: no lockfile; run "collider lock".') return os.EX_OK try: lockfile = Lockfile.from_path(lockfile_path) + except ColliderUserError as exc: + # An unreadable user lockfile is a user-data error: surface the carried + # exit code so direct prune invocations fail honestly instead of EX_OK. + logger.warning('prune skipped: unreadable lockfile; run "collider lock".') + return exc.exit_code except Exception: - logger.warning('collider.lock could not be read. Run "collider lock" to regenerate it.') + # Trailing one-liner so scripts can detect the skip without parsing mid-stream logs. + logger.warning('prune skipped: unreadable lockfile; run "collider lock".') return os.EX_OK managed = set(lockfile.all_packages.keys()) diff --git a/collider/subcommand/pkg/Remove.py b/collider/subcommand/pkg/Remove.py index 2691c56..1999bdf 100644 --- a/collider/subcommand/pkg/Remove.py +++ b/collider/subcommand/pkg/Remove.py @@ -123,7 +123,12 @@ def execute(self) -> int: if removed_artifacts: logger.info(f'Removed installed wrap state for "{self.package_name}".') + # Warn before pruning so run_prune's trailing skip summary stays the last log line. + warn_if_lockfile_needs_refresh(self.package_name) + if self.prune: + # Deliberately ignore run_prune's exit code: the remove itself succeeded, and + # issue #46 requires remove --prune to exit EX_OK when pruning is skipped. run_prune(self.context) elif still_needed is not True: self._inform_about_remaining_wraps( @@ -132,7 +137,6 @@ def execute(self) -> int: preserved_needed_name=self.package_name if still_needed is True else None, ) - warn_if_lockfile_needs_refresh(self.package_name) return os.EX_OK def _resolve_remaining_needed_packages(self, colliderfile: Colliderfile) -> Optional[set[str]]: diff --git a/collider/subcommand/pkg/Search.py b/collider/subcommand/pkg/Search.py index 819b4e1..d303a14 100644 --- a/collider/subcommand/pkg/Search.py +++ b/collider/subcommand/pkg/Search.py @@ -17,6 +17,7 @@ from packaging.version import InvalidVersion from collider.Context import Context +from collider.errors import ColliderUserError from collider.log import logger from collider.repository.entries import RepoPackageEntry from collider.repository.repository import search_packages @@ -98,8 +99,8 @@ def __init__(self, args: argparse.Namespace, context: Context): try: self.name_pattern: re.Pattern = re.compile(args.pattern) except re.error as e: - logger.critical(f'Invalid regex pattern: {e}') - raise e + logger.critical(msg := f'Invalid regex pattern: {e}') + raise ColliderUserError(msg, os.EX_USAGE) from e @override def execute(self) -> int: diff --git a/collider/subcommand/pkg/Upgrade.py b/collider/subcommand/pkg/Upgrade.py index 8acb706..323a25e 100644 --- a/collider/subcommand/pkg/Upgrade.py +++ b/collider/subcommand/pkg/Upgrade.py @@ -261,7 +261,12 @@ def _fetch_package( def _installed_wrap_matches(package_name: str, package: WrapPackage) -> bool: """Check whether the current wrap file already matches the fetched package.""" wrap_path = Path.cwd() / SUBPROJECTS_DIR / f'{package_name}.wrap' - return wrap_path.exists() and wrap_path.read_text(encoding='utf-8') == package.wrap_text + try: + return wrap_path.exists() and wrap_path.read_text(encoding='utf-8') == package.wrap_text + except (OSError, UnicodeDecodeError) as exc: + # An unreadable wrap cannot match; the reinstall path surfaces any real problem. + logger.debug(f'Cannot read wrap "{wrap_path}": {exc}') + return False def _install_downloaded_package(self, entry: RepoPackageEntry, package: WrapPackage) -> bool: """Write a fetched package into subprojects/.""" @@ -272,7 +277,7 @@ def _install_downloaded_package(self, entry: RepoPackageEntry, package: WrapPack Path.cwd() / SUBPROJECTS_DIR, offline=self.offline, ) - except (FileNotFoundError, ValueError) as exc: + except (OSError, ValueError) as exc: logger.critical(str(exc)) return False @@ -281,6 +286,9 @@ def _install_downloaded_package(self, entry: RepoPackageEntry, package: WrapPack except FileExistsError as exc: logger.critical(str(exc)) return False + except OSError as exc: + logger.critical(f'Could not install wrap file: {exc}') + return False logger.info(f'Upgraded "{entry.name}" to version "{package.version}".') return True diff --git a/collider/utils/packaging/resolver.py b/collider/utils/packaging/resolver.py index 3fd67f9..cb5ee0d 100644 --- a/collider/utils/packaging/resolver.py +++ b/collider/utils/packaging/resolver.py @@ -6,6 +6,7 @@ from __future__ import annotations import json +import os import re import subprocess import tarfile @@ -24,6 +25,7 @@ from tqdm import tqdm from collider.cache import WrapCache +from collider.errors import ColliderUserError from collider.log import logger, should_disable_progress from collider.repository import search_packages from collider.utils.core import assert_safe_path_segment @@ -54,14 +56,6 @@ def __init__(self, name: str, version_spec_str: Optional[str] = None): spec = SpecifierSet(version_spec_str) if version_spec_str else None object.__setattr__(self, 'version_spec', spec) - def __eq__(self, other: object) -> bool: - if not isinstance(other, Requirement): - return NotImplemented - return self.name == other.name - - def __hash__(self) -> int: - return hash(self.name) - def __repr__(self) -> str: if self.version_spec: return f'Requirement({self.name!r}, {str(self.version_spec)!r})' @@ -76,18 +70,6 @@ class Candidate: version: str repo_name: str - def __eq__(self, other: object) -> bool: - if not isinstance(other, Candidate): - return NotImplemented - return ( - self.name == other.name - and self.version == other.version - and self.repo_name == other.repo_name - ) - - def __hash__(self) -> int: - return hash((self.name, self.version, self.repo_name)) - def __repr__(self) -> str: return f'Candidate({self.name!r}, {self.version!r}, {self.repo_name!r})' @@ -410,8 +392,17 @@ def _scan_candidate(self, candidate: Candidate) -> list[ScannedDependency]: try: cache.prepare_packagecache(package, tmp, offline=self.offline) except (FileNotFoundError, ValueError) as e: + # FileNotFoundError means the archive is absent or could not be + # downloaded; ValueError covers hash mismatches and incomplete + # patch metadata. Both make this candidate unscannable. logger.warning(f'Could not prepare source for scan: {e}') return [] + except OSError as e: + # PermissionError, ENOSPC, and friends are real environment failures: + # fail loudly as a user error rather than masking them as an empty + # dependency set, since strict callers catch only resolver exceptions. + logger.critical(msg := f'Could not prepare source for scan: {e}') + raise ColliderUserError(msg, os.EX_IOERR) from e source_archive = tmp / 'packagecache' / package.source_filename if not source_archive.exists(): @@ -456,7 +447,15 @@ def _extract_archive(archive_path: Path, dest: Path) -> bool: try: with tarfile.open(archive_path) as tar: - tar.extractall(path=dest, filter='data') + if hasattr(tarfile, 'data_filter'): + tar.extractall(path=dest, filter='data') + else: + # Interpreters without the PEP 706 backport reject the filter kwarg. + logger.warning( + 'Extracting archive without a tar filter; ' + 'update Python for safer extraction.' + ) + tar.extractall(path=dest) # nosec B202 return True except (tarfile.TarError, OSError) as e: logger.warning(f'Tar extraction failed for "{archive_path.name}": {e}') diff --git a/collider/utils/project_state.py b/collider/utils/project_state.py index 66773ec..cc2e329 100644 --- a/collider/utils/project_state.py +++ b/collider/utils/project_state.py @@ -6,11 +6,13 @@ from __future__ import annotations import configparser +import os import shutil from pathlib import Path from typing import Optional +from collider.errors import ColliderUserError from collider.file_model.colliderfile import Colliderfile from collider.file_model.lockfile import Lockfile, compute_wrap_hash from collider.log import logger @@ -128,6 +130,7 @@ def remove_installed_artifacts(package_name: str) -> bool: subprojects/. :param package_name: Collider-managed package to remove. :return: True when any artifact was removed. + :raises ColliderUserError: When an artifact exists but cannot be deleted. """ if not is_safe_path_segment(package_name): logger.warning(f'Refusing to remove artifacts for unsafe package name "{package_name}".') @@ -139,17 +142,24 @@ def remove_installed_artifacts(package_name: str) -> bool: # Resolve the extracted directory from the wrap before unlinking it. declared_dir = _declared_subproject_dir(wrap_path, package_name) - removed_any = False - if wrap_path.exists() or wrap_path.is_symlink(): - wrap_path.unlink() - removed_any = True - - dir_names = [package_name] - if declared_dir is not None and declared_dir != package_name: - dir_names.append(declared_dir) - for name in dir_names: - if _remove_subproject_tree(subprojects_dir, name): + try: + removed_any = False + # Remove trees before the wrap: the wrap holds the `directory=` field, so keeping + # it until last leaves a failed removal retryable. + dir_names = [package_name] + if declared_dir is not None and declared_dir != package_name: + dir_names.append(declared_dir) + for name in dir_names: + if _remove_subproject_tree(subprojects_dir, name): + removed_any = True + + if wrap_path.exists() or wrap_path.is_symlink(): + wrap_path.unlink() removed_any = True + except OSError as exc: + # A file the user made undeletable is an environment problem, not a Collider bug. + logger.critical(msg := f'Could not remove installed files for "{package_name}": {exc}') + raise ColliderUserError(msg, os.EX_IOERR) from exc return removed_any @@ -170,16 +180,19 @@ def managed_package_names(sourcedir: Path) -> Optional[set[str]]: transitive wrap is recorded nowhere but the wrap file itself. :param sourcedir: Project source directory. :return: Managed package names, or None when collider.lock is absent. - :raises ValueError: When collider.lock exists but cannot be parsed. + :raises ColliderUserError: When collider.lock exists but cannot be read or parsed. """ lock_path = sourcedir / Lockfile.get_filename() if not lock_path.exists(): return None + # A present but unreadable/malformed lock is a hard error; from_path reports it accurately. + # Tolerate a TOCTOU delete between exists() and from_path(): a vanished lock is equivalent + # to "no lockfile" rather than a user-facing traceback. try: lockfile = Lockfile.from_path(lock_path) - except Exception as exc: - raise ValueError(f'collider.lock is malformed: {exc}') from exc + except FileNotFoundError: + return None names = set(lockfile.all_packages) @@ -188,7 +201,7 @@ def managed_package_names(sourcedir: Path) -> Optional[set[str]]: if colliderfile_path.exists(): try: colliderfile = Colliderfile.from_path(colliderfile_path) - except Exception as exc: + except ColliderUserError as exc: # The lockfile is authoritative; a broken colliderfile is caught later by setup # validation. Scope to lock-only names here rather than abort. logger.debug(f'Ignoring unreadable collider.json for force-fallback scoping: {exc}') @@ -208,16 +221,19 @@ def detect_locked_wrap_drift(sourcedir: Path) -> list[str]: not changes to the extracted subproject source tree. :param sourcedir: Project source directory. :return: Sorted names of wraps that drifted from the lock; empty when none or no lock. - :raises ValueError: When collider.lock exists but cannot be parsed. + :raises ColliderUserError: When collider.lock exists but cannot be read or parsed. """ lock_path = sourcedir / Lockfile.get_filename() if not lock_path.exists(): return [] + # A present but unreadable/malformed lock is a hard error; from_path reports it accurately. + # Tolerate a TOCTOU delete between exists() and from_path(): a vanished lock is equivalent + # to "no lockfile" rather than a user-facing traceback. try: lockfile = Lockfile.from_path(lock_path) - except Exception as exc: - raise ValueError(f'collider.lock is malformed: {exc}') from exc + except FileNotFoundError: + return [] subprojects_dir = sourcedir / SUBPROJECTS_DIR drifted: list[str] = [] diff --git a/test/config/test_config.py b/test/config/test_config.py index 714317d..ece34ed 100644 --- a/test/config/test_config.py +++ b/test/config/test_config.py @@ -291,4 +291,4 @@ def test_load_invalid_json_exits(mock_home: Path, caplog) -> None: config.load() assert excinfo.value.code == os.EX_DATAERR - assert f'Invalid JSON in "{config_path.as_posix()}"' in caplog.text + assert f'File "{config_path.as_posix()}" is invalid' in caplog.text diff --git a/test/contract/test_install_exit_codes.py b/test/contract/test_install_exit_codes.py index 5304be0..7d9d672 100644 --- a/test/contract/test_install_exit_codes.py +++ b/test/contract/test_install_exit_codes.py @@ -45,12 +45,14 @@ def _init_project(tmp_path: Path, dependencies: list[Dependency] | None = None) Colliderfile(dependencies=dependencies or []).save(tmp_path / Colliderfile.get_filename()) -def _make_context(tmp_path: Path, repo: RepositoryInterface) -> Context: +def _make_context( + tmp_path: Path, repo: RepositoryInterface, cache: WrapCache | None = None +) -> Context: """Build a Context whose single configured repository is the given mock.""" config = MagicMock() config.repositories = {'repo1': repo} config.offline = False - return Context(config=config, cache=WrapCache(tmp_path / 'cache'), offline=False) + return Context(config=config, cache=cache or WrapCache(tmp_path / 'cache'), offline=False) def _run(cmd: Install, tmp_path: Path) -> int: @@ -187,3 +189,62 @@ def test_install_ex_ok_no_collider_dependencies(tmp_path: Path) -> None: cmd = Install(argparse.Namespace(offline=False, frozen=False), context) assert _run(cmd, tmp_path) == os.EX_OK + + +def _init_locked_pkg_project(tmp_path: Path, package: WrapPackage) -> None: + """Write a meson.build, collider.json, and a lockfile pinning the given package.""" + _init_project( + tmp_path, + dependencies=[Dependency('pkg', DependencySource.COLLIDER, None)], + ) + Lockfile( + dependencies={ + 'pkg': LockedPackage( + version='1.0.0', + wrap_hash=compute_wrap_hash(package.wrap_text), + origin=ORIGIN, + ), + }, + ).save(tmp_path / Lockfile.get_filename()) + + +def _origin_repo(package: WrapPackage) -> RepositoryInterface: + """Build a mock origin repository that serves the given package.""" + repo = MagicMock(spec=RepositoryInterface) + repo.origin_url = ORIGIN + repo.requires_network.return_value = False + repo.get_package.return_value = package + return repo + + +def test_install_ex_ioerr_cache_prepare_permission(tmp_path: Path) -> None: + """`collider install` returns EX_IOERR when preparing the package cache is denied.""" + package = _make_package('pkg', '1.0.0', b'payload') + _init_locked_pkg_project(tmp_path, package) + + cache = MagicMock(spec=WrapCache) + cache.prepare_packagecache.side_effect = PermissionError('denied') + context = _make_context(tmp_path, _origin_repo(package), cache=cache) + + cmd = Install(argparse.Namespace(offline=False, frozen=False), context) + + assert _run(cmd, tmp_path) == os.EX_IOERR + + +def test_install_ex_ioerr_wrap_write_permission(tmp_path: Path, monkeypatch) -> None: + """`collider install` returns EX_IOERR when writing the wrap file fails.""" + package = _make_package('pkg', '1.0.0', b'payload') + _init_locked_pkg_project(tmp_path, package) + + # Cache is a no-op mock so prepare_packagecache cannot mask the wrap-write failure. + cache = MagicMock(spec=WrapCache) + context = _make_context(tmp_path, _origin_repo(package), cache=cache) + + def _deny_wrap_write(self: WrapPackage, path: Path) -> None: + raise PermissionError('denied') + + monkeypatch.setattr(WrapPackage, 'install_to_subproject', _deny_wrap_write) + + cmd = Install(argparse.Namespace(offline=False, frozen=False), context) + + assert _run(cmd, tmp_path) == os.EX_IOERR diff --git a/test/contract/test_pkg_search_exit_codes.py b/test/contract/test_pkg_search_exit_codes.py index 4c6dbc4..4af73e8 100644 --- a/test/contract/test_pkg_search_exit_codes.py +++ b/test/contract/test_pkg_search_exit_codes.py @@ -48,3 +48,8 @@ def test_pkg_search_ex_unavailable_cache_empty(): def test_pkg_search_ex_noinput_unknown_repository(): """`pkg search -r ` returns EX_NOINPUT when the repository is unknown.""" assert run_subcommand(Subcommand.PKG, ['search', '-r', 'nonexistent', '.*']) == os.EX_NOINPUT + + +def test_pkg_search_ex_usage_invalid_regex(): + """`pkg search` returns EX_USAGE when the pattern is not a valid regex.""" + assert run_subcommand(Subcommand.PKG, ['search', '[']) == os.EX_USAGE diff --git a/test/contract/test_repo_exit_codes.py b/test/contract/test_repo_exit_codes.py index 598b4fd..ef7432a 100644 --- a/test/contract/test_repo_exit_codes.py +++ b/test/contract/test_repo_exit_codes.py @@ -12,6 +12,7 @@ import pytest from collider.Context import Context +from collider.errors import ColliderUserError from collider.file_model.configfile import ConfigFile from collider.subcommand.Repo import Repo @@ -100,3 +101,18 @@ def test_repo_ex_noinput_remove_nonexistent() -> None: cmd = Repo(_repo_args(name='nonexistent', action='remove'), _make_context()) assert cmd.execute() == os.EX_NOINPUT + + +def test_repo_corrupt_config_propagates_user_error( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """`repo list` re-raises ColliderUserError with the carried exit code on a corrupt config.""" + config_path = tmp_path / 'config.json' + config_path.write_text('{ not valid json', encoding='utf-8') + monkeypatch.setattr('collider.config.get_config_path', lambda: config_path) + + cmd = Repo(_repo_args(action='list'), _make_context()) + + with pytest.raises(ColliderUserError) as excinfo: + cmd.execute() + assert excinfo.value.exit_code == os.EX_DATAERR diff --git a/test/file_model/test_colliderfile.py b/test/file_model/test_colliderfile.py index 7543d85..9a24eae 100644 --- a/test/file_model/test_colliderfile.py +++ b/test/file_model/test_colliderfile.py @@ -1,12 +1,11 @@ # SPDX-License-Identifier: Apache-2.0 # Copyright (c) 2026 MOG Robotics OÜ. -import json - from pathlib import Path import pytest +from collider.errors import ColliderUserError from collider.file_model.colliderfile import Colliderfile from collider.utils.packaging.Dependency import Dependency, DependencySource @@ -89,10 +88,10 @@ def test_colliderfile_validation() -> None: def test_colliderfile_validation_failure_invalid_json(tmp_path: Path) -> None: - """Test loading invalid JSON raises.""" + """Test loading invalid JSON raises a clean user error.""" path = tmp_path / 'collider.json' path.write_text('not json') - with pytest.raises((json.JSONDecodeError, TypeError)): + with pytest.raises(ColliderUserError): Colliderfile.from_path(path) diff --git a/test/file_model/test_configfile.py b/test/file_model/test_configfile.py index c7488e1..7db857c 100644 --- a/test/file_model/test_configfile.py +++ b/test/file_model/test_configfile.py @@ -7,6 +7,7 @@ import pytest +from collider.errors import ColliderUserError from collider.file_model.configfile import ConfigFile, RepoEntry from collider.repository import RepoImplRegistry @@ -171,9 +172,7 @@ def test_config_file_validation_failure(tmp_path: Path): invalid_data = {'repositories': [{'name': 'test', 'url': '/tmp/repo'}]} config_path.write_text(json.dumps(invalid_data)) - # from_path calls validate() which should fail for invalid data - # However, it seems prepare_ctor_kwargs raises TypeError if a required field is missing - # before validate() is even called on the instance. - - with pytest.raises(TypeError): + # prepare_ctor_kwargs rejects the missing required field before validate() runs; + # from_path reports either shape as a clean user error. + with pytest.raises(ColliderUserError): ConfigFile.from_path(config_path) diff --git a/test/file_model/test_file_model.py b/test/file_model/test_file_model.py index 8c724b8..ad12b2c 100644 --- a/test/file_model/test_file_model.py +++ b/test/file_model/test_file_model.py @@ -2,6 +2,7 @@ # Copyright 2026 MOG Robotics OÜ import json +import os from dataclasses import dataclass, field from enum import Enum @@ -11,6 +12,7 @@ import pytest +from collider.errors import ColliderUserError from collider.file_model.FileModelInterface import FileModelInterface @@ -110,8 +112,9 @@ def test_mock_file_model_invalid_json(tmp_path): file_path = tmp_path / 'invalid.json' file_path.write_text('not json', encoding='UTF-8') - with pytest.raises(json.JSONDecodeError): + with pytest.raises(ColliderUserError) as excinfo: MockFileModel.from_path(file_path) + assert excinfo.value.exit_code == os.EX_DATAERR def test_mock_file_model_missing_file(tmp_path): @@ -122,6 +125,20 @@ def test_mock_file_model_missing_file(tmp_path): MockFileModel.from_path(file_path) +@pytest.mark.skipif(os.geteuid() == 0, reason='root ignores file permissions') +def test_mock_file_model_unreadable_file(tmp_path): + """An unreadable file is a clean user error carrying EX_IOERR.""" + file_path = tmp_path / 'unreadable.json' + file_path.write_text('{"name": "x", "version": 1}', encoding='UTF-8') + file_path.chmod(0o000) + try: + with pytest.raises(ColliderUserError) as excinfo: + MockFileModel.from_path(file_path) + assert excinfo.value.exit_code == os.EX_IOERR + finally: + file_path.chmod(0o644) + + def test_enum_serialization_as_dict_and_from_dict(): """Test Enum serialization to values and deserialization back to instances.""" o = Outer(name='x', inner=Inner(tone=Color.RED), palette=[Color.BLUE, Color.RED]) diff --git a/test/file_model/test_lockfile.py b/test/file_model/test_lockfile.py index e3540c1..844546f 100644 --- a/test/file_model/test_lockfile.py +++ b/test/file_model/test_lockfile.py @@ -1,12 +1,11 @@ # SPDX-License-Identifier: Apache-2.0 # Copyright 2026 MOG Robotics OÜ -import json - from pathlib import Path import pytest +from collider.errors import ColliderUserError from collider.file_model.lockfile import LockedPackage, Lockfile, compute_wrap_hash @@ -146,10 +145,10 @@ def test_lockfile_validation_empty() -> None: def test_lockfile_validation_failure_invalid_json(tmp_path: Path) -> None: - """Test loading invalid JSON raises.""" + """Test loading invalid JSON raises a clean user error.""" path = tmp_path / 'collider.lock' path.write_text('not json') - with pytest.raises((json.JSONDecodeError, TypeError)): + with pytest.raises(ColliderUserError): Lockfile.from_path(path) diff --git a/test/repository/implementation/test_mesonwrapdb.py b/test/repository/implementation/test_mesonwrapdb.py index 9fba048..1aaa5c3 100644 --- a/test/repository/implementation/test_mesonwrapdb.py +++ b/test/repository/implementation/test_mesonwrapdb.py @@ -5,11 +5,13 @@ import json import os import time +import urllib.error import urllib.parse import urllib.request import pytest +from collider.errors import ColliderUserError from collider.Package import WrapPackage from collider.repository.entries import RepoPackageEntry from collider.repository.implementation.Wrap import ( @@ -257,8 +259,11 @@ def test_wrap_releases_cache_isolates_same_host_different_path(tmp_path): def test_wrap_from_url_offline_requires_cache(tmp_path): - with pytest.raises(ValueError, match='Offline mode requires cached wrap releases'): + with pytest.raises( + ColliderUserError, match='Offline mode requires cached wrap releases' + ) as excinfo: Wrap.from_url('https://wrapdb.mesonbuild.com/v2/', cache_path=tmp_path, offline=True) + assert excinfo.value.exit_code == os.EX_DATAERR def test_wrap_from_url_uses_ttl_cache_when_fresh(tmp_path, monkeypatch): @@ -282,6 +287,100 @@ def _fake_urlopen(url, **_kwargs): assert make_repo_key('foo', '1.0.0', PackageType.WRAP) in repo.packages +def test_wrap_from_url_corrupt_ttl_cache_refetches(tmp_path, monkeypatch): + """A corrupt within-TTL cache is treated as a miss and refreshed from the network.""" + cache_path = tmp_path / 'cache' + cache_file = _cache_file_for(cache_path, 'https://wrapdb.mesonbuild.com/v2/') + cache_file.parent.mkdir(parents=True, exist_ok=True) + cache_file.write_text('{ not valid json', encoding='utf-8') + + def _fake_urlopen(url, **_kwargs): + return _DummyResponse(json.dumps({'fresh': {'versions': ['3.0.0']}})) + + monkeypatch.setattr(urllib.request, 'urlopen', _fake_urlopen) + + repo = Wrap.from_url('https://wrapdb.mesonbuild.com/v2/', cache_path=cache_path) + assert make_repo_key('fresh', '3.0.0', PackageType.WRAP) in repo.packages + + +def test_wrap_from_url_network_failure_falls_back_to_stale_cache(tmp_path, monkeypatch, caplog): + """A failed refresh serves the stale cache with a warning.""" + cache_path = tmp_path / 'cache' + cache_file = _cache_file_for(cache_path, 'https://wrapdb.mesonbuild.com/v2/') + cache_file.parent.mkdir(parents=True, exist_ok=True) + cache_file.write_text(json.dumps({'stale': {'versions': ['0.1.0']}}), encoding='utf-8') + stale_mtime = time.time() - _RELEASES_TTL_SECONDS - 10 + os.utime(cache_file, (stale_mtime, stale_mtime)) + + def _fail_urlopen(url, **_kwargs): + raise urllib.error.URLError('network down') + + monkeypatch.setattr(urllib.request, 'urlopen', _fail_urlopen) + + with caplog.at_level('WARNING'): + repo = Wrap.from_url('https://wrapdb.mesonbuild.com/v2/', cache_path=cache_path) + assert make_repo_key('stale', '0.1.0', PackageType.WRAP) in repo.packages + assert 'using cached data' in caplog.text + + +def test_wrap_from_url_null_body_raises_user_error(tmp_path, monkeypatch): + """A 200 response with a `null` body is a user-facing data error, not an internal crash.""" + cache_path = tmp_path / 'cache' + + def _null_urlopen(url, **_kwargs): + return _DummyResponse('null') + + monkeypatch.setattr(urllib.request, 'urlopen', _null_urlopen) + + with pytest.raises(ColliderUserError) as excinfo: + Wrap.from_url('https://wrapdb.mesonbuild.com/v2/', cache_path=cache_path) + assert excinfo.value.exit_code == os.EX_DATAERR + + +def test_wrap_from_url_offline_non_object_cache_raises_user_error(tmp_path): + """Offline mode with a non-object cached releases.json is a clean data error.""" + cache_path = tmp_path / 'cache' + cache_file = _cache_file_for(cache_path, 'https://wrapdb.mesonbuild.com/v2/') + cache_file.parent.mkdir(parents=True, exist_ok=True) + cache_file.write_text('[1, 2, 3]', encoding='utf-8') + + with pytest.raises(ColliderUserError) as excinfo: + Wrap.from_url('https://wrapdb.mesonbuild.com/v2/', cache_path=cache_path, offline=True) + assert excinfo.value.exit_code == os.EX_DATAERR + + +def test_wrap_from_url_network_failure_with_corrupt_cache_raises(tmp_path, monkeypatch): + """A failed refresh re-raises the network error when the cache is unusable.""" + cache_path = tmp_path / 'cache' + cache_file = _cache_file_for(cache_path, 'https://wrapdb.mesonbuild.com/v2/') + cache_file.parent.mkdir(parents=True, exist_ok=True) + cache_file.write_text('{ not valid json', encoding='utf-8') + stale_mtime = time.time() - _RELEASES_TTL_SECONDS - 10 + os.utime(cache_file, (stale_mtime, stale_mtime)) + + def _fail_urlopen(url, **_kwargs): + raise urllib.error.URLError('network down') + + monkeypatch.setattr(urllib.request, 'urlopen', _fail_urlopen) + + with pytest.raises(urllib.error.URLError): + Wrap.from_url('https://wrapdb.mesonbuild.com/v2/', cache_path=cache_path) + + +def test_wrap_from_url_offline_corrupt_cache_errors(tmp_path): + """Offline mode with a corrupt cache raises the clean offline error, not a parse crash.""" + cache_path = tmp_path / 'cache' + cache_file = _cache_file_for(cache_path, 'https://wrapdb.mesonbuild.com/v2/') + cache_file.parent.mkdir(parents=True, exist_ok=True) + cache_file.write_text('{ not valid json', encoding='utf-8') + + with pytest.raises( + ColliderUserError, match='Offline mode requires cached wrap releases' + ) as excinfo: + Wrap.from_url('https://wrapdb.mesonbuild.com/v2/', cache_path=cache_path, offline=True) + assert excinfo.value.exit_code == os.EX_DATAERR + + def test_wrap_from_url_fetches_when_ttl_expired(tmp_path, monkeypatch): """Stale cached releases.json (past TTL) triggers a fresh HTTP fetch.""" cache_path = tmp_path / 'cache' diff --git a/test/subcommand/test_info.py b/test/subcommand/test_info.py index 91ad0e3..9eb8f99 100644 --- a/test/subcommand/test_info.py +++ b/test/subcommand/test_info.py @@ -14,6 +14,7 @@ from collider.cache import WrapCache from collider.Context import Context +from collider.errors import ColliderUserError from collider.file_model.colliderfile import Colliderfile from collider.Package import WrapPackage from collider.repository.entries import RepoPackageEntry @@ -190,3 +191,66 @@ def test_info_returns_unavailable_when_package_is_missing( os.chdir(cwd) assert 'No package matching query.' in caplog.text + + +def test_info_unreadable_wrap_due_to_encoding_yields_dataerr(tmp_path: Path) -> None: + """A wrap file with invalid UTF-8 is a data problem and exits with EX_DATAERR.""" + _init_project(tmp_path, [Dependency('demo', DependencySource.COLLIDER, None)]) + subprojects = tmp_path / 'subprojects' + subprojects.mkdir() + (subprojects / 'demo.wrap').write_bytes(b'\xff\xfe\x00bad') + + entry = RepoPackageEntry('demo', '2.0.0') + wrap_repo = Wrap( + urllib.parse.urlparse('https://wrapdb.example.com/v2/'), + {'demo@2.0.0#wrap': entry}, + ) + context = _make_context(tmp_path, {'wrapdb': wrap_repo}) + cmd = Info(argparse.Namespace(package='demo', repository=None), context) + + cwd = os.getcwd() + try: + os.chdir(tmp_path) + with patch( + 'collider.subcommand.pkg.Info.search_packages', + return_value={'wrapdb': {'demo@2.0.0#wrap': entry}}, + ): + with pytest.raises(ColliderUserError) as exc_info: + cmd.execute() + finally: + os.chdir(cwd) + + assert exc_info.value.exit_code == os.EX_DATAERR + + +def test_info_unreadable_wrap_due_to_oserror_yields_ioerr( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A wrap file that cannot be read is an IO problem and exits with EX_IOERR.""" + _init_project(tmp_path, [Dependency('demo', DependencySource.COLLIDER, None)]) + subprojects = tmp_path / 'subprojects' + subprojects.mkdir() + (subprojects / 'demo.wrap').write_text('[wrap-file]\n', encoding='utf-8') + + entry = RepoPackageEntry('demo', '2.0.0') + wrap_repo = Wrap( + urllib.parse.urlparse('https://wrapdb.example.com/v2/'), + {'demo@2.0.0#wrap': entry}, + ) + context = _make_context(tmp_path, {'wrapdb': wrap_repo}) + cmd = Info(argparse.Namespace(package='demo', repository=None), context) + + cwd = os.getcwd() + try: + os.chdir(tmp_path) + with patch( + 'collider.subcommand.pkg.Info.search_packages', + return_value={'wrapdb': {'demo@2.0.0#wrap': entry}}, + ): + with patch('pathlib.Path.read_text', side_effect=PermissionError('denied')): + with pytest.raises(ColliderUserError) as exc_info: + cmd.execute() + finally: + os.chdir(cwd) + + assert exc_info.value.exit_code == os.EX_IOERR diff --git a/test/subcommand/test_install.py b/test/subcommand/test_install.py index 7ee11e3..db5abca 100644 --- a/test/subcommand/test_install.py +++ b/test/subcommand/test_install.py @@ -518,6 +518,41 @@ def test_install_fetch_package_rejects_unsafe_name(tmp_path: Path) -> None: assert install._fetch_package(entry, repo, repo_key) is None +def _make_mock_cache_context(cache: WrapCache) -> Context: + """Build a context around a (possibly mocked) cache.""" + config = MagicMock() + config.repositories = {} + config.offline = False + return Context(config=config, cache=cache, offline=False) + + +def test_install_do_install_reports_cache_oserror(tmp_path: Path, monkeypatch) -> None: + """A permission error while preparing the package cache fails cleanly.""" + from collider.subcommand.Install import Install + + cache = MagicMock(spec=WrapCache) + cache.prepare_packagecache.side_effect = PermissionError('denied') + install = Install(argparse.Namespace(offline=False), _make_mock_cache_context(cache)) + + package = MagicMock(spec=WrapPackage) + monkeypatch.chdir(tmp_path) + assert install._do_install('shared', package) is False + + +def test_install_do_install_reports_wrap_write_oserror(tmp_path: Path, monkeypatch) -> None: + """A permission error while writing the wrap file fails cleanly.""" + from collider.subcommand.Install import Install + + install = Install( + argparse.Namespace(offline=False), _make_mock_cache_context(MagicMock(spec=WrapCache)) + ) + + package = MagicMock(spec=WrapPackage) + package.install_to_subproject.side_effect = PermissionError('denied') + monkeypatch.chdir(tmp_path) + assert install._do_install('shared', package) is False + + def test_install_adds_dependency_without_version(tmp_path: Path, monkeypatch) -> None: """New dependency in collider.json has no version pinned.""" _init_project(tmp_path) diff --git a/test/subcommand/test_prune.py b/test/subcommand/test_prune.py index 215f7c3..a5e60e5 100644 --- a/test/subcommand/test_prune.py +++ b/test/subcommand/test_prune.py @@ -196,14 +196,14 @@ def test_prune_warns_without_lockfile(tmp_path: Path, caplog: pytest.LogCaptureF os.chdir(cwd) assert (subprojects / 'abseil-cpp.wrap').exists() - assert 'No lockfile found' in caplog.text assert 'cannot safely determine which transitive wraps are orphaned' in caplog.text - assert 'Run "collider lock" to create ownership metadata for future operations' in caplog.text assert 'may still need to be removed manually' in caplog.text + # The trailing summary is the last line so scripts can detect the skip. + assert caplog.records[-1].message == 'prune skipped: no lockfile; run "collider lock".' def test_prune_warns_on_corrupt_lockfile(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: - """A corrupt lockfile triggers a warning and preserves all wraps.""" + """A corrupt lockfile fails honestly with the carried exit code and preserves all wraps.""" _init_project(tmp_path, dependencies=[]) subprojects = tmp_path / 'subprojects' subprojects.mkdir() @@ -218,12 +218,12 @@ def test_prune_warns_on_corrupt_lockfile(tmp_path: Path, caplog: pytest.LogCaptu cwd = os.getcwd() try: os.chdir(tmp_path) - assert cmd.execute() == os.EX_OK + assert cmd.execute() == os.EX_DATAERR finally: os.chdir(cwd) assert (subprojects / 'abseil-cpp.wrap').exists() - assert 'could not be read' in caplog.text + assert caplog.records[-1].message == 'prune skipped: unreadable lockfile; run "collider lock".' def test_prune_dry_run_lists_without_deleting( diff --git a/test/subcommand/test_remove.py b/test/subcommand/test_remove.py index 4031189..1e9a7c7 100644 --- a/test/subcommand/test_remove.py +++ b/test/subcommand/test_remove.py @@ -154,6 +154,27 @@ def test_remove_succeeds_when_only_declared_dependency_exists(tmp_path: Path) -> assert colliderfile.dependencies == [] +def test_remove_with_prune_and_no_lockfile_ends_with_skip_summary( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """`remove --prune` without a lockfile ends with the script-detectable skip summary.""" + _init_project( + tmp_path, + dependencies=[Dependency('shared', DependencySource.COLLIDER, None)], + ) + + cmd = Remove(argparse.Namespace(package='shared', prune=True), _make_context(tmp_path)) + + cwd = os.getcwd() + try: + os.chdir(tmp_path) + assert cmd.execute() == os.EX_OK + finally: + os.chdir(cwd) + + assert caplog.records[-1].message == 'prune skipped: no lockfile; run "collider lock".' + + def test_remove_missing_package_returns_noinput(tmp_path: Path) -> None: """Removing an unknown package should fail cleanly.""" _init_project(tmp_path, dependencies=[]) @@ -631,7 +652,8 @@ def test_remove_with_prune_warns_on_corrupt_lockfile( assert not (subprojects / 'grpc.wrap').exists() assert (subprojects / 'abseil-cpp.wrap').exists() - assert 'could not be read' in caplog.text + # The trailing summary is the last line so scripts can detect the skip. + assert caplog.records[-1].message == 'prune skipped: unreadable lockfile; run "collider lock".' def test_remove_keeps_artifacts_when_dependency_index_is_unavailable( diff --git a/test/subcommand/test_search.py b/test/subcommand/test_search.py index 19de845..53fa9bf 100644 --- a/test/subcommand/test_search.py +++ b/test/subcommand/test_search.py @@ -14,6 +14,7 @@ from collider.cache import WrapCache from collider.Context import Context +from collider.errors import ColliderUserError from collider.Package import WrapPackage from collider.repository.entries import RepoPackageEntry from collider.repository.implementation.RepositoryInterface import RepositoryInterface @@ -81,10 +82,11 @@ def test_search_init(mock_context): assert search_cmd.version_pattern == SpecifierSet('>=1.0.0') assert search_cmd.name_pattern.pattern == 'my-pkg.*' - # Test invalid regex (should fail when user fixes the implementation bug) + # An invalid regex is a usage error, reported cleanly instead of as an internal bug. args_invalid = argparse.Namespace(pattern='[', repository=None, version=None, cache=False) - with pytest.raises(re.error): + with pytest.raises(ColliderUserError) as excinfo: Search(args_invalid, mock_context) + assert excinfo.value.exit_code == os.EX_USAGE def test_search_execute_all_repos(mock_context, caplog): diff --git a/test/subcommand/test_setup.py b/test/subcommand/test_setup.py index 97eb3ce..d2b4ed1 100644 --- a/test/subcommand/test_setup.py +++ b/test/subcommand/test_setup.py @@ -14,6 +14,7 @@ import pytest from collider.Context import Context +from collider.errors import ColliderUserError from collider.file_model.lockfile import LockedPackage, Lockfile, compute_wrap_hash from collider.subcommand.Setup import Setup from collider.utils import meson @@ -168,14 +169,17 @@ def test_pass_args_to_meson(tmp_path: Path, meson_project: Path, capfd: pytest.C def test_missing_separator(tmp_path: Path, meson_project: Path, capfd: pytest.CaptureFixture): - """Test that missing -- separator for extra arguments causes failure.""" - with pytest.raises(ValueError): + """Test that missing -- separator for extra arguments causes a clean usage error.""" + assert ( run_subcommand( Subcommand.SETUP, ['--sourcedir', str(meson_project), '--builddir', str(tmp_path), 'reconfigure'], ) + == os.EX_USAGE + ) stdout, stderr = capfd.readouterr() assert 'Expected "--" separator' in stderr + assert 'probably a bug' not in stderr def test_builddir_cleanup_on_failure(tmp_path: Path): @@ -316,7 +320,7 @@ def test_setup_malformed_lock_errors(tmp_path: Path, capfd: pytest.CaptureFixtur ) stdout, stderr = capfd.readouterr() - assert 'collider.lock is malformed' in stderr + assert 'collider.lock' in stderr and 'is invalid' in stderr assert not builddir.exists() @@ -462,7 +466,7 @@ def test_force_args_malformed_lock_raises(tmp_path: Path) -> None: sourcedir = tmp_path / 'project' _write_wrapped_foo_project(sourcedir) (sourcedir / 'collider.lock').write_text('{ not json', encoding='utf-8') - with pytest.raises(ValueError, match='malformed'): + with pytest.raises(ColliderUserError, match='is invalid'): _force_fallback_args(sourcedir) @@ -471,7 +475,7 @@ def test_force_args_malformed_lock_wins_over_user_override(tmp_path: Path) -> No sourcedir = tmp_path / 'project' _write_wrapped_foo_project(sourcedir) (sourcedir / 'collider.lock').write_text('garbage', encoding='utf-8') - with pytest.raises(ValueError, match='malformed'): + with pytest.raises(ColliderUserError, match='is invalid'): _force_fallback_args(sourcedir, ['--force-fallback-for=x']) diff --git a/test/subcommand/test_status.py b/test/subcommand/test_status.py index 3cfd145..9224d6e 100644 --- a/test/subcommand/test_status.py +++ b/test/subcommand/test_status.py @@ -9,6 +9,8 @@ from pathlib import Path from unittest.mock import MagicMock, patch +import resolvelib + from collider.cache import WrapCache from collider.Context import Context from collider.file_model.colliderfile import Colliderfile @@ -39,7 +41,9 @@ def test_status_reports_tracked_and_untracked(tmp_path: Path, caplog) -> None: (subprojects_dir / 'alpha.wrap').write_text('[wrap-file]\n', encoding='utf-8') (subprojects_dir / 'gamma.wrap').write_text('[wrap-file]\n', encoding='utf-8') - context = MagicMock(spec=Context) + config = MagicMock() + config.repositories = {} + context = Context(config=config, cache=WrapCache(tmp_path / 'cache'), offline=False) args = argparse.Namespace() cmd = Status(args, context) @@ -221,7 +225,9 @@ def test_status_without_lockfile_shows_untracked(tmp_path: Path, caplog) -> None (subprojects_dir / 'grpc.wrap').write_text('[wrap-file]\n', encoding='utf-8') (subprojects_dir / 'abseil-cpp.wrap').write_text('[wrap-file]\n', encoding='utf-8') - context = MagicMock(spec=Context) + config = MagicMock() + config.repositories = {} + context = Context(config=config, cache=WrapCache(tmp_path / 'cache'), offline=False) cmd = Status(argparse.Namespace(), context) cwd = os.getcwd() @@ -294,6 +300,43 @@ def test_status_without_lockfile_resolves_transitive(tmp_path: Path, caplog) -> assert ' ‣ manual' in caplog.text +def test_status_warns_when_resolution_fails(tmp_path: Path, caplog) -> None: + """A failed resolution logs a warning instead of silently listing wraps as untracked.""" + dependencies = [Dependency('grpc', DependencySource.COLLIDER, None)] + _init_project(tmp_path, dependencies) + + subprojects_dir = tmp_path / 'subprojects' + subprojects_dir.mkdir() + (subprojects_dir / 'grpc.wrap').write_text('[wrap-file]\n', encoding='utf-8') + (subprojects_dir / 'abseil-cpp.wrap').write_text('[wrap-file]\n', encoding='utf-8') + + config = MagicMock() + config.repositories = {'wrapdb': MagicMock()} + context = Context(config=config, cache=WrapCache(tmp_path / 'cache'), offline=False) + cmd = Status(argparse.Namespace(), context) + + with ( + patch( + 'collider.subcommand.Status.resolve_all_dependencies', + side_effect=resolvelib.ResolutionTooDeep(1), + ), + patch( + 'collider.subcommand.Status.build_dep_name_index', + return_value={'grpc': 'wrapdb'}, + ), + ): + cwd = os.getcwd() + try: + os.chdir(tmp_path) + assert cmd.execute() == os.EX_OK + finally: + os.chdir(cwd) + + assert 'Version resolution failed' in caplog.text + assert '‣ untracked' in caplog.text + assert ' ‣ abseil-cpp' in caplog.text + + def test_status_passes_include_conditional_from_colliderfile( tmp_path: Path, caplog, diff --git a/test/subcommand/test_upgrade.py b/test/subcommand/test_upgrade.py index a41e7e3..1d38a0a 100644 --- a/test/subcommand/test_upgrade.py +++ b/test/subcommand/test_upgrade.py @@ -574,4 +574,4 @@ def test_upgrade_ignores_corrupt_lockfile_warning_check( os.chdir(cwd) assert (tmp_path / 'subprojects' / 'shared.wrap').exists() - assert 'Invalid JSON' in caplog.text + assert 'collider.lock" is invalid' in caplog.text diff --git a/test/utils/packaging/test_resolver.py b/test/utils/packaging/test_resolver.py index 01ece8f..b91a55a 100644 --- a/test/utils/packaging/test_resolver.py +++ b/test/utils/packaging/test_resolver.py @@ -3,6 +3,7 @@ """Tests for the transitive dependency resolver.""" +import os import tarfile import zipfile @@ -12,6 +13,7 @@ import pytest import resolvelib +from collider.errors import ColliderUserError from collider.repository.entries import ( RejectedEntry, RejectReason, @@ -163,6 +165,14 @@ def test_requirement_hash_consistent_with_equality() -> None: assert len({r1, r2}) == 1 +def test_requirement_inequality_by_version_spec() -> None: + """Same-name requirements with different constraints stay distinct.""" + r1 = Requirement('zlib', '>=1.2') + r2 = Requirement('zlib', '>=1.3') + assert r1 != r2 + assert len({r1, r2}) == 2 + + def test_requirement_repr_without_version() -> None: """Repr omits the version when none is set.""" r = Requirement('zlib') @@ -869,6 +879,24 @@ def test_extract_archive_corrupt_returns_false(tmp_path: Path) -> None: assert ColliderProvider._extract_archive(archive, dest) is False +def test_extract_archive_tar_without_data_filter( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Tar extraction works on interpreters without the PEP 706 filter backport.""" + archive = tmp_path / 'test.tar.gz' + content_dir = tmp_path / 'src' + content_dir.mkdir() + (content_dir / 'meson.build').write_text("project('test', 'c')") + with tarfile.open(archive, 'w:gz') as tar: + tar.add(content_dir / 'meson.build', arcname='src/meson.build') + + monkeypatch.delattr(tarfile, 'data_filter') + dest = tmp_path / 'out' + dest.mkdir() + assert ColliderProvider._extract_archive(archive, dest) is True + assert (dest / 'src' / 'meson.build').exists() + + # -- _patch_extract_target ----------------------------------------------------- @@ -1206,6 +1234,39 @@ def test_scan_candidate_online_fetches_from_repo_not_cache() -> None: mock_cache.load_wrap.assert_not_called() +def test_scan_candidate_reports_permission_error_as_user_error() -> None: + """A PermissionError while staging the source becomes a clean user error instead + of being swallowed as an empty dependency list: strict callers catch only resolver + exceptions, so a bare OSError would surface as an internal-bug banner.""" + from collider.cache import WrapCache + from collider.Package import WrapPackage + + package = WrapPackage.from_wrap_text( + 'zlib', + '1.3.1', + '[wrap-file]\n' + 'source_url=https://example.com/zlib-1.3.1.tar.xz\n' + 'source_filename=zlib-1.3.1.tar.xz\n' + 'source_hash=deadbeef\n', + ) + + packages = _make_packages(('zlib', '1.3.1', ['zlib'])) + repo = _make_repo(packages, requires_network=False) + repo.get_package.return_value = package + repos = {'local': repo} + dep_index = build_dep_name_index(repos) + + mock_cache = MagicMock(spec=WrapCache) + mock_cache.prepare_packagecache.side_effect = PermissionError('denied') + + provider = ColliderProvider(repos, dep_index, offline=False, wrap_cache=mock_cache) + candidate = Candidate('zlib', '1.3.1', 'local') + + with pytest.raises(ColliderUserError) as excinfo: + provider._scan_candidate(candidate) + assert excinfo.value.exit_code == os.EX_IOERR + + def test_get_dependencies_scan_cache_is_keyed_by_repo() -> None: """Two repos serving the same name+version are distinct packages, so their scans must not be shared: reusing one repo's scan for the other would bake the wrong dependency graph diff --git a/test/utils/test_project_state.py b/test/utils/test_project_state.py index cda8797..49c1ecd 100644 --- a/test/utils/test_project_state.py +++ b/test/utils/test_project_state.py @@ -3,10 +3,13 @@ """Tests for project-state helpers.""" +import os + from pathlib import Path import pytest +from collider.errors import ColliderUserError from collider.file_model.colliderfile import Colliderfile from collider.file_model.lockfile import LockedPackage, Lockfile, compute_wrap_hash from collider.utils.packaging.Dependency import Dependency, DependencySource @@ -154,8 +157,22 @@ def test_managed_names_unions_lock_and_colliderfile(tmp_path: Path) -> None: def test_managed_names_raises_on_malformed_lock(tmp_path: Path) -> None: """A malformed lockfile is a hard error, not a silent fallback.""" (tmp_path / Lockfile.get_filename()).write_text('{ not valid json', encoding='utf-8') - with pytest.raises(ValueError): + with pytest.raises(ColliderUserError) as excinfo: managed_package_names(tmp_path) + assert excinfo.value.exit_code == os.EX_DATAERR + + +def test_managed_names_tolerates_lock_vanishing_after_exists(tmp_path: Path, monkeypatch) -> None: + """A lock deleted between the exists() check and from_path() is treated as absent.""" + Lockfile( + dependencies={'foo': LockedPackage(version='1.0', wrap_hash=_HASH, origin=_ORIGIN)}, + ).save(tmp_path / Lockfile.get_filename()) + + def _raise_not_found(_path: Path) -> 'Lockfile': + raise FileNotFoundError(_path) + + monkeypatch.setattr(Lockfile, 'from_path', _raise_not_found) + assert managed_package_names(tmp_path) is None def test_managed_names_tolerates_malformed_colliderfile(tmp_path: Path) -> None: @@ -232,10 +249,21 @@ def test_drift_treats_non_utf8_wrap_as_drift(tmp_path: Path) -> None: def test_drift_raises_on_malformed_lock(tmp_path: Path) -> None: """A malformed lock is a hard error, mirroring managed_package_names.""" (tmp_path / Lockfile.get_filename()).write_text('{ not valid json', encoding='utf-8') - with pytest.raises(ValueError, match='malformed'): + with pytest.raises(ColliderUserError, match='is invalid'): detect_locked_wrap_drift(tmp_path) +def test_drift_tolerates_lock_vanishing_after_exists(tmp_path: Path, monkeypatch) -> None: + """A lock deleted between the exists() check and from_path() yields no drift.""" + _lock_with(tmp_path, foo='sha256:' + '0' * 64) + + def _raise_not_found(_path: Path) -> 'Lockfile': + raise FileNotFoundError(_path) + + monkeypatch.setattr(Lockfile, 'from_path', _raise_not_found) + assert detect_locked_wrap_drift(tmp_path) == [] + + # -- remove_installed_artifacts (#45) ----------------------------------------- @@ -249,6 +277,21 @@ def _absent_directory_wrap() -> str: ) +@pytest.mark.skipif(os.geteuid() == 0, reason='root ignores directory permissions') +def test_remove_artifacts_undeletable_wrap_raises_user_error(tmp_path: Path, monkeypatch) -> None: + """An undeletable artifact is a clean user error, not an internal bug.""" + monkeypatch.chdir(tmp_path) + subprojects = tmp_path / 'subprojects' + _write_wrap(subprojects, 'fmt', _wrap_file_text('fmt-10.0.0')) + subprojects.chmod(0o555) + try: + with pytest.raises(ColliderUserError) as excinfo: + remove_installed_artifacts('fmt') + assert excinfo.value.exit_code == os.EX_IOERR + finally: + subprojects.chmod(0o755) + + def test_remove_artifacts_deletes_directory_field_tree(tmp_path: Path, monkeypatch) -> None: """The extracted tree named by `directory=` (not ) is removed.""" monkeypatch.chdir(tmp_path)