diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bf9b31b..0630a06 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -12,7 +12,7 @@ repos: hooks: - id: prettier - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.0.289 + rev: v0.0.291 hooks: - id: ruff args: [--fix, --exit-non-zero-on-fix] diff --git a/README.md b/README.md index 6355ab5..8e3714d 100644 --- a/README.md +++ b/README.md @@ -23,3 +23,7 @@ For development and tests: ## Documentation - https://fedora-ci.github.io/rpmdeplint + +## RPM builds + +- https://copr.fedorainfracloud.org/coprs/g/osci/rpmdeplint diff --git a/docs/CHANGES.rst b/docs/CHANGES.rst index 81d4333..d62bcd1 100644 --- a/docs/CHANGES.rst +++ b/docs/CHANGES.rst @@ -1,6 +1,21 @@ Changelog --------- +2.0rc3 +~~~~~~ + +* Repodata cache now works with metalink/mirrorlist + (`RHBZ#1343247 `__). +* Don't leak directories in :file:`/var/tmp/rpmdeplint-*` + (`RHBZ#1343247 `__). +* Allow passing a metalink for --repo instead of baseurl + (`RHBZ#1454525 `__). +* Allow passing only a configured repo name for ``--repo``. +* Modernize deprecated stuff. +* Refactoring. +* Unit and acceptance tests moved to :file:`tests/`. +* Code from :file:`__init__.py` moved to :file:`analyzer.py`. + 2.0rc2 ~~~~~~ diff --git a/docs/rpmdeplint.rst b/docs/rpmdeplint.rst index f53e92b..5a3bbec 100644 --- a/docs/rpmdeplint.rst +++ b/docs/rpmdeplint.rst @@ -17,24 +17,35 @@ RPM packages against given repositories. Options ~~~~~~~ -.. option:: --repo NAME,URL, -r NAME,URL +.. option:: --repo NAME[,URL_OR_PATH], -r NAME[,URL_OR_PATH] - Load yum repo from the given URL. You can also specify a local filesystem - path instead of a URL. + Load yum repository specified by name or by URL/path. - The NAME is for descriptive purposes only. It has no impact on dependency - resolution. If rpmdeplint finds a dependency problem relating to a package - in this repo, the NAME will appear in the error message. + If the repo is already configured in :file:`/etc/yum.repos.d/{*}.repo` + you can just specify its name, like:: + + --repo=fedora - Note that the URL should point at a directory containing - ``repodata/repomd.xml``. For example:: + Otherwise, specify a name and the baseurl or metalink. The NAME is for + descriptive purposes only in this case. It has no impact on dependency + resolution. If rpmdeplint finds a dependency problem relating to a package + in this repo, the NAME will appear in the error message. Examples:: --repo=fedora,https://download.fedoraproject.org/pub/fedora/linux/development/rawhide/Everything/x86_64/os/ + --repo=fedora,https://mirrors.fedoraproject.org/metalink?repo=rawhide&arch=x86_64 + + You can also specify a local filesystem path instead of a URL, like:: + + --repo=myrepo,/home/me/my_repo + + Note that the URL/path should point at a directory containing + ``repodata/repomd.xml``. Examples:: + .. option:: --repos-from-system, -R - Use yum repos from the system-wide configuration in :file:`/etc/yum.conf` - and :file:`/etc/yum.repos.d/{*}.repo`. Repos which are disabled in the + Use yum repos from the system-wide configuration in + :file:`/etc/yum.repos.d/{*}.repo`. Repos which are disabled in the configuration (``enabled=0``) are ignored. This option can be combined with one or more :option:`--repo` options. @@ -137,6 +148,18 @@ to check if it will cause dependency errors in Fedora:: --repo=fedora,https://download.fedoraproject.org/pub/fedora/linux/development/rawhide/Everything/x86_64/os/ \ greenwave-0.6.1-0.git.2.2529bfb.fc29.noarch.rpm +You don't have to specify the URLs of the repos if they are already configured in `/etc/yum.repos.d/`:: + + rpmdeplint check \ + --repo=fedora --repo=updates \ + greenwave-0.6.1-0.git.2.2529bfb.fc29.noarch.rpm + +or use all configured repos:: + + rpmdeplint check \ + --repos-from-system \ + greenwave-0.6.1-0.git.2.2529bfb.fc29.noarch.rpm + You can also use a local filesystem path instead of an absolute URL for the repos to test against. For example, if you are offline you could re-use your local dnf cache. (Note that rpmdeplint may need to fetch packages for file diff --git a/plans/acceptance.fmf b/plans/acceptance.fmf index 63f0658..84bceda 100644 --- a/plans/acceptance.fmf +++ b/plans/acceptance.fmf @@ -10,4 +10,4 @@ prepare: - 'glibc-devel*i686' execute: script: - - pytest --verbose --showlocals acceptance_tests/ + - pytest --verbose --showlocals tests/acceptance/ diff --git a/plans/unit.fmf b/plans/unit.fmf index 4eb04af..1006823 100644 --- a/plans/unit.fmf +++ b/plans/unit.fmf @@ -7,4 +7,4 @@ prepare: - python3-rpmfluff execute: script: - - pytest --verbose --showlocals rpmdeplint/ + - pytest --verbose --showlocals tests/unit/ diff --git a/pyproject.toml b/pyproject.toml index 38ab6f8..aed6c23 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,7 +55,7 @@ rpmdeplint = "rpmdeplint.cli:main" [tool.hatch.version] source = "vcs" -raw-options.version_scheme = "post-release" +raw-options.version_scheme = "no-guess-dev" [tool.ruff] select = [ diff --git a/rpmdeplint.spec b/rpmdeplint.spec index 697a52a..9bad31d 100644 --- a/rpmdeplint.spec +++ b/rpmdeplint.spec @@ -1,5 +1,5 @@ Name: rpmdeplint -Version: 2.0 +Version: 2.0rc3 Release: %autorelease Summary: Tool to find errors in RPM packages in the context of their dependency graph License: GPL-2.0-or-later @@ -54,7 +54,7 @@ mv docs/_build/man/rpmdeplint.1 %{buildroot}%{_mandir}/man1/ %pyproject_save_files rpmdeplint %check -py.test-3 rpmdeplint -k "not TestDependencyAnalyzer" +%pytest tests/unit/ -k "not TestDependencyAnalyzer" # Acceptance tests do not work in mock because they require .i686 packages. %files diff --git a/rpmdeplint/__init__.py b/rpmdeplint/__init__.py index 483cf45..7bef063 100644 --- a/rpmdeplint/__init__.py +++ b/rpmdeplint/__init__.py @@ -4,538 +4,18 @@ # (at your option) any later version. -import ctypes -import logging -import os -import os.path -from collections import defaultdict -from collections.abc import Iterable -from typing import Any +from contextlib import suppress +from importlib.metadata import PackageNotFoundError, version -import rpm -import solv +from rpmdeplint.analyzer import DependencyAnalyzer, DependencySet +from rpmdeplint.repodata import Repo -logger = logging.getLogger(__name__) +with suppress(PackageNotFoundError): + __version__ = version("rpmdeplint") -installonlypkgs = [ - # The default 'installonlypkgs' from dnf - # https://github.com/rpm-software-management/dnf/blob/dnf-2.5.1-1/dnf/const.py.in#L28 - "kernel", - "kernel-PAE", - "installonlypkg(kernel)", - "installonlypkg(kernel-module)", - "installonlypkg(vm)", - # Additional names which yum 3.4.3 (RHEL7) has in its default 'installonlypkgs' - # https://github.com/rpm-software-management/yum/blob/cf8a5669165e958d56157abf40d0cdd552c8fbf9/yum/config.py#L650 - "kernel-bigmem", - "kernel-enterprise", - "kernel-smp", - "kernel-modules", - "kernel-debug", - "kernel-unsupported", - "kernel-source", - "kernel-devel", - "kernel-PAE-debug", +__all__ = [ + DependencyAnalyzer.__name__, + DependencySet.__name__, + Repo.__name__, ] - - -class UnreadablePackageError(Exception): - """ - Raised if an RPM package cannot be read from disk (it's corrupted, or the - file is not a valid RPM package, etc). - """ - - -class DependencySet: - """ - Contains dependency information from trying to install the packages under test. - """ - - def __init__(self) -> None: - self._packagedeps: defaultdict = defaultdict( - lambda: {"dependencies": [], "problems": []} - ) - self._packages_with_problems: set[str] = set() - self._overall_problems: set[str] = set() - - def add_package(self, pkg, dependencies: Iterable, problems: list): - nevra: str = str(pkg) - self._packagedeps[nevra]["dependencies"].extend(map(str, dependencies)) - if len(problems) != 0: - all_problems = [] - # For each problem, find all the problematic RPM "rules" which - # lead to the problem and also include them in - # the `overall_problems` description. - for problem in problems: - all_problems.append(str(problem)) - for rule in problem.findallproblemrules(): - all_problems.append(rule.info().problemstr()) - self._packagedeps[nevra]["problems"].extend(all_problems) - self._packages_with_problems.add(nevra) - self._overall_problems.update(all_problems) - - @property - def is_ok(self) -> bool: - return len(self._overall_problems) == 0 - - @property - def packages(self) -> list[str]: - return sorted(self._packagedeps.keys()) - - @property - def overall_problems(self) -> list[str]: - """ - List of str dependency problems found (if any) - """ - return sorted(self._overall_problems) - - @property - def packages_with_problems(self) -> list[str]: - """ - List of :py:class:`solv.Solvable` which had dependency problems - """ - return sorted(self._packages_with_problems) - - @property - def package_dependencies(self) -> dict[str, dict[str, list]]: - """ - Dict in the form {package: {'dependencies': list of packages, 'problems': list of problems}} - """ - return dict(self._packagedeps) - - -class DependencyAnalyzer: - """An object which checks packages against provided repos - for dependency satisfiability. - - Construct an instance for a particular set of packages you want to test, - with the repos you want to test against. Then call the individual checking - methods to perform each check. - """ - - def __init__(self, repos: Iterable, packages: Iterable[str], arch=None): - """ - :param repos: An iterable of :py:class:`rpmdeplint.repodata.Repo` instances - :param packages: An iterable of RPM package paths to be tested - """ - # delayed import to avoid circular dependency - from rpmdeplint.repodata import RepoDownloadError - - self.pool = solv.Pool() - self.pool.setarch(arch) - - #: List of :py:class:`solv.Solvable` to be tested (corresponding to *packages* parameter) - self.solvables = [] - self.commandline_repo = self.pool.add_repo("@commandline") - for rpmpath in packages: - solvable = self.commandline_repo.add_rpm(rpmpath) - if solvable is None: - # pool.errstr is already prefixed with the filename - raise UnreadablePackageError( - f"Failed to read package: {self.pool.errstr}" - ) - self.solvables.append(solvable) - - self.repos_by_name = ( - {} - ) #: Mapping of {repo name: :py:class:`rpmdeplint.repodata.Repo`} - for repo in repos: - try: - repo.download_repodata() - except RepoDownloadError as e: - if repo.skip_if_unavailable: - logger.warning("Skipping repo %s: %s", repo.name, e) - continue - raise - solv_repo = self.pool.add_repo(repo.name) - # solv.xfopen does not accept unicode filenames on Python 2 - solv_repo.add_rpmmd( - solv.xfopen_fd(str(repo.primary_url), repo.primary.fileno()), None - ) - solv_repo.add_rpmmd( - solv.xfopen_fd(str(repo.filelists_url), repo.filelists.fileno()), - None, - solv.Repo.REPO_EXTEND_SOLVABLES, - ) - self.repos_by_name[repo.name] = repo - - self.pool.addfileprovides() - self.pool.createwhatprovides() - - # Special handling for "installonly" packages: we create jobs to mark - # installonly package names as "multiversion" and then set those as - # pool jobs, which means the jobs are automatically applied whenever we - # run the solver on this pool. - multiversion_jobs = [] - for name in installonlypkgs: - selection = self.pool.select(name, solv.Selection.SELECTION_PROVIDES) - multiversion_jobs.extend(selection.jobs(solv.Job.SOLVER_MULTIVERSION)) - self.pool.setpooljobs(multiversion_jobs) - - # Context manager protocol is only implemented for backwards compatibility. - # There are actually no resources to acquire or release. - - def __enter__(self): - return self - - def __exit__(self, type, value, tb): - return - - def download_package_header(self, solvable) -> str: - if solvable in self.solvables: - # It's a package under test, nothing to download - return solvable.lookup_location()[0] - href = solvable.lookup_location()[0] - baseurl = solvable.lookup_str(self.pool.str2id("solvable:mediabase")) - repo = self.repos_by_name[solvable.repo.name] - return repo.download_package_header(href, baseurl) - - def try_to_install_all(self) -> DependencySet: - """ - Try to solve the goal of installing each of the packages under test, - starting from an empty package set. - - :return: dependency set - """ - solver = self.pool.Solver() - ds = DependencySet() - for solvable in self.solvables: - logger.debug("Solving install jobs for %s", solvable) - jobs = solvable.Selection().jobs(solv.Job.SOLVER_INSTALL) - if problems := solver.solve(jobs): - ds.add_package(solvable, [], problems) - else: - ds.add_package(solvable, solver.transaction().newsolvables(), []) - return ds - - def _select_obsoleted_by(self, solvables: Iterable) -> Any: - """ - Returns a solv.Selection matching every solvable which is "obsoleted" - by some solvable in the given list -- either due to an explicit - Obsoletes relationship, or because we have a solvable with the same - name with a higher epoch-version-release. - """ - # Start with an empty selection. - sel = self.pool.Selection() - for solvable in solvables: - # Select every solvable with the same name and lower EVR. - # XXX are there some special cases with arch-noarch upgrades which this does not handle? - sel.add( - self.pool.select( - f"{solvable.name}.{solvable.arch} < {solvable.evr}", - solv.Selection.SELECTION_NAME - | solv.Selection.SELECTION_DOTARCH - | solv.Selection.SELECTION_REL, - ) - ) - for obsoletes_rel in solvable.lookup_deparray( - self.pool.str2id("solvable:obsoletes") - ): - # Select every solvable matching the obsoletes relationship by name. - sel.add(obsoletes_rel.Selection_name()) - return sel - - def find_repoclosure_problems(self) -> list[str]: - """ - Checks for any package in the repos which would have unsatisfied - dependencies, if the packages under test were added to the repos. - - This applies some extra constraints to prevent the solver from finding - a solution which involves downgrading or installing an older package, - which is technically a valid solution but is not expected if the - packages are supposed to be updates. - - :return: List of str problem descriptions if any problems were found - """ - problems = [] - solver = self.pool.Solver() - # This selection matches packages obsoleted by our packages under test. - obs_sel = self._select_obsoleted_by(self.solvables) - # This selection matches packages obsoleted by other existing packages in the repo. - existing_obs_sel = self._select_obsoleted_by( - s for s in self.pool.solvables if s.repo.name != "@commandline" - ) - obsoleted = obs_sel.solvables() + existing_obs_sel.solvables() - logger.debug( - "Excluding the following obsoleted packages:\n%s", - "\n".join(f" {s}" for s in obsoleted), - ) - for solvable in self.pool.solvables: - if solvable in self.solvables: - continue # checked by check-sat command instead - if solvable in obsoleted: - continue # no reason to check it - if not self.pool.isknownarch(solvable.archid): - logger.debug( - "Skipping requirements for package %s arch does not match " - "Architecture under test", - str(solvable), - ) - continue - logger.debug("Checking requires for %s", solvable) - # XXX limit available packages to compatible arches? - # (use libsolv archpolicies somehow) - jobs = ( - solvable.Selection().jobs(solv.Job.SOLVER_INSTALL) - + obs_sel.jobs(solv.Job.SOLVER_ERASE) - + existing_obs_sel.jobs(solv.Job.SOLVER_ERASE) - ) - if solver_problems := solver.solve(jobs): - problem_msgs = [str(p) for p in solver_problems] - # If it's a pre-existing problem with repos (that is, the - # problem also exists when the packages under test are - # excluded) then warn about it here but don't consider it - # a problem. - jobs = solvable.Selection().jobs( - solv.Job.SOLVER_INSTALL - ) + existing_obs_sel.jobs(solv.Job.SOLVER_ERASE) - if existing_problems := solver.solve(jobs): - for p in existing_problems: - logger.warning( - "Ignoring pre-existing repoclosure problem: %s", p - ) - else: - problems.extend(problem_msgs) - return problems - - def _files_in_solvable(self, solvable) -> set: - iterator = solvable.Dataiterator( - self.pool.str2id("solvable:filelist"), - None, - solv.Dataiterator.SEARCH_FILES | solv.Dataiterator.SEARCH_COMPLETE_FILELIST, - ) - return {match.str for match in iterator} - - def _packages_can_be_installed_together(self, left, right) -> bool: - """ - Returns True if the given packages can be installed together. - """ - solver = self.pool.Solver() - left_install_jobs = left.Selection().jobs(solv.Job.SOLVER_INSTALL) - right_install_jobs = right.Selection().jobs(solv.Job.SOLVER_INSTALL) - # First check if each one can be installed on its own. If either of - # these fails it is a warning, because it means we have no way to know - # if they can be installed together or not. - if problems := solver.solve(left_install_jobs): - logger.warning( - "Ignoring conflict candidate %s " - "with pre-existing dependency problems: %s", - left, - problems[0], - ) - return False - if problems := solver.solve(right_install_jobs): - logger.warning( - "Ignoring conflict candidate %s " - "with pre-existing dependency problems: %s", - right, - problems[0], - ) - return False - if problems := solver.solve(left_install_jobs + right_install_jobs): - logger.debug( - "Conflict candidates %s and %s cannot be installed together: %s", - left, - right, - problems[0], - ) - return False - return True - - def _file_conflict_is_permitted(self, left, right, filename: str) -> bool: - """ - Returns True if rpm would allow both the given packages to share - ownership of the given filename. - """ - if not hasattr(rpm, "files"): - return self._file_conflict_is_permitted_rpm411(left, right, filename) - - ts = rpm.TransactionSet() - ts.setVSFlags(rpm._RPMVSF_NOSIGNATURES) - - left_hdr = ts.hdrFromFdno(open(left.lookup_location()[0], "rb")) - right_hdr = ts.hdrFromFdno(open(self.download_package_header(right), "rb")) - left_files = rpm.files(left_hdr) - right_files = rpm.files(right_hdr) - if left_files[filename].matches(right_files[filename]): - logger.debug( - "Conflict on %s between %s and %s permitted because files match", - filename, - left, - right, - ) - return True - if left_files[filename].color != right_files[filename].color: - logger.debug( - "Conflict on %s between %s and %s permitted because colors differ", - filename, - left, - right, - ) - return True - return False - - def _file_conflict_is_permitted_rpm411(self, left, right, filename: str) -> bool: - # In rpm 4.12+ the rpmfilesCompare() function is exposed nicely as the - # rpm.files.matches Python method. In earlier rpm versions there is - # nothing equivalent in the Python bindings, although we can use ctypes - # to poke around and call the older rpmfiCompare() C API directly... - librpm = ctypes.CDLL("librpm.so.3") - _rpm = ctypes.CDLL(os.path.join(os.path.dirname(rpm.__file__), "_rpm.so")) - - class rpmfi_s(ctypes.Structure): - pass - - librpm.rpmfiCompare.argtypes = [ - ctypes.POINTER(rpmfi_s), - ctypes.POINTER(rpmfi_s), - ] - librpm.rpmfiCompare.restype = ctypes.c_int - _rpm.fiFromFi.argtypes = [ctypes.py_object] - _rpm.fiFromFi.restype = ctypes.POINTER(rpmfi_s) - - ts = rpm.TransactionSet() - ts.setVSFlags(rpm._RPMVSF_NOSIGNATURES) - - left_hdr = ts.hdrFromFdno(open(left.lookup_location()[0], "rb")) - right_hdr = ts.hdrFromFdno(open(self.download_package_header(right), "rb")) - left_fi = rpm.fi(left_hdr) - try: - while left_fi.FN() != filename: - left_fi.next() - except StopIteration as e: - raise KeyError(f"Entry {filename} not found in {left}") from e - right_fi = rpm.fi(right_hdr) - try: - while right_fi.FN() != filename: - right_fi.next() - except StopIteration as exc: - raise KeyError(f"Entry {filename} not found in {right}") from exc - if librpm.rpmfiCompare(_rpm.fiFromFi(left_fi), _rpm.fiFromFi(right_fi)) == 0: - logger.debug( - "Conflict on %s between %s and %s permitted because files match", - filename, - left, - right, - ) - return True - if left_fi.FColor() != right_fi.FColor(): - logger.debug( - "Conflict on %s between %s and %s permitted because colors differ", - filename, - left, - right, - ) - return True - return False - - def find_conflicts(self) -> list[str]: - """ - Find undeclared file conflicts in the packages under test. - - :return: List of str describing each conflict found - (or empty list if no conflicts were found) - """ - # solver = self.pool.Solver() - problems = [] - for solvable in self.solvables: - logger.debug("Checking all files in %s for conflicts", solvable) - filenames = self._files_in_solvable(solvable) - # In libsolv, iterating all solvables is fast, and listing all - # files in a solvable is fast, but finding solvables which contain - # a given file is *very slow* (see bug 1465736). - # Hence this approach, where we visit each solvable and use Python - # set operations to look for any overlapping filenames. - for conflicting in self.pool.solvables: - # Conflicts cannot happen between identical solvables and also - # between solvables with the same name - such solvables cannot - # be installed next to each other. - if conflicting == solvable or conflicting.name == solvable.name: - continue - conflict_filenames = filenames.intersection( - self._files_in_solvable(conflicting) - ) - if not conflict_filenames: - continue - if not self._packages_can_be_installed_together(solvable, conflicting): - continue - for filename in conflict_filenames: - logger.debug( - "Considering conflict on %s with %s", filename, conflicting - ) - if not self._file_conflict_is_permitted( - solvable, conflicting, filename - ): - msg = ( - f"{solvable} provides {filename} " - f"which is also provided by {conflicting}" - ) - problems.append(msg) - if conflicting not in self.solvables: - # For each filename we are checking, we only want to - # check at most *one* package from the remote - # repositories. This is purely an optimization to save - # network bandwidth and time. We *are* potentially - # missing some real conflicts by doing this, but the - # cost of downloading every package in the distro for - # common directories like /usr/lib/debug is too high. - # Note however that we do always ensure at least one - # *remote* candidate is checked (that is, not from the - # set of packages under test) to catch problems like - # bug 1502458. - logger.debug( - "Skipping further checks on %s " - "to save network bandwidth", - filename, - ) - filenames.remove(filename) - return sorted(problems) - - def find_upgrade_problems(self) -> list[str]: - """ - Checks for any package in the repos which would upgrade or obsolete the - packages under test. - - :return: List of str describing each upgrade problem found (or - empty list if no problems were found) - """ - # Pretend the packages under test are installed, then solve a distupgrade. - # If any package under test would be erased, then it means some other - # package in the repos is better than it, and we have a problem. - self.pool.installed = self.commandline_repo - try: - jobs = self.pool.Selection_all().jobs(solv.Job.SOLVER_UPDATE) - solver = self.pool.Solver() - solver.set_flag(solver.SOLVER_FLAG_ALLOW_UNINSTALL, True) - solver_problems = solver.solve(jobs) - for problem in solver_problems: - # This is a warning, not an error, because it means there are - # some *other* problems with existing packages in the - # repository, not our packages under test. But it means our - # results here might not be valid. - logger.warning( - "Upgrade candidate has pre-existing dependency problem: %s", problem - ) - transaction = solver.transaction() - problems = [] - for solvable in self.solvables: - action = transaction.steptype( - solvable, transaction.SOLVER_TRANSACTION_SHOW_OBSOLETES - ) - other = transaction.othersolvable(solvable) - if action == transaction.SOLVER_TRANSACTION_IGNORE: - continue # it's kept, so no problem here - if action == transaction.SOLVER_TRANSACTION_UPGRADED: - problems.append( - f"{solvable} would be upgraded by {other} from repo {other.repo.name}" - ) - elif action == transaction.SOLVER_TRANSACTION_OBSOLETED: - problems.append( - f"{solvable} would be obsoleted by {other} from repo {other.repo.name}" - ) - else: - raise RuntimeError(f"Unrecognised transaction step type {action}") - return problems - finally: - self.pool.installed = None diff --git a/rpmdeplint/analyzer.py b/rpmdeplint/analyzer.py new file mode 100644 index 0000000..9348d0d --- /dev/null +++ b/rpmdeplint/analyzer.py @@ -0,0 +1,504 @@ +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. + + +from collections import defaultdict +from collections.abc import Iterable +from logging import getLogger +from typing import Optional + +import rpm +from solv import ( + Dataiterator, + Job, + Pool, + Problem, + Selection, + XSolvable, + xfopen_fd, +) +from solv import ( + Repo as SolvRepo, +) + +logger = getLogger(__name__) + + +installonlypkgs = [ + # The default 'installonlypkgs' from dnf + # https://github.com/rpm-software-management/dnf/blob/dnf-2.5.1-1/dnf/const.py.in#L28 + "kernel", + "kernel-PAE", + "installonlypkg(kernel)", + "installonlypkg(kernel-module)", + "installonlypkg(vm)", + # Additional names which yum 3.4.3 (RHEL7) has in its default 'installonlypkgs' + # https://github.com/rpm-software-management/yum/blob/cf8a5669165e958d56157abf40d0cdd552c8fbf9/yum/config.py#L650 + "kernel-bigmem", + "kernel-enterprise", + "kernel-smp", + "kernel-modules", + "kernel-debug", + "kernel-unsupported", + "kernel-source", + "kernel-devel", + "kernel-PAE-debug", +] + + +class UnreadablePackageError(Exception): + """ + Raised if an RPM package cannot be read from disk (it's corrupted, or the + file is not a valid RPM package, etc). + """ + + +class DependencySet: + """ + Contains dependency information from trying to install the packages under test. + """ + + def __init__(self) -> None: + self._packagedeps: defaultdict = defaultdict( + lambda: {"dependencies": [], "problems": []} + ) + self._packages_with_problems: set[str] = set() + self._overall_problems: set[str] = set() + + def add_package( + self, pkg: XSolvable, dependencies: Iterable, problems: list[Problem] + ): + nevra: str = str(pkg) + self._packagedeps[nevra]["dependencies"].extend(map(str, dependencies)) + if problems: + all_problems = [] + # For each problem, find all the problematic RPM "rules" which + # lead to the problem and also include them in + # the `overall_problems` description. + for problem in problems: + all_problems.append(str(problem)) + all_problems.extend( + rule.info().problemstr() for rule in problem.findallproblemrules() + ) + self._packagedeps[nevra]["problems"].extend(all_problems) + self._packages_with_problems.add(nevra) + self._overall_problems.update(all_problems) + + @property + def is_ok(self) -> bool: + return len(self._overall_problems) == 0 + + @property + def packages(self) -> list[str]: + return sorted(self._packagedeps.keys()) + + @property + def overall_problems(self) -> list[str]: + """ + List of str dependency problems found (if any) + """ + return sorted(self._overall_problems) + + @property + def packages_with_problems(self) -> list[str]: + """ + List of :py:class:`solv.Solvable` which had dependency problems + """ + return sorted(self._packages_with_problems) + + @property + def package_dependencies(self) -> dict[str, dict[str, list]]: + """ + Dict in the form {package: {'dependencies': list of packages, 'problems': list of problems}} + """ + return dict(self._packagedeps) + + +class DependencyAnalyzer: + """An object which checks packages against provided repos + for dependency satisfiability. + + Construct an instance for a particular set of packages you want to test, + with the repos you want to test against. Then call the individual checking + methods to perform each check. + """ + + def __init__( + self, + repos: Iterable, + packages: Iterable[str], + arch: Optional[str] = None, + ): + """ + :param repos: An iterable of :py:class:`rpmdeplint.repodata.Repo` instances + :param packages: An iterable of RPM package paths to be tested + """ + # delayed import to avoid circular dependency + from rpmdeplint.repodata import RepoDownloadError + + self.pool = Pool() + self.pool.setarch(arch) + + #: List of :py:class:`solv.Solvable` to be tested (corresponding to *packages* parameter) + self.solvables: list[XSolvable] = [] + self.commandline_repo = self.pool.add_repo("@commandline") + for rpmpath in packages: + solvable = self.commandline_repo.add_rpm(rpmpath) + if solvable is None: + # pool.errstr is already prefixed with the filename + raise UnreadablePackageError( + f"Failed to read package: {self.pool.errstr}" + ) + self.solvables.append(solvable) + + self.repos_by_name = ( + {} + ) #: Mapping of {repo name: :py:class:`rpmdeplint.repodata.Repo`} + for repo in repos: + try: + repo.download_repodata() + except RepoDownloadError as e: + if repo.skip_if_unavailable: + logger.warning("Skipping repo %s: %s", repo.name, e) + continue + raise + solv_repo = self.pool.add_repo(repo.name) + # solv.xfopen does not accept unicode filenames on Python 2 + solv_repo.add_rpmmd( + xfopen_fd(repo.primary_urls[0], repo.primary.fileno()), None + ) + solv_repo.add_rpmmd( + xfopen_fd(repo.filelists_urls[0], repo.filelists.fileno()), + None, + SolvRepo.REPO_EXTEND_SOLVABLES, + ) + self.repos_by_name[repo.name] = repo + + self.pool.addfileprovides() + self.pool.createwhatprovides() + + # Special handling for "installonly" packages: we create jobs to mark + # installonly package names as "multiversion" and then set those as + # pool jobs, which means the jobs are automatically applied whenever we + # run the solver on this pool. + multiversion_jobs = [] + for name in installonlypkgs: + selection = self.pool.select(name, Selection.SELECTION_PROVIDES) + multiversion_jobs.extend(selection.jobs(Job.SOLVER_MULTIVERSION)) + self.pool.setpooljobs(multiversion_jobs) + + # Context manager protocol is only implemented for backwards compatibility. + # There are actually no resources to acquire or release. + + def __enter__(self): + return self + + def __exit__(self, type, value, tb): + return + + def download_package_header(self, solvable: XSolvable) -> str: + if solvable in self.solvables: + # It's a package under test, nothing to download + return solvable.lookup_location()[0] + href = solvable.lookup_location()[0] + baseurl = solvable.lookup_str(self.pool.str2id("solvable:mediabase")) + repo = self.repos_by_name[solvable.repo.name] + return repo.download_package_header(href, baseurl) + + def try_to_install_all(self) -> DependencySet: + """ + Try to solve the goal of installing each of the packages under test, + starting from an empty package set. + + :return: dependency set + """ + solver = self.pool.Solver() + ds = DependencySet() + for solvable in self.solvables: + logger.debug("Solving install jobs for %s", solvable) + jobs = solvable.Selection().jobs(Job.SOLVER_INSTALL) + if problems := solver.solve(jobs): + ds.add_package(solvable, [], problems) + else: + ds.add_package(solvable, solver.transaction().newsolvables(), []) + return ds + + def _select_obsoleted_by(self, solvables: Iterable[XSolvable]) -> Selection: + """ + Returns a Selection matching every solvable which is "obsoleted" + by some solvable in the given list -- either due to an explicit + Obsoletes relationship, or because we have a solvable with the same + name with a higher epoch-version-release. + """ + # Start with an empty selection. + sel = self.pool.Selection() + for solvable in solvables: + # Select every solvable with the same name and lower EVR. + # XXX are there some special cases with arch-noarch upgrades which this does not handle? + sel.add( + self.pool.select( + f"{solvable.name}.{solvable.arch} < {solvable.evr}", + Selection.SELECTION_NAME + | Selection.SELECTION_DOTARCH + | Selection.SELECTION_REL, + ) + ) + for obsoletes_rel in solvable.lookup_deparray( + self.pool.str2id("solvable:obsoletes") + ): + # Select every solvable matching the obsoletes relationship by name. + sel.add(obsoletes_rel.Selection_name()) + return sel + + def find_repoclosure_problems(self) -> list[str]: + """ + Checks for any package in the repos which would have unsatisfied + dependencies, if the packages under test were added to the repos. + + This applies some extra constraints to prevent the solver from finding + a solution which involves downgrading or installing an older package, + which is technically a valid solution but is not expected if the + packages are supposed to be updates. + + :return: List of str problem descriptions if any problems were found + """ + problems = [] + solver = self.pool.Solver() + # This selection matches packages obsoleted by our packages under test. + obs_sel = self._select_obsoleted_by(self.solvables) + # This selection matches packages obsoleted by other existing packages in the repo. + existing_obs_sel = self._select_obsoleted_by( + s for s in self.pool.solvables if s.repo.name != "@commandline" + ) + obsoleted = obs_sel.solvables() + existing_obs_sel.solvables() + logger.debug( + "Excluding the following obsoleted packages:\n%s", + "\n".join(f" {s}" for s in obsoleted), + ) + for solvable in self.pool.solvables: + if solvable in self.solvables: + continue # checked by check-sat command instead + if solvable in obsoleted: + continue # no reason to check it + if not self.pool.isknownarch(solvable.archid): + logger.debug( + "Skipping requirements for package %s arch does not match " + "Architecture under test", + str(solvable), + ) + continue + logger.debug("Checking requires for %s", solvable) + # XXX limit available packages to compatible arches? + # (use libsolv archpolicies somehow) + jobs = ( + solvable.Selection().jobs(Job.SOLVER_INSTALL) + + obs_sel.jobs(Job.SOLVER_ERASE) + + existing_obs_sel.jobs(Job.SOLVER_ERASE) + ) + if solver_problems := solver.solve(jobs): + problem_msgs = [str(p) for p in solver_problems] + # If it's a pre-existing problem with repos (that is, the + # problem also exists when the packages under test are + # excluded) then warn about it here but don't consider it + # a problem. + jobs = solvable.Selection().jobs( + Job.SOLVER_INSTALL + ) + existing_obs_sel.jobs(Job.SOLVER_ERASE) + if existing_problems := solver.solve(jobs): + for p in existing_problems: + logger.warning( + "Ignoring pre-existing repoclosure problem: %s", p + ) + else: + problems.extend(problem_msgs) + return problems + + def _files_in_solvable(self, solvable: XSolvable) -> set[str]: + iterator = solvable.Dataiterator( + self.pool.str2id("solvable:filelist"), + None, + Dataiterator.SEARCH_FILES | Dataiterator.SEARCH_COMPLETE_FILELIST, + ) + return {match.str for match in iterator} + + def _packages_can_be_installed_together( + self, left: XSolvable, right: XSolvable + ) -> bool: + """ + Returns True if the given packages can be installed together. + """ + solver = self.pool.Solver() + left_install_jobs = left.Selection().jobs(Job.SOLVER_INSTALL) + right_install_jobs = right.Selection().jobs(Job.SOLVER_INSTALL) + # First check if each one can be installed on its own. If either of + # these fails it is a warning, because it means we have no way to know + # if they can be installed together or not. + if problems := solver.solve(left_install_jobs): + logger.warning( + "Ignoring conflict candidate %s " + "with pre-existing dependency problems: %s", + left, + problems[0], + ) + return False + if problems := solver.solve(right_install_jobs): + logger.warning( + "Ignoring conflict candidate %s " + "with pre-existing dependency problems: %s", + right, + problems[0], + ) + return False + if problems := solver.solve(left_install_jobs + right_install_jobs): + logger.debug( + "Conflict candidates %s and %s cannot be installed together: %s", + left, + right, + problems[0], + ) + return False + return True + + def _file_conflict_is_permitted( + self, left: XSolvable, right: XSolvable, filename: str + ) -> bool: + """ + Returns True if rpm would allow both the given packages to share + ownership of the given filename. + """ + ts = rpm.TransactionSet() + ts.setVSFlags(rpm._RPMVSF_NOSIGNATURES) + + left_hdr = ts.hdrFromFdno(open(left.lookup_location()[0], "rb")) + right_hdr = ts.hdrFromFdno(open(self.download_package_header(right), "rb")) + left_files = rpm.files(left_hdr) + right_files = rpm.files(right_hdr) + if left_files[filename].matches(right_files[filename]): + logger.debug( + "Conflict on %s between %s and %s permitted because files match", + filename, + left, + right, + ) + return True + if left_files[filename].color != right_files[filename].color: + logger.debug( + "Conflict on %s between %s and %s permitted because colors differ", + filename, + left, + right, + ) + return True + return False + + def find_conflicts(self) -> list[str]: + """ + Find undeclared file conflicts in the packages under test. + + :return: List of str describing each conflict found + (or empty list if no conflicts were found) + """ + # solver = self.pool.Solver() + problems = [] + for solvable in self.solvables: + logger.debug("Checking all files in %s for conflicts", solvable) + filenames = self._files_in_solvable(solvable) + # In libsolv, iterating all solvables is fast, and listing all + # files in a solvable is fast, but finding solvables which contain + # a given file is *very slow* (see bug 1465736). + # Hence this approach, where we visit each solvable and use Python + # set operations to look for any overlapping filenames. + for conflicting in self.pool.solvables: + # Conflicts cannot happen between identical solvables and also + # between solvables with the same name - such solvables cannot + # be installed next to each other. + if conflicting == solvable or conflicting.name == solvable.name: + continue + conflict_filenames = filenames.intersection( + self._files_in_solvable(conflicting) + ) + if not conflict_filenames: + continue + if not self._packages_can_be_installed_together(solvable, conflicting): + continue + for filename in conflict_filenames: + logger.debug( + "Considering conflict on %s with %s", filename, conflicting + ) + if not self._file_conflict_is_permitted( + solvable, conflicting, filename + ): + msg = ( + f"{solvable} provides {filename} " + f"which is also provided by {conflicting}" + ) + problems.append(msg) + if conflicting not in self.solvables: + # For each filename we are checking, we only want to + # check at most *one* package from the remote + # repositories. This is purely an optimization to save + # network bandwidth and time. We *are* potentially + # missing some real conflicts by doing this, but the + # cost of downloading every package in the distro for + # common directories like /usr/lib/debug is too high. + # Note however that we do always ensure at least one + # *remote* candidate is checked (that is, not from the + # set of packages under test) to catch problems like + # bug 1502458. + logger.debug( + "Skipping further checks on %s " + "to save network bandwidth", + filename, + ) + filenames.remove(filename) + return sorted(problems) + + def find_upgrade_problems(self) -> list[str]: + """ + Checks for any package in the repos which would upgrade or obsolete the + packages under test. + + :return: List of str describing each upgrade problem found (or + empty list if no problems were found) + """ + # Pretend the packages under test are installed, then solve a distupgrade. + # If any package under test would be erased, then it means some other + # package in the repos is better than it, and we have a problem. + self.pool.installed = self.commandline_repo + try: + jobs = self.pool.Selection_all().jobs(Job.SOLVER_UPDATE) + solver = self.pool.Solver() + solver.set_flag(solver.SOLVER_FLAG_ALLOW_UNINSTALL, True) + solver_problems = solver.solve(jobs) + for problem in solver_problems: + # This is a warning, not an error, because it means there are + # some *other* problems with existing packages in the + # repository, not our packages under test. But it means our + # results here might not be valid. + logger.warning( + "Upgrade candidate has pre-existing dependency problem: %s", problem + ) + transaction = solver.transaction() + problems = [] + for solvable in self.solvables: + action = transaction.steptype( + solvable, transaction.SOLVER_TRANSACTION_SHOW_OBSOLETES + ) + other = transaction.othersolvable(solvable) + if action == transaction.SOLVER_TRANSACTION_IGNORE: + continue # it's kept, so no problem here + if action == transaction.SOLVER_TRANSACTION_UPGRADED: + problems.append( + f"{solvable} would be upgraded by {other} from repo {other.repo.name}" + ) + elif action == transaction.SOLVER_TRANSACTION_OBSOLETED: + problems.append( + f"{solvable} would be obsoleted by {other} from repo {other.repo.name}" + ) + else: + raise RuntimeError(f"Unrecognised transaction step type {action}") + return problems + finally: + self.pool.installed = None diff --git a/rpmdeplint/cli.py b/rpmdeplint/cli.py index 3b6d1bc..a1b474d 100644 --- a/rpmdeplint/cli.py +++ b/rpmdeplint/cli.py @@ -9,15 +9,13 @@ import sys from collections.abc import Callable from enum import IntEnum -from importlib import metadata -from rpmdeplint import DependencyAnalyzer, UnreadablePackageError +from rpmdeplint import __version__ +from rpmdeplint.analyzer import DependencyAnalyzer, UnreadablePackageError from rpmdeplint.repodata import PackageDownloadError, Repo, RepoDownloadError logger = logging.getLogger(__name__) -version = metadata.version("rpmdeplint") - class ExitCode(IntEnum): OK = 0 @@ -143,13 +141,16 @@ def dependency_analyzer_from_args(args): return DependencyAnalyzer(repos, list(args.rpms), arch=args.arch) -def comma_separated_repo(value: str) -> Repo: - if "," not in value: - raise argparse.ArgumentTypeError( - f"Repo {value!r} is not in the form ," - ) - repo_name, baseurl = value.split(",", 1) - return Repo(repo_name, baseurl) +def repo(value: str) -> Repo: + if "," in value: + repo_name, url_or_path = value.split(",", 1) + if url_or_path.startswith("http") and "/metalink?" in url_or_path: + return Repo(name=repo_name, metalink=url_or_path) + return Repo(name=repo_name, baseurl=url_or_path) + + if repos := list(Repo.from_yum_config(name=value)): + return repos[0] + raise ValueError(f"Repo {value} is not configured") def add_common_dependency_analyzer_args(parser): @@ -162,12 +163,12 @@ def add_common_dependency_analyzer_args(parser): parser.add_argument( "-r", "--repo", - metavar="NAME,URL", - type=comma_separated_repo, + metavar="NAME[,URL_OR_PATH]", + type=repo, action="append", dest="repos", default=[], - help="Name and URL of a repo to test against", + help="Name and optional (baseurl or metalink or local path) of a repo to test against", ) parser.add_argument( "-R", @@ -213,7 +214,9 @@ def add_subparser( "--debug", action="store_true", help="Show detailed progress messages" ) parser.add_argument("--quiet", action="store_true", help="Show only errors") - parser.add_argument("--version", action="version", version=f"%(prog)s {version}") + parser.add_argument( + "--version", action="version", version=f"%(prog)s {__version__}" + ) subparsers = parser.add_subparsers(dest="subcommand", title="subcommands") subparsers.required = True diff --git a/rpmdeplint/repodata.py b/rpmdeplint/repodata.py index e48be07..1df1a24 100644 --- a/rpmdeplint/repodata.py +++ b/rpmdeplint/repodata.py @@ -4,27 +4,26 @@ # (at your option) any later version. -import configparser -import errno -import glob -import logging import os -import shutil -import tempfile -import time +from collections.abc import Iterator +from configparser import ConfigParser +from contextlib import suppress +from logging import getLogger from os import getenv from pathlib import Path -from typing import BinaryIO, Optional +from tempfile import mkdtemp, mkstemp +from time import time +from typing import Any, BinaryIO, Optional, Union import librepo -import requests import rpm +from requests import Session -logger = logging.getLogger(__name__) -requests_session = requests.Session() +logger = getLogger(__name__) +requests_session = Session() -REPO_CACHE_DIR = os.path.join(os.sep, "var", "tmp") +REPO_CACHE_DIR = "/var/tmp" REPO_CACHE_NAME_PREFIX = "rpmdeplint-" @@ -40,74 +39,87 @@ class RepoDownloadError(Exception): """ -def get_yumvars() -> dict[str, str]: - # This is not all the yumvars, but hopefully good enough... - - try: - import dnf.conf - import dnf.rpm - except ImportError: - pass - else: - installroot = "" - subst = dnf.conf.Conf().substitutions - subst["releasever"] = dnf.rpm.detect_releasever(installroot) - return subst - - try: - import rpmUtils - import yum - import yum.config - except ImportError: - pass - else: - return { - "arch": rpmUtils.arch.getCanonArch(), - "basearch": rpmUtils.arch.getBaseArch(), - "releasever": yum.config._getsysver( - "/", ["system-release(releasever)", "redhat-release"] - ), - } - - # Probably not going to work but there's not much else we can do... - return { - "arch": "$arch", - "basearch": "$basearch", - "releasever": "$releasever", - } - - -def substitute_yumvars(s: str, yumvars: dict[str, str]) -> str: - for name, value in yumvars.items(): - s = s.replace(f"${name}", value) - return s - - -def cache_base_path() -> Path: - default_cache_home = Path.home() / ".cache" - return Path(getenv("XDG_CACHE_HOME", default_cache_home)) / "rpmdeplint" - +class Cache: + @staticmethod + def base_path() -> Path: + default_cache_home = Path.home() / ".cache" + return Path(getenv("XDG_CACHE_HOME", default_cache_home)) / "rpmdeplint" + + @staticmethod + def entry_path(checksum: str) -> Path: + return Cache.base_path() / checksum[:1] / checksum[1:] + + @staticmethod + def clean(): + expiry_time = time() - float(getenv("RPMDEPLINT_EXPIRY_SECONDS", 604800)) + if not Cache.base_path().is_dir(): + return # nothing to do + for subdir in Cache.base_path().iterdir(): + # Should be a subdirectory named after the first checksum letter + if not subdir.is_dir(): + continue + for entry in subdir.iterdir(): + if not entry.is_file(): + continue + if entry.stat().st_mtime < expiry_time: + logger.debug("Purging expired cache file %s", entry) + entry.unlink() + + @staticmethod + def download_repodata_file(checksum: str, urls: list[str]) -> BinaryIO: + """ + Each created file in cache becomes immutable, and is referenced in + the directory tree within XDG_CACHE_HOME as + $XDG_CACHE_HOME/rpmdeplint// -def cache_entry_path(checksum: str) -> Path: - return cache_base_path() / checksum[:1] / checksum[1:] + Both metadata and the files to be cached are written to a tempdir first + then renamed to the cache dir atomically to avoid them potentially being + accessed before written to cache. + """ + if not urls: + raise ValueError("No urls specified to download repodata from") + filepath_in_cache: Path = Cache.entry_path(checksum) + if filepath_in_cache.is_file(): + f = filepath_in_cache.open(mode="rb") + logger.debug("Using cached file %s for %s", filepath_in_cache, urls[0]) + # Bump the modtime on the cache file we are using, + # since our cache expiry is LRU based on modtime. + os.utime(f.fileno()) + return f -def clean_cache(): - expiry_time = time.time() - float( - os.environ.get("RPMDEPLINT_EXPIRY_SECONDS", "604800") - ) - if not cache_base_path().is_dir(): - return # nothing to do - for subdir in cache_base_path().iterdir(): - # Should be a subdirectory named after the first checksum letter - if not subdir.is_dir(): - continue - for entry in subdir.iterdir(): - if not entry.is_file(): - continue - if entry.stat().st_mtime < expiry_time: - logger.debug("Purging expired cache file %s", entry) - entry.unlink() + filepath_in_cache.parent.mkdir(parents=True, exist_ok=True) + fd, temp_path = mkstemp(dir=filepath_in_cache.parent, text=False) + try: + f = os.fdopen(fd, "wb+") + except Exception: + os.close(fd) + raise + try: + for index, url in enumerate(urls): + try: + logger.debug("Downloading %s", url) + response = requests_session.get(url, stream=True) + response.raise_for_status() + for chunk in response.raw.stream(decode_content=False): + f.write(chunk) + response.close() + break + except OSError as e: + msg = f"Failed to download repodata file {url}, {e}" + if index == len(urls) - 1: # last url in the list + raise RepoDownloadError(msg) from e + logger.debug(msg) + f.flush() + f.seek(0) + os.fchmod(f.fileno(), 0o644) + os.rename(temp_path, filepath_in_cache) + logger.debug("Cached as %s", filepath_in_cache) + return f + except Exception: + f.close() + os.unlink(temp_path) + raise class Repo: @@ -115,20 +127,42 @@ class Repo: Represents a Yum ("repomd") package repository to test dependencies against. """ - yum_main_config_path = "/etc/yum.conf" - yum_repos_config_glob = "/etc/yum.repos.d/*.repo" + yum_repos_d = Path("/etc/yum.repos.d/") + + @staticmethod + def get_yumvars() -> dict[str, str]: + with suppress(ModuleNotFoundError): + import dnf.conf + import dnf.rpm + + subst = dnf.conf.Conf().substitutions + subst["releasever"] = dnf.rpm.detect_releasever(installroot="") + return subst + + # Probably not going to work but there's not much else we can do... + return { + "arch": "$arch", + "basearch": "$basearch", + "releasever": "$releasever", + } @classmethod - def from_yum_config(cls): + def from_yum_config(cls, name: str = "") -> Iterator["Repo"]: """ - Yields Repo instances loaded from the system-wide Yum - configuration in :file:`/etc/yum.conf` and :file:`/etc/yum.repos.d/`. + Yields Repo instances loaded from the system-wide + configuration in :file:`/etc/yum.repos.d/`. """ - yumvars = get_yumvars() - config = configparser.RawConfigParser() - config.read([cls.yum_main_config_path, *glob.glob(cls.yum_repos_config_glob)]) + + def substitute_yumvars(s: str, _yumvars: dict[str, str]) -> str: + for name, value in _yumvars.items(): + s = s.replace(f"${name}", value) + return s + + yumvars = cls.get_yumvars() + config = ConfigParser() + config.read(cls.yum_repos_d.glob("*.repo")) for section in config.sections(): - if section == "main": + if name and section != name: continue if config.has_option(section, "enabled") and not config.getboolean( section, "enabled" @@ -164,13 +198,13 @@ def from_yum_config(cls): def __init__( self, - repo_name: str, + name: str, baseurl: Optional[str] = None, metalink: Optional[str] = None, skip_if_unavailable: bool = False, ): """ - :param repo_name: Name of the repository, for example "fedora-updates" + :param name: Name of the repository, for example "fedora-updates" (used in problems and error messages) :param baseurl: URL or filesystem path to the base of the repository (there should be a repodata subdirectory under this) @@ -181,121 +215,79 @@ def __init__( Exactly one of the *baseurl* or *metalink* parameters must be supplied. """ - self.name = repo_name + self.name = name if not baseurl and not metalink: - raise RuntimeError("Must specify either baseurl or metalink for repo") - self.baseurl = baseurl + raise ValueError("Must specify either baseurl or metalink for repo") + if baseurl and not baseurl.startswith("http"): + baseurl = baseurl.removeprefix("file://") + if not Path(baseurl).is_dir(): + raise ValueError(f"baseurl {baseurl!r} is not a local directory") + self.urls = [baseurl] if baseurl else [] self.metalink = metalink self.skip_if_unavailable = skip_if_unavailable + self.is_local = baseurl is not None and Path(baseurl).is_dir() + self._librepo_handle = librepo.Handle() + self._rpmmd_repomd: Optional[dict[str, Any]] = None + self.primary: Optional[BinaryIO] = None + self.filelists: Optional[BinaryIO] = None + def download_repodata(self): - clean_cache() + Cache.clean() + self._download_metadata_result() + if self.is_local: + self.primary = open(self.primary_urls[0], "rb") + self.filelists = open(self.filelists_urls[0], "rb") + else: + self.primary = Cache.download_repodata_file( + self.primary_checksum, self.primary_urls + ) + self.filelists = Cache.download_repodata_file( + self.filelists_checksum, self.filelists_urls + ) + + def _download_metadata_result(self) -> None: + def perform() -> librepo.Result: + try: + return self._librepo_handle.perform() + except librepo.LibrepoException as ex: + raise RepoDownloadError( + f"Failed to download repo metadata for {self!r}: {ex.args[1]}" + ) from ex + logger.debug( - "Loading repodata for %s from %s", self.name, self.baseurl or self.metalink + "Loading repodata for repo %r from %s", + self.name, + self.urls or self.metalink, ) - self.librepo_handle = h = librepo.Handle() - r = librepo.Result() + + h = self._librepo_handle h.repotype = librepo.LR_YUMREPO - if self.baseurl: - h.urls = [self.baseurl] + if self.urls: + h.urls = self.urls if self.metalink: - h.mirrorlist = self.metalink - h.setopt( - librepo.LRO_DESTDIR, - tempfile.mkdtemp( - self.name, prefix=REPO_CACHE_NAME_PREFIX, dir=REPO_CACHE_DIR - ), - ) - h.setopt(librepo.LRO_INTERRUPTIBLE, True) - h.setopt(librepo.LRO_YUMDLIST, []) - if self.baseurl and os.path.isdir(self.baseurl): - self._download_metadata_result(h, r) - self._yum_repomd = r.yum_repomd - self._root_path = self.baseurl - self.primary = open(self.primary_url, "rb") - self.filelists = open(self.filelists_url, "rb") + h.metalinkurl = self.metalink + if self.is_local: + # no files will be downloaded + h.local = True else: - self._root_path = h.destdir = tempfile.mkdtemp( + # tempdir for repomd.xml (metadata) and downloaded rpms + h.destdir = mkdtemp( self.name, prefix=REPO_CACHE_NAME_PREFIX, dir=REPO_CACHE_DIR ) - self._download_metadata_result(h, r) - self._yum_repomd = r.yum_repomd - self.primary = self._download_repodata_file( - self.primary_checksum, self.primary_url - ) - self.filelists = self._download_repodata_file( - self.filelists_checksum, self.filelists_url - ) + h.interruptible = True # Download is interruptible + h.yumdlist = [] # Download only repomd.xml from repodata/ - def _download_metadata_result(self, handle: librepo.Handle, result: librepo.Result): - try: - handle.perform(result) - except librepo.LibrepoException as ex: - raise RepoDownloadError( - f"Failed to download repodata for {self!r}: {ex.args[1]}" - ) from ex - - def _download_repodata_file(self, checksum: str, url: str) -> BinaryIO: - """ - Each created file in cache becomes immutable, and is referenced in - the directory tree within XDG_CACHE_HOME as - $XDG_CACHE_HOME/rpmdeplint// + result = perform() + self._rpmmd_repomd = result.rpmmd_repomd - Both metadata and the files to be cached are written to a tempdir first - then renamed to the cache dir atomically to avoid them potentially being - accessed before written to cache. - """ - filepath_in_cache: Path = cache_entry_path(checksum) - try: - f = open(filepath_in_cache, "rb") - except OSError as e: - if e.errno == errno.ENOENT: - pass # cache entry does not exist, we will download it - elif e.errno == errno.EISDIR: - # This is the original cache directory layout, merged in commit - # 6f11c3708, although it didn't appear in any released version - # of rpmdeplint. To be helpful we will fix it up, by just - # deleting the directory and letting it be replaced by a file. - shutil.rmtree(filepath_in_cache, ignore_errors=True) - else: - raise - else: - logger.debug("Using cached file %s for %s", filepath_in_cache, url) - # Bump the modtime on the cache file we are using, - # since our cache expiry is LRU based on modtime. - os.utime(f.fileno()) # Python 3.3+ - return f - filepath_in_cache.parent.mkdir(parents=True, exist_ok=True) - fd, temp_path = tempfile.mkstemp(dir=filepath_in_cache.parent, text=False) - logger.debug("Downloading %s to cache temp file %s", url, temp_path) - try: - f = os.fdopen(fd, "wb+") - except Exception: - os.close(fd) - raise - try: - try: - response = requests_session.get(url, stream=True) - response.raise_for_status() - for chunk in response.raw.stream(decode_content=False): - f.write(chunk) - response.close() - except OSError as e: - raise RepoDownloadError( - f"Failed to download repodata file {os.path.basename(url)} for {self!r}: {e}" - ) from e - f.flush() - f.seek(0) - os.fchmod(f.fileno(), 0o644) - os.rename(temp_path, filepath_in_cache) - logger.debug("Using cached file %s for %s", filepath_in_cache, url) - return f - except Exception: - f.close() - os.unlink(temp_path) - raise + if self.metalink: + # Only download & parse metalink/mirrorlist + h.fetchmirrors = True + perform() + self.urls.extend(m for m in h.mirrors if m.startswith("http")) - def _is_header_complete(self, local_path: str) -> bool: + def _is_header_complete(self, local_path: Union[Path, str]) -> bool: """ Returns `True` if the RPM file `local_path` has complete RPM header. """ @@ -343,7 +335,7 @@ def download_package_header(self, location: str, baseurl: str) -> str: only when complete RPM file is downloaded. """ local_path = os.path.join(self._root_path, os.path.basename(location)) - if self.librepo_handle.local: + if self.is_local: logger.debug("Using package %s from local filesystem directly", local_path) return local_path @@ -358,7 +350,7 @@ def download_package_header(self, location: str, baseurl: str) -> str: location, base_url=baseurl, dest=self._root_path, - handle=self.librepo_handle, + handle=self._librepo_handle, byterangestart=0, byterangeend=byterangeend, ) @@ -379,36 +371,41 @@ def download_package_header(self, location: str, baseurl: str) -> str: return target.local_path @property - def yum_repomd(self): - return self._yum_repomd + def _root_path(self) -> str: + # Path to the local repo dir or + # tempdir with downloaded repomd.xml and rpms + return self.urls[0] if self.is_local else self._librepo_handle.destdir @property - def repomd_fn(self) -> str: - return os.path.join(self._root_path, "repodata", "repomd.xml") + def repomd(self) -> dict[str, Any]: + if not self._rpmmd_repomd: + raise RuntimeError("_rpmmd_repomd is not set") + return self._rpmmd_repomd @property - def primary_url(self) -> str: - if not self.baseurl: - raise RuntimeError("baseurl not specified") - return os.path.join(self.baseurl, self.yum_repomd["primary"]["location_href"]) + def primary_checksum(self) -> str: + return self.repomd["records"]["primary"]["checksum"] @property - def primary_checksum(self) -> str: - return self.yum_repomd["primary"]["checksum"] + def primary_urls(self) -> list[str]: + location_href = self.repomd["records"]["primary"]["location_href"] + return [os.path.join(url, location_href) for url in self.urls] @property def filelists_checksum(self) -> str: - return self.yum_repomd["filelists"]["checksum"] + return self.repomd["records"]["filelists"]["checksum"] @property - def filelists_url(self) -> str: - if not self.baseurl: - raise RuntimeError("baseurl not specified") - return os.path.join(self.baseurl, self.yum_repomd["filelists"]["location_href"]) + def filelists_urls(self) -> list[str]: + location_href = self.repomd["records"]["filelists"]["location_href"] + return [os.path.join(url, location_href) for url in self.urls] def __repr__(self): - if self.baseurl: - return f"{self.__class__.__name__}(repo_name={self.name!r}, baseurl={self.baseurl!r})" - if self.metalink: - return f"{self.__class__.__name__}(repo_name={self.name!r}, metalink={self.metalink!r})" - return f"{self.__class__.__name__}(repo_name={self.name!r})" + return ( + "Repo(" + f"name={self.name!r}, " + f"urls={self.urls!r}, " + f"metalink={self.metalink!r}, " + f"skip_if_unavailable={self.skip_if_unavailable!r}, " + f"is_local={self.is_local!r})" + ) diff --git a/rpmdeplint/tests/test_repodata.py b/rpmdeplint/tests/test_repodata.py deleted file mode 100644 index c9e012b..0000000 --- a/rpmdeplint/tests/test_repodata.py +++ /dev/null @@ -1,135 +0,0 @@ -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. - -import os -import platform - -import pytest - -from rpmdeplint.repodata import Repo, RepoDownloadError, get_yumvars - - -@pytest.fixture() -def yumdir(tmpdir, monkeypatch): - tmpdir.join("yum.conf").write("[main]\n") - monkeypatch.setattr(Repo, "yum_main_config_path", str(tmpdir.join("yum.conf"))) - monkeypatch.setattr( - Repo, "yum_repos_config_glob", str(tmpdir.join("yum.repos.d", "*.repo")) - ) - return tmpdir - - -def test_loads_system_yum_repo_with_baseurl(yumdir): - yumdir.join("yum.repos.d", "dummy.repo").write( - "[dummy]\nname=Dummy\nbaseurl=http://example.invalid/dummy\n", ensure=True - ) - - repos = list(Repo.from_yum_config()) - assert len(repos) == 1 - assert repos[0].name == "dummy" - assert repos[0].baseurl == "http://example.invalid/dummy" - assert repos[0].metalink is None - - -def test_loads_system_yum_repo_with_metalink(yumdir): - yumdir.join("yum.repos.d", "dummy.repo").write( - "[dummy]\nname=Dummy\nmetalink=http://example.invalid/dummy\n", ensure=True - ) - - repos = list(Repo.from_yum_config()) - assert len(repos) == 1 - assert repos[0].name == "dummy" - assert repos[0].baseurl is None - assert repos[0].metalink == "http://example.invalid/dummy" - - -def test_loads_system_yum_repo_with_mirrorlist(yumdir): - yumdir.join("yum.repos.d", "dummy.repo").write( - "[dummy]\nname=Dummy\nmirrorlist=http://example.invalid/dummy\n", ensure=True - ) - - repos = list(Repo.from_yum_config()) - assert len(repos) == 1 - assert repos[0].name == "dummy" - assert repos[0].baseurl is None - assert repos[0].metalink == "http://example.invalid/dummy" - - -def test_skips_disabled_system_yum_repo(yumdir): - yumdir.join("yum.repos.d", "dummy.repo").write( - "[dummy]\nname=Dummy\nbaseurl=http://example.invalid/dummy\nenabled=0\n", - ensure=True, - ) - - repos = list(Repo.from_yum_config()) - assert not repos - - -def test_loads_system_yum_repo_with_substitutions(yumdir, monkeypatch): - yumdir.join("yum.repos.d", "dummy.repo").write( - "[dummy]\nname=Dummy\nbaseurl=http://example.invalid/$releasever/$basearch/\n", - ensure=True, - ) - monkeypatch.setattr( - "rpmdeplint.repodata.get_yumvars", - lambda: { - "releasever": "21", - "basearch": "s390x", - }, - ) - - repos = list(Repo.from_yum_config()) - assert len(repos) == 1 - assert repos[0].name == "dummy" - assert repos[0].baseurl == "http://example.invalid/21/s390x/" - - -def test_yumvars(): - # The expected values are dependent on the system where we are running, and - # also will be different in mock for example (where neither yum nor dnf are - # present). So the best we can do is touch the code path and makes sure it - # gives back some values. - yumvars = get_yumvars() - if ( - "ID=fedora\nVERSION_ID=25\n" in open("/etc/os-release").read() - and os.path.exists("/usr/bin/dnf") - and platform.machine() == "x86_64" - ): - # The common case on developer's machines - assert yumvars["arch"] == "x86_64" - assert yumvars["basearch"] == "x86_64" - assert yumvars["releasever"] == "25" - else: - # Everywhere else, just assume it's fine - assert "arch" in yumvars - assert "basearch" in yumvars - assert "releasever" in yumvars - - -def test_bad_repo_url_raises_error(yumdir): - yumdir.join("yum.repos.d", "dummy.repo").write( - "[dummy]\nname=Dummy\nbaseurl=http://example.invalid/dummy\nenabled=1\n", - ensure=True, - ) - - repos = list(Repo.from_yum_config()) - assert len(repos) == 1 - with pytest.raises(RepoDownloadError) as rde: - repos[0].download_repodata() - assert "Cannot download repomd.xml" in str(rde.value) - assert "repo_name='dummy'" in str(rde.value) - - -def test_skip_if_unavailable_is_obeyed(yumdir): - yumdir.join("yum.repos.d", "dummy.repo").write( - "[dummy]\nname=Dummy\nbaseurl=http://example.invalid/dummy\n" - "enabled=1\nskip_if_unavailable=1\n", - ensure=True, - ) - - repos = list(Repo.from_yum_config()) - assert len(repos) == 1 - assert repos[0].name == "dummy" - assert repos[0].skip_if_unavailable is True diff --git a/acceptance_tests/conftest.py b/tests/acceptance/conftest.py similarity index 100% rename from acceptance_tests/conftest.py rename to tests/acceptance/conftest.py diff --git a/acceptance_tests/data/b-0.1-1.i386.rpm b/tests/acceptance/data/b-0.1-1.i386.rpm similarity index 100% rename from acceptance_tests/data/b-0.1-1.i386.rpm rename to tests/acceptance/data/b-0.1-1.i386.rpm diff --git a/acceptance_tests/data_setup.py b/tests/acceptance/data_setup.py similarity index 100% rename from acceptance_tests/data_setup.py rename to tests/acceptance/data_setup.py diff --git a/acceptance_tests/test_check.py b/tests/acceptance/test_check.py similarity index 84% rename from acceptance_tests/test_check.py rename to tests/acceptance/test_check.py index 1416c3a..a702bae 100644 --- a/acceptance_tests/test_check.py +++ b/tests/acceptance/test_check.py @@ -12,10 +12,10 @@ from rpmfluff import SimpleRpmBuild, SourceFile from rpmfluff.yumrepobuild import YumRepoBuild -from rpmdeplint.repodata import cache_entry_path +from rpmdeplint.repodata import Cache -def expected_cache_path(repodir: str, name: str, old=False) -> Path: +def expected_cache_path(repodir: str, name: str) -> Path: """ For the test repo located in *repodir*, return the path within the rpmdeplint cache where we expect the metadata file with given suffix @@ -23,7 +23,7 @@ def expected_cache_path(repodir: str, name: str, old=False) -> Path: """ file = next(Path(repodir, "repodata").glob(f"*-{name}.*")) checksum = file.name.split("-", 1)[0] - return cache_entry_path(checksum) / file.name if old else cache_entry_path(checksum) + return Cache.entry_path(checksum) def test_finds_all_problems(request, dir_server): @@ -61,7 +61,7 @@ def test_finds_all_problems(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) for p in repo_packages + test_packages: - shutil.rmtree(p.get_base_dir()) + p.clean() request.addfinalizer(cleanUp) @@ -98,8 +98,8 @@ def test_guesses_arch_when_combined_with_noarch_package(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p_noarch.get_base_dir()) - shutil.rmtree(p_archful.get_base_dir()) + p_noarch.clean() + p_archful.clean() request.addfinalizer(cleanUp) @@ -128,7 +128,7 @@ def test_cache_is_used_when_available(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p1.get_base_dir()) + p1.clean() request.addfinalizer(cleanUp) @@ -183,7 +183,7 @@ def test_cache_doesnt_grow_unboundedly(request, dir_server): def cleanup(): shutil.rmtree(firstrepo.repoDir) - shutil.rmtree(p1.get_base_dir()) + p1.clean() request.addfinalizer(cleanup) @@ -210,7 +210,7 @@ def cleanup(): def cleanup2(): shutil.rmtree(secondrepo.repoDir) - shutil.rmtree(p2.get_base_dir()) + p2.clean() request.addfinalizer(cleanup2) @@ -239,46 +239,13 @@ def cleanup2(): assert second_filelists_cache_path.exists() -def test_migrates_old_cache_layout(request, dir_server): - p1 = SimpleRpmBuild("a", "0.1", "1", ["i386"]) - repo = YumRepoBuild([p1]) - repo.make("i386") - dir_server.basepath = repo.repoDir - - def cleanUp(): - shutil.rmtree(repo.repoDir) - shutil.rmtree(p1.get_base_dir()) - - request.addfinalizer(cleanUp) - - old_cache_path = expected_cache_path(repo.repoDir, "primary.xml", old=True) - new_cache_path = expected_cache_path(repo.repoDir, "primary.xml") - - # Simulate the old cache path left over from an older version of rpmdeplint - old_cache_path.parent.mkdir(parents=True) - old_cache_path.write_text("lol\n") - - exitcode, out, err = run_rpmdeplint( - [ - "rpmdeplint", - "check", - f"--repo=base,{dir_server.url}", - p1.get_built_rpm("i386"), - ] - ) - assert exitcode == 0 - assert err == "" - assert not old_cache_path.exists() - assert new_cache_path.is_file() - - def test_prints_error_on_repo_download_failure(request): # Specifically we don't want an unhandled exception, because that triggers abrt. test_tool_rpm = SimpleRpmBuild("test-tool", "10", "3.el6", ["x86_64"]) test_tool_rpm.make() def cleanUp(): - shutil.rmtree(test_tool_rpm.get_base_dir()) + test_tool_rpm.clean() request.addfinalizer(cleanUp) @@ -292,7 +259,7 @@ def cleanUp(): ) assert exitcode == 1 - assert err.startswith("Failed to download repodata") + assert err.startswith("Failed to download repo metadata") assert "Traceback" not in err @@ -311,7 +278,7 @@ def test_prints_error_on_repodata_file_download_failure(request, dir_server): def cleanUp(): shutil.rmtree(repo.repoDir) - shutil.rmtree(p1.get_base_dir()) + p1.clean() request.addfinalizer(cleanUp) @@ -325,6 +292,5 @@ def cleanUp(): ) assert exitcode == 1 - assert err.startswith("Failed to download repodata") - assert "404" in err + assert err.startswith("Failed to download repodata file") assert "Traceback" not in err diff --git a/acceptance_tests/test_check_conflicts.py b/tests/acceptance/test_check_conflicts.py similarity index 93% rename from acceptance_tests/test_check_conflicts.py rename to tests/acceptance/test_check_conflicts.py index 8b40275..0dba02e 100644 --- a/acceptance_tests/test_check_conflicts.py +++ b/tests/acceptance/test_check_conflicts.py @@ -33,8 +33,8 @@ def test_finds_undeclared_file_conflict(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -71,8 +71,8 @@ def test_finds_undeclared_file_conflict_with_repo_on_local_filesystem(request): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -110,8 +110,8 @@ def test_package_does_not_conflict_with_earlier_version_of_itself(request, dir_s def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -150,8 +150,8 @@ def test_conflict_is_ignored_for_rpm_level_conflicts(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -188,8 +188,8 @@ def test_conflict_is_ignored_if_files_match(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -240,10 +240,10 @@ def test_conflict_not_ignored_if_contents_match_but_perms_differ(request, dir_se def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(basepackage.get_base_dir()) - shutil.rmtree(different_mode.get_base_dir()) - shutil.rmtree(different_owner.get_base_dir()) - shutil.rmtree(different_group.get_base_dir()) + basepackage.clean() + different_mode.clean() + different_owner.clean() + different_group.clean() request.addfinalizer(cleanUp) @@ -292,8 +292,8 @@ def test_conflict_is_ignored_if_file_colors_are_different(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -333,7 +333,7 @@ def test_does_not_fail_with_signed_rpms(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) + p2.clean() request.addfinalizer(cleanUp) @@ -372,8 +372,8 @@ def test_conflict_is_ignored_if_not_installable_concurrently(request, dir_server def cleanUp(): shutil.rmtree(repo.repoDir) - shutil.rmtree(glib_28.get_base_dir()) - shutil.rmtree(glib_26.get_base_dir()) + glib_28.clean() + glib_26.clean() request.addfinalizer(cleanUp) @@ -415,8 +415,8 @@ def test_finds_conflicts_in_installonly_packages(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(kernel1.get_base_dir()) - shutil.rmtree(kernel2.get_base_dir()) + kernel1.clean() + kernel2.clean() request.addfinalizer(cleanUp) @@ -465,8 +465,8 @@ def test_finds_conflict_against_older_subpackage(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(oldvim.get_base_dir()) - shutil.rmtree(newvim.get_base_dir()) + oldvim.clean() + newvim.clean() request.addfinalizer(cleanUp) @@ -521,8 +521,8 @@ def test_obeys_xml_base_when_downloading_packages(request, tmpdir, dir_server): p1.make() def cleanUp(): - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) diff --git a/acceptance_tests/test_check_repoclosure.py b/tests/acceptance/test_check_repoclosure.py similarity index 88% rename from acceptance_tests/test_check_repoclosure.py rename to tests/acceptance/test_check_repoclosure.py index eb1c7f3..109e6a2 100644 --- a/acceptance_tests/test_check_repoclosure.py +++ b/tests/acceptance/test_check_repoclosure.py @@ -28,9 +28,9 @@ def test_catches_soname_change(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p_depending.get_base_dir()) - shutil.rmtree(p_older.get_base_dir()) - shutil.rmtree(p_newer.get_base_dir()) + p_depending.clean() + p_older.clean() + p_newer.clean() request.addfinalizer(cleanUp) @@ -67,9 +67,9 @@ def test_catches_soname_change_with_package_rename(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p_depending.get_base_dir()) - shutil.rmtree(p_older.get_base_dir()) - shutil.rmtree(p_newer.get_base_dir()) + p_depending.clean() + p_older.clean() + p_newer.clean() request.addfinalizer(cleanUp) @@ -102,8 +102,8 @@ def test_ignores_dependency_problems_in_packages_under_test(request, dir_server) def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -140,10 +140,10 @@ def test_ignores_problems_in_older_packages(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p_older_depending.get_base_dir()) - shutil.rmtree(p_newer_depending.get_base_dir()) - shutil.rmtree(p_older.get_base_dir()) - shutil.rmtree(p_newer.get_base_dir()) + p_older_depending.clean() + p_newer_depending.clean() + p_older.clean() + p_newer.clean() request.addfinalizer(cleanUp) @@ -179,10 +179,10 @@ def test_ignores_problems_in_obsoleted_packages(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p_obsolete_depending.get_base_dir()) - shutil.rmtree(p_newer_depending.get_base_dir()) - shutil.rmtree(p_older.get_base_dir()) - shutil.rmtree(p_newer.get_base_dir()) + p_obsolete_depending.clean() + p_newer_depending.clean() + p_older.clean() + p_newer.clean() request.addfinalizer(cleanUp) @@ -212,8 +212,8 @@ def test_warns_on_preexisting_repoclosure_problems(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -247,9 +247,9 @@ def test_works_on_different_platform_to_current(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(grep.get_base_dir()) - shutil.rmtree(needs_grep.get_base_dir()) - shutil.rmtree(package_to_test.get_base_dir()) + grep.clean() + needs_grep.clean() + package_to_test.clean() request.addfinalizer(cleanUp) @@ -282,9 +282,9 @@ def test_arch_set_manually_is_passed_to_sack(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(grep.get_base_dir()) - shutil.rmtree(needs_grep.get_base_dir()) - shutil.rmtree(package_to_test.get_base_dir()) + grep.clean() + needs_grep.clean() + package_to_test.clean() request.addfinalizer(cleanUp) diff --git a/acceptance_tests/test_check_sat.py b/tests/acceptance/test_check_sat.py similarity index 78% rename from acceptance_tests/test_check_sat.py rename to tests/acceptance/test_check_sat.py index 23a2856..ef83722 100644 --- a/acceptance_tests/test_check_sat.py +++ b/tests/acceptance/test_check_sat.py @@ -22,8 +22,8 @@ def test_shows_error_for_rpms(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -43,12 +43,9 @@ def cleanUp(): assert out == "" -def test_error_if_repository_names_not_provided(tmpdir): +def test_error_if_repository_names_not_provided(tmp_path): exitcode, out, err = run_rpmdeplint( - ["rpmdeplint", "check-sat", f"--repo={tmpdir.dirpath()}"] + ["rpmdeplint", "check-sat", f"--repo={tmp_path}"] ) assert exitcode == 2 - assert ( - f"error: argument -r/--repo: Repo '{tmpdir.dirpath()}' is not in the form ," - in err - ) + assert f"error: argument -r/--repo: invalid repo value: '{tmp_path}'" in err diff --git a/acceptance_tests/test_check_upgrade.py b/tests/acceptance/test_check_upgrade.py similarity index 91% rename from acceptance_tests/test_check_upgrade.py rename to tests/acceptance/test_check_upgrade.py index 57263b6..377051a 100644 --- a/acceptance_tests/test_check_upgrade.py +++ b/tests/acceptance/test_check_upgrade.py @@ -22,8 +22,8 @@ def test_finds_newer_version_in_repo(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -56,8 +56,8 @@ def test_finds_obsoleting_package_in_repo(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -89,8 +89,8 @@ def test_epoch(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) diff --git a/acceptance_tests/test_list_deps.py b/tests/acceptance/test_list_deps.py similarity index 90% rename from acceptance_tests/test_list_deps.py rename to tests/acceptance/test_list_deps.py index 07744cb..beb8a9d 100644 --- a/acceptance_tests/test_list_deps.py +++ b/tests/acceptance/test_list_deps.py @@ -22,8 +22,8 @@ def test_lists_dependencies_for_rpms(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -53,8 +53,8 @@ def test_lists_dependencies_for_rpms_served_from_filesystem(request): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) - shutil.rmtree(p1.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -81,8 +81,8 @@ def test_errors_out_for_unsatisfiable_deps(request, dir_server): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p1.get_base_dir()) - shutil.rmtree(p2.get_base_dir()) + p2.clean() + p1.clean() request.addfinalizer(cleanUp) @@ -97,13 +97,13 @@ def cleanUp(): assert exitcode == 3 -def test_rpmdeplint_errors_on_unavailble_url(request): +def test_rpmdeplint_errors_on_unavailable_url(request): url = "http://example.test" p1 = SimpleRpmBuild("a", "0.1", "1", ["i386"]) p1.make() def cleanUp(): - shutil.rmtree(p1.get_base_dir()) + p1.clean() request.addfinalizer(cleanUp) @@ -139,7 +139,7 @@ def test_handles_invalid_rpm_without_crashing(request, dir_server, tmpdir): def cleanUp(): shutil.rmtree(baserepo.repoDir) - shutil.rmtree(p2.get_base_dir()) + p2.clean() request.addfinalizer(cleanUp) diff --git a/acceptance_tests/test_usage.py b/tests/acceptance/test_usage.py similarity index 100% rename from acceptance_tests/test_usage.py rename to tests/acceptance/test_usage.py diff --git a/rpmdeplint/tests/__init__.py b/tests/unit/__init__.py similarity index 100% rename from rpmdeplint/tests/__init__.py rename to tests/unit/__init__.py diff --git a/rpmdeplint/tests/test_dependency_analyzer.py b/tests/unit/test_dependency_analyzer.py similarity index 93% rename from rpmdeplint/tests/test_dependency_analyzer.py rename to tests/unit/test_dependency_analyzer.py index be03291..2618b3c 100644 --- a/rpmdeplint/tests/test_dependency_analyzer.py +++ b/tests/unit/test_dependency_analyzer.py @@ -8,7 +8,7 @@ from rpmfluff import SimpleRpmBuild from rpmfluff.yumrepobuild import YumRepoBuild -from rpmdeplint import DependencyAnalyzer +from rpmdeplint.analyzer import DependencyAnalyzer from rpmdeplint.repodata import Repo @@ -46,7 +46,7 @@ def test_repos(self): self.addCleanup(lemon_meringue_pie.clean) da = DependencyAnalyzer( - repos=[Repo(repo_name="base_1", baseurl=base_1_repo.repoDir)], + repos=[Repo(name="base_1", baseurl=base_1_repo.repoDir)], packages=[ apple.get_built_rpm("x86_64"), lemon_meringue_pie.get_built_rpm("x86_64"), @@ -74,8 +74,8 @@ def test_repos(self): da = DependencyAnalyzer( repos=[ - Repo(repo_name="base_1", baseurl=base_1_repo.repoDir), - Repo(repo_name="base_2", baseurl=base_2_repo.repoDir), + Repo(name="base_1", baseurl=base_1_repo.repoDir), + Repo(name="base_2", baseurl=base_2_repo.repoDir), ], packages=[ apple.get_built_rpm("x86_64"), diff --git a/rpmdeplint/tests/test_dependency_set.py b/tests/unit/test_dependency_set.py similarity index 98% rename from rpmdeplint/tests/test_dependency_set.py rename to tests/unit/test_dependency_set.py index 8fc6b68..a673aac 100644 --- a/rpmdeplint/tests/test_dependency_set.py +++ b/tests/unit/test_dependency_set.py @@ -5,7 +5,7 @@ from typing import ClassVar from unittest import TestCase -from rpmdeplint import DependencySet +from rpmdeplint.analyzer import DependencySet class test_pkg: diff --git a/tests/unit/test_repodata.py b/tests/unit/test_repodata.py new file mode 100644 index 0000000..fffa689 --- /dev/null +++ b/tests/unit/test_repodata.py @@ -0,0 +1,183 @@ +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. + +import platform +import shutil +from pathlib import Path + +import pytest +from rpmfluff import SimpleRpmBuild +from rpmfluff.yumrepobuild import YumRepoBuild + +from rpmdeplint.repodata import Repo, RepoDownloadError + + +@pytest.fixture() +def yumdir(tmp_path, monkeypatch): + monkeypatch.setattr(Repo, "yum_repos_d", tmp_path) + return tmp_path + + +def test_loads_system_yum_repo_with_baseurl(yumdir): + yumdir.joinpath("dummy.repo").write_text( + "[dummy]\nname=Dummy\nbaseurl=http://example.invalid/dummy\n" + ) + + repos = list(Repo.from_yum_config()) + assert len(repos) == 1 + assert repos[0].name == "dummy" + assert repos[0].urls[0] == "http://example.invalid/dummy" + assert repos[0].metalink is None + + +def test_loads_system_yum_repo_specific(yumdir): + yumdir.joinpath("first.repo").write_text( + "[first]\nname=First\nbaseurl=http://example.invalid/first\n" + ) + yumdir.joinpath("dummy.repo").write_text( + "[dummy]\nname=Dummy\nbaseurl=http://example.invalid/dummy\n\n" + "[dummy-debuginfo]\nname=Dummy debuginfo\nbaseurl=http://example.invalid/other\n" + ) + yumdir.joinpath("last.repo").write_text( + "[last]\nname=Last\nbaseurl=http://example.invalid/last\n" + ) + + repos = list(Repo.from_yum_config(name="dummy")) + assert len(repos) == 1 + assert repos[0].name == "dummy" + assert repos[0].urls[0] == "http://example.invalid/dummy" + assert repos[0].metalink is None + + +def test_loads_system_yum_repo_with_metalink(yumdir): + yumdir.joinpath("dummy.repo").write_text( + "[dummy]\nname=Dummy\nmetalink=http://example.invalid/dummy\n" + ) + + repos = list(Repo.from_yum_config()) + assert len(repos) == 1 + assert repos[0].name == "dummy" + assert not repos[0].urls + assert repos[0].metalink == "http://example.invalid/dummy" + + +def test_loads_system_yum_repo_with_mirrorlist(yumdir): + yumdir.joinpath("dummy.repo").write_text( + "[dummy]\nname=Dummy\nmirrorlist=http://example.invalid/dummy\n" + ) + + repos = list(Repo.from_yum_config()) + assert len(repos) == 1 + assert repos[0].name == "dummy" + assert not repos[0].urls + assert repos[0].metalink == "http://example.invalid/dummy" + + +def test_loads_system_yum_repo_local(yumdir): + local_repo = yumdir / "local_repo" + local_repo.mkdir() + yumdir.joinpath("dummy.repo").write_text( + f"[dummy]\nname=Dummy\nbaseurl=file://{local_repo}\n" + ) + + repos = list(Repo.from_yum_config()) + assert len(repos) == 1 + assert repos[0].name == "dummy" + assert repos[0].urls[0] == str(local_repo) + assert repos[0].is_local + assert repos[0].metalink is None + + +def test_skips_disabled_system_yum_repo(yumdir): + yumdir.joinpath("dummy.repo").write_text( + "[dummy]\nname=Dummy\nbaseurl=http://example.invalid/dummy\nenabled=0\n" + ) + + repos = list(Repo.from_yum_config()) + assert not repos + + +def test_loads_system_yum_repo_with_substitutions(yumdir, monkeypatch): + yumdir.joinpath("dummy.repo").write_text( + "[dummy]\nname=Dummy\nbaseurl=http://example.invalid/$releasever/$basearch/\n" + ) + monkeypatch.setattr( + "rpmdeplint.repodata.Repo.get_yumvars", + lambda: { + "releasever": "21", + "basearch": "s390x", + }, + ) + + repos = list(Repo.from_yum_config()) + assert len(repos) == 1 + assert repos[0].name == "dummy" + assert repos[0].urls[0] == "http://example.invalid/21/s390x/" + + +def test_yumvars(): + # The expected values are dependent on the system where we are running, and + # also will be different in mock for example (where neither yum nor dnf are + # present). So the best we can do is touch the code path and makes sure it + # gives back some values. + yumvars = Repo.get_yumvars() + if Path("/usr/bin/dnf").is_file(): + # The common case on developer's machines + assert yumvars["arch"] == platform.machine() + assert yumvars["basearch"] == platform.machine() + assert yumvars["releasever"] + else: + # Everywhere else, just assume it's fine + assert "arch" in yumvars + assert "basearch" in yumvars + assert "releasever" in yumvars + + +def test_bad_repo_url_raises_error(yumdir): + yumdir.joinpath("dummy.repo").write_text( + "[dummy]\nname=Dummy\nbaseurl=http://example.invalid/dummy\nenabled=1\n", + ) + + repos = list(Repo.from_yum_config()) + assert len(repos) == 1 + with pytest.raises(RepoDownloadError) as rde: + repos[0].download_repodata() + assert "Cannot download repomd.xml" in str(rde.value) + assert "name='dummy'" in str(rde.value) + + +def test_skip_if_unavailable_is_obeyed(yumdir): + yumdir.joinpath("dummy.repo").write_text( + "[dummy]\nname=Dummy\nbaseurl=http://example.invalid/dummy\n" + "enabled=1\nskip_if_unavailable=1\n", + ) + + repos = list(Repo.from_yum_config()) + assert len(repos) == 1 + assert repos[0].name == "dummy" + assert repos[0].skip_if_unavailable is True + + +def test_download_repodata_from_local_repo(request): + satori = SimpleRpmBuild("satori", "1", "3", ["noarch"]) + repobuild = YumRepoBuild([satori]) + repobuild.make("noarch") + + def cleanUp(): + shutil.rmtree(repobuild.repoDir) + satori.clean() + + request.addfinalizer(cleanUp) + + repo = Repo(name="dummy", baseurl=repobuild.repoDir) + assert repo.is_local + repo.download_repodata() + assert repo.repomd + assert repo.primary_checksum + assert repo.primary_urls + assert repo.primary + assert repo.filelists_checksum + assert repo.filelists_urls + assert repo.filelists