From 97a2759d548ce804cc0a04cbeb8a9284ea2c7dd6 Mon Sep 17 00:00:00 2001 From: Vendula Poncova Date: Fri, 20 Jan 2023 12:25:03 +0100 Subject: [PATCH 01/40] Use the DNF5 API Use DNF5 in the Anaconda system installer for the package installation. --- anaconda.spec.in | 4 +- .../payloads/payload/dnf/dnf_manager.py | 526 ++++++---- .../payloads/payload/dnf/download_progress.py | 76 +- .../payloads/payload/dnf/initialization.py | 21 +- .../payload/dnf/transaction_progress.py | 148 ++- .../payloads/payload/dnf/validation.py | 25 +- .../test_module_payload_dnf5_manager.py | 982 ++++++++++++++++++ .../test_module_payload_dnf_manager.py | 913 +--------------- 8 files changed, 1450 insertions(+), 1245 deletions(-) create mode 100644 tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py diff --git a/anaconda.spec.in b/anaconda.spec.in index a7f0b85b452..cf6b359a676 100644 --- a/anaconda.spec.in +++ b/anaconda.spec.in @@ -26,7 +26,7 @@ Source0: https://github.com/rhinstaller/%{name}/releases/download/%{name}-%{vers %endif %define dasbusver 1.3 %define dbusver 1.2.3 -%define dnfver 3.6.0 +%define dnfver 5.0.5-1 %define dracutver 102-3 %define fcoeutilsver 1.0.12-3.20100323git %define gettextver 0.19.8 @@ -90,7 +90,7 @@ Requires: python3-libs %if 0%{?rhel} > 10 || 0%{?fedora} > 40 Requires: python3-crypt-r %endif -Requires: python3-dnf >= %{dnfver} +Requires: python3-libdnf5 >= %{dnfver} Requires: python3-blivet >= %{pythonblivetver} Requires: python3-blockdev >= %{libblockdevver} Requires: python3-meh >= %{mehver} diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 44860bfe534..87a30fe3cc8 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -22,12 +22,7 @@ import threading import traceback -import dnf -import dnf.exceptions -import dnf.module.module_base -import dnf.repo -import dnf.subject -import libdnf.conf +import libdnf5 from blivet.size import Size from pyanaconda.anaconda_loggers import get_module_logger @@ -40,7 +35,7 @@ URL_TYPE_METALINK, URL_TYPE_MIRRORLIST, ) -from pyanaconda.core.i18n import _ +from pyanaconda.core.path import join_paths from pyanaconda.core.payload import ProxyString, ProxyStringError from pyanaconda.core.util import get_os_release_value from pyanaconda.modules.common.errors.installation import PayloadInstallationError @@ -55,6 +50,7 @@ ) from pyanaconda.modules.common.structures.packages import PackagesConfigurationData from pyanaconda.modules.common.structures.payload import RepoConfigurationData +from pyanaconda.modules.common.structures.validation import ValidationReport from pyanaconda.modules.payloads.constants import DNF_REPO_DIRS from pyanaconda.modules.payloads.payload.dnf.download_progress import DownloadProgress from pyanaconda.modules.payloads.payload.dnf.transaction_progress import ( @@ -104,16 +100,53 @@ class InvalidSelectionError(DNFManagerError): """The software selection couldn't be resolved.""" +class DNFConfigWrapper(): + """This is a temporary wrapper of a DNF config object.""" + + def __init__(self, config): + """Wrap the DNF config object.""" + self._config = config + + def __getattr__(self, name): + """Get the attribute. + + Called when an attribute lookup has not found + the attribute in the usual places. + """ + option = getattr(self._config, name)() + return option.get_value() + + def __setattr__(self, name, value): + """Set the attribute. + + Called when an attribute assignment is attempted. + """ + if name in ["_config"]: + return super().__setattr__(name, value) + + option = getattr(self._config, name)() + option.set(value) + + +def simplify_config(config): + """Simplify the specified DNF config object.""" + return DNFConfigWrapper(config) + + class DNFManager: """The abstraction of the DNF base.""" def __init__(self): self.__base = None + self.__goal = None + # Protect access to _base.repos to ensure that the dictionary is not # modified while another thread is attempting to iterate over it. The # lock only needs to be held during operations that change the number # of repos or that iterate over the repos. self._lock = threading.RLock() + + self._transaction = None self._ignore_missing_packages = False self._ignore_broken_packages = False self._download_location = None @@ -128,18 +161,29 @@ def _base(self): return self.__base + @property + def _goal(self): + """The DNF goal.""" + if self.__goal is None: + self.__goal = libdnf5.base.Goal(self._base) + + return self.__goal + @classmethod def _create_base(cls): """Create a new DNF base.""" - base = dnf.Base() - base.conf.read() - base.conf.cachedir = DNF_CACHE_DIR - base.conf.pluginconfpath = DNF_PLUGINCONF_DIR - base.conf.logdir = '/tmp/' + base = libdnf5.base.Base() + base.load_config_from_file() + + config = simplify_config(base.get_config()) + config.reposdir = DNF_REPO_DIRS + config.cachedir = DNF_CACHE_DIR + config.pluginconfpath = DNF_PLUGINCONF_DIR + config.logdir = '/tmp/' # Set installer defaults - base.conf.gpgcheck = False - base.conf.skip_if_unavailable = False + config.gpgcheck = False + config.skip_if_unavailable = False # Set the default release version. base.conf.releasever = get_product_release_version() @@ -148,22 +192,25 @@ def _create_base(cls): base.conf.substitutions.update_from_etc("/") # Set the installation root. - base.conf.installroot = conf.target.system_root - base.conf.prepend_installroot('persistdir') + config.installroot = conf.target.system_root + config.persistdir = join_paths( + conf.target.system_root, + config.persistdir + ) # Set the platform id based on the /os/release present # in the installation environment. platform_id = get_os_release_value("PLATFORM_ID") if platform_id is not None: - base.conf.module_platform_id = platform_id + config.module_platform_id = platform_id - # Start with an empty comps so we can go ahead and use - # the environment and group properties. Unset reposdir - # to ensure dnf has nothing it can check automatically. - base.conf.reposdir = [] - base.read_comps(arch_filter=True) - base.conf.reposdir = DNF_REPO_DIRS + # Load vars and do other initialization based on the + # configuration. The method is supposed to be called + # after configuration is updated, but before repositories + # are loaded or any query created. + # FIXME: Should we do that here? + #base.setup() log.debug("The DNF base has been created.") return base @@ -175,18 +222,14 @@ def reset_base(self): * Reset all attributes of the DNF manager. * The new DNF base will be created on demand. """ - base = self.__base self.__base = None - - if base is not None: - base.close() - + self.__goal = None + self._transaction = None self._ignore_missing_packages = False self._ignore_broken_packages = False self._download_location = None self._md_hashes = {} self._enabled_system_repositories = [] - log.debug("The DNF base has been reset.") def configure_base(self, data: PackagesConfigurationData): @@ -194,18 +237,21 @@ def configure_base(self, data: PackagesConfigurationData): :param data: a packages configuration data """ - base = self._base - base.conf.multilib_policy = data.multilib_policy + config = simplify_config(self._base.get_config()) + config.multilib_policy = data.multilib_policy if data.timeout != DNF_DEFAULT_TIMEOUT: - base.conf.timeout = data.timeout + config.timeout = data.timeout if data.retries != DNF_DEFAULT_RETRIES: - base.conf.retries = data.retries + config.retries = data.retries self._ignore_missing_packages = data.missing_ignored self._ignore_broken_packages = data.broken_ignored + # FIXME: Set up skip broken? + # config.skip_broken = data.broken_ignored + if self._ignore_broken_packages: log.warning( "\n***********************************************\n" @@ -217,7 +263,7 @@ def configure_base(self, data: PackagesConfigurationData): # Two reasons to turn this off: # 1. Minimal installs don't want all the extras this brings in. # 2. Installs aren't reproducible due to weak deps. failing silently. - base.conf.install_weak_deps = not data.weakdeps_excluded + config.install_weak_deps = not data.weakdeps_excluded @property def default_environment(self): @@ -241,18 +287,21 @@ def environments(self): :return: a list of ids """ - return [env.id for env in self._base.comps.environments] + environments = libdnf5.comps.EnvironmentQuery(self._base) + return [env.get_environmentid() for env in environments] def _get_environment(self, environment_name): """Translate the given environment name to a DNF object. :param environment_name: an identifier of an environment - :return: a DNF object or None + :return libdnf5.comps.Environment: a DNF object or None """ if not environment_name: return None - return self._base.comps.environment_by_pattern(environment_name) + environments = libdnf5.comps.EnvironmentQuery(self._base) + environments.filter_name(environment_name) + return next(iter(environments), None) def resolve_environment(self, environment_name): """Translate the given environment name to a group ID. @@ -265,7 +314,7 @@ def resolve_environment(self, environment_name): if not env: return None - return env.id + return env.get_environmentid() def get_environment_data(self, environment_name) -> CompsEnvironmentData: """Get the data of the specified environment. @@ -288,23 +337,27 @@ def _get_environment_data(self, env) -> CompsEnvironmentData: :return: an instance of CompsEnvironmentData """ data = CompsEnvironmentData() - data.id = env.id or "" - data.name = env.ui_name or "" - data.description = env.ui_description or "" + data.id = env.get_environmentid() or "" + data.name = env.get_translated_name() or "" + data.description = env.get_translated_description() or "" - optional = {i.name for i in env.option_ids} - default = {i.name for i in env.option_ids if i.default} + available_groups = libdnf5.comps.GroupQuery(self._base) + optional_groups = set(env.get_optional_groups()) - for grp in self._base.comps.groups: + for group in available_groups: + group_id = group.get_groupid() + visible = group.get_uservisible() + default = group.get_default() + optional = group_id in optional_groups - if grp.id in optional: - data.optional_groups.append(grp.id) + if visible: + data.visible_groups.append(group_id) - if grp.visible: - data.visible_groups.append(grp.id) + if optional: + data.optional_groups.append(group_id) - if grp.id in default: - data.default_groups.append(grp.id) + if optional and default: + data.default_groups.append(group_id) return data @@ -314,15 +367,18 @@ def groups(self): :return: a list of IDs """ - return [g.id for g in self._base.comps.groups] + groups = libdnf5.comps.GroupQuery(self._base) + return [g.get_groupid() for g in groups] def _get_group(self, group_name): """Translate the given group name into a DNF object. :param group_name: an identifier of a group - :return: a DNF object or None + :return libdnf5.comps.Group: a DNF object or None """ - return self._base.comps.group_by_pattern(group_name) + groups = libdnf5.comps.GroupQuery(self._base) + groups.filter_name(group_name) + return next(iter(groups), None) def resolve_group(self, group_name): """Translate the given group name into a group ID. @@ -335,7 +391,7 @@ def resolve_group(self, group_name): if not grp: return None - return grp.id + return grp.get_groupid() def get_group_data(self, group_name) -> CompsGroupData: """Get the data of the specified group. @@ -359,9 +415,9 @@ def _get_group_data(grp) -> CompsGroupData: :return: an instance of CompsGroupData """ data = CompsGroupData() - data.id = grp.id or "" - data.name = grp.ui_name or "" - data.description = grp.ui_description or "" + data.id = grp.get_groupid() or "" + data.name = grp.get_translated_name() or "" + data.description = grp.get_translated_description() or "" return data def configure_proxy(self, url): @@ -369,12 +425,12 @@ def configure_proxy(self, url): :param url: a proxy URL or None """ - base = self._base + config = simplify_config(self._base.get_config()) # Reset the proxy configuration. - base.conf.proxy = "" - base.conf.proxy_username = "" - base.conf.proxy_password = "" + config.proxy = "" + config.proxy_username = "" + config.proxy_password = "" # Parse the given URL. proxy = self._parse_proxy(url) @@ -384,9 +440,9 @@ def configure_proxy(self, url): # Set the proxy configuration. log.info("Using '%s' as a proxy.", url) - base.conf.proxy = proxy.noauth_url - base.conf.proxy_username = proxy.username or "" - base.conf.proxy_password = proxy.password or "" + config.proxy = proxy.noauth_url + config.proxy_username = proxy.username or "" + config.proxy_password = proxy.password or "" def _parse_proxy(self, url): """Parse the given proxy URL. @@ -409,9 +465,9 @@ def dump_configuration(self): log.debug( "DNF configuration:" "\n%s" - "\nsubstitutions = %s", - self._base.conf.dump().strip(), - self._base.conf.substitutions + "\nvariables = %s", + str(self._base.get_config()), + str(self._base.get_vars()) ) def substitute(self, text): @@ -425,9 +481,10 @@ def substitute(self, text): if not text: return "" - return libdnf.conf.ConfigParser.substitute( - text, self._base.conf.substitutions - ) + # FIXME: Call base.setup() to set up all variables. + + variables = self._base.get_vars() + return variables.substitute(text) def configure_substitution(self, release_version): """Set up the substitution variables. @@ -437,7 +494,8 @@ def configure_substitution(self, release_version): if not release_version: return - self._base.conf.releasever = release_version + variables = self._base.get_vars() + variables.set("releasever", release_version) log.debug("The $releasever variable is set to '%s'.", release_version) def get_installation_size(self): @@ -449,14 +507,16 @@ def get_installation_size(self): packages_size = Size(0) files_number = 0 - if self._base.transaction is None: + if self._transaction is None: return Size("3000 MiB") - for tsi in self._base.transaction: + for tspkg in self._transaction.get_transaction_packages(): + # Get a package. + package = tspkg.get_package() # Space taken by all files installed by the packages. - packages_size += tsi.pkg.installsize + packages_size += package.get_install_size() # Number of files installed on the system. - files_number += len(tsi.pkg.files) + files_number += len(package.get_files()) # Calculate the files size depending on number of files. files_size = Size(files_number * DNF_EXTRA_SIZE_PER_FILE) @@ -473,14 +533,15 @@ def get_download_size(self): :return: a space required for packages :rtype: an instance of Size """ - if self._base.transaction is None: + if self._transaction is None: return Size(0) download_size = Size(0) # Calculate the download size. - for tsi in self._base.transaction: - download_size += tsi.pkg.downloadsize + for tspkg in self._transaction.get_transaction_packages(): + package = tspkg.get_package() + download_size += package.get_package_size() # Get the total size. Reserve extra space. total_space = download_size + Size("150 MiB") @@ -490,10 +551,14 @@ def get_download_size(self): def clear_cache(self): """Clear the DNF cache.""" + self.clear_selection() self._enabled_system_repositories = [] shutil.rmtree(DNF_CACHE_DIR, ignore_errors=True) shutil.rmtree(DNF_PLUGINCONF_DIR, ignore_errors=True) - self._base.reset(sack=True, repos=True, goal=True) + + # FIXME: Reset sacks. Should we just drop the base? + # self._base.reset(sack=True, repos=True, goal=True) + log.debug("The DNF cache has been cleared.") def is_package_available(self, package_spec): @@ -502,12 +567,15 @@ def is_package_available(self, package_spec): :param package_spec: a package spec :return: True if the package can be installed, otherwise False """ - if not self._base.sack: - log.warning("There is no metadata about packages!") - return False + #if not self._base.sack: + # log.warning("There is no metadata about packages!") + # return False - subject = dnf.subject.Subject(package_spec) - return bool(subject.get_best_query(self._base.sack)) + query = libdnf5.rpm.PackageQuery(self._base) + query.filter_name([package_spec]) + query.filter_available() + + return bool(query) def match_available_packages(self, pattern): """Find available packages that match the specified pattern. @@ -515,12 +583,15 @@ def match_available_packages(self, pattern): :param pattern: a pattern for package names :return: a list of matched package names """ - if not self._base.sack: - log.warning("There is no metadata about packages!") - return [] + #if not self._base.sack: + # log.warning("There is no metadata about packages!") + # return [] + + query = libdnf5.rpm.PackageQuery(self._base) + query.filter_name([pattern], libdnf5.common.QueryCmp_GLOB) + query.filter_available() - packages = self._base.sack.query().available().filter(name__glob=pattern) - return [p.name for p in packages] + return [p.get_name() for p in query] def apply_specs(self, include_list, exclude_list): """Mark packages, groups and modules for installation. @@ -531,76 +602,48 @@ def apply_specs(self, include_list, exclude_list): :raise BrokenSpecsError: if there are broken specs """ log.info("Including specs: %s", include_list) - log.info("Excluding specs: %s", exclude_list) - - try: - self._base.install_specs( - install=include_list, - exclude=exclude_list, - strict=not self._ignore_broken_packages - ) - except dnf.exceptions.MarkingErrors as e: - log.error("Failed to apply specs!\n%s", str(e)) - self._handle_marking_errors(e, self._ignore_missing_packages) + for spec in include_list: + self._goal.add_install(spec) - def _handle_marking_errors(self, exception, ignore_missing_packages=False): - """Handle the dnf.exceptions.MarkingErrors exception. - - :param exception: a exception - :param ignore_missing_packages: can missing specs be ignored? - :raise MissingSpecsError: if there are missing specs - :raise BrokenSpecsError: if there are broken specs - """ - # There are only some missing specs. They can be ignored. - if self._is_missing_specs_error(exception): - - if ignore_missing_packages: - log.info("Ignoring missing packages, groups or modules.") - return - - message = _("Some packages, groups or modules are missing.") - raise MissingSpecsError(message + "\n\n" + str(exception).strip()) from None - - # There are some broken specs. Raise an exception. - message = _("Some packages, groups or modules are broken.") - raise BrokenSpecsError(message + "\n\n" + str(exception).strip()) from None - - def _is_missing_specs_error(self, exception): - """Is it a missing specs error? - - :param exception: an exception - :return: True or False - """ - return isinstance(exception, dnf.exceptions.MarkingErrors) \ - and not exception.error_group_specs \ - and not exception.error_pkg_specs \ - and not exception.module_depsolv_errors + log.info("Excluding specs: %s", exclude_list) + for spec in exclude_list: + self._goal.add_remove(spec) def resolve_selection(self): """Resolve the software selection. :raise InvalidSelectionError: if the selection cannot be resolved """ - log.debug("Resolving the software selection.",) + report = ValidationReport() - try: - self._base.resolve() - except dnf.exceptions.DepsolveError as e: - log.error("The software couldn't be resolved!\n%s", str(e)) + log.debug("Resolving the software selection.") + self._transaction = self._goal.resolve() - message = _( - "The following software marked for installation has errors.\n" - "This is likely caused by an error with your installation source." - ) + # FIXME: Ignore missing packages. Otherwise, report as warning. + if self._ignore_missing_packages: + pass + + # FIXME: Ignore broken packages. Otherwise, report as error. + if self._ignore_broken_packages: + pass - raise InvalidSelectionError(message + "\n\n" + str(e).strip()) from None + # FIXME: If other problems, report all as errors. + # FIXME: If no problems, but some logs, report all as warnings. + if self._transaction.get_problems() != libdnf5.base.GoalProblem_NO_PROBLEM: + for message in self._transaction.get_resolve_logs_as_strings(): + report.error_messages.append(message) - log.info("The software selection has been resolved (%d packages selected).", - len(self._base.transaction)) + if report.is_valid(): + log.info("The software selection has been resolved (%d packages selected).", + len(self._transaction.get_transaction_packages())) + + log.debug("Resolving has been completed: %s", report) + return report def clear_selection(self): """Clear the software selection.""" - self._base.reset(goal=True) + self.__goal = None + self._transaction = None log.debug("The software selection has been cleared.") @property @@ -613,8 +656,9 @@ def set_download_location(self, path): :param path: a path to the package directory """ - for repo in self._base.repos.iter_enabled(): - repo.pkgdir = path + # FIXME: Reimplement the assignment. + # for repo in self._base.repos.iter_enabled(): + # repo.pkgdir = path self._download_location = path @@ -624,17 +668,43 @@ def download_packages(self, callback): :param callback: a callback for progress reporting :raise PayloadInstallationError: if the download fails """ - packages = self._base.transaction.install_set # pylint: disable=no-member - progress = DownloadProgress(callback=callback) + # Set up the download callbacks. + progress = DownloadProgress(callback) + self._set_download_callbacks(progress) + + # Prepare packages for download. + downloader = libdnf5.repo.PackageDownloader() + packages = self._get_download_packages() + destination = self.download_location - log.info("Downloading packages to %s.", self.download_location) + for package in packages: + downloader.add(package, destination=destination) + + # Download the packages. + log.info("Downloading packages to %s.", destination) try: - self._base.download_packages(packages, progress) - except dnf.exceptions.DownloadError as e: + downloader.download(fail_fast=True, resume=False) + except RuntimeError as e: msg = "Failed to download the following packages: " + str(e) raise PayloadInstallationError(msg) from None + def _set_download_callbacks(self, callbacks): + """Set up the download callbacks.""" + self._base.set_download_callbacks( + libdnf5.repo.DownloadCallbacksUniquePtr(callbacks) + ) + + def _get_download_packages(self): + """Get a list of resolved packages to download.""" + if not self._transaction: + raise RuntimeError("There is no transaction to use!") + + return [ + tspkg.get_package() for tspkg in self._transaction.get_transaction_packages() + if libdnf5.base.transaction.transaction_item_action_is_inbound(tspkg.get_action()) + ] + def install_packages(self, callback, timeout=20): """Install the packages. @@ -646,10 +716,10 @@ def install_packages(self, callback, timeout=20): :raise PayloadInstallationError: if the installation fails """ queue = multiprocessing.Queue() - display = TransactionProgress(queue) + progress = TransactionProgress(queue) process = multiprocessing.Process( target=self._run_transaction, - args=(self._base, display) + args=(self._base, progress) ) # Start the transaction. @@ -669,27 +739,29 @@ def install_packages(self, callback, timeout=20): log.debug("The transaction process exited with %s.", process.exitcode) @staticmethod - def _run_transaction(base, display): + def _run_transaction(base, transaction, progress): """Run the DNF transaction. Execute the DNF transaction and catch any errors. :param base: the DNF base - :param display: the DNF progress-reporting object + :param progress: the DNF progress-reporting object """ log.debug("Running the transaction...") try: - base.do_transaction(display) - if transaction_has_errors(base.transaction): - display.error("The transaction process has ended with errors.") + callbacks = libdnf5.rpm.TransactionCallbacksUniquePtr(progress) + result = transaction.run(callbacks, description="", user_id=None, comment=None) + log.debug("The transaction finished with %s", result) + if transaction_has_errors(transaction): + progress.error("The transaction process has ended with errors.") except BaseException as e: # pylint: disable=broad-except - display.error("The transaction process has ended abruptly: {}\n{}".format( + progress.error("The transaction process has ended abruptly: {}\n{}".format( str(e), traceback.format_exc())) finally: log.debug("The transaction has ended.") - base.close() # Always close this base. - display.quit("DNF quit") + # base.close() # Always close this base. + progress.quit("DNF quit") @property def repositories(self): @@ -698,7 +770,8 @@ def repositories(self): :return: a list of IDs """ with self._lock: - return [r.id for r in self._base.repos.values()] + repositories = libdnf5.repo.RepoQuery(self._base) + return sorted(r.get_id() for r in repositories) @property def enabled_repositories(self): @@ -707,7 +780,9 @@ def enabled_repositories(self): :return: a list of IDs """ with self._lock: - return [r.id for r in self._base.repos.iter_enabled()] + repositories = libdnf5.repo.RepoQuery(self._base) + repositories.filter_enabled(True) + return sorted(r.get_id() for r in repositories) def get_matching_repositories(self, pattern): """Get a list of repositories that match the specified pattern. @@ -719,21 +794,24 @@ def get_matching_repositories(self, pattern): :return: a list of matching IDs """ with self._lock: - return [r.id for r in self._base.repos.get_matching(pattern)] + repositories = libdnf5.repo.RepoQuery(self._base) + repositories.filter_id(pattern, libdnf5.common.QueryCmp_GLOB) + return sorted(r.get_id() for r in repositories) def _get_repository(self, repo_id): """Translate the given repository name to a DNF object. :param repo_id: an identifier of a repository - :return: a DNF object + :return libdnf5.repo.Repo: a DNF object :raise: UnknownRepositoryError if no repo is found """ - repo = self._base.repos.get(repo_id) - - if not repo: - raise UnknownRepositoryError(repo_id) + repositories = libdnf5.repo.RepoQuery(self._base) + repositories.filter_id(repo_id) - return repo + try: + return repositories.get() + except RuntimeError: + raise UnknownRepositoryError(repo_id) from None def add_repository(self, data: RepoConfigurationData): """Add a repository. @@ -742,18 +820,21 @@ def add_repository(self, data: RepoConfigurationData): :param RepoConfigurationData data: a repo configuration """ - # Create a new repository. - repo = self._create_repository(data) with self._lock: + # Create a new repository. + repo = self._create_repository(data) + + # FIXME: How to handle existing repositories? # Remove an existing repository. - if repo.id in self._base.repos: - self._base.repos.pop(repo.id) + # + # if repo.id in self._base.repos: + # self._base.repos.pop(repo.id) # Add the new repository. - self._base.repos.add(repo) + #self._base.repos.add(repo) - log.info("Added the '%s' repository: %s", repo.id, repo) + log.info("Added the '%s' repository: %s", repo.get_id(), repo) def _create_repository(self, data: RepoConfigurationData): """Create a DNF repository. @@ -761,7 +842,9 @@ def _create_repository(self, data: RepoConfigurationData): :param RepoConfigurationData data: a repo configuration return dnf.repo.Repo: a DNF repository """ - repo = dnf.repo.Repo(data.name, self._base.conf) + repo_sack = self._base.get_repo_sack() + repo = repo_sack.create_repo(data.name) + config = simplify_config(repo.get_config()) # Disable the repo if requested. if not data.enabled: @@ -771,43 +854,43 @@ def _create_repository(self, data: RepoConfigurationData): url = self.substitute(data.url) if data.type == URL_TYPE_BASEURL: - repo.baseurl = [url] + config.baseurl = [url] if data.type == URL_TYPE_MIRRORLIST: - repo.mirrorlist = url + config.mirrorlist = url if data.type == URL_TYPE_METALINK: - repo.metalink = url + config.metalink = url # Set the proxy configuration. proxy = self._parse_proxy(data.proxy) if proxy: - repo.proxy = proxy.noauth_url - repo.proxy_username = proxy.username or "" - repo.proxy_password = proxy.password or "" + config.proxy = proxy.noauth_url + config.proxy_username = proxy.username or "" + config.proxy_password = proxy.password or "" # Set the repo configuration. if data.cost != DNF_DEFAULT_REPO_COST: - repo.cost = data.cost + config.cost = data.cost if data.included_packages: - repo.includepkgs = data.included_packages + config.includepkgs = data.included_packages if data.excluded_packages: - repo.excludepkgs = data.excluded_packages + config.excludepkgs = data.excluded_packages # Set up the SSL configuration. - repo.sslverify = conf.payload.verify_ssl and data.ssl_verification_enabled + config.sslverify = conf.payload.verify_ssl and data.ssl_verification_enabled if data.ssl_configuration.ca_cert_path: - repo.sslcacert = data.ssl_configuration.ca_cert_path + config.sslcacert = data.ssl_configuration.ca_cert_path if data.ssl_configuration.client_cert_path: - repo.sslclientcert = data.ssl_configuration.client_cert_path + config.sslclientcert = data.ssl_configuration.client_cert_path if data.ssl_configuration.client_key_path: - repo.sslclientkey = data.ssl_configuration.client_key_path + config.sslclientkey = data.ssl_configuration.client_key_path return repo @@ -877,7 +960,7 @@ def set_repository_enabled(self, repo_id, enabled): repo = self._get_repository(repo_id) # Skip if the repository is already set to the right value. - if repo.enabled == enabled: + if repo.is_enabled() == enabled: return if enabled: @@ -903,13 +986,21 @@ def read_system_repositories(self): raise RuntimeError("The DNF repo cache is not cleared.") log.debug("Read system repositories.") - self._base.read_all_repos() + repo_sack = self._base.get_repo_sack() + repo_sack.create_repos_from_system_configuration() + + log.debug("Disable system repositories.") + repositories = libdnf5.repo.RepoQuery(self._base) + repositories.filter_enabled(True) # Remember enabled system repositories. - self._enabled_system_repositories = list(self.enabled_repositories) + self._enabled_system_repositories = sorted( + r.get_id() for r in repositories + ) - log.debug("Disable system repositories.") - self._base.repos.all().disable() + # Disable all system repositories. + for repo in repositories: + repo.disable() def restore_system_repositories(self): """Restore the system repositories. @@ -940,16 +1031,22 @@ def load_repository(self, repo_id): log.debug("Load metadata for the '%s' repository.", repo_id) repo = self._get_repository(repo_id) - url = repo.baseurl or repo.mirrorlist or repo.metalink + config = simplify_config(repo.get_config()) + url = config.baseurl or config.mirrorlist or config.metalink - if not repo.enabled: + if not repo.is_enabled(): log.debug("Don't load metadata from a disabled repository.") return try: + repo.fetch_metadata() + + repo = self._get_repository(repo_id) repo.load() - except dnf.exceptions.RepoError as e: + except RuntimeError as e: log.debug("Failed to load metadata from '%s': %s", url, str(e)) + # FIXME: Get a new object to avoid a crash. + repo = self._get_repository(repo_id) repo.disable() raise MetadataError(str(e)) from None @@ -962,17 +1059,10 @@ def load_packages_metadata(self): It will update the cache that provides information about available packages, modules, groups and environments. """ - # Load all enabled repositories. - # Set up the package sack. - self._base.fill_sack( - load_system_repo=False, - load_available_repos=True, - ) - # Load the comps metadata. - self._base.read_comps( - arch_filter=True - ) - + repositories = libdnf5.repo.RepoQuery(self._base) + repositories.filter_enabled(True) + repo_sack = self._base.get_repo_sack() + repo_sack.update_and_load_repos(repositories) log.info("Loaded packages and group metadata.") def load_repomd_hashes(self): @@ -995,12 +1085,14 @@ def _get_repomd_hashes(self): :return: a dictionary of repo ids and repomd.xml hashes """ + repositories = libdnf5.repo.RepoQuery(self._base) + repositories.filter_enabled(True) md_hashes = {} - for repo in self._base.repos.iter_enabled(): + for repo in repositories: content = self._get_repomd_content(repo) md_hash = calculate_hash(content) if content else None - md_hashes[repo.id] = md_hash + md_hashes[repo.get_id()] = md_hash log.debug("Loaded repomd.xml hashes: %s", md_hashes) return md_hashes @@ -1011,12 +1103,16 @@ def _get_repomd_content(self, repo): :param repo: a DNF repo :return: a content of the repomd.xml file """ - for url in repo.baseurl: + config = simplify_config(repo.get_config()) + urls = config.baseurl + + for url in urls: try: repomd_url = "{}/repodata/repomd.xml".format(url) - with self._base.urlopen(repomd_url, repo=repo, mode="w+t") as f: - return f.read() + # FIXME: Should we use is_repomd_in_sync instead? + # with self._base.urlopen(repomd_url, repo=repo, mode="w+t") as f: + # return f.read() except OSError as e: log.debug("Can't download repomd.xml from: %s", str(e)) diff --git a/pyanaconda/modules/payloads/payload/dnf/download_progress.py b/pyanaconda/modules/payloads/payload/dnf/download_progress.py index e2694d5f627..30e7f14e2d7 100644 --- a/pyanaconda/modules/payloads/payload/dnf/download_progress.py +++ b/pyanaconda/modules/payloads/payload/dnf/download_progress.py @@ -18,8 +18,7 @@ import collections import time -import dnf -import dnf.callback +import libdnf5 from blivet.size import Size from pyanaconda.anaconda_loggers import get_module_logger @@ -41,7 +40,7 @@ def paced_fn(self, *args): return paced_fn -class DownloadProgress(dnf.callback.DownloadProgress): +class DownloadProgress(libdnf5.repo.DownloadCallbacks): """The class for receiving information about an ongoing download.""" def __init__(self, callback): @@ -51,47 +50,42 @@ def __init__(self, callback): """ super().__init__() self.callback = callback - self.downloads = collections.defaultdict(int) - self.last_time = time.time() - self.total_files = 0 - self.total_size = Size(0) - self.downloaded_size = Size(0) - - @paced - def _report_progress(self): - # Update the downloaded size. - self.downloaded_size = Size(sum(self.downloads.values())) - - # Report the progress. - msg = _( - 'Downloading {total_files} RPMs, ' - '{downloaded_size} / {total_size} ' - '({total_percent}%) done.' - ).format( - downloaded_size=self.downloaded_size, - total_percent=int(100 * self.downloaded_size / self.total_size), - total_files=self.total_files, - total_size=self.total_size - ) - self.callback(msg) + def add_new_download(self, user_data, description, total_to_download): + """Notify the client that a new download has been created. - def end(self, payload, status, msg): - nevra = str(payload) + :param user_data: user data entered together with a package to download + :param str description: a message describing the package + :param float total_to_download: a total number of bytes to download + :return: associated user data for the new package download + """ + self._report_progress("Downloading {} - {} bytes".format( + description, total_to_download + )) + return description - if status is dnf.callback.STATUS_OK: - self.downloads[nevra] = payload.download_size - self._report_progress() - return + def progress(self, user_cb_data, total_to_download, downloaded): + """Download progress callback. - log.warning("Failed to download '%s': %d - %s", nevra, status, msg) + :param user_cb_data: associated user data obtained from add_new_download + :param float total_to_download: a total number of bytes to download + :param float downloaded: a number of bytes downloaded + """ + self._report_progress("Downloading {} - {}/{} bytes".format( + user_cb_data, downloaded, total_to_download + )) - def progress(self, payload, done): - nevra = str(payload) - self.downloads[nevra] = done - self._report_progress() + def end(self, user_cb_data, status, msg): + """End of download callback. - def start(self, total_files, total_size, total_drpms=0): - del total_drpms - self.total_files = total_files - self.total_size = Size(total_size) + :param user_cb_data: associated user data obtained from add_new_download + :param status: the transfer status + :param msg: the error message in case of error + """ + self._report_progress("Downloaded {} - {} ({})".format( + user_cb_data, status, msg + )) + + def _report_progress(self, msg): + log.debug(msg) + self.callback(msg) diff --git a/pyanaconda/modules/payloads/payload/dnf/initialization.py b/pyanaconda/modules/payloads/payload/dnf/initialization.py index 0d198caee00..ba180343f1f 100644 --- a/pyanaconda/modules/payloads/payload/dnf/initialization.py +++ b/pyanaconda/modules/payloads/payload/dnf/initialization.py @@ -18,8 +18,8 @@ import logging from collections import namedtuple -import dnf.logging -import libdnf +DNF_LIBREPO_LOG = "/tmp/dnf.librepo.log" +DNF_LOGGER = "dnf" from pyanaconda.anaconda_loggers import get_module_logger from pyanaconda.core.constants import BASE_REPO_NAME, REPO_ORIGIN_SYSTEM @@ -44,27 +44,28 @@ ) from pyanaconda.modules.payloads.payload.dnf.tree_info import LoadTreeInfoMetadataTask -DNF_LIBREPO_LOG = "/tmp/dnf.librepo.log" -DNF_LOGGER = "dnf" log = get_module_logger(__name__) - def configure_dnf_logging(): """Configure the DNF logging.""" + + # FIXME: Set up the librepo logger. # Set up librepo. # This is still required even when the librepo has a separate logger because # DNF needs to have callbacks that the librepo log is written to be able to # process that log. - libdnf.repo.LibrepoLog.removeAllHandlers() - libdnf.repo.LibrepoLog.addHandler(DNF_LIBREPO_LOG) + # libdnf.repo.LibrepoLog.removeAllHandlers() + # libdnf.repo.LibrepoLog.addHandler(DNF_LIBREPO_LOG) + # FIXME: Set up the DNF logger. # Set up DNF. Increase the log level to the custom DDEBUG level. - dnf_logger = logging.getLogger(DNF_LOGGER) - dnf_logger.setLevel(dnf.logging.DDEBUG) - + # dnf_logger = logging.getLogger(DNF_LOGGER) + # dnf_logger.setLevel(dnf.logging.DDEBUG) # The result of the SetUpDNFSourcesTask task. + + SetUpDNFSourcesResult = namedtuple( "LoadTreeInfoMetadataResult", [ "dnf_manager", diff --git a/pyanaconda/modules/payloads/payload/dnf/transaction_progress.py b/pyanaconda/modules/payloads/payload/dnf/transaction_progress.py index 7cb297aa89d..5d0b96942f0 100644 --- a/pyanaconda/modules/payloads/payload/dnf/transaction_progress.py +++ b/pyanaconda/modules/payloads/payload/dnf/transaction_progress.py @@ -15,8 +15,7 @@ # License and may only be used or replicated with the express permission of # Red Hat, Inc. # -import dnf.callback -import dnf.transaction +import libdnf5 from pyanaconda.anaconda_loggers import get_module_logger from pyanaconda.core.i18n import _ @@ -57,7 +56,7 @@ def process_transaction_progress(queue, callback): (token, msg) = queue.get() -class TransactionProgress(dnf.callback.TransactionProgress): +class TransactionProgress(libdnf5.rpm.TransactionCallbacks): """The class for receiving information about an ongoing transaction.""" def __init__(self, queue): @@ -67,56 +66,39 @@ def __init__(self, queue): """ super().__init__() self._queue = queue - self._last_ts = None - self._postinst_phase = False - self.cnt = 0 - - def progress(self, package, action, ti_done, _ti_total, ts_done, ts_total): - """Report ongoing progress on the given transaction item. - - :param package: the DNF package object - :param action: the ID of the current action - :param ti_done: the number of processed bytes of the transaction item - :param _ti_total: the total number of bytes of the transaction item - :param ts_done: the number of actions processed in the whole transaction - :param ts_total: the total number of actions in the whole transaction - """ - # Process DNF actions, communicating with anaconda via the queue - # A normal installation consists of 'install' messages followed by - # the 'post' message. - if action == dnf.transaction.PKG_INSTALL and ti_done == 0: - # do not report same package twice - if self._last_ts == ts_done: - return - self._last_ts = ts_done - - msg = '%s.%s (%d/%d)' % \ - (package.name, package.arch, ts_done, ts_total) - self.cnt += 1 - self._queue.put(('install', msg)) - - # Log the exact package nevra, build time and checksum - nevra = "%s-%s.%s" % (package.name, package.evr, package.arch) - log_msg = "Installed: %s %s %s" % (nevra, package.buildtime, package.returnIdSum()[1]) - self._queue.put(('log', log_msg)) - - elif action == dnf.transaction.TRANS_POST: - self._queue.put(('post', None)) - log_msg = "Post installation setup phase started." - self._queue.put(('log', log_msg)) - self._postinst_phase = True - - elif action == dnf.transaction.PKG_SCRIPTLET: - # Log the exact package nevra, build time and checksum - nevra = "%s-%s.%s" % (package.name, package.evr, package.arch) - log_msg = "Configuring (running scriptlet for): %s %s %s" % (nevra, package.buildtime, - package.returnIdSum()[1]) - self._queue.put(('log', log_msg)) - - # only show progress in UI for post-installation scriptlets - if self._postinst_phase: - msg = '%s.%s' % (package.name, package.arch) - self._queue.put(('configure', msg)) + + def install_start(self, item, total=0): + log.debug("Installing - %s", item.get_package().get_nevra()) + log.debug(libdnf5.base.transaction.transaction_item_action_to_string(item.get_action())) + self._queue.put(('install', item.get_package().get_nevra())) + + def install_progress(self, item, amount, total): + log.debug("Installing - %s (%s/%s)", item.get_package().get_nevra(), amount, total) + + def verify_progress(self, amount, total): + log.debug("Verify %s/%s", amount, total) + self._queue.put(('verify', 'packages')) + + def script_start(self, item, nevra, type): + log.debug("Configuring - %s, %s, %s", item.get_package().get_nevra(), nevra, type) + self._queue.put(('configure', nevra)) + + def transaction_stop(self, total): + log.debug("Done - %s", total) + self._queue.put(('done', None)) + self._queue.close() + + def cpio_error(self, item): + log.debug("Error - %s", item.get_package().get_nevra()) + self._queue.put(('error', item.get_package().get_nevra())) + + def script_error(self, item, nevra, type, return_code): + log.debug("Error - %s, %s, %s, %s", item.get_package().get_nevra(), nevra, type, return_code) + self._queue.put(('error', item.get_package().get_nevra())) + + def unpack_error(self, item): + log.debug("Error - %s", item.get_package().get_nevra()) + self._queue.put(('error', item.get_package().get_nevra())) def error(self, message): """Report an error that occurred during the transaction. @@ -132,3 +114,63 @@ def quit(self, message): """ self._queue.put(('quit', message)) self._queue.close() + + # def progress(self, package, action, ti_done, ti_total, ts_done, ts_total): + # """Report ongoing progress on the given transaction item. + # + # :param package: the DNF package object + # :param action: the ID of the current action + # :param ti_done: the number of processed bytes of the transaction item + # :param ti_total: the total number of bytes of the transaction item + # :param ts_done: the number of actions processed in the whole transaction + # :param ts_total: the total number of actions in the whole transaction + # """ + # # Process DNF actions, communicating with anaconda via the queue + # # A normal installation consists of 'install' messages followed by + # # the 'post' message. + # if action == dnf.transaction.PKG_INSTALL and ti_done == 0: + # # do not report same package twice + # if self._last_ts == ts_done: + # return + # self._last_ts = ts_done + # + # msg = '%s.%s (%d/%d)' % \ + # (package.name, package.arch, ts_done, ts_total) + # self.cnt += 1 + # self._queue.put(('install', msg)) + # + # # Log the exact package nevra, build time and checksum + # nevra = "%s-%s.%s" % (package.name, package.evr, package.arch) + # log_msg = "Installed: %s %s %s" % (nevra, package.buildtime, package.returnIdSum()[1]) + # self._queue.put(('log', log_msg)) + # + # elif action == dnf.transaction.TRANS_POST: + # self._queue.put(('post', None)) + # log_msg = "Post installation setup phase started." + # self._queue.put(('log', log_msg)) + # self._postinst_phase = True + # + # elif action == dnf.transaction.PKG_SCRIPTLET: + # # Log the exact package nevra, build time and checksum + # nevra = "%s-%s.%s" % (package.name, package.evr, package.arch) + # log_msg = "Configuring (running scriptlet for): %s %s %s" % (nevra, package.buildtime, + # package.returnIdSum()[1]) + # self._queue.put(('log', log_msg)) + # + # # only show progress in UI for post-installation scriptlets + # if self._postinst_phase: + # msg = '%s.%s' % (package.name, package.arch) + # self._queue.put(('configure', msg)) + # + # elif action == dnf.transaction.PKG_VERIFY: + # msg = '%s.%s (%d/%d)' % (package.name, package.arch, ts_done, ts_total) + # self._queue.put(('verify', msg)) + # + # # Log the exact package nevra, build time and checksum + # nevra = "%s-%s.%s" % (package.name, package.evr, package.arch) + # log_msg = "Verifying: %s %s %s" % (nevra, package.buildtime, package.returnIdSum()[1]) + # self._queue.put(('log', log_msg)) + # + # # Once the last package is verified the transaction is over + # if ts_done == ts_total: + # self._queue.put(('done', None)) diff --git a/pyanaconda/modules/payloads/payload/dnf/validation.py b/pyanaconda/modules/payloads/payload/dnf/validation.py index 407a7d3eb7d..7db5027bcd3 100644 --- a/pyanaconda/modules/payloads/payload/dnf/validation.py +++ b/pyanaconda/modules/payloads/payload/dnf/validation.py @@ -133,28 +133,11 @@ def _collect_required_specs(self): def _resolve_selection(self): """Resolve the new selection.""" log.debug("Resolving the software selection.") - report = ValidationReport() - - with self._reported_errors(report): - self._dnf_manager.apply_specs(self._include_list, self._exclude_list) - with self._reported_errors(report): - self._dnf_manager.resolve_selection() + # Set up the selection. + self._dnf_manager.apply_specs(self._include_list, self._exclude_list) + # Resolve the selection. + report = self._dnf_manager.resolve_selection() log.debug("Resolving has been completed: %s", report) return report - - @contextmanager - def _reported_errors(self, report): - """Add exceptions into the validation report. - - :param report: a validation report - """ - try: - yield - except MissingSpecsError as e: - report.warning_messages.append(str(e)) - except BrokenSpecsError as e: - report.error_messages.append(str(e)) - except InvalidSelectionError as e: - report.error_messages.append(str(e)) diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py new file mode 100644 index 00000000000..8a5f9a36a04 --- /dev/null +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -0,0 +1,982 @@ +# +# Copyright (C) 2020 Red Hat, Inc. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions of +# the GNU General Public License v.2, or (at your option) any later version. +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY expressed or implied, including the implied warranties of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. You should have received a copy of the +# GNU General Public License along with this program; if not, write to the +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the +# source code or documentation are not subject to the GNU General Public +# License and may only be used or replicated with the express permission of +# Red Hat, Inc. +# +import os +import subprocess +import unittest +from tempfile import TemporaryDirectory +from textwrap import dedent +from unittest.mock import patch, Mock + +import libdnf5 +import pytest +from blivet.size import ROUND_UP, Size + +from pyanaconda.core.constants import ( + MULTILIB_POLICY_ALL, + URL_TYPE_BASEURL, + URL_TYPE_METALINK, + URL_TYPE_MIRRORLIST, +) +from pyanaconda.modules.common.errors.payload import UnknownRepositoryError +from pyanaconda.modules.common.structures.packages import PackagesConfigurationData +from pyanaconda.modules.common.structures.payload import RepoConfigurationData +from pyanaconda.modules.payloads.payload.dnf.dnf_manager import ( + DNFManager, + simplify_config, + MetadataError, +) + + +class DNF5TestCase(unittest.TestCase): + """Test the DNF5 library.""" + + def test_runtime_error(self): + base = libdnf5.base.Base() + query = libdnf5.repo.RepoQuery(base) + + with pytest.raises(RuntimeError): + query.get() + + @pytest.mark.skip("Fatal Python error: Aborted") + def test_undefined_variables(self): + base = libdnf5.base.Base() + variables = base.get_vars() + variables.get_value("undefined") + + @pytest.mark.skip("Fatal Python error: Aborted") + def test_resolve_without_setup(self): + """Call resolve without setting up the base.""" + base = libdnf5.base.Base() + goal = libdnf5.base.Goal(base) + goal.resolve() + + @pytest.mark.skip("Fatal Python error: Aborted") + def test_environment_query(self): + base = libdnf5.base.Base() + libdnf5.comps.EnvironmentQuery(base) + + @pytest.mark.skip("Fatal Python error: Aborted") + def test_group_query(self): + base = libdnf5.base.Base() + libdnf5.comps.GroupQuery(base) + + @pytest.mark.skip("Fatal Python error: Aborted") + def test_disable_failed_repository(self): + base = libdnf5.base.Base() + sack = base.get_repo_sack() + sack.create_repo("r1") + base.setup() + + repo = self._get_repo(base, "r1") + + try: + repo.fetch_metadata() + repo.load() + except RuntimeError: + print("Print something") + # It will crash here. + repo.disable() + + def _get_repo(self, base, repo_id): + repos = libdnf5.repo.RepoQuery(base) + repos.filter_id(repo_id) + return repos.get() + + def test_simplify_config(self): + """Test the simplify_config function.""" + base = libdnf5.base.Base() + config = simplify_config( + base.get_config() + ) + + config.installroot = "/my/install/root" + assert config.installroot == "/my/install/root" + + +class DNFManagerTestCase(unittest.TestCase): + """Test the DNFManager class.""" + + def setUp(self): + self.maxDiff = None + self.dnf_manager = DNFManager() + + def _get_configuration(self): + """Get the configuration of the DNF base.""" + return simplify_config(self.dnf_manager._base.get_config()) + + def _check_variables(self, **expected_variables): + """Check values of the expected substitution variables.""" + variables = self.dnf_manager._base.get_vars() + + for name, value in expected_variables.items(): + assert variables.get_value(name) == value + + def _check_base_setup(self): + """Make sure that the base is set up.""" + # FIXME: Figure this out on the manager level. + base = self.dnf_manager._base + assert base + base.setup() + + def test_create_base(self): + """Test the creation of the DNF base.""" + assert self.dnf_manager._base is not None + + def test_reset_base(self): + """Test the reset of the DNF base.""" + base_1 = self.dnf_manager._base + assert self.dnf_manager._base == base_1 + self.dnf_manager.reset_base() + + base_2 = self.dnf_manager._base + assert self.dnf_manager._base == base_2 + assert self.dnf_manager._base != base_1 + + def test_clear_cache(self): + """Test the clear_cache method.""" + self.dnf_manager.clear_cache() + + def test_set_default_configuration(self): + """Test the default configuration of the DNF base.""" + config = self._get_configuration() + assert config.gpgcheck is False + assert config.skip_if_unavailable is False + assert config.cachedir == "/tmp/dnf.cache" + assert config.pluginconfpath == "/tmp/dnf.pluginconf" + assert config.logdir == "/tmp/" + assert config.installroot == "/mnt/sysroot" + assert config.persistdir == "/mnt/sysroot/var/lib/dnf" + assert config.reposdir == ( + "/etc/yum.repos.d", + "/etc/anaconda.repos.d" + ) + self._check_variables(releasever="rawhide") + + @patch("pyanaconda.modules.payloads.payload.dnf.dnf_manager.get_os_release_value") + def test_set_module_platform_id(self, get_platform_id): + """Test the configuration of module_platform_id.""" + get_platform_id.return_value = "platform:f32" + + self.dnf_manager.reset_base() + config = self._get_configuration() + + assert config.module_platform_id == "platform:f32" + + def test_configure_proxy(self): + """Test the proxy configuration.""" + config = self._get_configuration() + + self.dnf_manager.configure_proxy("http://user:pass@example.com/proxy") + assert config.proxy == "http://example.com:3128" + assert config.proxy_username == "user" + assert config.proxy_password == "pass" + + self.dnf_manager.configure_proxy("@:/invalid") + assert config.proxy == "" + assert config.proxy_username == "" + assert config.proxy_password == "" + + self.dnf_manager.configure_proxy("http://example.com/proxy") + assert config.proxy == "http://example.com:3128" + assert config.proxy_username == "" + assert config.proxy_password == "" + + self.dnf_manager.configure_proxy(None) + assert config.proxy == "" + assert config.proxy_username == "" + assert config.proxy_password == "" + + def test_configure_base_default(self): + """Test the default configuration of the DNF base.""" + data = PackagesConfigurationData() + self.dnf_manager.configure_base(data) + config = self._get_configuration() + + assert config.multilib_policy == "best" + assert config.timeout == 30 + assert config.retries == 10 + assert config.install_weak_deps is True + + assert self.dnf_manager._ignore_broken_packages is False + assert self.dnf_manager._ignore_missing_packages is False + + def test_configure_base(self): + """Test the configuration of the DNF base.""" + data = PackagesConfigurationData() + data.multilib_policy = MULTILIB_POLICY_ALL + data.timeout = 100 + data.retries = 5 + data.broken_ignored = True + data.missing_ignored = True + data.weakdeps_excluded = True + + self.dnf_manager.configure_base(data) + config = self._get_configuration() + + assert config.multilib_policy == "all" + assert config.timeout == 100 + assert config.retries == 5 + assert config.install_weak_deps is False + + assert self.dnf_manager._ignore_broken_packages is True + assert self.dnf_manager._ignore_missing_packages is True + + @pytest.mark.skip("Dump is unsupported.") + def test_dump_configuration(self): + """Test the dump of the DNF configuration.""" + with self.assertLogs(level="DEBUG") as cm: + self.dnf_manager.dump_configuration() + + msg = "DNF configuration:" + assert any(map(lambda x: msg in x, cm.output)) + + msg = "installroot = /mnt/sysroot" + assert any(map(lambda x: msg in x, cm.output)) + + def test_get_installation_size(self): + """Test the get_installation_size method.""" + # No transaction. + size = self.dnf_manager.get_installation_size() + assert size == Size("3000 MiB") + + # Fake transaction. + self.dnf_manager._transaction = self._get_transaction() + size = self.dnf_manager.get_installation_size() + size = size.round_to_nearest("KiB", ROUND_UP) + assert size == Size("528 KiB") + + def test_get_download_size(self): + """Test the get_download_size method.""" + # No transaction. + size = self.dnf_manager.get_download_size() + assert size == Size(0) + + # Fake transaction. + self.dnf_manager._transaction = self._get_transaction() + size = self.dnf_manager.get_download_size() + assert size == Size("450 MiB") + + def _get_transaction(self, packages=2): + """Create a mocked DNF transaction with some packages.""" + tspkgs = [] + + for i in range(1, packages+1): + # Create a package. + pkg = Mock(spec=libdnf5.rpm.Package) + pkg.get_package_size.return_value = 1024 * 1024 * 100 * i + pkg.get_install_size.return_value = 1024 * 100 * i + pkg.get_files.return_value = ["/file"] * 10 * i + + # Create a transaction package. + tspkg = Mock(spec=libdnf5.base.TransactionPackage) + tspkg.get_package.return_value = pkg + tspkgs.append(tspkg) + + # Create a transaction. + transaction = Mock(spec=libdnf5.base.Transaction) + transaction.get_transaction_packages.return_value = tspkgs + return transaction + + def test_apply_specs(self): + """Test the apply_specs method.""" + self.dnf_manager.apply_specs( + include_list=["@g1", "p1"], + exclude_list=["@g2", "p2"] + ) + + # FIXME: Check the goal. + assert self.dnf_manager._goal + + def test_resolve_no_selection(self): + """Test the resolve_selection method with no selection.""" + self._check_base_setup() + + with self.assertLogs(level="INFO") as cm: + report = self.dnf_manager.resolve_selection() + + expected = "The software selection has been resolved (0 packages selected)." + assert expected in "\n".join(cm.output) + assert report.error_messages == [] + assert report.warning_messages == [] + + def test_resolve_missing_selection(self): + """Test the resolve selection method with missing selection.""" + self._check_base_setup() + + self.dnf_manager.apply_specs( + include_list=["@g1", "p1"], + exclude_list=["@g2", "p2"] + ) + + report = self.dnf_manager.resolve_selection() + assert report.error_messages == [ + 'No match for argument: p1', + 'No packages to remove for argument: p2', + 'No match for argument: g1', + 'No groups to remove for argument: g2', + ] + assert report.warning_messages == [] + + @pytest.mark.skip("Not implemented") + def test_ignore_missing_packages(self): + """Test the ignore_missing_packages attribute.""" + + @pytest.mark.skip("Not implemented") + def test_ignore_broken_packages(self): + """Test the ignore_missing_packages attribute.""" + + def test_clear_selection(self): + """Test the clear_selection method.""" + self._check_base_setup() + self.dnf_manager.resolve_selection() + + g = self.dnf_manager._goal + t = self.dnf_manager._transaction + + self.dnf_manager.clear_selection() + assert g is not self.dnf_manager._goal + assert t is not self.dnf_manager._transaction + + def test_substitute(self): + """Test the substitute method.""" + # No variables. + assert self.dnf_manager.substitute(None) == "" + assert self.dnf_manager.substitute("") == "" + assert self.dnf_manager.substitute("/") == "/" + assert self.dnf_manager.substitute("/text") == "/text" + + # Unknown variables. + assert self.dnf_manager.substitute("/$unknown") == "/$unknown" + + # FIXME: Some variables are defined by base.setup(). + assert self.dnf_manager.substitute("/$arch") == "/$arch" + assert self.dnf_manager.substitute("/$basearch") == "/$basearch" + assert self.dnf_manager.substitute("/$releasever") != "/$releasever" + + # Supported variables. + self._check_base_setup() + assert self.dnf_manager.substitute("/$arch") != "/$arch" + assert self.dnf_manager.substitute("/$basearch") != "/$basearch" + assert self.dnf_manager.substitute("/$releasever") != "/$releasever" + + def test_configure_substitution(self): + """Test the configure_substitution function.""" + self.dnf_manager.configure_substitution(release_version="35") + self._check_variables(releasever="35") + + def test_reset_substitution(self): + """Test the reset_substitution method.""" + self.dnf_manager.configure_substitution(release_version="35") + self._check_variables(releasever="35") + + self.dnf_manager.reset_substitution() + self._check_variables(releasever="rawhide") + + +class DNFManagerCompsTestCase(unittest.TestCase): + """Test the comps abstraction of the DNF base.""" + + def setUp(self): + self.maxDiff = None + self.dnf_manager = DNFManager() + # self.dnf_manager._base._comps = self._create_comps() + + @pytest.mark.skip("Fatal Python error: Aborted") + def test_groups(self): + """Test the groups property.""" + # FIXME: Handle unlocked base. + assert self.dnf_manager.groups == [] + + @pytest.mark.skip("Fatal Python error: Aborted") + def test_no_default_environment(self): + """Test the default_environment property with no environments.""" + # FIXME: Handle unlocked base. + assert self.dnf_manager.default_environment is None + + @pytest.mark.skip("Fatal Python error: Aborted") + def test_environments(self): + """Test the environments property.""" + # FIXME: Handle unlocked base. + assert self.dnf_manager.environments == [] + + +class DNFManagerReposTestCase(unittest.TestCase): + """Test the repo abstraction of the DNF base.""" + + def setUp(self): + self.maxDiff = None + self.dnf_manager = DNFManager() + + def _check_base_setup(self): + """Make sure that the base is set up.""" + # FIXME: Figure this out on the manager level. + base = self.dnf_manager._base + assert base + base.setup() + + def _add_repository(self, repo_id, repo_dir=None, **kwargs): + """Add the DNF repository with the specified id.""" + data = RepoConfigurationData() + data.name = repo_id + self.dnf_manager.add_repository(data) + + if repo_dir: + # Generate repo data. + os.makedirs(os.path.join(repo_dir), exist_ok=True) + subprocess.run(["createrepo_c", "."], cwd=repo_dir) + + # Update the baseurl. + baseurl = kwargs.get("baseurl", []) + baseurl.append("file://" + repo_dir) + kwargs["baseurl"] = baseurl + + config = self._get_configuration(repo_id) + for name, value in kwargs.items(): + setattr(config, name, value) + + return self._get_repository(repo_id) + + def _get_repository(self, repo_id): + """Get the DNF repository.""" + return self.dnf_manager._get_repository(repo_id) + + def _get_configuration(self, repo_id): + """Get a configuration of the DNF repository.""" + repo = self._get_repository(repo_id) + return simplify_config(repo.get_config()) + + def test_repositories(self): + """Test the repositories property.""" + assert self.dnf_manager.repositories == [] + + self._add_repository("r1") + self._add_repository("r2") + self._add_repository("r3") + + assert self.dnf_manager.repositories == ["r1", "r2", "r3"] + + def test_enabled_repositories(self): + """Test the enabled_repositories property.""" + assert self.dnf_manager.enabled_repositories == [] + + self._add_repository("r1").disable() + self._add_repository("r2").enable() + self._add_repository("r3").disable() + self._add_repository("r4").enable() + + assert self.dnf_manager.enabled_repositories == ["r2", "r4"] + + def test_get_matching_repositories(self): + """Test the get_matching_repositories method.""" + assert self.dnf_manager.get_matching_repositories("r*") == [] + + self._add_repository("r1") + self._add_repository("r20") + self._add_repository("r21") + self._add_repository("r3") + + assert self.dnf_manager.get_matching_repositories("") == [] + assert self.dnf_manager.get_matching_repositories("*1") == ["r1", "r21"] + assert self.dnf_manager.get_matching_repositories("*2*") == ["r20", "r21"] + assert self.dnf_manager.get_matching_repositories("r3") == ["r3"] + assert self.dnf_manager.get_matching_repositories("r4") == [] + assert self.dnf_manager.get_matching_repositories("r*") == ["r1", "r20", "r21", "r3"] + + def test_set_repository_enabled(self): + """Test the set_repository_enabled function.""" + self._add_repository("r1").disable() + + # Enable a disabled repository. + with self.assertLogs(level="INFO") as cm: + self.dnf_manager.set_repository_enabled("r1", True) + + msg = "The 'r1' repository is enabled." + assert any(map(lambda x: msg in x, cm.output)) + assert "r1" in self.dnf_manager.enabled_repositories + + # Enable an enabled repository. + with self.assertNoLogs(level="INFO"): + self.dnf_manager.set_repository_enabled("r1", True) + + # Disable an enabled repository. + with self.assertLogs(level="INFO") as cm: + self.dnf_manager.set_repository_enabled("r1", False) + + msg = "The 'r1' repository is disabled." + assert any(map(lambda x: msg in x, cm.output)) + assert "r1" not in self.dnf_manager.enabled_repositories + + # Disable a disabled repository. + with self.assertNoLogs(level="INFO"): + self.dnf_manager.set_repository_enabled("r1", False) + + # Enable an unknown repository. + with pytest.raises(UnknownRepositoryError): + self.dnf_manager.set_repository_enabled("r2", True) + + def test_add_repository_default(self): + """Test the add_repository method with defaults.""" + data = RepoConfigurationData() + data.name = "r1" + + self.dnf_manager.add_repository(data) + repo = self._get_repository("r1") + config = self._get_configuration("r1") + + assert repo.get_id() == "r1" + assert repo.get_name() == "" + assert repo.is_enabled() + + assert config.baseurl == ("", ) + assert config.proxy == "" + assert config.sslverify is True + assert config.sslcacert == "" + assert config.sslclientcert == "" + assert config.sslclientkey == "" + assert config.cost == 1000 + assert config.includepkgs == () + assert config.excludepkgs == () + + def test_add_repository_enabled(self): + """Test the add_repository method with enabled repo.""" + data = RepoConfigurationData() + data.name = "r1" + data.enabled = True + + self.dnf_manager.add_repository(data) + repo = self._get_repository("r1") + assert repo.is_enabled() is True + + def test_add_repository_disabled(self): + """Test the add_repository method with disabled repo.""" + data = RepoConfigurationData() + data.name = "r1" + data.enabled = False + + self.dnf_manager.add_repository(data) + repo = self._get_repository("r1") + assert repo.is_enabled() is False + + def test_add_repository_baseurl(self): + """Test the add_repository method with baseurl.""" + data = RepoConfigurationData() + data.name = "r1" + data.type = URL_TYPE_BASEURL + data.url = "http://repo" + + self.dnf_manager.add_repository(data) + config = self._get_configuration("r1") + assert config.baseurl == ("http://repo", ) + + def test_add_repository_mirrorlist(self): + """Test the add_repository method with mirrorlist.""" + data = RepoConfigurationData() + data.name = "r1" + data.type = URL_TYPE_MIRRORLIST + data.url = "http://mirror" + + self.dnf_manager.add_repository(data) + config = self._get_configuration("r1") + assert config.mirrorlist == "http://mirror" + + def test_add_repository_metalink(self): + """Test the add_repository method with metalink.""" + data = RepoConfigurationData() + data.name = "r1" + data.type = URL_TYPE_METALINK + data.url = "http://metalink" + + self.dnf_manager.add_repository(data) + config = self._get_configuration("r1") + assert config.metalink == "http://metalink" + + def test_add_repository_no_ssl_configuration(self): + """Test the add_repository method without the ssl configuration.""" + data = RepoConfigurationData() + data.name = "r1" + data.ssl_verification_enabled = False + + self.dnf_manager.add_repository(data) + config = self._get_configuration("r1") + assert config.sslverify is False + + def test_add_repository_ssl_configuration(self): + """Test the add_repository method with the ssl configuration.""" + data = RepoConfigurationData() + data.name = "r1" + data.ssl_verification_enabled = True + data.ssl_configuration.ca_cert_path = "file:///ca-cert" + data.ssl_configuration.client_cert_path = "file:///client-cert" + data.ssl_configuration.client_key_path = "file:///client-key" + + self.dnf_manager.add_repository(data) + config = self._get_configuration("r1") + assert config.sslverify is True + assert config.sslcacert == "file:///ca-cert" + assert config.sslclientcert == "file:///client-cert" + assert config.sslclientkey == "file:///client-key" + + def test_add_repository_invalid_proxy(self): + """Test the add_repository method the invalid proxy configuration.""" + data = RepoConfigurationData() + data.name = "r1" + data.proxy = "@:/invalid" + + self.dnf_manager.add_repository(data) + config = self._get_configuration("r1") + assert config.proxy == "" + + def test_add_repository_no_auth_proxy(self): + """Test the add_repository method the no auth proxy configuration.""" + data = RepoConfigurationData() + data.name = "r1" + data.proxy = "http://example.com:1234" + + self.dnf_manager.add_repository(data) + config = self._get_configuration("r1") + assert config.proxy == "http://example.com:1234" + + def test_add_repository_proxy(self): + """Test the add_repository method with the proxy configuration.""" + data = RepoConfigurationData() + data.name = "r1" + data.proxy = "http://user:pass@example.com:1234" + + self.dnf_manager.add_repository(data) + config = self._get_configuration("r1") + assert config.proxy == "http://example.com:1234" + assert config.proxy_username == "user" + assert config.proxy_password == "pass" + + def test_add_repository_cost(self): + """Test the add_repository method with a cost.""" + data = RepoConfigurationData() + data.name = "r1" + data.cost = 256 + + self.dnf_manager.add_repository(data) + config = self._get_configuration("r1") + assert config.cost == 256 + + def test_add_repository_packages(self): + """Test the add_repository method with packages.""" + data = RepoConfigurationData() + data.name = "r1" + data.included_packages = ["p1", "p2"] + data.excluded_packages = ["p3", "p4"] + + self.dnf_manager.add_repository(data) + config = self._get_configuration("r1") + assert config.includepkgs == ("p1", "p2") + assert config.excludepkgs == ("p3", "p4") + + @pytest.mark.skip("Not implemented") + def test_add_repository_replace(self): + """Test the add_repository method with a replacement.""" + data = RepoConfigurationData() + data.name = "r1" + data.url = "http://u1" + + self.dnf_manager.add_repository(data) + config = self._get_configuration("r1") + assert config.baseurl == ("http://u1",) + + data.url = "http://u2" + + self.dnf_manager.add_repository(data) + config = self._get_configuration("r1") + assert config.baseurl == ("http://u2",) + + @pytest.mark.skip("Not implemented") + def test_remove_repository(self): + """Test the remove_repository method.""" + assert self.dnf_manager.repositories == [] + + self._add_repository("r1") + self._add_repository("r2") + + assert self.dnf_manager.repositories == ["r1", "r2"] + + self.dnf_manager.remove_repository("r1") + assert self.dnf_manager.repositories == ["r2"] + + self.dnf_manager.remove_repository("r3") + assert self.dnf_manager.repositories == ["r2"] + + self.dnf_manager.remove_repository("r2") + assert self.dnf_manager.repositories == [] + + def test_generate_repo_file_baseurl(self): + """Test the generate_repo_file method with baseurl.""" + data = RepoConfigurationData() + data.name = "r1" + data.type = URL_TYPE_BASEURL + data.url = "http://repo" + data.proxy = "http://example.com:1234" + data.cost = 256 + + self._check_repo_file_content( + data, + """ + [r1] + name = r1 + enabled = 1 + baseurl = http://repo + proxy = http://example.com:1234 + cost = 256 + """ + ) + + def test_generate_repo_file_mirrorlist(self): + """Test the generate_repo_file method with mirrorlist.""" + data = RepoConfigurationData() + data.name = "r1" + data.type = URL_TYPE_MIRRORLIST + data.url = "http://mirror" + data.ssl_verification_enabled = False + data.proxy = "http://user:pass@example.com:1234" + + self._check_repo_file_content( + data, + """ + [r1] + name = r1 + enabled = 1 + mirrorlist = http://mirror + sslverify = 0 + proxy = http://example.com:1234 + proxy_username = user + proxy_password = pass + """ + ) + + def test_generate_repo_file_metalink(self): + """Test the generate_repo_file method with metalink.""" + data = RepoConfigurationData() + data.name = "r1" + data.enabled = False + data.type = URL_TYPE_METALINK + data.url = "http://metalink" + data.included_packages = ["p1", "p2"] + data.excluded_packages = ["p3", "p4"] + + self._check_repo_file_content( + data, + """ + [r1] + name = r1 + enabled = 0 + metalink = http://metalink + includepkgs = p1, p2 + excludepkgs = p3, p4 + """ + ) + + def _check_repo_file_content(self, repo_data, expected_content): + """Check the generated content of the .repo file.""" + # Generate the content of the .repo file. + expected_content = dedent(expected_content).strip() + content = self.dnf_manager.generate_repo_file(repo_data) + assert content == expected_content + + # FIXME: Try to recreate the generated repository. + # expected_attrs = expected_content.splitlines(keepends=False) + # self.dnf_manager.add_repository(repo_data) + # self._check_repo(repo_data.name, expected_attrs) + + def test_read_system_repositories(self): + """Test the read_system_repositories method.""" + self.dnf_manager.read_system_repositories() + + # There should be some repositories in the testing environment. + assert self.dnf_manager.repositories + + # All these repositories should be disabled. + assert not self.dnf_manager.enabled_repositories + + # However, we should remember which ones were enabled. + assert self.dnf_manager._enabled_system_repositories + + for repo_id in self.dnf_manager._enabled_system_repositories: + assert repo_id in self.dnf_manager.repositories + + # Don't read system repositories again. + with pytest.raises(RuntimeError): + self.dnf_manager.read_system_repositories() + + # FIXME: Unless we cleared the cache. + # self.dnf_manager.clear_cache() + # assert not self.dnf_manager._enabled_system_repositories + # self.dnf_manager.read_system_repositories() + + # Or reset the base. + self.dnf_manager.reset_base() + assert not self.dnf_manager._enabled_system_repositories + self.dnf_manager.read_system_repositories() + + def test_restore_system_repositories(self): + """Test the restore_system_repositories.""" + # Read repositories from the testing environment and disable them. + self.dnf_manager.read_system_repositories() + assert not self.dnf_manager.enabled_repositories + assert self.dnf_manager._enabled_system_repositories + + # Re-enable repositories from the testing environment. + self.dnf_manager.restore_system_repositories() + assert self.dnf_manager.enabled_repositories + assert self.dnf_manager._enabled_system_repositories + + assert self.dnf_manager.enabled_repositories == \ + self.dnf_manager._enabled_system_repositories + + # Skip unknown repositories. + self.dnf_manager._enabled_system_repositories.append("r1") + self.dnf_manager.restore_system_repositories() + + def test_load_repository_unknown(self): + """Test the load_repository method with an unknown repo.""" + self._check_base_setup() + + with pytest.raises(UnknownRepositoryError): + self.dnf_manager.load_repository("r1") + + def test_load_repository_failed(self): + """Test the load_repository method with a failure.""" + self._check_base_setup() + self._add_repository("r1") + + with pytest.raises(MetadataError, match="Failed to download metadata"): + self.dnf_manager.load_repository("r1") + + repo = self._get_repository("r1") + assert repo.is_enabled() is False + + def test_load_repository_disabled(self): + """Test the load_repository method with a disabled repo.""" + self._check_base_setup() + + repo = self._add_repository("r1") + repo.disable() + + self.dnf_manager.load_repository("r1") + + repo = self._get_repository("r1") + assert repo.is_enabled() is False + + def test_load_repository(self): + """Test the load_repository method.""" + self._check_base_setup() + + with TemporaryDirectory() as d: + self._add_repository("r1", repo_dir=d) + self.dnf_manager.load_repository("r1") + + repo = self._get_repository("r1") + assert repo.is_enabled() is True + + def test_load_packages_metadata(self): + """Test the load_packages_metadata method.""" + self._check_base_setup() + self.dnf_manager.load_packages_metadata() + + def test_load_no_repomd_hashes(self): + """Test the load_repomd_hashes method with no repositories.""" + self.dnf_manager.load_repomd_hashes() + assert self.dnf_manager._md_hashes == {} + + @pytest.mark.skip("Not implemented") + def test_load_one_repomd_hash(self): + """Test the load_repomd_hashes method with one repository.""" + with TemporaryDirectory() as d: + self._add_repository("r1", repo_dir=d) + self.dnf_manager.load_repomd_hashes() + assert self.dnf_manager._md_hashes == { + 'r1': b"\x90\xa0\xb7\xce\xc2H\x85#\xa3\xfci" + b"\x9e+\xf4\xe2\x19D\xbc\x9b'\xeb\xb7" + b"\x90\x1d\xcey\xb3\xd4p\xc3\x1d\xfb", + } + + @pytest.mark.skip("Not implemented") + def test_load_repomd_hashes(self): + """Test the load_repomd_hashes method.""" + with TemporaryDirectory() as d: + self._add_repository( + repo_id="r1", + baseurl=[ + "file://nonexistent/1", + "file://nonexistent/2", + "file://nonexistent/3", + ], + repo_dir=d + "/r1", + ) + self._add_repository( + repo_id="r2", + baseurl=[ + "file://nonexistent/1", + "file://nonexistent/2", + "file://nonexistent/3", + ] + ) + self._add_repository( + repo_id="r3", + metalink="file://metalink" + ) + self._add_repository( + repo_id="r4", + mirrorlist="file://mirrorlist" + ) + self.dnf_manager.load_repomd_hashes() + assert self.dnf_manager._md_hashes == { + 'r1': b"\x90\xa0\xb7\xce\xc2H\x85#\xa3\xfci" + b"\x9e+\xf4\xe2\x19D\xbc\x9b'\xeb\xb7" + b"\x90\x1d\xcey\xb3\xd4p\xc3\x1d\xfb", + 'r2': None, + 'r3': None, + 'r4': None, + } + + @pytest.mark.skip("Not implemented") + def test_verify_repomd_hashes(self): + """Test the verify_repomd_hashes method.""" + with TemporaryDirectory() as d: + # Test no repository. + assert self.dnf_manager.verify_repomd_hashes() is False + + # Create a repository. + self._add_repository(repo_id="r1", repo_dir=d) + + # Test no loaded repository. + assert self.dnf_manager.verify_repomd_hashes() is False + + # Test a loaded repository. + self.dnf_manager.load_repomd_hashes() + assert self.dnf_manager.verify_repomd_hashes() is True + + # Test a different content of metadata. + with open(os.path.join(d, "repodata", "repomd.xml"), 'w') as f: + f.write("Different metadata for r1.") + + assert self.dnf_manager.verify_repomd_hashes() is False + + # Test a reloaded repository. + self.dnf_manager.load_repomd_hashes() + assert self.dnf_manager.verify_repomd_hashes() is True + + # Test the base reset. + self.dnf_manager.reset_base() + assert self.dnf_manager.verify_repomd_hashes() is False diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py index 1aac9a3a266..bda1f3477df 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py @@ -21,17 +21,19 @@ from textwrap import dedent from unittest.mock import Mock, call, patch -import libdnf.transaction +import libdnf5 + +#import libdnf.transaction import pytest from blivet.size import ROUND_UP, Size from dasbus.structure import compare_data -from dnf.callback import PKG_SCRIPTLET, STATUS_FAILED, STATUS_OK -from dnf.comps import Comps, Environment, Group -from dnf.exceptions import DepsolveError, MarkingErrors, RepoError -from dnf.package import Package -from dnf.repo import Repo -from dnf.transaction import PKG_INSTALL, TRANS_POST +#from dnf.callback import PKG_SCRIPTLET, STATUS_FAILED, STATUS_OK +#from dnf.comps import Comps, Environment, Group +#from dnf.exceptions import DepsolveError, MarkingErrors, RepoError +#from dnf.package import Package +#from dnf.repo import Repo +#from dnf.transaction import PKG_INSTALL, TRANS_POST from pyanaconda.core.constants import ( MULTILIB_POLICY_ALL, URL_TYPE_BASEURL, @@ -56,280 +58,11 @@ InvalidSelectionError, MetadataError, MissingSpecsError, + simplify_config, ) class DNFManagerTestCase(unittest.TestCase): - """Test the abstraction of the DNF base.""" - - def setUp(self): - self.maxDiff = None - self.dnf_manager = DNFManager() - - def _check_configuration(self, *attributes): - """Check the DNF configuration.""" - configuration = self.dnf_manager._base.conf.dump() - configuration = configuration.splitlines(keepends=False) - - for attribute in attributes: - assert attribute in configuration - - def _check_substitutions(self, substitutions): - """Check the DNF substitutions.""" - assert dict(self.dnf_manager._base.conf.substitutions) == substitutions - - def test_create_base(self): - """Test the creation of the DNF base.""" - assert self.dnf_manager._base is not None - - def test_reset_base(self): - """Test the reset of the DNF base.""" - base_1 = self.dnf_manager._base - assert self.dnf_manager._base == base_1 - self.dnf_manager.reset_base() - - base_2 = self.dnf_manager._base - assert self.dnf_manager._base == base_2 - assert self.dnf_manager._base != base_1 - - assert base_1._closed - assert not base_2._closed - - def test_clear_cache(self): - """Test the clear_cache method.""" - self.dnf_manager.clear_cache() - - def test_set_default_configuration(self): - """Test the default configuration of the DNF base.""" - self._check_configuration( - "gpgcheck = 0", - "skip_if_unavailable = 0" - ) - self._check_configuration( - "cachedir = /tmp/dnf.cache", - "pluginconfpath = /tmp/dnf.pluginconf", - "logdir = /tmp/", - ) - self._check_configuration( - "installroot = /mnt/sysroot", - "persistdir = /mnt/sysroot/var/lib/dnf" - ) - self._check_configuration( - "reposdir = " - "/etc/yum.repos.d, " - "/etc/anaconda.repos.d" - ) - self._check_substitutions({ - "arch": "x86_64", - "basearch": "x86_64", - "releasever": "rawhide", - "releasever_major": "rawhide", - "releasever_minor": "", - }) - - @patch("pyanaconda.modules.payloads.payload.dnf.dnf_manager.get_os_release_value") - def test_set_module_platform_id(self, get_platform_id): - """Test the configuration of module_platform_id.""" - get_platform_id.return_value = "platform:f32" - self.dnf_manager.reset_base() - self._check_configuration("module_platform_id = platform:f32") - - def test_configure_proxy(self): - """Test the proxy configuration.""" - self.dnf_manager.configure_proxy("http://user:pass@example.com/proxy") - self._check_configuration( - "proxy = http://example.com:3128", - "proxy_username = user", - "proxy_password = pass", - ) - - self.dnf_manager.configure_proxy("@:/invalid") - self._check_configuration( - "proxy = ", - "proxy_username = ", - "proxy_password = ", - ) - - self.dnf_manager.configure_proxy("http://example.com/proxy") - self._check_configuration( - "proxy = http://example.com:3128", - "proxy_username = ", - "proxy_password = ", - ) - - self.dnf_manager.configure_proxy(None) - self._check_configuration( - "proxy = ", - "proxy_username = ", - "proxy_password = ", - ) - - def test_configure_base(self): - """Test the configuration of the DNF base.""" - data = PackagesConfigurationData() - - self.dnf_manager.configure_base(data) - self._check_configuration( - "multilib_policy = best", - "timeout = 30", - "retries = 10", - "install_weak_deps = 1", - ) - - assert self.dnf_manager._ignore_broken_packages is False - assert self.dnf_manager._ignore_missing_packages is False - - data.multilib_policy = MULTILIB_POLICY_ALL - data.timeout = 100 - data.retries = 5 - data.broken_ignored = True - data.missing_ignored = True - data.weakdeps_excluded = True - - self.dnf_manager.configure_base(data) - self._check_configuration( - "multilib_policy = all", - "timeout = 100", - "retries = 5", - "install_weak_deps = 0", - ) - - assert self.dnf_manager._ignore_broken_packages is True - assert self.dnf_manager._ignore_missing_packages is True - - def test_dump_configuration(self): - """Test the dump of the DNF configuration.""" - with self.assertLogs(level="DEBUG") as cm: - self.dnf_manager.dump_configuration() - - msg = "DNF configuration:" - assert any(map(lambda x: msg in x, cm.output)) - - msg = "installroot = /mnt/sysroot" - assert any(map(lambda x: msg in x, cm.output)) - - def test_get_installation_size(self): - """Test the get_installation_size method.""" - # No transaction. - size = self.dnf_manager.get_installation_size() - assert size == Size("3000 MiB") - - # Fake transaction. - tsi_1 = Mock() - tsi_1.pkg.installsize = 1024 * 100 - tsi_1.pkg.files = ["/file"] * 10 - - tsi_2 = Mock() - tsi_2.pkg.installsize = 1024 * 200 - tsi_2.pkg.files = ["/file"] * 20 - - self.dnf_manager._base.transaction = [tsi_1, tsi_2] - size = self.dnf_manager.get_installation_size() - size = size.round_to_nearest("KiB", ROUND_UP) - - assert size == Size("528 KiB") - - def test_get_download_size(self): - """Test the get_download_size method.""" - # No transaction. - size = self.dnf_manager.get_download_size() - assert size == Size(0) - - # Fake transaction. - tsi_1 = Mock() - tsi_1.pkg.downloadsize = 1024 * 1024 * 100 - - tsi_2 = Mock() - tsi_2.pkg.downloadsize = 1024 * 1024 * 200 - - self.dnf_manager._base.transaction = [tsi_1, tsi_2] - size = self.dnf_manager.get_download_size() - - assert size == Size("450 MiB") - - @patch("dnf.base.Base.install_specs") - def test_apply_specs(self, install_specs): - """Test the apply_specs method.""" - self.dnf_manager.apply_specs( - include_list=["@g1", "p1"], - exclude_list=["@g2", "p2"] - ) - - install_specs.assert_called_once_with( - install=["@g1", "p1"], - exclude=["@g2", "p2"], - strict=True - ) - - @patch("dnf.base.Base.install_specs") - def test_apply_specs_error(self, install_specs): - """Test the apply_specs method with an error.""" - install_specs.side_effect = MarkingErrors( - error_group_specs=["@g1"] - ) - - with pytest.raises(BrokenSpecsError): - self.dnf_manager.apply_specs( - include_list=["@g1", "p1"], - exclude_list=["@g2", "p2"] - ) - - install_specs.side_effect = MarkingErrors( - no_match_group_specs=["@g1"] - ) - - with pytest.raises(MissingSpecsError): - self.dnf_manager.apply_specs( - include_list=["@g1", "p1"], - exclude_list=["@g2", "p2"] - ) - - @patch("dnf.base.Base.install_specs") - def test_apply_specs_ignore_broken(self, install_specs): - """Test the apply_specs method with ignored broken packages.""" - self.dnf_manager._ignore_broken_packages = True - self.dnf_manager.apply_specs( - include_list=["@g1", "p1"], - exclude_list=["@g2", "p2"] - ) - - install_specs.assert_called_once_with( - install=["@g1", "p1"], - exclude=["@g2", "p2"], - strict=False - ) - - @patch("dnf.base.Base.install_specs") - def test_apply_specs_ignore_missing(self, install_specs): - """Test the apply_specs method with ignored missing packages.""" - self.dnf_manager._ignore_missing_packages = True - - # Ignore a missing package. - install_specs.side_effect = MarkingErrors( - no_match_pkg_specs=["p1"] - ) - - self.dnf_manager.apply_specs( - include_list=["@g1", "p1"], - exclude_list=["@g2", "p2"] - ) - - install_specs.assert_called_once_with( - install=["@g1", "p1"], - exclude=["@g2", "p2"], - strict=True - ) - - # Don't ignore a broken transaction. - install_specs.side_effect = MarkingErrors( - error_pkg_specs=["p1"] - ) - - with pytest.raises(BrokenSpecsError): - self.dnf_manager.apply_specs( - include_list=["@g1", "p1"], - exclude_list=["@g2", "p2"] - ) @patch("dnf.base.Base.download_packages") @patch("dnf.base.Base.transaction") @@ -557,46 +290,6 @@ def test_download_location(self): self.dnf_manager.reset_base() assert self.dnf_manager.download_location is None - def test_substitute(self): - """Test the substitute method.""" - # No variables. - assert self.dnf_manager.substitute(None) == "" - assert self.dnf_manager.substitute("") == "" - assert self.dnf_manager.substitute("/") == "/" - assert self.dnf_manager.substitute("/text") == "/text" - - # Unknown variables. - assert self.dnf_manager.substitute("/$unknown") == "/$unknown" - - # Supported variables. - assert self.dnf_manager.substitute("/$basearch") != "/$basearch" - assert self.dnf_manager.substitute("/$releasever") != "/$releasever" - - def test_configure_substitution(self): - """Test the configure_substitution function.""" - self.dnf_manager.configure_substitution( - release_version="123" - ) - self._check_substitutions({ - "arch": "x86_64", - "basearch": "x86_64", - "releasever": "123", - "releasever_major": "123", - "releasever_minor": "", - }) - - # Ignore an undefined release version. - self.dnf_manager.configure_substitution( - release_version="" - ) - self._check_substitutions({ - "arch": "x86_64", - "basearch": "x86_64", - "releasever": "123", - "releasever_major": "123", - "releasever_minor": "", - }) - @patch("dnf.subject.Subject.get_best_query") def test_is_package_available(self, get_best_query): """Test the is_package_available method.""" @@ -644,38 +337,6 @@ def test_match_available_packages(self): msg = "There is no metadata about packages!" assert any(map(lambda x: msg in x, cm.output)) - @patch("dnf.base.Base.resolve") - def test_resolve_selection(self, resolve): - """Test the resolve_selection method.""" - self.dnf_manager._base.transaction = [Mock(), Mock()] - - with self.assertLogs(level="INFO") as cm: - self.dnf_manager.resolve_selection() - - expected = "The software selection has been resolved (2 packages selected)." - assert expected in "\n".join(cm.output) - - resolve.assert_called_once() - - @patch("dnf.base.Base.resolve") - def test_resolve_selection_failed(self, resolve): - """Test the failed resolve_selection method.""" - resolve.side_effect = DepsolveError("e1") - - with pytest.raises(InvalidSelectionError) as cm: - self.dnf_manager.resolve_selection() - - expected = \ - "The following software marked for installation has errors.\n" \ - "This is likely caused by an error with your installation source.\n\n" \ - "e1" - - assert expected == str(cm.value) - - def test_clear_selection(self): - """Test the clear_selection method.""" - self.dnf_manager.clear_selection() - class DNFManagerCompsTestCase(unittest.TestCase): """Test the comps abstraction of the DNF base.""" @@ -890,557 +551,3 @@ def test_environment_data_available_groups(self): assert data.get_available_groups() == [ "g1", "g2", "g3", "g4", "g5" ] - - -class DNFManagerReposTestCase(unittest.TestCase): - """Test the repo abstraction of the DNF base.""" - - def setUp(self): - self.maxDiff = None - self.dnf_manager = DNFManager() - - def _add_repo(self, repo_id): - """Add a mocked repo with the specified id.""" - repo = Repo(repo_id, self.dnf_manager._base.conf) - self.dnf_manager._base.repos.add(repo) - return repo - - def _check_repo(self, repo_id, attributes): - """Check the DNF repo configuration.""" - repo = self.dnf_manager._base.repos[repo_id] - repo_conf = repo.dump() - repo_conf = repo_conf.splitlines(keepends=False) - - print(repo.dump()) - - for attribute in attributes: - assert attribute in repo_conf - - def _check_content(self, repo_data, expected_content): - """Check the generated content of the .repo file.""" - expected_content = dedent(expected_content).strip() - content = self.dnf_manager.generate_repo_file(repo_data) - assert content == expected_content - - expected_attrs = expected_content.splitlines(keepends=False) - self.dnf_manager.add_repository(repo_data) - self._check_repo(repo_data.name, expected_attrs) - - def test_repositories(self): - """Test the repositories property.""" - assert self.dnf_manager.repositories == [] - - self._add_repo("r1") - self._add_repo("r2") - self._add_repo("r3") - - assert self.dnf_manager.repositories == ["r1", "r2", "r3"] - - def test_enabled_repositories(self): - """Test the enabled_repositories property.""" - assert self.dnf_manager.enabled_repositories == [] - - self._add_repo("r1").disable() - self._add_repo("r2").enable() - self._add_repo("r3").disable() - self._add_repo("r4").enable() - - assert self.dnf_manager.enabled_repositories == ["r2", "r4"] - - def test_get_matching_repositories(self): - """Test the get_matching_repositories method.""" - assert self.dnf_manager.get_matching_repositories("r*") == [] - - self._add_repo("r1") - self._add_repo("r20") - self._add_repo("r21") - self._add_repo("r3") - - assert self.dnf_manager.get_matching_repositories("") == [] - assert self.dnf_manager.get_matching_repositories("*1") == ["r1", "r21"] - assert self.dnf_manager.get_matching_repositories("*2*") == ["r20", "r21"] - assert self.dnf_manager.get_matching_repositories("r3") == ["r3"] - assert self.dnf_manager.get_matching_repositories("r4") == [] - assert self.dnf_manager.get_matching_repositories("r*") == ["r1", "r20", "r21", "r3"] - - def test_set_repository_enabled(self): - """Test the set_repository_enabled function.""" - self._add_repo("r1").disable() - - # Enable a disabled repository. - with self.assertLogs(level="INFO") as cm: - self.dnf_manager.set_repository_enabled("r1", True) - - msg = "The 'r1' repository is enabled." - assert any(map(lambda x: msg in x, cm.output)) - assert "r1" in self.dnf_manager.enabled_repositories - - # Enable an enabled repository. - with self.assertNoLogs(level="INFO"): - self.dnf_manager.set_repository_enabled("r1", True) - - # Disable an enabled repository. - with self.assertLogs(level="INFO") as cm: - self.dnf_manager.set_repository_enabled("r1", False) - - msg = "The 'r1' repository is disabled." - assert any(map(lambda x: msg in x, cm.output)) - assert "r1" not in self.dnf_manager.enabled_repositories - - # Disable a disabled repository. - with self.assertNoLogs(level="INFO"): - self.dnf_manager.set_repository_enabled("r1", False) - - # Enable an unknown repository. - with pytest.raises(UnknownRepositoryError): - self.dnf_manager.set_repository_enabled("r2", True) - - def test_add_repository_default(self): - """Test the add_repository method with defaults.""" - data = RepoConfigurationData() - data.name = "r1" - - self.dnf_manager.add_repository(data) - - self._check_repo("r1", [ - "baseurl = ", - "proxy = ", - "sslverify = 1", - "sslcacert = ", - "sslclientcert = ", - "sslclientkey = ", - "cost = 1000", - "includepkgs = ", - "excludepkgs = ", - ]) - - def test_add_repository_enabled(self): - """Test the add_repository method with enabled repo.""" - data = RepoConfigurationData() - data.name = "r1" - data.enabled = True - - self.dnf_manager.add_repository(data) - - self._check_repo("r1", [ - "enabled = 1", - ]) - - def test_add_repository_disabled(self): - """Test the add_repository method with disabled repo.""" - data = RepoConfigurationData() - data.name = "r1" - data.enabled = False - - self.dnf_manager.add_repository(data) - - self._check_repo("r1", [ - "enabled = 0", - ]) - - def test_add_repository_baseurl(self): - """Test the add_repository method with baseurl.""" - data = RepoConfigurationData() - data.name = "r1" - data.type = URL_TYPE_BASEURL - data.url = "http://repo" - - self.dnf_manager.add_repository(data) - - self._check_repo("r1", [ - "baseurl = http://repo", - ]) - - def test_add_repository_mirrorlist(self): - """Test the add_repository method with mirrorlist.""" - data = RepoConfigurationData() - data.name = "r1" - data.type = URL_TYPE_MIRRORLIST - data.url = "http://mirror" - - self.dnf_manager.add_repository(data) - self._check_repo("r1", [ - "mirrorlist = http://mirror", - ]) - - def test_add_repository_metalink(self): - """Test the add_repository method with metalink.""" - data = RepoConfigurationData() - data.name = "r1" - data.type = URL_TYPE_METALINK - data.url = "http://metalink" - - self.dnf_manager.add_repository(data) - self._check_repo("r1", [ - "metalink = http://metalink", - ]) - - def test_add_repository_no_ssl_configuration(self): - """Test the add_repository method without the ssl configuration.""" - data = RepoConfigurationData() - data.name = "r1" - data.ssl_verification_enabled = False - - self.dnf_manager.add_repository(data) - self._check_repo("r1", [ - "sslverify = 0", - ]) - - def test_add_repository_ssl_configuration(self): - """Test the add_repository method with the ssl configuration.""" - data = RepoConfigurationData() - data.name = "r1" - data.ssl_verification_enabled = True - data.ssl_configuration.ca_cert_path = "file:///ca-cert" - data.ssl_configuration.client_cert_path = "file:///client-cert" - data.ssl_configuration.client_key_path = "file:///client-key" - - self.dnf_manager.add_repository(data) - self._check_repo("r1", [ - "sslverify = 1", - "sslcacert = file:///ca-cert", - "sslclientcert = file:///client-cert", - "sslclientkey = file:///client-key", - ]) - - def test_add_repository_invalid_proxy(self): - """Test the add_repository method the invalid proxy configuration.""" - data = RepoConfigurationData() - data.name = "r1" - data.proxy = "@:/invalid" - - self.dnf_manager.add_repository(data) - self._check_repo("r1", [ - "proxy = ", - ]) - - def test_add_repository_no_auth_proxy(self): - """Test the add_repository method the no auth proxy configuration.""" - data = RepoConfigurationData() - data.name = "r1" - data.proxy = "http://example.com:1234" - - self.dnf_manager.add_repository(data) - self._check_repo("r1", [ - "proxy = http://example.com:1234", - ]) - - def test_add_repository_proxy(self): - """Test the add_repository method with the proxy configuration.""" - data = RepoConfigurationData() - data.name = "r1" - data.proxy = "http://user:pass@example.com:1234" - - self.dnf_manager.add_repository(data) - self._check_repo("r1", [ - "proxy = http://example.com:1234", - "proxy_username = user", - "proxy_password = pass", - ]) - - def test_add_repository_cost(self): - """Test the add_repository method with a cost.""" - data = RepoConfigurationData() - data.name = "r1" - data.cost = 256 - - self.dnf_manager.add_repository(data) - self._check_repo("r1", [ - "cost = 256" - ]) - - def test_add_repository_packages(self): - """Test the add_repository method with packages.""" - data = RepoConfigurationData() - data.name = "r1" - data.included_packages = ["p1", "p2"] - data.excluded_packages = ["p3", "p4"] - - self.dnf_manager.add_repository(data) - self._check_repo("r1", [ - "includepkgs = p1, p2", - "excludepkgs = p3, p4", - ]) - - def test_add_repository_replace(self): - """Test the add_repository method with a replacement.""" - data = RepoConfigurationData() - data.name = "r1" - data.url = "http://u1" - - self.dnf_manager.add_repository(data) - self._check_repo("r1", [ - "baseurl = http://u1", - ]) - - data.url = "http://u2" - - self.dnf_manager.add_repository(data) - self._check_repo("r1", [ - "baseurl = http://u2", - ]) - - def test_generate_repo_file_baseurl(self): - """Test the generate_repo_file method with baseurl.""" - data = RepoConfigurationData() - data.name = "r1" - data.type = URL_TYPE_BASEURL - data.url = "http://repo" - data.proxy = "http://example.com:1234" - data.cost = 256 - - self._check_content( - data, - """ - [r1] - name = r1 - enabled = 1 - baseurl = http://repo - proxy = http://example.com:1234 - cost = 256 - """ - ) - - def test_generate_repo_file_mirrorlist(self): - """Test the generate_repo_file method with mirrorlist.""" - data = RepoConfigurationData() - data.name = "r1" - data.type = URL_TYPE_MIRRORLIST - data.url = "http://mirror" - data.ssl_verification_enabled = False - data.proxy = "http://user:pass@example.com:1234" - - self._check_content( - data, - """ - [r1] - name = r1 - enabled = 1 - mirrorlist = http://mirror - sslverify = 0 - proxy = http://example.com:1234 - proxy_username = user - proxy_password = pass - """ - ) - - def test_generate_repo_file_metalink(self): - """Test the generate_repo_file method with metalink.""" - data = RepoConfigurationData() - data.name = "r1" - data.enabled = False - data.type = URL_TYPE_METALINK - data.url = "http://metalink" - data.included_packages = ["p1", "p2"] - data.excluded_packages = ["p3", "p4"] - - self._check_content( - data, - """ - [r1] - name = r1 - enabled = 0 - metalink = http://metalink - includepkgs = p1, p2 - excludepkgs = p3, p4 - """ - ) - - def test_read_system_repositories(self): - """Test the read_system_repositories method.""" - self.dnf_manager.read_system_repositories() - - # There should be some repositories in the testing environment. - assert self.dnf_manager.repositories - - # All these repositories should be disabled. - assert not self.dnf_manager.enabled_repositories - - # However, we should remember which ones were enabled. - assert self.dnf_manager._enabled_system_repositories - - for repo_id in self.dnf_manager._enabled_system_repositories: - assert repo_id in self.dnf_manager.repositories - - # Don't read system repositories again. - with pytest.raises(RuntimeError): - self.dnf_manager.read_system_repositories() - - # Unless we cleared the cache. - self.dnf_manager.clear_cache() - assert not self.dnf_manager._enabled_system_repositories - self.dnf_manager.read_system_repositories() - - # Or reset the base. - self.dnf_manager.reset_base() - assert not self.dnf_manager._enabled_system_repositories - self.dnf_manager.read_system_repositories() - - def test_restore_system_repositories(self): - """Test the restore_system_repositories.""" - # Read repositories from the testing environment and disable them. - self.dnf_manager.read_system_repositories() - assert not self.dnf_manager.enabled_repositories - assert self.dnf_manager._enabled_system_repositories - - # Re-enable repositories from the testing environment. - self.dnf_manager.restore_system_repositories() - assert self.dnf_manager.enabled_repositories - assert self.dnf_manager._enabled_system_repositories - - assert self.dnf_manager.enabled_repositories == \ - self.dnf_manager._enabled_system_repositories - - # Skip unknown repositories. - self.dnf_manager._enabled_system_repositories.append("r1") - self.dnf_manager.restore_system_repositories() - - def test_load_repository_unknown(self): - """Test the load_repository method with an unknown repo.""" - with pytest.raises(UnknownRepositoryError): - self.dnf_manager.load_repository("r1") - - def test_load_repository_failed(self): - """Test the load_repository method with a failure.""" - repo = self._add_repo("r1") - repo.load = Mock(side_effect=RepoError("Fake error!")) - repo.enable() - - with pytest.raises(MetadataError) as cm: - self.dnf_manager.load_repository("r1") - - repo.load.assert_called_once() - assert repo.enabled is False - assert str(cm.value) == "Fake error!" - - def test_load_repository_disabled(self): - """Test the load_repository method with a disabled repo.""" - repo = self._add_repo("r1") - repo.load = Mock() - repo.disable() - - self.dnf_manager.load_repository("r1") - - repo.load.assert_not_called() - assert repo.enabled is False - - def test_load_repository(self): - """Test the load_repository method.""" - repo = self._add_repo("r1") - repo.load = Mock() - repo.enable() - - self.dnf_manager.load_repository("r1") - - repo.load.assert_called_once() - assert repo.enabled is True - - def test_load_packages_metadata(self): - """Test the load_packages_metadata method.""" - sack = self.dnf_manager._base.sack - comps = self.dnf_manager._base.comps - - self.dnf_manager.load_packages_metadata() - - # The metadata should be reloaded. - assert sack != self.dnf_manager._base.sack - assert comps != self.dnf_manager._base.comps - - def _create_repo(self, repo, repo_dir): - """Generate fake metadata for the repo.""" - # Create the repodata directory. - os.makedirs(os.path.join(repo_dir, "repodata")) - - # Create the repomd.xml file. - md_path = os.path.join(repo_dir, "repodata", "repomd.xml") - md_content = "Metadata for {}.".format(repo.id) - - with open(md_path, 'w') as f: - f.write(md_content) - - # Set up the baseurl. - repo.baseurl.append("file://" + repo_dir) - - def test_load_no_repomd_hashes(self): - """Test the load_repomd_hashes method with no repositories.""" - self.dnf_manager.load_repomd_hashes() - assert self.dnf_manager._md_hashes == {} - - def test_load_one_repomd_hash(self): - """Test the load_repomd_hashes method with one repository.""" - with TemporaryDirectory() as d: - r1 = self._add_repo("r1") - self._create_repo(r1, d) - - self.dnf_manager.load_repomd_hashes() - assert self.dnf_manager._md_hashes == { - 'r1': b"\x90\xa0\xb7\xce\xc2H\x85#\xa3\xfci" - b"\x9e+\xf4\xe2\x19D\xbc\x9b'\xeb\xb7" - b"\x90\x1d\xcey\xb3\xd4p\xc3\x1d\xfb", - } - - def test_load_repomd_hashes(self): - """Test the load_repomd_hashes method.""" - with TemporaryDirectory() as d: - r1 = self._add_repo("r1") - r1.baseurl = [ - "file://nonexistent/1", - "file://nonexistent/2", - "file://nonexistent/3", - ] - self._create_repo(r1, d + "/r1") - - r2 = self._add_repo("r2") - r2.baseurl = [ - "file://nonexistent/1", - "file://nonexistent/2", - "file://nonexistent/3", - ] - - r3 = self._add_repo("r3") - r3.metalink = "file://metalink" - - r4 = self._add_repo("r4") - r4.mirrorlist = "file://mirrorlist" - - self.dnf_manager.load_repomd_hashes() - - assert self.dnf_manager._md_hashes == { - 'r1': b"\x90\xa0\xb7\xce\xc2H\x85#\xa3\xfci" - b"\x9e+\xf4\xe2\x19D\xbc\x9b'\xeb\xb7" - b"\x90\x1d\xcey\xb3\xd4p\xc3\x1d\xfb", - 'r2': None, - 'r3': None, - 'r4': None, - } - - def test_verify_repomd_hashes(self): - """Test the verify_repomd_hashes method.""" - with TemporaryDirectory() as d: - # Test no repository. - assert self.dnf_manager.verify_repomd_hashes() is False - - # Create a repository. - r = self._add_repo("r1") - self._create_repo(r, d) - - # Test no loaded repository. - assert self.dnf_manager.verify_repomd_hashes() is False - - # Test a loaded repository. - self.dnf_manager.load_repomd_hashes() - assert self.dnf_manager.verify_repomd_hashes() is True - - # Test a different content of metadata. - with open(os.path.join(d, "repodata", "repomd.xml"), 'w') as f: - f.write("Different metadata for r1.") - - assert self.dnf_manager.verify_repomd_hashes() is False - - # Test a reloaded repository. - self.dnf_manager.load_repomd_hashes() - assert self.dnf_manager.verify_repomd_hashes() is True - - # Test the base reset. - self.dnf_manager.reset_base() - assert self.dnf_manager.verify_repomd_hashes() is False From c0292fb2cd91da86326c66e13bdc8e3b17e62f90 Mon Sep 17 00:00:00 2001 From: Vendula Poncova Date: Wed, 15 Feb 2023 15:20:52 +0100 Subject: [PATCH 02/40] Use repo.get() --- .../payloads/payload/dnf/dnf_manager.py | 17 +++++------ .../test_module_payload_dnf5_manager.py | 28 +++++++++++-------- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 87a30fe3cc8..2328fc7901d 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -288,7 +288,7 @@ def environments(self): :return: a list of ids """ environments = libdnf5.comps.EnvironmentQuery(self._base) - return [env.get_environmentid() for env in environments] + return [env.get_environmentid() for env in environments.list()] def _get_environment(self, environment_name): """Translate the given environment name to a DNF object. @@ -301,7 +301,7 @@ def _get_environment(self, environment_name): environments = libdnf5.comps.EnvironmentQuery(self._base) environments.filter_name(environment_name) - return next(iter(environments), None) + return next(iter(environments.list()), None) def resolve_environment(self, environment_name): """Translate the given environment name to a group ID. @@ -368,7 +368,7 @@ def groups(self): :return: a list of IDs """ groups = libdnf5.comps.GroupQuery(self._base) - return [g.get_groupid() for g in groups] + return [g.get_groupid() for g in groups.list()] def _get_group(self, group_name): """Translate the given group name into a DNF object. @@ -378,7 +378,7 @@ def _get_group(self, group_name): """ groups = libdnf5.comps.GroupQuery(self._base) groups.filter_name(group_name) - return next(iter(groups), None) + return next(iter(groups.list()), None) def resolve_group(self, group_name): """Translate the given group name into a group ID. @@ -467,7 +467,7 @@ def dump_configuration(self): "\n%s" "\nvariables = %s", str(self._base.get_config()), - str(self._base.get_vars()) + str(self._base.get_vars()), ) def substitute(self, text): @@ -809,7 +809,8 @@ def _get_repository(self, repo_id): repositories.filter_id(repo_id) try: - return repositories.get() + weak_repo_ref = repositories.get() + return weak_repo_ref.get() except RuntimeError: raise UnknownRepositoryError(repo_id) from None @@ -1040,13 +1041,9 @@ def load_repository(self, repo_id): try: repo.fetch_metadata() - - repo = self._get_repository(repo_id) repo.load() except RuntimeError as e: log.debug("Failed to load metadata from '%s': %s", url, str(e)) - # FIXME: Get a new object to avoid a crash. - repo = self._get_repository(repo_id) repo.disable() raise MetadataError(str(e)) from None diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index 8a5f9a36a04..7a81675a087 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -75,7 +75,6 @@ def test_group_query(self): base = libdnf5.base.Base() libdnf5.comps.GroupQuery(base) - @pytest.mark.skip("Fatal Python error: Aborted") def test_disable_failed_repository(self): base = libdnf5.base.Base() sack = base.get_repo_sack() @@ -95,7 +94,8 @@ def test_disable_failed_repository(self): def _get_repo(self, base, repo_id): repos = libdnf5.repo.RepoQuery(base) repos.filter_id(repo_id) - return repos.get() + weak_ref = repos.get() + return weak_ref.get() def test_simplify_config(self): """Test the simplify_config function.""" @@ -394,24 +394,30 @@ class DNFManagerCompsTestCase(unittest.TestCase): def setUp(self): self.maxDiff = None self.dnf_manager = DNFManager() - # self.dnf_manager._base._comps = self._create_comps() - @pytest.mark.skip("Fatal Python error: Aborted") + def _check_base_setup(self): + """Make sure that the base is set up.""" + # FIXME: Figure this out on the manager level. + base = self.dnf_manager._base + assert base + base.setup() + + @pytest.mark.skip("'SwigPyObject' object is not iterable") def test_groups(self): """Test the groups property.""" - # FIXME: Handle unlocked base. + self._check_base_setup() assert self.dnf_manager.groups == [] - @pytest.mark.skip("Fatal Python error: Aborted") + @pytest.mark.skip("'SwigPyObject' object is not iterable") def test_no_default_environment(self): """Test the default_environment property with no environments.""" - # FIXME: Handle unlocked base. + self._check_base_setup() assert self.dnf_manager.default_environment is None - @pytest.mark.skip("Fatal Python error: Aborted") + @pytest.mark.skip("'SwigPyObject' object is not iterable") def test_environments(self): """Test the environments property.""" - # FIXME: Handle unlocked base. + self._check_base_setup() assert self.dnf_manager.environments == [] @@ -886,8 +892,8 @@ def test_load_repository(self): self._add_repository("r1", repo_dir=d) self.dnf_manager.load_repository("r1") - repo = self._get_repository("r1") - assert repo.is_enabled() is True + repo = self._get_repository("r1") + assert repo.is_enabled() is True def test_load_packages_metadata(self): """Test the load_packages_metadata method.""" From 3655f619cd23406f795adbaf9ab2f00f097012cd Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Mon, 15 Apr 2024 08:51:48 +0200 Subject: [PATCH 03/40] Remove the DNFConfigWrapper and access the configuration options directly The dnf5 python bindings were changed to contain shortcuts for the configuration options in dnf5 commit: --- .../payloads/payload/dnf/dnf_manager.py | 46 +++---------------- .../test_module_payload_dnf5_manager.py | 14 ++---- .../test_module_payload_dnf_manager.py | 1 - 3 files changed, 11 insertions(+), 50 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 2328fc7901d..7ef778e09fc 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -100,39 +100,6 @@ class InvalidSelectionError(DNFManagerError): """The software selection couldn't be resolved.""" -class DNFConfigWrapper(): - """This is a temporary wrapper of a DNF config object.""" - - def __init__(self, config): - """Wrap the DNF config object.""" - self._config = config - - def __getattr__(self, name): - """Get the attribute. - - Called when an attribute lookup has not found - the attribute in the usual places. - """ - option = getattr(self._config, name)() - return option.get_value() - - def __setattr__(self, name, value): - """Set the attribute. - - Called when an attribute assignment is attempted. - """ - if name in ["_config"]: - return super().__setattr__(name, value) - - option = getattr(self._config, name)() - option.set(value) - - -def simplify_config(config): - """Simplify the specified DNF config object.""" - return DNFConfigWrapper(config) - - class DNFManager: """The abstraction of the DNF base.""" @@ -175,7 +142,7 @@ def _create_base(cls): base = libdnf5.base.Base() base.load_config_from_file() - config = simplify_config(base.get_config()) + config = base.get_config() config.reposdir = DNF_REPO_DIRS config.cachedir = DNF_CACHE_DIR config.pluginconfpath = DNF_PLUGINCONF_DIR @@ -237,7 +204,7 @@ def configure_base(self, data: PackagesConfigurationData): :param data: a packages configuration data """ - config = simplify_config(self._base.get_config()) + config = self._base.get_config() config.multilib_policy = data.multilib_policy if data.timeout != DNF_DEFAULT_TIMEOUT: @@ -425,7 +392,7 @@ def configure_proxy(self, url): :param url: a proxy URL or None """ - config = simplify_config(self._base.get_config()) + config = self._base.get_config() # Reset the proxy configuration. config.proxy = "" @@ -845,7 +812,7 @@ def _create_repository(self, data: RepoConfigurationData): """ repo_sack = self._base.get_repo_sack() repo = repo_sack.create_repo(data.name) - config = simplify_config(repo.get_config()) + config = repo.get_config() # Disable the repo if requested. if not data.enabled: @@ -1032,7 +999,7 @@ def load_repository(self, repo_id): log.debug("Load metadata for the '%s' repository.", repo_id) repo = self._get_repository(repo_id) - config = simplify_config(repo.get_config()) + config = repo.get_config() url = config.baseurl or config.mirrorlist or config.metalink if not repo.is_enabled(): @@ -1100,8 +1067,7 @@ def _get_repomd_content(self, repo): :param repo: a DNF repo :return: a content of the repomd.xml file """ - config = simplify_config(repo.get_config()) - urls = config.baseurl + urls = repo.get_config().baseurl for url in urls: try: diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index 7a81675a087..ad5b46e3c52 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -37,7 +37,6 @@ from pyanaconda.modules.common.structures.payload import RepoConfigurationData from pyanaconda.modules.payloads.payload.dnf.dnf_manager import ( DNFManager, - simplify_config, MetadataError, ) @@ -97,12 +96,10 @@ def _get_repo(self, base, repo_id): weak_ref = repos.get() return weak_ref.get() - def test_simplify_config(self): - """Test the simplify_config function.""" + def test_config(self): + """Test accessing the dnf config.""" base = libdnf5.base.Base() - config = simplify_config( - base.get_config() - ) + config = base.get_config() config.installroot = "/my/install/root" assert config.installroot == "/my/install/root" @@ -117,7 +114,7 @@ def setUp(self): def _get_configuration(self): """Get the configuration of the DNF base.""" - return simplify_config(self.dnf_manager._base.get_config()) + return self.dnf_manager._base.get_config() def _check_variables(self, **expected_variables): """Check values of the expected substitution variables.""" @@ -463,8 +460,7 @@ def _get_repository(self, repo_id): def _get_configuration(self, repo_id): """Get a configuration of the DNF repository.""" - repo = self._get_repository(repo_id) - return simplify_config(repo.get_config()) + return self._get_repository(repo_id).get_config() def test_repositories(self): """Test the repositories property.""" diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py index bda1f3477df..dc7bf8c8015 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py @@ -58,7 +58,6 @@ InvalidSelectionError, MetadataError, MissingSpecsError, - simplify_config, ) From 9c622f4f84dd2e6c75da9e569f9609a3ed2bdc47 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Mon, 15 Apr 2024 10:21:06 +0200 Subject: [PATCH 04/40] Call get_download_size instead of get_package_size because of rename --- pyanaconda/modules/payloads/payload/dnf/dnf_manager.py | 2 +- .../payloads/payload/test_module_payload_dnf5_manager.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 7ef778e09fc..93b7f9c824a 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -508,7 +508,7 @@ def get_download_size(self): # Calculate the download size. for tspkg in self._transaction.get_transaction_packages(): package = tspkg.get_package() - download_size += package.get_package_size() + download_size += package.get_download_size() # Get the total size. Reserve extra space. total_space = download_size + Size("150 MiB") diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index ad5b46e3c52..f98d3cb8d10 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -275,7 +275,7 @@ def _get_transaction(self, packages=2): for i in range(1, packages+1): # Create a package. pkg = Mock(spec=libdnf5.rpm.Package) - pkg.get_package_size.return_value = 1024 * 1024 * 100 * i + pkg.get_download_size.return_value = 1024 * 1024 * 100 * i pkg.get_install_size.return_value = 1024 * 100 * i pkg.get_files.return_value = ["/file"] * 10 * i From ab3d3819ce0fcf6654152e73e6f8c8d6c2165481 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Mon, 15 Apr 2024 10:08:30 +0200 Subject: [PATCH 05/40] Fix setting releasever in dnf5 base --- pyanaconda/modules/payloads/payload/dnf/dnf_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 93b7f9c824a..304821dc5e4 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -153,7 +153,7 @@ def _create_base(cls): config.skip_if_unavailable = False # Set the default release version. - base.conf.releasever = get_product_release_version() + base.get_vars().set("releasever", get_product_release_version()) # Load variables from the host (rhbz#1920735). base.conf.substitutions.update_from_etc("/") From 516dc18f511b056bff0b33cda8889b2b6fee304e Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Wed, 24 Apr 2024 09:35:30 +0200 Subject: [PATCH 06/40] Call dnf5 base setup at a proper place --- .../payloads/payload/dnf/dnf_manager.py | 22 ++++++---- .../payloads/payload/dnf/initialization.py | 1 + .../payload/test_module_payload_dnf.py | 1 + .../test_module_payload_dnf5_manager.py | 43 +++++-------------- 4 files changed, 25 insertions(+), 42 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 304821dc5e4..bc5f43c33be 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -172,16 +172,22 @@ def _create_base(cls): if platform_id is not None: config.module_platform_id = platform_id - # Load vars and do other initialization based on the - # configuration. The method is supposed to be called - # after configuration is updated, but before repositories - # are loaded or any query created. - # FIXME: Should we do that here? - #base.setup() - log.debug("The DNF base has been created.") return base + def setup_base(self): + """Set up the DNF base. + + Load vars and do other initialization based on the configuration. + + Must be called after configuration and vars are updated, application plugins applied, their + pre configuration modification in configuration, but before repositories are loaded or any + Package r Advisory query created. + + :raise: RuntimeError if called the second time + """ + self._base.setup() + def reset_base(self): """Reset the DNF base. @@ -448,8 +454,6 @@ def substitute(self, text): if not text: return "" - # FIXME: Call base.setup() to set up all variables. - variables = self._base.get_vars() return variables.substitute(text) diff --git a/pyanaconda/modules/payloads/payload/dnf/initialization.py b/pyanaconda/modules/payloads/payload/dnf/initialization.py index ba180343f1f..44958f6366a 100644 --- a/pyanaconda/modules/payloads/payload/dnf/initialization.py +++ b/pyanaconda/modules/payloads/payload/dnf/initialization.py @@ -211,6 +211,7 @@ def _configure_dnf_manager(self): dnf_manager.configure_base(self._configuration) dnf_manager.configure_proxy(self._proxy) dnf_manager.configure_substitution(self._release_version) + dnf_manager.setup_base() dnf_manager.dump_configuration() dnf_manager.read_system_repositories() return dnf_manager diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf.py index 6002f53a8d7..89547d3fd5e 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf.py @@ -692,6 +692,7 @@ def setUp(self): payload=self.module, payload_intf=self.interface ) + self.interface.implementation._dnf_manager.setup_base() def test_type(self): """Test the Type property.""" diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index f98d3cb8d10..036950307b3 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -123,13 +123,6 @@ def _check_variables(self, **expected_variables): for name, value in expected_variables.items(): assert variables.get_value(name) == value - def _check_base_setup(self): - """Make sure that the base is set up.""" - # FIXME: Figure this out on the manager level. - base = self.dnf_manager._base - assert base - base.setup() - def test_create_base(self): """Test the creation of the DNF base.""" assert self.dnf_manager._base is not None @@ -291,6 +284,8 @@ def _get_transaction(self, packages=2): def test_apply_specs(self): """Test the apply_specs method.""" + self.dnf_manager.setup_base() + self.dnf_manager.apply_specs( include_list=["@g1", "p1"], exclude_list=["@g2", "p2"] @@ -301,7 +296,7 @@ def test_apply_specs(self): def test_resolve_no_selection(self): """Test the resolve_selection method with no selection.""" - self._check_base_setup() + self.dnf_manager.setup_base() with self.assertLogs(level="INFO") as cm: report = self.dnf_manager.resolve_selection() @@ -313,7 +308,7 @@ def test_resolve_no_selection(self): def test_resolve_missing_selection(self): """Test the resolve selection method with missing selection.""" - self._check_base_setup() + self.dnf_manager.setup_base() self.dnf_manager.apply_specs( include_list=["@g1", "p1"], @@ -339,7 +334,8 @@ def test_ignore_broken_packages(self): def test_clear_selection(self): """Test the clear_selection method.""" - self._check_base_setup() + self.dnf_manager.setup_base() + self.dnf_manager.resolve_selection() g = self.dnf_manager._goal @@ -351,6 +347,8 @@ def test_clear_selection(self): def test_substitute(self): """Test the substitute method.""" + self.dnf_manager.setup_base() + # No variables. assert self.dnf_manager.substitute(None) == "" assert self.dnf_manager.substitute("") == "" @@ -366,7 +364,6 @@ def test_substitute(self): assert self.dnf_manager.substitute("/$releasever") != "/$releasever" # Supported variables. - self._check_base_setup() assert self.dnf_manager.substitute("/$arch") != "/$arch" assert self.dnf_manager.substitute("/$basearch") != "/$basearch" assert self.dnf_manager.substitute("/$releasever") != "/$releasever" @@ -391,30 +388,21 @@ class DNFManagerCompsTestCase(unittest.TestCase): def setUp(self): self.maxDiff = None self.dnf_manager = DNFManager() - - def _check_base_setup(self): - """Make sure that the base is set up.""" - # FIXME: Figure this out on the manager level. - base = self.dnf_manager._base - assert base - base.setup() + self.dnf_manager.setup_base() @pytest.mark.skip("'SwigPyObject' object is not iterable") def test_groups(self): """Test the groups property.""" - self._check_base_setup() assert self.dnf_manager.groups == [] @pytest.mark.skip("'SwigPyObject' object is not iterable") def test_no_default_environment(self): """Test the default_environment property with no environments.""" - self._check_base_setup() assert self.dnf_manager.default_environment is None @pytest.mark.skip("'SwigPyObject' object is not iterable") def test_environments(self): """Test the environments property.""" - self._check_base_setup() assert self.dnf_manager.environments == [] @@ -424,13 +412,7 @@ class DNFManagerReposTestCase(unittest.TestCase): def setUp(self): self.maxDiff = None self.dnf_manager = DNFManager() - - def _check_base_setup(self): - """Make sure that the base is set up.""" - # FIXME: Figure this out on the manager level. - base = self.dnf_manager._base - assert base - base.setup() + self.dnf_manager.setup_base() def _add_repository(self, repo_id, repo_dir=None, **kwargs): """Add the DNF repository with the specified id.""" @@ -859,7 +841,6 @@ def test_load_repository_unknown(self): def test_load_repository_failed(self): """Test the load_repository method with a failure.""" - self._check_base_setup() self._add_repository("r1") with pytest.raises(MetadataError, match="Failed to download metadata"): @@ -870,8 +851,6 @@ def test_load_repository_failed(self): def test_load_repository_disabled(self): """Test the load_repository method with a disabled repo.""" - self._check_base_setup() - repo = self._add_repository("r1") repo.disable() @@ -882,8 +861,6 @@ def test_load_repository_disabled(self): def test_load_repository(self): """Test the load_repository method.""" - self._check_base_setup() - with TemporaryDirectory() as d: self._add_repository("r1", repo_dir=d) self.dnf_manager.load_repository("r1") From 587798e0746863ad32f55a10adced440fdfc16cd Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Mon, 15 Apr 2024 09:26:02 +0200 Subject: [PATCH 07/40] Load all repositories at once The dnf5 methods `Repo::fetch_metadata` and `Repo::load` were both removed from API. See: rpm-software-management/dnf5/commit/cc2ae250c30bdbd8fadb99a4eb9d817b074dc6b2 rpm-software-management/dnf5/commit/b2ece236b62d121c4e2ddd91272804db17ee7d12 Instead use `RepoSack::load_repos` to load all repositories. This method must be called only once. --- .../payloads/payload/dnf/dnf_manager.py | 48 ++++--------------- .../payloads/payload/dnf/initialization.py | 37 ++++++-------- .../payloads/payload/dnf/repositories.py | 10 +--- .../payloads/payload/test_dnf_repositories.py | 1 - .../test_module_payload_dnf5_manager.py | 45 ++++++----------- 5 files changed, 40 insertions(+), 101 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index bc5f43c33be..1a0d25d46c1 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -988,50 +988,22 @@ def restore_system_repositories(self): except UnknownRepositoryError: log.debug("There is no '%s' repository to enable.", repo_id) - def load_repository(self, repo_id): - """Download repo metadata. + def load_repositories(self): + """Load all enabled repositories. - If the repo is enabled, load its metadata to verify that - the repo is valid. An invalid repo will be disabled. + Load all enabled repositories, including system repositories, and + process their metadata. It will update the cache that provides + information about available packages, modules, groups and environments. - This method will by default not try to refresh already - loaded data if called repeatedly. - - :param str repo_id: an identifier of a repository - :raise: MetadataError if the metadata cannot be loaded + Can be called only once per each RepoSack. """ - log.debug("Load metadata for the '%s' repository.", repo_id) - - repo = self._get_repository(repo_id) - config = repo.get_config() - url = config.baseurl or config.mirrorlist or config.metalink - - if not repo.is_enabled(): - log.debug("Don't load metadata from a disabled repository.") - return - + repo_sack = self._base.get_repo_sack() try: - repo.fetch_metadata() - repo.load() + repo_sack.load_repos(False) except RuntimeError as e: - log.debug("Failed to load metadata from '%s': %s", url, str(e)) - repo.disable() + log.warning(str(e)) raise MetadataError(str(e)) from None - - log.info("Loaded metadata from '%s'.", url) - - def load_packages_metadata(self): - """Load metadata about packages in available repositories. - - Load all enabled repositories and process their metadata. - It will update the cache that provides information about - available packages, modules, groups and environments. - """ - repositories = libdnf5.repo.RepoQuery(self._base) - repositories.filter_enabled(True) - repo_sack = self._base.get_repo_sack() - repo_sack.update_and_load_repos(repositories) - log.info("Loaded packages and group metadata.") + log.info("Loaded repositories.") def load_repomd_hashes(self): """Load a hash of the repomd.xml file for each enabled repository.""" diff --git a/pyanaconda/modules/payloads/payload/dnf/initialization.py b/pyanaconda/modules/payloads/payload/dnf/initialization.py index 44958f6366a..bf6acdfde4b 100644 --- a/pyanaconda/modules/payloads/payload/dnf/initialization.py +++ b/pyanaconda/modules/payloads/payload/dnf/initialization.py @@ -132,12 +132,15 @@ def run(self): # Load additional sources. self._load_additional_sources(dnf_manager, sources, repositories) - # Validate enabled repositories. - self._validate_repositories(dnf_manager) + # Check there are enabled repositories. + self._check_enabled_repositories(dnf_manager) - # Load package and group metadata. - self.report_progress(_("Downloading group metadata...")) - self._load_metadata(dnf_manager) + # Load repositories. + self.report_progress(_("Loading repositories...")) + try: + self._load_repositories(dnf_manager) + except MetadataError as e: + raise SourceSetupError(str(e)) from None return SetUpDNFSourcesResult( dnf_manager=dnf_manager, @@ -254,35 +257,23 @@ def _load_additional_sources(self, dnf_manager, sources, repositories): enable_existing_repository(dnf_manager, repository) @staticmethod - def _validate_repositories(dnf_manager): - """Validate all enabled repositories. - - Collect error messages about invalid repositories. - All invalid repositories are disabled. - - The user repositories are validated when we add them - to DNF, so this covers invalid system repositories. + def _check_enabled_repositories(dnf_manager): + """Check there is at least one enabled repository. :param DNFManager dnf_manager: a configured DNF manager """ - # Check if there is at least one enabled repository. if not dnf_manager.enabled_repositories: raise SourceSetupError(_("No repository is configured.")) - # Load all enabled repositories and report warnings if any. - for repo_id in dnf_manager.enabled_repositories: - try: - dnf_manager.load_repository(repo_id) - except MetadataError as e: - log.warning(str(e)) - @staticmethod - def _load_metadata(dnf_manager): + def _load_repositories(dnf_manager): """Load metadata of configured repositories. + Can be called only once per each RepoSack. + :param DNFManager dnf_manager: a configured DNF manager """ - dnf_manager.load_packages_metadata() + dnf_manager.load_repositories() dnf_manager.load_repomd_hashes() diff --git a/pyanaconda/modules/payloads/payload/dnf/repositories.py b/pyanaconda/modules/payloads/payload/dnf/repositories.py index 56b9020f63d..f79e0a99720 100644 --- a/pyanaconda/modules/payloads/payload/dnf/repositories.py +++ b/pyanaconda/modules/payloads/payload/dnf/repositories.py @@ -33,7 +33,6 @@ ) from pyanaconda.modules.common.structures.payload import RepoConfigurationData from pyanaconda.modules.payloads.constants import SourceType -from pyanaconda.modules.payloads.payload.dnf.dnf_manager import MetadataError from pyanaconda.modules.payloads.source.factory import SourceFactory log = get_module_logger(__name__) @@ -213,18 +212,13 @@ def enable_matching_repositories(dnf_manager, patterns, enabled=True): def create_repository(dnf_manager, repository): - """Create a new repository and check its validity. + """Create a new repository. :param DNFManager dnf_manager: a configured DNF manager :param RepoConfigurationData repository: a resolved repository data """ log.debug("Add the '%s' repository (%s).", repository.name, repository) - try: - dnf_manager.add_repository(repository) - dnf_manager.load_repository(repository.name) - except MetadataError as e: - msg = _("Failed to add the '{name}' repository: {details}") - raise SourceSetupError(msg.format(name=repository.name, details=str(e))) from None + dnf_manager.add_repository(repository) def enable_existing_repository(dnf_manager, repository): diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_repositories.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_repositories.py index 1d5c0c226d0..e16ad083ccd 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_repositories.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_repositories.py @@ -312,7 +312,6 @@ def test_create_repository(self): create_repository(dnf_manager, repository) dnf_manager.add_repository.assert_called_once_with(repository) - dnf_manager.load_repository.assert_called_once_with("test") def test_enable_existing_repository(self): """Test the enable_existing_repository function.""" diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index 036950307b3..296722466bf 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -80,15 +80,13 @@ def test_disable_failed_repository(self): sack.create_repo("r1") base.setup() + # First check that load_repos fails (because of missing baseurl of the r1 repo) + with pytest.raises(RuntimeError): + sack.load_repos() + # When the repo is disabled, load_repos succeeds repo = self._get_repo(base, "r1") - - try: - repo.fetch_metadata() - repo.load() - except RuntimeError: - print("Print something") - # It will crash here. - repo.disable() + repo.disable() + sack.load_repos() def _get_repo(self, base, repo_id): repos = libdnf5.repo.RepoQuery(base) @@ -832,47 +830,32 @@ def test_restore_system_repositories(self): self.dnf_manager._enabled_system_repositories.append("r1") self.dnf_manager.restore_system_repositories() - def test_load_repository_unknown(self): - """Test the load_repository method with an unknown repo.""" - self._check_base_setup() - - with pytest.raises(UnknownRepositoryError): - self.dnf_manager.load_repository("r1") - def test_load_repository_failed(self): - """Test the load_repository method with a failure.""" + """Test the load_repositories method with a failure.""" self._add_repository("r1") with pytest.raises(MetadataError, match="Failed to download metadata"): - self.dnf_manager.load_repository("r1") + self.dnf_manager.load_repositories() - repo = self._get_repository("r1") - assert repo.is_enabled() is False - - def test_load_repository_disabled(self): - """Test the load_repository method with a disabled repo.""" + def test_load_repositories_disabled(self): + """Test the load_repositories method with a disabled repo.""" repo = self._add_repository("r1") repo.disable() - self.dnf_manager.load_repository("r1") + self.dnf_manager.load_repositories() repo = self._get_repository("r1") assert repo.is_enabled() is False - def test_load_repository(self): - """Test the load_repository method.""" + def test_load_repositories(self): + """Test the load_repositories method.""" with TemporaryDirectory() as d: self._add_repository("r1", repo_dir=d) - self.dnf_manager.load_repository("r1") + self.dnf_manager.load_repositories() repo = self._get_repository("r1") assert repo.is_enabled() is True - def test_load_packages_metadata(self): - """Test the load_packages_metadata method.""" - self._check_base_setup() - self.dnf_manager.load_packages_metadata() - def test_load_no_repomd_hashes(self): """Test the load_repomd_hashes method with no repositories.""" self.dnf_manager.load_repomd_hashes() From 05488880bc812e7975458c576c3bf0fd1d25ea65 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Tue, 7 May 2024 09:40:21 +0200 Subject: [PATCH 08/40] Adapt test_dnf_initialization.py tests for dnf5 --- .../payloads/payload/test_dnf_initialization.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_initialization.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_initialization.py index 21744571433..b3bed62c49b 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_initialization.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_initialization.py @@ -154,7 +154,7 @@ def test_source_proxy(self): dnf_manager = result.dnf_manager # The DNF manager should use the proxy of the source. - assert dnf_manager._base.conf.proxy == "http://proxy:3128" + assert dnf_manager._base.get_config().proxy == "http://proxy:3128" def test_invalid_source(self): """Set up an invalid source.""" @@ -170,8 +170,8 @@ def test_invalid_source(self): with pytest.raises(SourceSetupError) as cm: self._run_task(source) - msg = "Failed to add the 'anaconda' repository:" - assert str(cm.value).startswith(msg) + assert str(cm.value).startswith("Failed to download metadata") + assert str(cm.value).endswith("for repository \"anaconda\"") def test_valid_repository(self): """Set up a valid additional repository.""" @@ -208,8 +208,8 @@ def test_invalid_repository(self): with pytest.raises(SourceSetupError) as cm: self._run_task(source, [repository]) - msg = "Failed to add the 'test' repository:" - assert str(cm.value).startswith(msg) + assert str(cm.value).startswith("Failed to download metadata") + assert str(cm.value).endswith("for repository \"test\"") def test_system_repository(self): """Set up a system repository.""" @@ -262,16 +262,17 @@ def test_treeinfo_repositories(self): dnf_manager = result.dnf_manager # The treeinfo release version is used. - assert dnf_manager._base.conf.releasever == "8.5" + assert dnf_manager._base.get_vars().get_value("releasever") == "8.5" # The treeinfo repositories are configured. assert dnf_manager.enabled_repositories == [ - "anaconda", "AppStream" + "AppStream", "anaconda" ] # The treeinfo base repository is configured. repo_object = dnf_manager._get_repository("anaconda") - assert repo_object.baseurl == ["file://{}/baseos".format(path)] + repo_config = repo_object.get_config() + assert repo_config.baseurl == ("file://{}/baseos".format(path),) # Check the generated treeinfo repository. repository = RepoConfigurationData() From 74d4788bd1d6fe873c520d759a5cb46f6711b510 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Tue, 7 May 2024 14:33:43 +0200 Subject: [PATCH 09/40] Adapt test_module_payload_dnf_manager download tests for dnf5 --- .../payloads/payload/dnf/dnf_manager.py | 2 +- .../test_module_payload_dnf5_manager.py | 103 +++++++++++++++++- .../test_module_payload_dnf_manager.py | 86 --------------- 3 files changed, 103 insertions(+), 88 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 1a0d25d46c1..d9dd033eefe 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -644,7 +644,7 @@ def download_packages(self, callback): self._set_download_callbacks(progress) # Prepare packages for download. - downloader = libdnf5.repo.PackageDownloader() + downloader = libdnf5.repo.PackageDownloader(self._base) packages = self._get_download_packages() destination = self.download_location diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index 296722466bf..daf57aa3863 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -20,7 +20,7 @@ import unittest from tempfile import TemporaryDirectory from textwrap import dedent -from unittest.mock import patch, Mock +from unittest.mock import Mock, call, patch import libdnf5 import pytest @@ -380,6 +380,107 @@ def test_reset_substitution(self): self._check_variables(releasever="rawhide") + # For this test, mocked Transaction is needed, but it can't be easily + # created, because it doesn't have a public constructor, it's supposed + # to be taken from resolved Goal. + @pytest.mark.skip("There is no transaction to use") + @patch("dnf.base.Base.download_packages") + @patch("dnf.base.Base.transaction") + def test_download_packages(self, transaction, download_packages): + """Test the download_packages method.""" + self.dnf_manager.setup_base() + + callback = Mock() + transaction.install_set = ["p1", "p2", "p3"] + download_packages.side_effect = self._download_packages + + self.dnf_manager.download_packages(callback) + + callback.assert_has_calls([ + call('Downloading 3 RPMs, 25 B / 300 B (8%) done.'), + call('Downloading 3 RPMs, 75 B / 300 B (25%) done.'), + call('Downloading 3 RPMs, 100 B / 300 B (33%) done.'), + call('Downloading 3 RPMs, 125 B / 300 B (41%) done.'), + call('Downloading 3 RPMs, 175 B / 300 B (58%) done.'), + call('Downloading 3 RPMs, 200 B / 300 B (66%) done.'), + call('Downloading 3 RPMs, 225 B / 300 B (75%) done.'), + call('Downloading 3 RPMs, 275 B / 300 B (91%) done.'), + call('Downloading 3 RPMs, 300 B / 300 B (100%) done.') + ]) + + def _download_packages(self, packages, progress): + """Simulate the download of packages.""" + progress.start(total_files=3, total_size=300) + + for name in packages: + payload = Mock() + payload.__str__ = Mock(return_value=name) + payload.download_size = 100 + + progress.last_time = 0 + progress.progress(payload, 25) + + progress.last_time += 3600 + progress.progress(payload, 50) + + progress.last_time = 0 + progress.progress(payload, 75) + + progress.last_time = 0 + progress.end( + payload, libdnf5.repo.DownloadCallbacks.TransferStatus_SUCCESSFUL, "Message!" + ) + + assert progress.downloads == { + "p1": 100, + "p2": 100, + "p3": 100 + } + + # For this test, mocked Transaction is needed, but it can't be easily + # created, because it doesn't have a public constructor, it's supposed + # to be taken from resolved Goal. + @pytest.mark.skip("There is no transaction to use") + @patch("dnf.base.Base.download_packages") + @patch("dnf.base.Base.transaction") + def test_download_packages_failed(self, transaction, download_packages): + """Test the download_packages method with failed packages.""" + self.dnf_manager.setup_base() + + callback = Mock() + transaction.install_set = ["p1", "p2", "p3"] + download_packages.side_effect = self._download_packages_failed + + self.dnf_manager.download_packages(callback) + + callback.assert_has_calls([ + call('Downloading 3 RPMs, 25 B / 300 B (8%) done.'), + call('Downloading 3 RPMs, 50 B / 300 B (16%) done.'), + call('Downloading 3 RPMs, 75 B / 300 B (25%) done.') + ]) + + def _download_packages_failed(self, packages, progress): + """Simulate the failed download of packages.""" + progress.start(total_files=3, total_size=300) + + for name in packages: + payload = Mock() + payload.__str__ = Mock(return_value=name) + payload.download_size = 100 + + progress.last_time = 0 + progress.progress(payload, 25) + + progress.last_time = 0 + progress.end(payload, libdnf5.repo.DownloadCallbacks.TransferStatus_ERROR, "Message!") + + assert progress.downloads == { + "p1": 25, + "p2": 25, + "p3": 25 + } + + class DNFManagerCompsTestCase(unittest.TestCase): """Test the comps abstraction of the DNF base.""" diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py index dc7bf8c8015..af69f05a6d7 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py @@ -63,92 +63,6 @@ class DNFManagerTestCase(unittest.TestCase): - @patch("dnf.base.Base.download_packages") - @patch("dnf.base.Base.transaction") - def test_download_packages(self, transaction, download_packages): - """Test the download_packages method.""" - callback = Mock() - transaction.install_set = ["p1", "p2", "p3"] - download_packages.side_effect = self._download_packages - - self.dnf_manager.download_packages(callback) - - callback.assert_has_calls([ - call('Downloading 3 RPMs, 25 B / 300 B (8%) done.'), - call('Downloading 3 RPMs, 75 B / 300 B (25%) done.'), - call('Downloading 3 RPMs, 100 B / 300 B (33%) done.'), - call('Downloading 3 RPMs, 125 B / 300 B (41%) done.'), - call('Downloading 3 RPMs, 175 B / 300 B (58%) done.'), - call('Downloading 3 RPMs, 200 B / 300 B (66%) done.'), - call('Downloading 3 RPMs, 225 B / 300 B (75%) done.'), - call('Downloading 3 RPMs, 275 B / 300 B (91%) done.'), - call('Downloading 3 RPMs, 300 B / 300 B (100%) done.') - ]) - - def _download_packages(self, packages, progress): - """Simulate the download of packages.""" - progress.start(total_files=3, total_size=300) - - for name in packages: - payload = Mock() - payload.__str__ = Mock(return_value=name) - payload.download_size = 100 - - progress.last_time = 0 - progress.progress(payload, 25) - - progress.last_time += 3600 - progress.progress(payload, 50) - - progress.last_time = 0 - progress.progress(payload, 75) - - progress.last_time = 0 - progress.end(payload, STATUS_OK, "Message!") - - assert progress.downloads == { - "p1": 100, - "p2": 100, - "p3": 100 - } - - @patch("dnf.base.Base.download_packages") - @patch("dnf.base.Base.transaction") - def test_download_packages_failed(self, transaction, download_packages): - """Test the download_packages method with failed packages.""" - callback = Mock() - transaction.install_set = ["p1", "p2", "p3"] - download_packages.side_effect = self._download_packages_failed - - self.dnf_manager.download_packages(callback) - - callback.assert_has_calls([ - call('Downloading 3 RPMs, 25 B / 300 B (8%) done.'), - call('Downloading 3 RPMs, 50 B / 300 B (16%) done.'), - call('Downloading 3 RPMs, 75 B / 300 B (25%) done.') - ]) - - def _download_packages_failed(self, packages, progress): - """Simulate the failed download of packages.""" - progress.start(total_files=3, total_size=300) - - for name in packages: - payload = Mock() - payload.__str__ = Mock(return_value=name) - payload.download_size = 100 - - progress.last_time = 0 - progress.progress(payload, 25) - - progress.last_time = 0 - progress.end(payload, STATUS_FAILED, "Message!") - - assert progress.downloads == { - "p1": 25, - "p2": 25, - "p3": 25 - } - @patch("dnf.base.Base.do_transaction") def test_install_packages(self, do_transaction): """Test the install_packages method.""" From c8a75fe41fee711882502376216fc54edb91e2cb Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Thu, 9 May 2024 12:41:13 +0200 Subject: [PATCH 10/40] Remove test for reset_substitution method The method was removed by commit: 0ff5e44d3f5bca32ef18694249531f4ab4fcd62a --- .../payloads/payload/test_module_payload_dnf5_manager.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index daf57aa3863..c40634ecced 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -371,15 +371,6 @@ def test_configure_substitution(self): self.dnf_manager.configure_substitution(release_version="35") self._check_variables(releasever="35") - def test_reset_substitution(self): - """Test the reset_substitution method.""" - self.dnf_manager.configure_substitution(release_version="35") - self._check_variables(releasever="35") - - self.dnf_manager.reset_substitution() - self._check_variables(releasever="rawhide") - - # For this test, mocked Transaction is needed, but it can't be easily # created, because it doesn't have a public constructor, it's supposed # to be taken from resolved Goal. From f00139a094d7a0aa360afa524abaad2e7ffe2c55 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Thu, 9 May 2024 12:43:48 +0200 Subject: [PATCH 11/40] Remove variable loading, since it's done in base.setup() The test is adjusted because the base.setup() was already called. This might need to change if the place of calling base.setup() changes. --- pyanaconda/modules/payloads/payload/dnf/dnf_manager.py | 2 +- .../payloads/payload/test_module_payload_dnf5_manager.py | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index d9dd033eefe..4c931bf1ca0 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -156,7 +156,7 @@ def _create_base(cls): base.get_vars().set("releasever", get_product_release_version()) # Load variables from the host (rhbz#1920735). - base.conf.substitutions.update_from_etc("/") + # Vars are now loaded during base.setup() # Set the installation root. config.installroot = conf.target.system_root diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index c40634ecced..7b1ef5310e2 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -356,11 +356,6 @@ def test_substitute(self): # Unknown variables. assert self.dnf_manager.substitute("/$unknown") == "/$unknown" - # FIXME: Some variables are defined by base.setup(). - assert self.dnf_manager.substitute("/$arch") == "/$arch" - assert self.dnf_manager.substitute("/$basearch") == "/$basearch" - assert self.dnf_manager.substitute("/$releasever") != "/$releasever" - # Supported variables. assert self.dnf_manager.substitute("/$arch") != "/$arch" assert self.dnf_manager.substitute("/$basearch") != "/$basearch" From 3d8fa9e1fa85cfc86048348b448f1b68f1cbf72b Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Tue, 14 May 2024 13:22:42 +0200 Subject: [PATCH 12/40] Replace `base.load_config_from_file` with `base.load_config` method The method was removed by commit: https://github.com/rpm-software-management/dnf5/commit/bfb6f32e15c359011e078c81b78d160ef10708dd --- pyanaconda/modules/payloads/payload/dnf/dnf_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 4c931bf1ca0..026ff39536a 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -140,7 +140,7 @@ def _goal(self): def _create_base(cls): """Create a new DNF base.""" base = libdnf5.base.Base() - base.load_config_from_file() + base.load_config() config = base.get_config() config.reposdir = DNF_REPO_DIRS From 32c0e9361fed29f06ee0ab1da70db3c22cef8ab3 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Thu, 23 May 2024 10:52:33 +0200 Subject: [PATCH 13/40] Fix use of group and environment iterators --- pyanaconda/modules/payloads/payload/dnf/dnf_manager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 026ff39536a..d9c62623932 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -261,7 +261,7 @@ def environments(self): :return: a list of ids """ environments = libdnf5.comps.EnvironmentQuery(self._base) - return [env.get_environmentid() for env in environments.list()] + return [env.get_environmentid() for env in environments] def _get_environment(self, environment_name): """Translate the given environment name to a DNF object. @@ -274,7 +274,7 @@ def _get_environment(self, environment_name): environments = libdnf5.comps.EnvironmentQuery(self._base) environments.filter_name(environment_name) - return next(iter(environments.list()), None) + return next(iter(environments), None) def resolve_environment(self, environment_name): """Translate the given environment name to a group ID. @@ -341,7 +341,7 @@ def groups(self): :return: a list of IDs """ groups = libdnf5.comps.GroupQuery(self._base) - return [g.get_groupid() for g in groups.list()] + return [g.get_groupid() for g in groups] def _get_group(self, group_name): """Translate the given group name into a DNF object. @@ -351,7 +351,7 @@ def _get_group(self, group_name): """ groups = libdnf5.comps.GroupQuery(self._base) groups.filter_name(group_name) - return next(iter(groups.list()), None) + return next(iter(groups), None) def resolve_group(self, group_name): """Translate the given group name into a group ID. From 338615c328f5478eaebbf0fc67eb884106855b18 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Thu, 23 May 2024 14:40:45 +0200 Subject: [PATCH 14/40] TEMPORARY: Disable testing messages from resolving package selections There was a change regarding creation of the `ValidationReport`. It's no longer created in `CheckPackagesSelectionTask._resolve_selection()` but in `DNFManager.resolve_selection()`. The change lookgs good and it was probably even necessary because of changes in dnf reports from the transaction resolving, but it prevents testing the report messages, because the report is now also mocked. Since it's not a critical problem, I am disabling the message checks for now. The changes that caused the issue: https://github.com/poncovka/anaconda/commit/faccc47ff21fae1eb4abe0b1d1da2d5f8033f451#diff-08489d103d5780dd5279cb390820b5eeb83963642783cd82773cef081803b1b7L620-L641 https://github.com/poncovka/anaconda/commit/faccc47ff21fae1eb4abe0b1d1da2d5f8033f451#diff-57dffd56427be4eca900ee75e0a130414fb51527d206b00116ef9501f0784127L96-L114 --- .../payloads/payload/test_dnf_validation.py | 69 ++++++++++++++----- 1 file changed, 52 insertions(+), 17 deletions(-) diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_validation.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_validation.py index 19d7c6b9033..4def6a83a02 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_validation.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_validation.py @@ -19,11 +19,13 @@ from unittest.mock import Mock, patch from pyanaconda.modules.common.structures.packages import PackagesSelectionData -from pyanaconda.modules.payloads.payload.dnf.dnf_manager import ( - DNFManager, - InvalidSelectionError, - MissingSpecsError, -) + +# from pyanaconda.modules.payloads.payload.dnf.dnf_manager import ( +# DNFManager, +# InvalidSelectionError, +# MissingSpecsError, +# ) +from pyanaconda.modules.payloads.payload.dnf.dnf_manager import DNFManager from pyanaconda.modules.payloads.payload.dnf.validation import ( CheckPackagesSelectionTask, VerifyRepomdHashesTask, @@ -45,12 +47,21 @@ def test_check_no_selection(self, kernel_getter): selection.core_group_enabled = False task = CheckPackagesSelectionTask(dnf_manager, selection) - report = task.run() + task.run() + # FIXME: The `ValidationReport` object used to be created in + # `CheckPackagesSelectionTask._resolve_selection()`, but now it's in + # `DNFManager.resolve_selection()`. And since the `DNFManager` is mocked, it + # produces only mocked report that cannot be checked for messages. + # report = task.run() dnf_manager.clear_selection.assert_called_once_with() dnf_manager.apply_specs.assert_called_once_with([], ["@core"]) dnf_manager.resolve_selection.assert_called_once_with() - assert report.get_messages() == [] + # FIXME: The `ValidationReport` object used to be created in + # `CheckPackagesSelectionTask._resolve_selection()`, but now it's in + # `DNFManager.resolve_selection()`. And since the `DNFManager` is mocked, it + # produces only mocked report that cannot be checked for messages. + # assert report.get_messages() == [] @patch("pyanaconda.modules.payloads.payload.dnf.validation.get_kernel_package") def test_check_default_selection(self, kernel_getter): @@ -64,14 +75,23 @@ def test_check_default_selection(self, kernel_getter): selection.core_group_enabled = True task = CheckPackagesSelectionTask(dnf_manager, selection) - report = task.run() + task.run() + # FIXME: The `ValidationReport` object used to be created in + # `CheckPackagesSelectionTask._resolve_selection()`, but now it's in + # `DNFManager.resolve_selection()`. And since the `DNFManager` is mocked, it + # produces only mocked report that cannot be checked for messages. + # report = task.run() dnf_manager.clear_selection.assert_called_once_with() dnf_manager.apply_specs.assert_called_once_with( ["@environment", "@core", "kernel"], [] ) dnf_manager.resolve_selection.assert_called_once_with() - assert report.get_messages() == [] + # FIXME: The `ValidationReport` object used to be created in + # `CheckPackagesSelectionTask._resolve_selection()`, but now it's in + # `DNFManager.resolve_selection()`. And since the `DNFManager` is mocked, it + # produces only mocked report that cannot be checked for messages. + # assert report.get_messages() == [] @patch("pyanaconda.modules.payloads.payload.dnf.validation.get_kernel_package") def test_check_selection(self, kernel_getter): @@ -92,7 +112,12 @@ def test_check_selection(self, kernel_getter): selection.disabled_modules = ["m3", "m4"] task = CheckPackagesSelectionTask(dnf_manager, selection) - report = task.run() + task.run() + # FIXME: The `ValidationReport` object used to be created in + # `CheckPackagesSelectionTask._resolve_selection()`, but now it's in + # `DNFManager.resolve_selection()`. And since the `DNFManager` is mocked, it + # produces only mocked report that cannot be checked for messages. + # report = task.run() dnf_manager.clear_selection.assert_called_once_with() dnf_manager.apply_specs.assert_called_once_with( @@ -100,7 +125,11 @@ def test_check_selection(self, kernel_getter): ["@core", "@g3", "@g4", "p3", "p4"] ) dnf_manager.resolve_selection.assert_called_once_with() - assert report.get_messages() == [] + # FIXME: The `ValidationReport` object used to be created in + # `CheckPackagesSelectionTask._resolve_selection()`, but now it's in + # `DNFManager.resolve_selection()`. And since the `DNFManager` is mocked, it + # produces only mocked report that cannot be checked for messages. + # assert report.get_messages() == [] @patch("pyanaconda.modules.payloads.payload.dnf.validation.get_kernel_package") def test_check_invalid_selection(self, kernel_getter): @@ -108,14 +137,20 @@ def test_check_invalid_selection(self, kernel_getter): selection = PackagesSelectionData() dnf_manager = Mock() - dnf_manager.apply_specs.side_effect = MissingSpecsError("e3") - dnf_manager.resolve_selection.side_effect = InvalidSelectionError("e4") + # FIXME: The `CheckPackagesSelectionTask._resolve_selection()` no longer catches the + # errors. They are instead processed and logged directly in the mocked `DNFManager`. + # dnf_manager.apply_specs.side_effect = MissingSpecsError("e3") + # dnf_manager.resolve_selection.side_effect = InvalidSelectionError("e4") task = CheckPackagesSelectionTask(dnf_manager, selection) - report = task.run() - - assert report.error_messages == ["e4"] - assert report.warning_messages == ["e3"] + task.run() + # FIXME: The `ValidationReport` object used to be created in + # `CheckPackagesSelectionTask._resolve_selection()`, but now it's in + # `DNFManager.resolve_selection()`. And since the `DNFManager` is mocked, it + # produces only mocked report that cannot be checked for messages. + # report = task.run() + # assert report.error_messages == ["e4"] + # assert report.warning_messages == ["e3"] class VerifyRepomdHashesTaskTestCase(unittest.TestCase): From 2de138a66df72462835ba3c18619759d3157c160 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Wed, 12 Jun 2024 14:54:14 +0200 Subject: [PATCH 15/40] Update TransactionItemState_ERROR import to libdnf5 --- pyanaconda/modules/payloads/payload/dnf/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/utils.py b/pyanaconda/modules/payloads/payload/dnf/utils.py index f0b765de57b..e7ab8fa5b2b 100644 --- a/pyanaconda/modules/payloads/payload/dnf/utils.py +++ b/pyanaconda/modules/payloads/payload/dnf/utils.py @@ -21,7 +21,7 @@ import rpm from blivet.size import Size -from libdnf.transaction import TransactionItemState_ERROR +from libdnf5.transaction import TransactionItemState_ERROR from pyanaconda.anaconda_loggers import get_module_logger from pyanaconda.core.configuration.anaconda import conf From e7e7734df371350126485a13fe2957d4e7ee23dc Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Wed, 12 Jun 2024 15:04:14 +0200 Subject: [PATCH 16/40] Remove unused _create_group method from DNFRequirementsTestCase --- .../payload/test_module_payload_dnf_requirements.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_requirements.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_requirements.py index 74c88747c38..e398a071335 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_requirements.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_requirements.py @@ -42,12 +42,6 @@ class DNFRequirementsTestCase(unittest.TestCase): - def _create_group(self, name): - """Create a mocked group object.""" - group = Mock() - group.id = name - return group - def _create_requirement(self, name, reason, req_type=REQUIREMENT_TYPE_PACKAGE): """Create a new requirement.""" requirement = Requirement() From a0079437f2a20949a9aecf302275eb1f9391a6ef Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Wed, 19 Jun 2024 09:54:50 +0200 Subject: [PATCH 17/40] TEMPORARY: Store comps queries The comps objects are owned by the queries, so when a query is destroyed, so are the contained objects. There is a bug for dnf: https://github.com/rpm-software-management/dnf5/issues/1530 This is a temporary workaround. --- .../payloads/payload/dnf/dnf_manager.py | 42 ++++++++++++++----- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index d9c62623932..82a7d9e736d 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -119,6 +119,8 @@ def __init__(self): self._download_location = None self._md_hashes = {} self._enabled_system_repositories = [] + self._query_environments = None + self._query_groups = None @property def _base(self): @@ -254,14 +256,19 @@ def default_environment(self): return None + @property + def _environments(self): + if self._query_environments is None: + self._query_environments = libdnf5.comps.EnvironmentQuery(self._base) + return self._query_environments + @property def environments(self): """Environments defined in comps.xml file. :return: a list of ids """ - environments = libdnf5.comps.EnvironmentQuery(self._base) - return [env.get_environmentid() for env in environments] + return [env.get_environmentid() for env in self._environments] def _get_environment(self, environment_name): """Translate the given environment name to a DNF object. @@ -272,9 +279,13 @@ def _get_environment(self, environment_name): if not environment_name: return None - environments = libdnf5.comps.EnvironmentQuery(self._base) - environments.filter_name(environment_name) - return next(iter(environments), None) + for env in self._environments: + if environment_name == env.get_name(): + return env + if environment_name == env.get_environmentid(): + return env + + return None def resolve_environment(self, environment_name): """Translate the given environment name to a group ID. @@ -314,7 +325,7 @@ def _get_environment_data(self, env) -> CompsEnvironmentData: data.name = env.get_translated_name() or "" data.description = env.get_translated_description() or "" - available_groups = libdnf5.comps.GroupQuery(self._base) + available_groups = self._groups optional_groups = set(env.get_optional_groups()) for group in available_groups: @@ -334,14 +345,19 @@ def _get_environment_data(self, env) -> CompsEnvironmentData: return data + @property + def _groups(self): + if self._query_groups is None: + self._query_groups = libdnf5.comps.GroupQuery(self._base) + return self._query_groups + @property def groups(self): """Groups defined in comps.xml file. :return: a list of IDs """ - groups = libdnf5.comps.GroupQuery(self._base) - return [g.get_groupid() for g in groups] + return [g.get_groupid() for g in self._groups] def _get_group(self, group_name): """Translate the given group name into a DNF object. @@ -349,9 +365,13 @@ def _get_group(self, group_name): :param group_name: an identifier of a group :return libdnf5.comps.Group: a DNF object or None """ - groups = libdnf5.comps.GroupQuery(self._base) - groups.filter_name(group_name) - return next(iter(groups), None) + for group in self._groups: + if group_name == group.get_name(): + return group + if group_name == group.get_groupid(): + return group + + return None def resolve_group(self, group_name): """Translate the given group name into a group ID. From 44e5a8a0dfd4167ca3109d844e7826b238818e31 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Wed, 19 Jun 2024 10:01:13 +0200 Subject: [PATCH 18/40] Fix setting of libdnf5.repo.PackageDownloader --- pyanaconda/modules/payloads/payload/dnf/dnf_manager.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 82a7d9e736d..2d5b1d2ae6e 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -669,13 +669,16 @@ def download_packages(self, callback): destination = self.download_location for package in packages: - downloader.add(package, destination=destination) + downloader.add(package, destination) + + downloader.set_fail_fast(True) + downloader.set_resume(False) # Download the packages. log.info("Downloading packages to %s.", destination) try: - downloader.download(fail_fast=True, resume=False) + downloader.download() except RuntimeError as e: msg = "Failed to download the following packages: " + str(e) raise PayloadInstallationError(msg) from None From 863606b2ef87c09fa197fe0b32341cf11a622a13 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Wed, 19 Jun 2024 10:02:24 +0200 Subject: [PATCH 19/40] Enable comps tests --- .../payloads/payload/test_module_payload_dnf5_manager.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index 7b1ef5310e2..b48bbb00760 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -475,17 +475,14 @@ def setUp(self): self.dnf_manager = DNFManager() self.dnf_manager.setup_base() - @pytest.mark.skip("'SwigPyObject' object is not iterable") def test_groups(self): """Test the groups property.""" assert self.dnf_manager.groups == [] - @pytest.mark.skip("'SwigPyObject' object is not iterable") def test_no_default_environment(self): """Test the default_environment property with no environments.""" assert self.dnf_manager.default_environment is None - @pytest.mark.skip("'SwigPyObject' object is not iterable") def test_environments(self): """Test the environments property.""" assert self.dnf_manager.environments == [] From ca9e024814c1ed9d96834a8df6733a430550dbb7 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Fri, 21 Jun 2024 10:24:45 +0200 Subject: [PATCH 20/40] Adapt checking transaction errors to DNF5 --- pyanaconda/modules/payloads/payload/dnf/utils.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/utils.py b/pyanaconda/modules/payloads/payload/dnf/utils.py index e7ab8fa5b2b..e66e69429b9 100644 --- a/pyanaconda/modules/payloads/payload/dnf/utils.py +++ b/pyanaconda/modules/payloads/payload/dnf/utils.py @@ -421,8 +421,16 @@ def transaction_has_errors(transaction): :return: True if the transaction has any error, otherwise False """ has_errors = False - for tsi in transaction: - if tsi.state == TransactionItemState_ERROR: - log.error("The transaction contains item %s in error state.", tsi) + for environment in transaction.get_transaction_environments(): + if environment.get_state() == TransactionItemState_ERROR: + log.error("The transaction contains environment %s in error state.", environment) + has_errors = True + for group in transaction.get_transaction_groups(): + if group.get_state() == TransactionItemState_ERROR: + log.error("The transaction contains group %s in error state.", group) + has_errors = True + for package in transaction.get_transaction_packages(): + if package.get_state() == TransactionItemState_ERROR: + log.error("The transaction contains package %s in error state.", package) has_errors = True return has_errors From 5ef5c8371175816e1c6596de44e330a84ebca747 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Mon, 24 Jun 2024 13:35:30 +0200 Subject: [PATCH 21/40] Adapt test_module_payload_dnf_installation for dnf5 The ValidationReport is now created within the mocked DNFManager.resolve_selection, so it must be mocked as well. Also, there are no longer exceptions raised from the DNF transaction, instead, the problems are stored in a report, so there are no exceptions to mock. --- .../test_module_payload_dnf_installation.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_installation.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_installation.py index f5931f2c23f..f412280819c 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_installation.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_installation.py @@ -34,11 +34,8 @@ ) from pyanaconda.modules.common.structures.payload import RepoConfigurationData from pyanaconda.modules.common.structures.requirement import Requirement -from pyanaconda.modules.payloads.payload.dnf.dnf_manager import ( - DNFManager, - InvalidSelectionError, - MissingSpecsError, -) +from pyanaconda.modules.common.structures.validation import ValidationReport +from pyanaconda.modules.payloads.payload.dnf.dnf_manager import DNFManager from pyanaconda.modules.payloads.payload.dnf.installation import ( CleanUpDownloadLocationTask, DownloadPackagesTask, @@ -389,6 +386,7 @@ def test_resolve(self, kernel_getter, req_getter1, req_getter2, req_getter3, req dnf_manager = Mock() dnf_manager.default_environment = None data = PackagesConfigurationData() + dnf_manager.resolve_selection.return_value = ValidationReport() task = ResolvePackagesTask(dnf_manager, selection, data) task.run() @@ -397,7 +395,7 @@ def test_resolve(self, kernel_getter, req_getter1, req_getter2, req_getter3, req dnf_manager.apply_specs.assert_called_once_with( ["@core", "@r1", "@r2", "r4", "r5"], ["@r3", "r6"] ) - dnf_manager.resolve_selection.assert_called_once_with() + dnf_manager.resolve_selection.assert_called_once() @patch("pyanaconda.modules.payloads.payload.dnf.installation.collect_driver_disk_requirements") @patch("pyanaconda.modules.payloads.payload.dnf.installation.collect_platform_requirements") @@ -417,7 +415,9 @@ def test_fail(self, kernel_getter, req_getter1, req_getter2, req_getter3, req_ge dnf_manager = Mock() dnf_manager.default_environment = None - dnf_manager.apply_specs.side_effect = MissingSpecsError("e2") + report = ValidationReport() + report.warning_messages = ["e2"] + dnf_manager.resolve_selection.return_value = report with pytest.raises(NonCriticalInstallationError) as cm: data = PackagesConfigurationData() @@ -427,7 +427,7 @@ def test_fail(self, kernel_getter, req_getter1, req_getter2, req_getter3, req_ge expected = "e2" assert str(cm.value) == expected - dnf_manager.resolve_selection.side_effect = InvalidSelectionError("e4") + report.error_messages = ["e4"] with pytest.raises(PayloadInstallationError) as cm: data = PackagesConfigurationData() From 7604541f5d3ce9b3aafa68d8ca1537fa2df6f968 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Thu, 4 Jul 2024 09:05:41 +0200 Subject: [PATCH 22/40] Fix setting up and running transaction --- pyanaconda/modules/payloads/payload/dnf/dnf_manager.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 2d5b1d2ae6e..f16ae03f13d 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -713,7 +713,7 @@ def install_packages(self, callback, timeout=20): progress = TransactionProgress(queue) process = multiprocessing.Process( target=self._run_transaction, - args=(self._base, progress) + args=(self._base, self._transaction, progress) ) # Start the transaction. @@ -745,7 +745,8 @@ def _run_transaction(base, transaction, progress): try: callbacks = libdnf5.rpm.TransactionCallbacksUniquePtr(progress) - result = transaction.run(callbacks, description="", user_id=None, comment=None) + transaction.set_callbacks(callbacks) + result = transaction.run() log.debug("The transaction finished with %s", result) if transaction_has_errors(transaction): progress.error("The transaction process has ended with errors.") From e9845028b1377a5a9348f2c75638d977131e3c75 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Thu, 4 Jul 2024 09:40:18 +0200 Subject: [PATCH 23/40] Adapt tests of DNFManager.install_packages for dnf5 --- .../test_module_payload_dnf5_manager.py | 80 +++++++++++++++++++ .../test_module_payload_dnf_manager.py | 56 ------------- 2 files changed, 80 insertions(+), 56 deletions(-) diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index b48bbb00760..02dc6fc8a65 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -32,6 +32,7 @@ URL_TYPE_METALINK, URL_TYPE_MIRRORLIST, ) +from pyanaconda.modules.common.errors.installation import PayloadInstallationError from pyanaconda.modules.common.errors.payload import UnknownRepositoryError from pyanaconda.modules.common.structures.packages import PackagesConfigurationData from pyanaconda.modules.common.structures.payload import RepoConfigurationData @@ -466,6 +467,85 @@ def _download_packages_failed(self, packages, progress): "p3": 25 } + @patch.object(DNFManager, '_run_transaction') + def test_install_packages(self, run_transaction): + """Test the install_packages method.""" + self.dnf_manager.setup_base() + + calls = [] + + run_transaction.side_effect = self._install_packages + + self.dnf_manager.install_packages(calls.append) + + assert calls == [ + 'Installing p1-1.2-3.x86_64', + 'Configuring p1-1.2-3.x86_64', + 'Installing p2-1.2-3.x86_64', + 'Configuring p2-1.2-3.x86_64', + 'Installing p3-1.2-3.x86_64', + 'Configuring p3-1.2-3.x86_64', + 'Configuring p1-1.2-3.x86_64', + 'Configuring p2-1.2-3.x86_64', + 'Configuring p3-1.2-3.x86_64' + ] + + def _get_package(self, name, action=libdnf5.transaction.TransactionItemAction_INSTALL): + """Get a mocked package of the specified name.""" + package = Mock(spec=libdnf5.transaction.Package) + package.get_name.return_value = name + package.get_epoch.return_value = "0" + package.get_release.return_value = "3" + package.get_arch.return_value = "x86_64" + package.get_version.return_value = "1.2" + package.to_string.return_value = name + "-1.2-3.x86_64" + package.get_action.return_value = action + return package + + def _install_packages(self, base, transaction, progress): + """Simulate the installation of packages.""" + packages = list(map(self._get_package, ["p1", "p2", "p3"])) + ts_total = len(packages) + for ts_done, package in enumerate(packages): + progress.install_start(package, ts_total) + progress.install_progress(package, ts_done, ts_total) + progress.script_start( + package, + package.to_string(), + libdnf5.rpm.TransactionCallbacks.ScriptType_PRE_INSTALL + ) + progress.install_progress(package, ts_done + 1, ts_total) + + for ts_done, package in enumerate(packages): + progress.script_start( + package, + package.to_string(), + libdnf5.rpm.TransactionCallbacks.ScriptType_POST_TRANSACTION + ) + + progress.quit("DNF quit") + + @patch.object(DNFManager, '_run_transaction') + def test_install_packages_failed(self, run_transaction): + """Test the failed install_packages method.""" + self.dnf_manager.setup_base() + + calls = [] + run_transaction.side_effect = self._install_packages_failed + + with pytest.raises(PayloadInstallationError) as cm: + self.dnf_manager.install_packages(calls.append) + + msg = "An error occurred during the transaction: " \ + "The p1 package couldn't be installed!" + + assert str(cm.value) == msg + assert calls == [] + + def _install_packages_failed(self, base, transaction, progress): + """Simulate the failed installation of packages.""" + progress.error("The p1 package couldn't be installed!") + class DNFManagerCompsTestCase(unittest.TestCase): """Test the comps abstraction of the DNF base.""" diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py index af69f05a6d7..8610a8ba835 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py @@ -63,27 +63,6 @@ class DNFManagerTestCase(unittest.TestCase): - @patch("dnf.base.Base.do_transaction") - def test_install_packages(self, do_transaction): - """Test the install_packages method.""" - calls = [] - do_transaction.side_effect = self._install_packages - - # Fake transaction. - self.dnf_manager._base.transaction = [Mock(), Mock(), Mock()] - - self.dnf_manager.install_packages(calls.append) - - assert calls == [ - 'Installing p1.x86_64 (0/3)', - 'Installing p2.x86_64 (1/3)', - 'Installing p3.x86_64 (2/3)', - 'Performing post-installation setup tasks', - 'Configuring p1.x86_64', - 'Configuring p2.x86_64', - 'Configuring p3.x86_64', - ] - def _get_package(self, name): """Get a mocked package of the specified name.""" package = Mock(spec=Package) @@ -94,41 +73,6 @@ def _get_package(self, name): package.returnIdSum.return_value = ("", "1a2b3c") return package - def _install_packages(self, progress): - """Simulate the installation of packages.""" - packages = list(map(self._get_package, ["p1", "p2", "p3"])) - ts_total = len(packages) - - for ts_done, package in enumerate(packages): - progress.progress(package, PKG_INSTALL, 0, 100, ts_done, ts_total) - progress.progress(package, PKG_INSTALL, 50, 100, ts_done, ts_total) - progress.progress(package, PKG_SCRIPTLET, 75, 100, ts_done, ts_total) - progress.progress(package, PKG_INSTALL, 100, 100, ts_done + 1, ts_total) - - progress.progress(None, TRANS_POST, None, None, None, None) - - for ts_done, package in enumerate(packages): - progress.progress(package, PKG_SCRIPTLET, 100, 100, ts_done + 1, ts_total) - - @patch("dnf.base.Base.do_transaction") - def test_install_packages_failed(self, do_transaction): - """Test the failed install_packages method.""" - calls = [] - do_transaction.side_effect = self._install_packages_failed - - with pytest.raises(PayloadInstallationError) as cm: - self.dnf_manager.install_packages(calls.append) - - msg = "An error occurred during the transaction: " \ - "The p1 package couldn't be installed!" - - assert str(cm.value) == msg - assert calls == [] - - def _install_packages_failed(self, progress): - """Simulate the failed installation of packages.""" - progress.error("The p1 package couldn't be installed!") - @patch("dnf.base.Base.do_transaction") def test_install_packages_dnf_ts_item_error(self, do_transaction): """Test install_packages method failing on transaction item error.""" From b282fcc59ecc2c70344a26fe5d23d1b54cfd6f3b Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Thu, 4 Jul 2024 09:05:16 +0200 Subject: [PATCH 24/40] Fix transaction callbacks The transaction callbacks take libdnf5::base::TransactionPackage, not libdnf5::transaction::Package. --- .../payload/dnf/transaction_progress.py | 124 +++++++----------- .../test_module_payload_dnf5_manager.py | 35 +++-- 2 files changed, 71 insertions(+), 88 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/transaction_progress.py b/pyanaconda/modules/payloads/payload/dnf/transaction_progress.py index 5d0b96942f0..fbe4aaf6f83 100644 --- a/pyanaconda/modules/payloads/payload/dnf/transaction_progress.py +++ b/pyanaconda/modules/payloads/payload/dnf/transaction_progress.py @@ -68,20 +68,36 @@ def __init__(self, queue): self._queue = queue def install_start(self, item, total=0): - log.debug("Installing - %s", item.get_package().get_nevra()) - log.debug(libdnf5.base.transaction.transaction_item_action_to_string(item.get_action())) - self._queue.put(('install', item.get_package().get_nevra())) + log.debug("Installing - %s", item.get_package().to_string()) + self._queue.put(('install', item.get_package().to_string())) def install_progress(self, item, amount, total): - log.debug("Installing - %s (%s/%s)", item.get_package().get_nevra(), amount, total) + log.debug("Installing - %s (%s/%s)", item.get_package().to_string(), amount, total) def verify_progress(self, amount, total): log.debug("Verify %s/%s", amount, total) self._queue.put(('verify', 'packages')) - def script_start(self, item, nevra, type): - log.debug("Configuring - %s, %s, %s", item.get_package().get_nevra(), nevra, type) - self._queue.put(('configure', nevra)) + def script_start(self, item, nevra, type): # pylint: disable=redefined-builtin + log.debug( + "Configuring - %s, %s, %s", + # In case of the script_start callback, the item can be a nullpointer. + # There reason is some scriptlets (namely file triggers) can be run for a package + # that is not part of the transaction. + item.get_package().to_string() if item else "unknown", + # FIXME: Get nevra instead of composing it from all the fields. + # See issue: https://github.com/rpm-software-management/dnf5/issues/1644 + #libdnf5.rpm.to_full_nevra_string(nevra), + "{name}-{epoch}:{version}-{release}.{arch}".format( + name=nevra.get_name(), + epoch=nevra.get_epoch(), + version=nevra.get_version(), + release=nevra.get_release(), + arch=nevra.get_arch() + ), + libdnf5.rpm.TransactionCallbacks.script_type_to_string(type) + ) + self._queue.put(('configure', nevra.get_name())) def transaction_stop(self, total): log.debug("Done - %s", total) @@ -89,16 +105,34 @@ def transaction_stop(self, total): self._queue.close() def cpio_error(self, item): - log.debug("Error - %s", item.get_package().get_nevra()) - self._queue.put(('error', item.get_package().get_nevra())) - - def script_error(self, item, nevra, type, return_code): - log.debug("Error - %s, %s, %s, %s", item.get_package().get_nevra(), nevra, type, return_code) - self._queue.put(('error', item.get_package().get_nevra())) + log.debug("Error - %s", item.get_package().to_string()) + self._queue.put(('error', item.get_package().to_string())) + + def script_error(self, item, nevra, type, return_code): # pylint: disable=redefined-builtin + log.debug( + "Error - %s, %s, %s, %s", + # In case of the script_error callback, the item can be a nullpointer. + # There reason is some scriptlets (namely file triggers) can be run for a package + # that is not part of the transaction. + item.get_package().to_string() if item else "unknown", + # FIXME: Get nevra instead of composing it from all the fields. + # See issue: https://github.com/rpm-software-management/dnf5/issues/1644 + #libdnf5.rpm.to_full_nevra_string(nevra), + "{name}-{epoch}:{version}-{release}.{arch}".format( + name=nevra.get_name(), + epoch=nevra.get_epoch(), + version=nevra.get_version(), + release=nevra.get_release(), + arch=nevra.get_arch() + ), + libdnf5.rpm.TransactionCallbacks.script_type_to_string(type), + return_code + ) + self._queue.put(('error', item.get_package().to_string())) def unpack_error(self, item): - log.debug("Error - %s", item.get_package().get_nevra()) - self._queue.put(('error', item.get_package().get_nevra())) + log.debug("Error - %s", item.get_package().to_string()) + self._queue.put(('error', item.get_package().to_string())) def error(self, message): """Report an error that occurred during the transaction. @@ -114,63 +148,3 @@ def quit(self, message): """ self._queue.put(('quit', message)) self._queue.close() - - # def progress(self, package, action, ti_done, ti_total, ts_done, ts_total): - # """Report ongoing progress on the given transaction item. - # - # :param package: the DNF package object - # :param action: the ID of the current action - # :param ti_done: the number of processed bytes of the transaction item - # :param ti_total: the total number of bytes of the transaction item - # :param ts_done: the number of actions processed in the whole transaction - # :param ts_total: the total number of actions in the whole transaction - # """ - # # Process DNF actions, communicating with anaconda via the queue - # # A normal installation consists of 'install' messages followed by - # # the 'post' message. - # if action == dnf.transaction.PKG_INSTALL and ti_done == 0: - # # do not report same package twice - # if self._last_ts == ts_done: - # return - # self._last_ts = ts_done - # - # msg = '%s.%s (%d/%d)' % \ - # (package.name, package.arch, ts_done, ts_total) - # self.cnt += 1 - # self._queue.put(('install', msg)) - # - # # Log the exact package nevra, build time and checksum - # nevra = "%s-%s.%s" % (package.name, package.evr, package.arch) - # log_msg = "Installed: %s %s %s" % (nevra, package.buildtime, package.returnIdSum()[1]) - # self._queue.put(('log', log_msg)) - # - # elif action == dnf.transaction.TRANS_POST: - # self._queue.put(('post', None)) - # log_msg = "Post installation setup phase started." - # self._queue.put(('log', log_msg)) - # self._postinst_phase = True - # - # elif action == dnf.transaction.PKG_SCRIPTLET: - # # Log the exact package nevra, build time and checksum - # nevra = "%s-%s.%s" % (package.name, package.evr, package.arch) - # log_msg = "Configuring (running scriptlet for): %s %s %s" % (nevra, package.buildtime, - # package.returnIdSum()[1]) - # self._queue.put(('log', log_msg)) - # - # # only show progress in UI for post-installation scriptlets - # if self._postinst_phase: - # msg = '%s.%s' % (package.name, package.arch) - # self._queue.put(('configure', msg)) - # - # elif action == dnf.transaction.PKG_VERIFY: - # msg = '%s.%s (%d/%d)' % (package.name, package.arch, ts_done, ts_total) - # self._queue.put(('verify', msg)) - # - # # Log the exact package nevra, build time and checksum - # nevra = "%s-%s.%s" % (package.name, package.evr, package.arch) - # log_msg = "Verifying: %s %s %s" % (nevra, package.buildtime, package.returnIdSum()[1]) - # self._queue.put(('log', log_msg)) - # - # # Once the last package is verified the transaction is over - # if ts_done == ts_total: - # self._queue.put(('done', None)) diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index 02dc6fc8a65..94aecb15422 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -490,7 +490,7 @@ def test_install_packages(self, run_transaction): 'Configuring p3-1.2-3.x86_64' ] - def _get_package(self, name, action=libdnf5.transaction.TransactionItemAction_INSTALL): + def _get_transaction_item(self, name, action=libdnf5.transaction.TransactionItemAction_INSTALL): """Get a mocked package of the specified name.""" package = Mock(spec=libdnf5.transaction.Package) package.get_name.return_value = name @@ -500,26 +500,35 @@ def _get_package(self, name, action=libdnf5.transaction.TransactionItemAction_IN package.get_version.return_value = "1.2" package.to_string.return_value = name + "-1.2-3.x86_64" package.get_action.return_value = action - return package + + nevra = Mock(spec=libdnf5.rpm.Nevra) + nevra.get_name.return_value = name + "-1.2-3.x86_64" + + item = Mock(spec=libdnf5.base.TransactionPackage) + item.get_package.return_value = package + item.nevra = nevra + item.get_action.return_value = action + + return item def _install_packages(self, base, transaction, progress): """Simulate the installation of packages.""" - packages = list(map(self._get_package, ["p1", "p2", "p3"])) - ts_total = len(packages) - for ts_done, package in enumerate(packages): - progress.install_start(package, ts_total) - progress.install_progress(package, ts_done, ts_total) + transaction_items = list(map(self._get_transaction_item, ["p1", "p2", "p3"])) + ts_total = len(transaction_items) + for ts_done, item in enumerate(transaction_items): + progress.install_start(item, ts_total) + progress.install_progress(item, ts_done, ts_total) progress.script_start( - package, - package.to_string(), + item, + item.nevra, libdnf5.rpm.TransactionCallbacks.ScriptType_PRE_INSTALL ) - progress.install_progress(package, ts_done + 1, ts_total) + progress.install_progress(item, ts_done + 1, ts_total) - for ts_done, package in enumerate(packages): + for ts_done, item in enumerate(transaction_items): progress.script_start( - package, - package.to_string(), + item, + item.nevra, libdnf5.rpm.TransactionCallbacks.ScriptType_POST_TRANSACTION ) From 0c6e2d09d08586dc9ae9bd3bea4b2d708db0b22f Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Mon, 8 Jul 2024 15:49:17 +0200 Subject: [PATCH 25/40] Report warnings in DNFManager.resolve_selection --- pyanaconda/modules/payloads/payload/dnf/dnf_manager.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index f16ae03f13d..6e987014e5b 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -618,11 +618,12 @@ def resolve_selection(self): if self._ignore_broken_packages: pass - # FIXME: If other problems, report all as errors. - # FIXME: If no problems, but some logs, report all as warnings. if self._transaction.get_problems() != libdnf5.base.GoalProblem_NO_PROBLEM: for message in self._transaction.get_resolve_logs_as_strings(): report.error_messages.append(message) + else: + for message in self._transaction.get_resolve_logs_as_strings(): + report.warning_messages.append(message) if report.is_valid(): log.info("The software selection has been resolved (%d packages selected).", From 0ed56aac9eb54cab3749176dcd6cc7600c4264bb Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Wed, 10 Jul 2024 15:50:12 +0200 Subject: [PATCH 26/40] Remove the test_dnf_tear_down The base is no longer being closed and the resetting of base is tested in test_module_payload_dnf5_manager.DNFManagerTestCase.test_reset_base. --- .../payloads/payload/test_dnf_tear_down.py | 37 ------------------- 1 file changed, 37 deletions(-) delete mode 100644 tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_tear_down.py diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_tear_down.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_tear_down.py deleted file mode 100644 index 2d6aeb2e2f2..00000000000 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_dnf_tear_down.py +++ /dev/null @@ -1,37 +0,0 @@ -# -# Copyright (C) 2022 Red Hat, Inc. -# -# This copyrighted material is made available to anyone wishing to use, -# modify, copy, or redistribute it subject to the terms and conditions of -# the GNU General Public License v.2, or (at your option) any later version. -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY expressed or implied, including the implied warranties of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General -# Public License for more details. You should have received a copy of the -# GNU General Public License along with this program; if not, write to the -# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA -# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the -# source code or documentation are not subject to the GNU General Public -# License and may only be used or replicated with the express permission of -# Red Hat, Inc. -# -import unittest - -from pyanaconda.modules.payloads.payload.dnf.dnf_manager import DNFManager -from pyanaconda.modules.payloads.payload.dnf.tear_down import ResetDNFManagerTask - - -class ResetDNFManagerTaskTestCase(unittest.TestCase): - """Test the installation task for setting the RPM macros.""" - - def test_reset_dnf_manager_task(self): - """Test the ResetDNFManagerTask task.""" - dnf_manager = DNFManager() - dnf_base = dnf_manager._base - - task = ResetDNFManagerTask( - dnf_manager=dnf_manager - ) - task.run() - - assert dnf_base._closed From 7589a9455efac5af56447a7d7d67c2a021ec8516 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Wed, 10 Jul 2024 16:26:37 +0200 Subject: [PATCH 27/40] Enable some tests in test_module_payload_dnf5_manager These tests probably didn't work at the time they were marked as skipped, but are working now. --- .../test_module_payload_dnf5_manager.py | 67 +++++++++++++++++-- .../test_module_payload_dnf_manager.py | 65 ++---------------- 2 files changed, 66 insertions(+), 66 deletions(-) diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index 94aecb15422..1060f5bf0ba 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -33,7 +33,12 @@ URL_TYPE_MIRRORLIST, ) from pyanaconda.modules.common.errors.installation import PayloadInstallationError -from pyanaconda.modules.common.errors.payload import UnknownRepositoryError +from pyanaconda.modules.common.errors.payload import ( + UnknownCompsEnvironmentError, + UnknownCompsGroupError, + UnknownRepositoryError, +) +from pyanaconda.modules.common.structures.comps import CompsEnvironmentData from pyanaconda.modules.common.structures.packages import PackagesConfigurationData from pyanaconda.modules.common.structures.payload import RepoConfigurationData from pyanaconda.modules.payloads.payload.dnf.dnf_manager import ( @@ -52,27 +57,29 @@ def test_runtime_error(self): with pytest.raises(RuntimeError): query.get() - @pytest.mark.skip("Fatal Python error: Aborted") def test_undefined_variables(self): base = libdnf5.base.Base() variables = base.get_vars() - variables.get_value("undefined") - @pytest.mark.skip("Fatal Python error: Aborted") + with pytest.raises(IndexError): + variables.get_value("undefined") + def test_resolve_without_setup(self): """Call resolve without setting up the base.""" base = libdnf5.base.Base() goal = libdnf5.base.Goal(base) - goal.resolve() - @pytest.mark.skip("Fatal Python error: Aborted") + with pytest.raises(RuntimeError): + goal.resolve() + def test_environment_query(self): base = libdnf5.base.Base() + base.setup() libdnf5.comps.EnvironmentQuery(base) - @pytest.mark.skip("Fatal Python error: Aborted") def test_group_query(self): base = libdnf5.base.Base() + base.setup() libdnf5.comps.GroupQuery(base) def test_disable_failed_repository(self): @@ -555,6 +562,29 @@ def _install_packages_failed(self, base, transaction, progress): """Simulate the failed installation of packages.""" progress.error("The p1 package couldn't be installed!") + @pytest.mark.skip("Not implemented") + def test_set_download_location(self): + """Test the set_download_location method.""" + r1 = self._add_repo("r1") + r2 = self._add_repo("r2") + r3 = self._add_repo("r3") + + self.dnf_manager.set_download_location("/my/download/location") + + assert r1.pkgdir == "/my/download/location" + assert r2.pkgdir == "/my/download/location" + assert r3.pkgdir == "/my/download/location" + + def test_download_location(self): + """Test the download_location property.""" + assert self.dnf_manager.download_location is None + + self.dnf_manager.set_download_location("/my/location") + assert self.dnf_manager.download_location == "/my/location" + + self.dnf_manager.reset_base() + assert self.dnf_manager.download_location is None + class DNFManagerCompsTestCase(unittest.TestCase): """Test the comps abstraction of the DNF base.""" @@ -568,6 +598,11 @@ def test_groups(self): """Test the groups property.""" assert self.dnf_manager.groups == [] + def test_get_group_data_error(self): + """Test the failed get_group_data method.""" + with pytest.raises(UnknownCompsGroupError): + self.dnf_manager.get_group_data("g1") + def test_no_default_environment(self): """Test the default_environment property with no environments.""" assert self.dnf_manager.default_environment is None @@ -576,6 +611,24 @@ def test_environments(self): """Test the environments property.""" assert self.dnf_manager.environments == [] + def test_get_environment_data_error(self): + """Test the failed get_environment_data method.""" + with pytest.raises(UnknownCompsEnvironmentError): + self.dnf_manager.get_environment_data("e1") + + def test_environment_data_available_groups(self): + """Test the get_available_groups method.""" + data = CompsEnvironmentData() + assert data.get_available_groups() == [] + + data.optional_groups = ["g1", "g2", "g3"] + data.visible_groups = ["g3", "g4", "g5"] + data.default_groups = ["g1", "g3"] + + assert data.get_available_groups() == [ + "g1", "g2", "g3", "g4", "g5" + ] + class DNFManagerReposTestCase(unittest.TestCase): """Test the repo abstraction of the DNF base.""" diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py index 8610a8ba835..0ecbcb229bb 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py @@ -73,6 +73,9 @@ def _get_package(self, name): package.returnIdSum.return_value = ("", "1a2b3c") return package + # For this test, mocked Transaction is needed, but it can't be easily + # created, because it doesn't have a public constructor, it's supposed + # to be taken from resolved Goal. @patch("dnf.base.Base.do_transaction") def test_install_packages_dnf_ts_item_error(self, do_transaction): """Test install_packages method failing on transaction item error.""" @@ -95,6 +98,9 @@ def test_install_packages_dnf_ts_item_error(self, do_transaction): assert str(cm.value) == msg assert calls == [] + # For this test, mocked Transaction is needed, but it can't be easily + # created, because it doesn't have a public constructor, it's supposed + # to be taken from resolved Goal. @patch("dnf.base.Base.do_transaction") def test_install_packages_quit(self, do_transaction): """Test the terminated install_packages method.""" @@ -115,38 +121,6 @@ def _install_packages_quit(self, progress): """Simulate the terminated installation of packages.""" raise IOError("Something went wrong with the p1 package!") - def _add_repo(self, name, enabled=True): - """Add the DNF repo object.""" - repo = Repo(name, self.dnf_manager._base.conf) - self.dnf_manager._base.repos.add(repo) - - if enabled: - repo.enable() - - return repo - - def test_set_download_location(self): - """Test the set_download_location method.""" - r1 = self._add_repo("r1") - r2 = self._add_repo("r2") - r3 = self._add_repo("r3") - - self.dnf_manager.set_download_location("/my/download/location") - - assert r1.pkgdir == "/my/download/location" - assert r2.pkgdir == "/my/download/location" - assert r3.pkgdir == "/my/download/location" - - def test_download_location(self): - """Test the download_location property.""" - assert self.dnf_manager.download_location is None - - self.dnf_manager.set_download_location("/my/location") - assert self.dnf_manager.download_location == "/my/location" - - self.dnf_manager.reset_base() - assert self.dnf_manager.download_location is None - @patch("dnf.subject.Subject.get_best_query") def test_is_package_available(self, get_best_query): """Test the is_package_available method.""" @@ -283,11 +257,6 @@ def test_resolve_group(self): assert self.dnf_manager.resolve_group("g1") == "g1" assert self.dnf_manager.resolve_group("g2") is None - def test_get_group_data_error(self): - """Test the failed get_group_data method.""" - with pytest.raises(UnknownCompsGroupError): - self.dnf_manager.get_group_data("g1") - def test_get_group_data(self): """Test the get_group_data method.""" self._add_group("g1") @@ -301,10 +270,6 @@ def test_get_group_data(self): assert isinstance(data, CompsGroupData) assert compare_data(data, expected) - def test_no_default_environment(self): - """Test the default_environment property with no environments.""" - assert self.dnf_manager.default_environment is None - def test_default_environment(self): """Test the default_environment property with some environments.""" self._add_environment("e1") @@ -342,11 +307,6 @@ def test_resolve_environment(self): assert self.dnf_manager.resolve_environment("e1") == "e1" assert self.dnf_manager.resolve_environment("e2") is None - def test_get_environment_data_error(self): - """Test the failed get_environment_data method.""" - with pytest.raises(UnknownCompsEnvironmentError): - self.dnf_manager.get_environment_data("e1") - def test_get_environment_data(self): """Test the get_environment_data method.""" self._add_environment("e1") @@ -395,16 +355,3 @@ def test_get_environment_data_default_groups(self): data = self.dnf_manager.get_environment_data("e1") assert data.default_groups == ["g1", "g3"] - - def test_environment_data_available_groups(self): - """Test the get_available_groups method.""" - data = CompsEnvironmentData() - assert data.get_available_groups() == [] - - data.optional_groups = ["g1", "g2", "g3"] - data.visible_groups = ["g3", "g4", "g5"] - data.default_groups = ["g1", "g3"] - - assert data.get_available_groups() == [ - "g1", "g2", "g3", "g4", "g5" - ] From 7850e8e2dc0c00b6c0c00130f72f697a27600c7b Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Tue, 16 Jul 2024 14:21:10 +0200 Subject: [PATCH 28/40] Log also the message of the dnf transaction result --- pyanaconda/modules/payloads/payload/dnf/dnf_manager.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 6e987014e5b..7acb832e8f1 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -748,7 +748,10 @@ def _run_transaction(base, transaction, progress): callbacks = libdnf5.rpm.TransactionCallbacksUniquePtr(progress) transaction.set_callbacks(callbacks) result = transaction.run() - log.debug("The transaction finished with %s", result) + log.debug( + "The transaction finished with %s (%s)", + result, transaction.transaction_result_to_string(result) + ) if transaction_has_errors(transaction): progress.error("The transaction process has ended with errors.") except BaseException as e: # pylint: disable=broad-except From 56104b5265273e3c8a4e008454010bc1e1139f63 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Tue, 16 Jul 2024 14:22:21 +0200 Subject: [PATCH 29/40] Report error when the dnf transaction fails The transaction can fail even if it doesn't contain any transaction items with an error status. --- pyanaconda/modules/payloads/payload/dnf/dnf_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 7acb832e8f1..a00adcc521f 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -752,7 +752,7 @@ def _run_transaction(base, transaction, progress): "The transaction finished with %s (%s)", result, transaction.transaction_result_to_string(result) ) - if transaction_has_errors(transaction): + if result != 0 or transaction_has_errors(transaction): progress.error("The transaction process has ended with errors.") except BaseException as e: # pylint: disable=broad-except progress.error("The transaction process has ended abruptly: {}\n{}".format( From c4e24e33bdf8a68005fb405e67e388fade6e8b5f Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Tue, 16 Jul 2024 14:36:39 +0200 Subject: [PATCH 30/40] Log messages from dnf into /tmp/dnf.log --- .../modules/payloads/payload/dnf/dnf.py | 4 ---- .../payloads/payload/dnf/dnf_manager.py | 1 + .../payloads/payload/dnf/initialization.py | 22 ------------------- 3 files changed, 1 insertion(+), 26 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf.py b/pyanaconda/modules/payloads/payload/dnf/dnf.py index 6f6ee29d436..771fb55b4f7 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf.py @@ -39,7 +39,6 @@ SetUpDNFSourcesResult, SetUpDNFSourcesTask, TearDownDNFSourcesTask, - configure_dnf_logging, ) from pyanaconda.modules.payloads.payload.dnf.installation import ( CleanUpDownloadLocationTask, @@ -69,9 +68,6 @@ # Set up the modules logger. log = get_module_logger(__name__) -# Configure the DNF logging. -configure_dnf_logging() - __all__ = ["DNFModule"] diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index a00adcc521f..c94c0331eb6 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -149,6 +149,7 @@ def _create_base(cls): config.cachedir = DNF_CACHE_DIR config.pluginconfpath = DNF_PLUGINCONF_DIR config.logdir = '/tmp/' + base.get_logger().add_logger(libdnf5.logger.create_file_logger(base, "dnf.log")) # Set installer defaults config.gpgcheck = False diff --git a/pyanaconda/modules/payloads/payload/dnf/initialization.py b/pyanaconda/modules/payloads/payload/dnf/initialization.py index bf6acdfde4b..7c8f8935f19 100644 --- a/pyanaconda/modules/payloads/payload/dnf/initialization.py +++ b/pyanaconda/modules/payloads/payload/dnf/initialization.py @@ -15,12 +15,8 @@ # License and may only be used or replicated with the express permission of # Red Hat, Inc. # -import logging from collections import namedtuple -DNF_LIBREPO_LOG = "/tmp/dnf.librepo.log" -DNF_LOGGER = "dnf" - from pyanaconda.anaconda_loggers import get_module_logger from pyanaconda.core.constants import BASE_REPO_NAME, REPO_ORIGIN_SYSTEM from pyanaconda.core.i18n import _ @@ -44,28 +40,10 @@ ) from pyanaconda.modules.payloads.payload.dnf.tree_info import LoadTreeInfoMetadataTask - log = get_module_logger(__name__) -def configure_dnf_logging(): - """Configure the DNF logging.""" - - # FIXME: Set up the librepo logger. - # Set up librepo. - # This is still required even when the librepo has a separate logger because - # DNF needs to have callbacks that the librepo log is written to be able to - # process that log. - # libdnf.repo.LibrepoLog.removeAllHandlers() - # libdnf.repo.LibrepoLog.addHandler(DNF_LIBREPO_LOG) - # FIXME: Set up the DNF logger. - # Set up DNF. Increase the log level to the custom DDEBUG level. - - # dnf_logger = logging.getLogger(DNF_LOGGER) - # dnf_logger.setLevel(dnf.logging.DDEBUG) # The result of the SetUpDNFSourcesTask task. - - SetUpDNFSourcesResult = namedtuple( "LoadTreeInfoMetadataResult", [ "dnf_manager", From cdbbca81d5a57694051738e3d3f6e3e03e2307bb Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Wed, 28 Aug 2024 13:44:15 +0200 Subject: [PATCH 31/40] Don't close the multiprocessing queue too soon The transaction_stop is actually for RPMCALLBACK_TRANS_STOP from rpm, which is only the end of preparation phase. The after_complete is called by dnf after the whole transaction completes. Also, the queue cannot be closed at this point, because transaction errors are written there after the transaction completes and then TransactionProgress.quit is called, which closes the queue as well. --- .../modules/payloads/payload/dnf/transaction_progress.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/transaction_progress.py b/pyanaconda/modules/payloads/payload/dnf/transaction_progress.py index fbe4aaf6f83..da60b6dcb8f 100644 --- a/pyanaconda/modules/payloads/payload/dnf/transaction_progress.py +++ b/pyanaconda/modules/payloads/payload/dnf/transaction_progress.py @@ -99,10 +99,9 @@ def script_start(self, item, nevra, type): # pylint: disable=redefined-builtin ) self._queue.put(('configure', nevra.get_name())) - def transaction_stop(self, total): - log.debug("Done - %s", total) + def after_complete(self, success): + log.debug("Done - %s", success) self._queue.put(('done', None)) - self._queue.close() def cpio_error(self, item): log.debug("Error - %s", item.get_package().to_string()) From fd6dba1be20008a08aeefb1506b93c16bffe8e51 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Thu, 19 Sep 2024 10:04:33 +0200 Subject: [PATCH 32/40] Always call progress.quit in mocked installing packages in tests --- .../test_module_payload_dnf5_manager.py | 42 ++++++++++--------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index 1060f5bf0ba..fc02d6de77d 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -520,26 +520,28 @@ def _get_transaction_item(self, name, action=libdnf5.transaction.TransactionItem def _install_packages(self, base, transaction, progress): """Simulate the installation of packages.""" - transaction_items = list(map(self._get_transaction_item, ["p1", "p2", "p3"])) - ts_total = len(transaction_items) - for ts_done, item in enumerate(transaction_items): - progress.install_start(item, ts_total) - progress.install_progress(item, ts_done, ts_total) - progress.script_start( - item, - item.nevra, - libdnf5.rpm.TransactionCallbacks.ScriptType_PRE_INSTALL - ) - progress.install_progress(item, ts_done + 1, ts_total) - - for ts_done, item in enumerate(transaction_items): - progress.script_start( - item, - item.nevra, - libdnf5.rpm.TransactionCallbacks.ScriptType_POST_TRANSACTION - ) - - progress.quit("DNF quit") + try: + transaction_items = list(map(self._get_transaction_item, ["p1", "p2", "p3"])) + ts_total = len(transaction_items) + for ts_done, item in enumerate(transaction_items): + progress.install_start(item, ts_total) + progress.install_progress(item, ts_done, ts_total) + progress.script_start( + item, + item.nevra, + libdnf5.rpm.TransactionCallbacks.ScriptType_PRE_INSTALL + ) + progress.install_progress(item, ts_done + 1, ts_total) + + for ts_done, item in enumerate(transaction_items): + progress.script_start( + item, + item.nevra, + libdnf5.rpm.TransactionCallbacks.ScriptType_POST_TRANSACTION + ) + finally: + # The quit must be called even if there is an error, otherwise the process never quits. + progress.quit("DNF quit") @patch.object(DNFManager, '_run_transaction') def test_install_packages_failed(self, run_transaction): From 67a98500558c073cda94b719becbfe1228466beb Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Mon, 23 Sep 2024 09:06:08 +0200 Subject: [PATCH 33/40] Set the destdir option --- pyanaconda/modules/payloads/payload/dnf/dnf_manager.py | 5 +---- .../payload/test_module_payload_dnf5_manager.py | 10 +--------- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index c94c0331eb6..6bfd1d380b3 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -649,10 +649,7 @@ def set_download_location(self, path): :param path: a path to the package directory """ - # FIXME: Reimplement the assignment. - # for repo in self._base.repos.iter_enabled(): - # repo.pkgdir = path - + self._base.get_config().destdir = path self._download_location = path def download_packages(self, callback): diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index fc02d6de77d..ba459bea422 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -564,18 +564,10 @@ def _install_packages_failed(self, base, transaction, progress): """Simulate the failed installation of packages.""" progress.error("The p1 package couldn't be installed!") - @pytest.mark.skip("Not implemented") def test_set_download_location(self): """Test the set_download_location method.""" - r1 = self._add_repo("r1") - r2 = self._add_repo("r2") - r3 = self._add_repo("r3") - self.dnf_manager.set_download_location("/my/download/location") - - assert r1.pkgdir == "/my/download/location" - assert r2.pkgdir == "/my/download/location" - assert r3.pkgdir == "/my/download/location" + assert self._get_configuration().destdir == "/my/download/location" def test_download_location(self): """Test the download_location property.""" From c0c7410c146f252df0aa7a382df5e066a1053f26 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Thu, 26 Sep 2024 13:58:57 +0200 Subject: [PATCH 34/40] Remember if repositories were loaded In libdnf5, the `repos_updated_and_loaded` bool attribute, which serves the same purpose, is private. Check if the repositories are loaded before making queries. --- .../payloads/payload/dnf/dnf_manager.py | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 6bfd1d380b3..1c9aac10324 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -119,6 +119,7 @@ def __init__(self): self._download_location = None self._md_hashes = {} self._enabled_system_repositories = [] + self._repositories_loaded = False self._query_environments = None self._query_groups = None @@ -206,6 +207,7 @@ def reset_base(self): self._download_location = None self._md_hashes = {} self._enabled_system_repositories = [] + self._repositories_loaded = False log.debug("The DNF base has been reset.") def configure_base(self, data: PackagesConfigurationData): @@ -259,6 +261,9 @@ def default_environment(self): @property def _environments(self): + if not self._repositories_loaded: + log.warning("There is no metadata about environments and groups!") + return [] if self._query_environments is None: self._query_environments = libdnf5.comps.EnvironmentQuery(self._base) return self._query_environments @@ -348,6 +353,9 @@ def _get_environment_data(self, env) -> CompsEnvironmentData: @property def _groups(self): + if not self._repositories_loaded: + log.warning("There is no metadata about environments and groups!") + return [] if self._query_groups is None: self._query_groups = libdnf5.comps.GroupQuery(self._base) return self._query_groups @@ -559,9 +567,9 @@ def is_package_available(self, package_spec): :param package_spec: a package spec :return: True if the package can be installed, otherwise False """ - #if not self._base.sack: - # log.warning("There is no metadata about packages!") - # return False + if not self._repositories_loaded: + log.warning("There is no metadata about packages!") + return False query = libdnf5.rpm.PackageQuery(self._base) query.filter_name([package_spec]) @@ -575,9 +583,9 @@ def match_available_packages(self, pattern): :param pattern: a pattern for package names :return: a list of matched package names """ - #if not self._base.sack: - # log.warning("There is no metadata about packages!") - # return [] + if not self._repositories_loaded: + log.warning("There is no metadata about packages!") + return [] query = libdnf5.rpm.PackageQuery(self._base) query.filter_name([pattern], libdnf5.common.QueryCmp_GLOB) @@ -840,6 +848,9 @@ def _create_repository(self, data: RepoConfigurationData): :param RepoConfigurationData data: a repo configuration return dnf.repo.Repo: a DNF repository """ + if self._repositories_loaded: + raise RuntimeError("Cannot create a new repository. Repositories were already loaded.") + repo_sack = self._base.get_repo_sack() repo = repo_sack.create_repo(data.name) config = repo.get_config() @@ -1029,6 +1040,7 @@ def load_repositories(self): except RuntimeError as e: log.warning(str(e)) raise MetadataError(str(e)) from None + self._repositories_loaded = True log.info("Loaded repositories.") def load_repomd_hashes(self): From 6da04d5cc3caacf275fe863e2e70c2af0882f21c Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Wed, 6 Nov 2024 13:58:16 +0100 Subject: [PATCH 35/40] Reset the dnf base when clearing cache To reset the repo sack, the whole base must be created anew, because it is not possible to lead repositories multiple times in dnf5. --- pyanaconda/modules/payloads/payload/dnf/dnf_manager.py | 3 +-- .../payloads/payload/test_module_payload_dnf5_manager.py | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 1c9aac10324..29c6af74682 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -556,8 +556,7 @@ def clear_cache(self): shutil.rmtree(DNF_CACHE_DIR, ignore_errors=True) shutil.rmtree(DNF_PLUGINCONF_DIR, ignore_errors=True) - # FIXME: Reset sacks. Should we just drop the base? - # self._base.reset(sack=True, repos=True, goal=True) + self.reset_base() log.debug("The DNF cache has been cleared.") diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index ba459bea422..88a3139b23a 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -1021,10 +1021,10 @@ def test_read_system_repositories(self): with pytest.raises(RuntimeError): self.dnf_manager.read_system_repositories() - # FIXME: Unless we cleared the cache. - # self.dnf_manager.clear_cache() - # assert not self.dnf_manager._enabled_system_repositories - # self.dnf_manager.read_system_repositories() + # Unless we cleared the cache. + self.dnf_manager.clear_cache() + assert not self.dnf_manager._enabled_system_repositories + self.dnf_manager.read_system_repositories() # Or reset the base. self.dnf_manager.reset_base() From f89d9b812c278980e297af428132718115c6a1c5 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Fri, 22 Nov 2024 14:18:07 +0100 Subject: [PATCH 36/40] Implement replacing repositories There is no way to remove a repository from the RepoSack without dropping the entire Base. Therefore, a repository can be only configured again, but not replaced. Since it cannot be removed, the test for repository removal was removed as well. --- .../payloads/payload/dnf/dnf_manager.py | 41 +++++++++++-------- .../test_module_payload_dnf5_manager.py | 20 --------- 2 files changed, 25 insertions(+), 36 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 29c6af74682..ea0a6c835b1 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -825,33 +825,29 @@ def add_repository(self, data: RepoConfigurationData): :param RepoConfigurationData data: a repo configuration """ + repositories = libdnf5.repo.RepoQuery(self._base) + repositories.filter_id(data.name) with self._lock: - # Create a new repository. - repo = self._create_repository(data) - - # FIXME: How to handle existing repositories? - # Remove an existing repository. - # - # if repo.id in self._base.repos: - # self._base.repos.pop(repo.id) - - # Add the new repository. - #self._base.repos.add(repo) + if repositories.empty(): + # Create a new repository. + repo = self._create_repository(data) + else: + # Replace the existing repository with a new one. + repo = self._configure_repository(repositories.get(), data) log.info("Added the '%s' repository: %s", repo.get_id(), repo) - def _create_repository(self, data: RepoConfigurationData): - """Create a DNF repository. + def _configure_repository(self, repo: libdnf5.repo.Repo, data: RepoConfigurationData): + """Configure a DNF repository. + :param libdnf5.repo.Repo repo:existing repository :param RepoConfigurationData data: a repo configuration - return dnf.repo.Repo: a DNF repository + return libdnf5.repo.Repo: a DNF repository """ if self._repositories_loaded: raise RuntimeError("Cannot create a new repository. Repositories were already loaded.") - repo_sack = self._base.get_repo_sack() - repo = repo_sack.create_repo(data.name) config = repo.get_config() # Disable the repo if requested. @@ -902,6 +898,19 @@ def _create_repository(self, data: RepoConfigurationData): return repo + def _create_repository(self, data: RepoConfigurationData): + """Create a DNF repository. + + :param RepoConfigurationData data: a repo configuration + return libdnf5.repo.Repo: a DNF repository + """ + if self._repositories_loaded: + raise RuntimeError("Cannot create a new repository. Repositories were already loaded.") + + repo = self._base.get_repo_sack().create_repo(data.name) + + return self._configure_repository(repo, data) + def generate_repo_file(self, data: RepoConfigurationData): """Generate a content of the .repo file. diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index 88a3139b23a..64bbca7f0e1 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -887,7 +887,6 @@ def test_add_repository_packages(self): assert config.includepkgs == ("p1", "p2") assert config.excludepkgs == ("p3", "p4") - @pytest.mark.skip("Not implemented") def test_add_repository_replace(self): """Test the add_repository method with a replacement.""" data = RepoConfigurationData() @@ -904,25 +903,6 @@ def test_add_repository_replace(self): config = self._get_configuration("r1") assert config.baseurl == ("http://u2",) - @pytest.mark.skip("Not implemented") - def test_remove_repository(self): - """Test the remove_repository method.""" - assert self.dnf_manager.repositories == [] - - self._add_repository("r1") - self._add_repository("r2") - - assert self.dnf_manager.repositories == ["r1", "r2"] - - self.dnf_manager.remove_repository("r1") - assert self.dnf_manager.repositories == ["r2"] - - self.dnf_manager.remove_repository("r3") - assert self.dnf_manager.repositories == ["r2"] - - self.dnf_manager.remove_repository("r2") - assert self.dnf_manager.repositories == [] - def test_generate_repo_file_baseurl(self): """Test the generate_repo_file method with baseurl.""" data = RepoConfigurationData() From 7b847a803223618b05dc89cc3dc9f893cfdf4539 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Wed, 27 Nov 2024 12:52:09 +0100 Subject: [PATCH 37/40] Respect the missing_ignored and broken_ignored configuration --- .../payloads/payload/dnf/dnf_manager.py | 12 ++-------- .../test_module_payload_dnf5_manager.py | 22 +++++++++++++++++-- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index ea0a6c835b1..9d2eb01d3cc 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -227,8 +227,8 @@ def configure_base(self, data: PackagesConfigurationData): self._ignore_missing_packages = data.missing_ignored self._ignore_broken_packages = data.broken_ignored - # FIXME: Set up skip broken? - # config.skip_broken = data.broken_ignored + config.skip_unavailable = data.missing_ignored + config.skip_broken = data.broken_ignored if self._ignore_broken_packages: log.warning( @@ -618,14 +618,6 @@ def resolve_selection(self): log.debug("Resolving the software selection.") self._transaction = self._goal.resolve() - # FIXME: Ignore missing packages. Otherwise, report as warning. - if self._ignore_missing_packages: - pass - - # FIXME: Ignore broken packages. Otherwise, report as error. - if self._ignore_broken_packages: - pass - if self._transaction.get_problems() != libdnf5.base.GoalProblem_NO_PROBLEM: for message in self._transaction.get_resolve_logs_as_strings(): report.error_messages.append(message) diff --git a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py index 64bbca7f0e1..03360bc5760 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py +++ b/tests/unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf5_manager.py @@ -330,13 +330,31 @@ def test_resolve_missing_selection(self): ] assert report.warning_messages == [] - @pytest.mark.skip("Not implemented") def test_ignore_missing_packages(self): """Test the ignore_missing_packages attribute.""" + data = PackagesConfigurationData() + data.missing_ignored = True + self.dnf_manager.configure_base(data) + self.dnf_manager.setup_base() + + self.dnf_manager.apply_specs( + include_list=["@g1", "p1"], + exclude_list=["@g2", "p2"] + ) + + report = self.dnf_manager.resolve_selection() + + assert report.error_messages == [] + assert report.warning_messages == [ + 'No match for argument: p1', + 'No packages to remove for argument: p2', + 'No match for argument: g1', + 'No groups to remove for argument: g2', + ] @pytest.mark.skip("Not implemented") def test_ignore_broken_packages(self): - """Test the ignore_missing_packages attribute.""" + """Test the ignore_broken_packages attribute.""" def test_clear_selection(self): """Test the clear_selection method.""" From 7c9d59d3e1fc15f9dd8026130af33f0b7a571fda Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Mon, 9 Dec 2024 10:32:29 +0100 Subject: [PATCH 38/40] Report downloading progress Progress is reported as summary of how much is downloaded out of total. This also removes the debug logs for all the progress messages and adds only logs for starting and finishing the downloads. The debug logs cannot be in the _report_progress class, because that one is paced. --- .../payloads/payload/dnf/download_progress.py | 61 +++++++++++++++---- 1 file changed, 48 insertions(+), 13 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/download_progress.py b/pyanaconda/modules/payloads/payload/dnf/download_progress.py index 30e7f14e2d7..0f0e19ba3ac 100644 --- a/pyanaconda/modules/payloads/payload/dnf/download_progress.py +++ b/pyanaconda/modules/payloads/payload/dnf/download_progress.py @@ -15,7 +15,6 @@ # License and may only be used or replicated with the express permission of # Red Hat, Inc. # -import collections import time import libdnf5 @@ -50,6 +49,12 @@ def __init__(self, callback): """ super().__init__() self.callback = callback + self.user_cb_data_container = [] # Hold references to user_cb_data + self.last_time = time.time() # Used to pace _report_progress + self.total_files = 0 + self.total_size = Size(0) + self.downloads = {} # Hold number of bytes downloaded for each nevra + self.downloaded_size = 0 def add_new_download(self, user_data, description, total_to_download): """Notify the client that a new download has been created. @@ -59,10 +64,12 @@ def add_new_download(self, user_data, description, total_to_download): :param float total_to_download: a total number of bytes to download :return: associated user data for the new package download """ - self._report_progress("Downloading {} - {} bytes".format( - description, total_to_download - )) - return description + self.user_cb_data_container.append(description) + self.total_files += 1 + self.total_size += total_to_download + log.debug("Started downloading '%s' - %s bytes", description, total_to_download) + self._report_progress() + return len(self.user_cb_data_container) - 1 def progress(self, user_cb_data, total_to_download, downloaded): """Download progress callback. @@ -71,9 +78,13 @@ def progress(self, user_cb_data, total_to_download, downloaded): :param float total_to_download: a total number of bytes to download :param float downloaded: a number of bytes downloaded """ - self._report_progress("Downloading {} - {}/{} bytes".format( - user_cb_data, downloaded, total_to_download - )) + nevra = self.user_cb_data_container[user_cb_data] + self.downloads[nevra] = downloaded + + if total_to_download > 0: + self._report_progress() + + return 0 # Not used, but int is expected to be returned. def end(self, user_cb_data, status, msg): """End of download callback. @@ -82,10 +93,34 @@ def end(self, user_cb_data, status, msg): :param status: the transfer status :param msg: the error message in case of error """ - self._report_progress("Downloaded {} - {} ({})".format( - user_cb_data, status, msg - )) + nevra = self.user_cb_data_container[user_cb_data] + + if status is libdnf5.repo.DownloadCallbacks.TransferStatus_SUCCESSFUL: + log.debug("Downloaded '%s'", nevra) + self._report_progress() + elif status is libdnf5.repo.DownloadCallbacks.TransferStatus_ALREADYEXISTS: + log.debug("Skipping to download '%s': %s", nevra, msg) + self._report_progress() + else: + log.warning("Failed to download '%s': %s", nevra, msg) + + return 0 # Not used, but int is expected to be returned. + + @paced + def _report_progress(self): + # Update the downloaded size. + self.downloaded_size = Size(sum(self.downloads.values())) + + # Report the progress. + msg = _( + 'Downloading {total_files} RPMs, ' + '{downloaded_size} / {total_size} ' + '({total_percent}%) done.' + ).format( + downloaded_size=self.downloaded_size, + total_percent=int(100 * int(self.downloaded_size) / int(self.total_size)), + total_files=self.total_files, + total_size=self.total_size + ) - def _report_progress(self, msg): - log.debug(msg) self.callback(msg) From 320ccfc17dbb3f86c2261f6fb273da36b2010f49 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Mon, 6 Jan 2025 11:55:38 +0100 Subject: [PATCH 39/40] Remove unused imports --- pyanaconda/modules/payloads/payload/dnf/validation.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/validation.py b/pyanaconda/modules/payloads/payload/dnf/validation.py index 7db5027bcd3..5c0ed38244e 100644 --- a/pyanaconda/modules/payloads/payload/dnf/validation.py +++ b/pyanaconda/modules/payloads/payload/dnf/validation.py @@ -15,18 +15,11 @@ # License and may only be used or replicated with the express permission of # Red Hat, Inc. # -from contextlib import contextmanager - from pyanaconda.anaconda_loggers import get_module_logger from pyanaconda.core.i18n import _ from pyanaconda.modules.common.structures.packages import PackagesSelectionData from pyanaconda.modules.common.structures.validation import ValidationReport from pyanaconda.modules.common.task import ValidationTask -from pyanaconda.modules.payloads.payload.dnf.dnf_manager import ( - BrokenSpecsError, - InvalidSelectionError, - MissingSpecsError, -) from pyanaconda.modules.payloads.payload.dnf.utils import ( get_installation_specs, get_kernel_package, From 48132b1efeb4bd6293f10cb92c3f47ee78cb1741 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Mon, 6 Jan 2025 15:01:59 +0100 Subject: [PATCH 40/40] Remove the dnf_manager exceptions related to selection resolving The exceptions are not actually raised anywhere and the report is now constructed right away in the resolve_selection method, so it doesn't make sense to raise these and catch elsewhere to add the messages to the report as it was done previously. --- .../payloads/payload/dnf/dnf_manager.py | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py index 9d2eb01d3cc..90aba78f39d 100644 --- a/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py +++ b/pyanaconda/modules/payloads/payload/dnf/dnf_manager.py @@ -88,18 +88,6 @@ class MetadataError(DNFManagerError): """Metadata couldn't be loaded.""" -class MissingSpecsError(DNFManagerError): - """Some packages, groups or modules are missing.""" - - -class BrokenSpecsError(DNFManagerError): - """Some packages, groups or modules are broken.""" - - -class InvalidSelectionError(DNFManagerError): - """The software selection couldn't be resolved.""" - - class DNFManager: """The abstraction of the DNF base.""" @@ -597,8 +585,6 @@ def apply_specs(self, include_list, exclude_list): :param include_list: a list of specs for inclusion :param exclude_list: a list of specs for exclusion - :raise MissingSpecsError: if there are missing specs - :raise BrokenSpecsError: if there are broken specs """ log.info("Including specs: %s", include_list) for spec in include_list: @@ -609,10 +595,7 @@ def apply_specs(self, include_list, exclude_list): self._goal.add_remove(spec) def resolve_selection(self): - """Resolve the software selection. - - :raise InvalidSelectionError: if the selection cannot be resolved - """ + """Resolve the software selection.""" report = ValidationReport() log.debug("Resolving the software selection.")