From 25ba11e5fea593dd89543c830dc5f6824c0bd0ba Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Fri, 2 Sep 2022 13:07:46 -0700 Subject: [PATCH] Group transactions: optionally report missing group packages This modifies `base.resolve()` so a caller may pass in a dict. If so, the dict will be populated with information on any 'missing' group packages: that is, cases where a group that was included in the transaction specifies a package name, but no package by that name could be found in the repositories. The dict has four keys matching the four libcomps package 'types', and the value for each key is a list of the missing package names of that type (if any). The intent of this change is to enable callers to treat missing group packages as a fatal error (or do anything else they like with the information), without changing default behaviour, API, or the behaviour of the dnf CLI. Links to some of the the history behind this change: https://bugzilla.redhat.com/show_bug.cgi?id=1292892 https://bugzilla.redhat.com/show_bug.cgi?id=1427365 https://bugzilla.redhat.com/show_bug.cgi?id=1461539 https://github.com/rpm-software-management/dnf/pull/1038 Signed-off-by: Adam Williamson --- dnf/base.py | 16 ++++++++++++++-- dnf/comps.py | 3 +++ tests/test_groups.py | 43 ++++++++++++++++++++++++++++++++++--------- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/dnf/base.py b/dnf/base.py index 154eb4e37b..cf39e9eace 100644 --- a/dnf/base.py +++ b/dnf/base.py @@ -75,6 +75,7 @@ import gc import hawkey import itertools +import libcomps import logging import math import os @@ -880,11 +881,14 @@ def _set_excludes_from_weak_to_goal(self): query = query.available() self._goal.add_exclude_from_weak(query) - def resolve(self, allow_erasing=False): + def resolve(self, allow_erasing=False, missing_dict=None): # :api """Build the transaction set.""" exc = None - self._finalize_comps_trans() + missing_group_pkgs = self._finalize_comps_trans() + if isinstance(missing_dict, dict): + for (key, value) in missing_group_pkgs.items(): + missing_dict[key] = value timer = dnf.logging.Timer('depsolve') self._ds_callback.start() @@ -1695,6 +1699,12 @@ def trans_remove(query, remove_query, comps_pkg): (trans.upgrade, trans_upgrade), (trans.remove, trans_remove)) + missing_group_pkgs = { + libcomps.PACKAGE_TYPE_OPTIONAL: [], + libcomps.PACKAGE_TYPE_DEFAULT: [], + libcomps.PACKAGE_TYPE_MANDATORY: [], + libcomps.PACKAGE_TYPE_CONDITIONAL: [] + } for (attr, fn) in attr_fn: for comps_pkg in attr: query_args = {'name': comps_pkg.name} @@ -1707,11 +1717,13 @@ def trans_remove(query, remove_query, comps_pkg): if comps_pkg.basearchonly: package_string += '.' + basearch logger.warning(_('No match for group package "{}"').format(package_string)) + missing_group_pkgs[comps_pkg.type].append(package_string) continue remove_query = fn(q, remove_query, comps_pkg) self._goal.group_members.add(comps_pkg.name) self._remove_if_unneeded(remove_query) + return missing_group_pkgs def _build_comps_solver(self): def reason_fn(pkgname): diff --git a/dnf/comps.py b/dnf/comps.py index 19df04dc0a..a8896a63e2 100644 --- a/dnf/comps.py +++ b/dnf/comps.py @@ -452,6 +452,7 @@ def __init__(self, pkg_or_name): self.name = pkg_or_name self.optional = True self.requires = None + self.type = libcomps.PACKAGE_TYPE_OPTIONAL elif isinstance(pkg_or_name, libdnf.transaction.CompsGroupPackage): # from swdb package # TODO: @@ -462,12 +463,14 @@ def __init__(self, pkg_or_name): # TODO: self.requires = None # self.requires = pkg_or_name.requires + self.type = pkg_or_name.getPackageType() else: # from comps package self.basearchonly = pkg_or_name.basearchonly self.name = pkg_or_name.name self.optional = pkg_or_name.type & libcomps.PACKAGE_TYPE_OPTIONAL self.requires = pkg_or_name.requires + self.type = pkg_or_name.type def __eq__(self, other): return (self.name == other.name and diff --git a/tests/test_groups.py b/tests/test_groups.py index 407cd91408..06a951d022 100644 --- a/tests/test_groups.py +++ b/tests/test_groups.py @@ -22,6 +22,7 @@ import operator +import libcomps import libdnf.transaction import dnf.comps @@ -200,10 +201,12 @@ def test_group_remove(self): class ProblemGroupTest(tests.support.ResultTestCase): """Test some cases involving problems in groups: packages that don't exist, and packages that exist but cannot be installed. The - "broken" group lists three packages. "meaning-of-life", explicitly - 'default', does not exist. "lotus", implicitly 'mandatory' (no - explicit type), exists and is installable. "brokendeps", - explicitly 'optional', exists but has broken dependencies. See + "broken" group lists four packages. "meaning-of-life", explicitly + 'mandatory', does not exist. "lotus", explicitly 'mandatory' (no + explicit type), exists and is installable. "librita", explicitly + 'default', exists, but requires non-existent "no-such-package". + "brokendeps", explicitly 'optional', exists but has broken + dependencies. See https://bugzilla.redhat.com/show_bug.cgi?id=1292892, https://bugzilla.redhat.com/show_bug.cgi?id=1337731, https://bugzilla.redhat.com/show_bug.cgi?id=1427365, and @@ -218,8 +221,9 @@ class ProblemGroupTest(tests.support.ResultTestCase): def test_group_install_broken_mandatory(self): """Here we will test installing the group with only mandatory packages. We expect this to succeed, leaving out the - non-existent 'meaning-of-life': it should also log a warning, - but we don't test that. + non-existent 'meaning-of-life', and populating the 'missing + dict' with information on the missing package. It should also + log a warning, but we don't test that. """ comps_group = self.base.comps.group_by_pattern('Broken Group') swdb_group = self.history.group.get(comps_group.id) @@ -227,7 +231,8 @@ def test_group_install_broken_mandatory(self): cnt = self.base.group_install(comps_group.id, ('mandatory',)) self._swdb_commit() - self.base.resolve() + missing_dict = {} + self.base.resolve(missing_dict=missing_dict) # this counts packages *listed* in the group, so 2 self.assertEqual(cnt, 2) @@ -235,6 +240,14 @@ def test_group_install_broken_mandatory(self): # the above should work, but only 'lotus' actually installed self.assertLength(inst, 1) self.assertEmpty(removed) + # the dict should record that 'meaning-of-life' was missing + expected_dict = { + libcomps.PACKAGE_TYPE_OPTIONAL: [], + libcomps.PACKAGE_TYPE_DEFAULT: [], + libcomps.PACKAGE_TYPE_MANDATORY: ['meaning-of-life'], + libcomps.PACKAGE_TYPE_CONDITIONAL: [] + } + self.assertEqual(missing_dict, expected_dict) def test_group_install_broken_default(self): """Here we will test installing the group with only mandatory @@ -242,7 +255,9 @@ def test_group_install_broken_default(self): factor is an entry pulling in librita if no-such-package is also included or installed. We expect this not to actually pull in librita (as no-such-package obviously *isn't* there), - but also not to cause a fatal error. + but also not to cause a fatal error. We also expect the + missing dict to record 'meaning-of-life', but not 'librita', + as missing. """ comps_group = self.base.comps.group_by_pattern('Broken Group') swdb_group = self.history.group.get(comps_group.id) @@ -250,7 +265,8 @@ def test_group_install_broken_default(self): cnt = self.base.group_install(comps_group.id, ('mandatory', 'default')) self._swdb_commit() - self.base.resolve() + missing_dict = {} + self.base.resolve(missing_dict=missing_dict) # this counts packages *listed* in the group, so 3 self.assertEqual(cnt, 3) @@ -258,6 +274,15 @@ def test_group_install_broken_default(self): # the above should work, but only 'lotus' actually installed self.assertLength(inst, 1) self.assertEmpty(removed) + # the dict should record that 'meaning-of-life' was missing, + # but *not* librita + expected_dict = { + libcomps.PACKAGE_TYPE_OPTIONAL: [], + libcomps.PACKAGE_TYPE_DEFAULT: [], + libcomps.PACKAGE_TYPE_MANDATORY: ['meaning-of-life'], + libcomps.PACKAGE_TYPE_CONDITIONAL: [] + } + self.assertEqual(missing_dict, expected_dict) def test_group_install_broken_optional(self): """Here we test installing the group with optional packages