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
#1038

Signed-off-by: Adam Williamson <[email protected]>

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):

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

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,46 +221,68 @@ 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)
         self.assertIsNone(swdb_group)
 
         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)
 
         inst, removed = self.installed_removed(self.base)
         # 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
         and default packages. Again we expect this to succeed: the new
         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)
         self.assertIsNone(swdb_group)
 
         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)
 
         inst, removed = self.installed_removed(self.base)
         # 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