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

tests/repos/main_comps.xml

@@ -43,17 +43,25 @@
     <id>broken-group</id>
     <name>Broken Group</name>
     <packagelist>
-      <packagereq type="mandatory">meaning-of-life</packagereq>
       <packagereq type="mandatory">lotus</packagereq>
       <packagereq type="default" requires="no-such-package">librita</packagereq>
       <packagereq type="optional">brokendeps</packagereq>
+      <packagereq type="optional">no-such-package</packagereq>
+    </packagelist>
+  </group>
+  <group>
+    <id>broken-group-2</id>
+    <name>Broken Group 2</name>
+    <packagelist>
+      <packagereq type="mandatory">no-such-package</packagereq>
+      <packagereq type="default">no-such-package-2</packagereq>
     </packagelist>
   </group>
   <group>
     <id>missing-name-group</id>
     <name></name>
     <packagelist>
-      <packagereq type="mandatory">meaning-of-life</packagereq>
+      <packagereq type="mandatory">lotus</packagereq>
     </packagelist>
   </group>
   <category>

tests/support.py

@@ -95,7 +95,7 @@ def mock_open(mock=None, data=None):
 MAIN_NSOLVABLES = 9
 UPDATES_NSOLVABLES = 4
 AVAILABLE_NSOLVABLES = MAIN_NSOLVABLES + UPDATES_NSOLVABLES
-TOTAL_GROUPS = 5
+TOTAL_GROUPS = 6
 TOTAL_NSOLVABLES = SYSTEM_NSOLVABLES + AVAILABLE_NSOLVABLES
 
 

tests/test_comps.py

@@ -107,15 +107,15 @@ def test_group_packages(self):
     def test_iteration(self):
         comps = self.comps
         self.assertEqual([g.name for g in comps.groups_iter()],
-                         ['Base', 'Solid Ground', "Pepper's", "Broken Group", None])
+                         ['Base', 'Solid Ground', "Pepper's", "Broken Group", "Broken Group 2", None])
         self.assertEqual([c.name for c in comps.categories_iter()],
                          ['Base System'])
         g = dnf.util.first(comps.groups_iter())
         self.assertEqual(g.desc_by_lang['cs'], TRANSLATION)
 
     def test_group_display_order(self):
         self.assertEqual([g.name for g in self.comps.groups],
-                         ["Pepper's", 'Base', 'Solid Ground', 'Broken Group', None])
+                         ["Pepper's", 'Base', 'Solid Ground', 'Broken Group', 'Broken Group 2', None])
 
     def test_packages(self):
         comps = self.comps
@@ -127,7 +127,7 @@ def test_packages(self):
 
     def test_size(self):
         comps = self.comps
-        self.assertLength(comps, 7)
+        self.assertLength(comps, 8)
         self.assertLength(comps.groups, tests.support.TOTAL_GROUPS)
         self.assertLength(comps.categories, 1)
         self.assertLength(comps.environments, 1)

tests/test_groups.py

@@ -199,27 +199,53 @@ 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
+    don't exist, and packages that exist but cannot be installed.
+    broken_group lists four packages. "lotus", explicitly
+    'mandatory', exists and is installable. "librita", explicitly
+    'default', is a conditional entry that exists, but requires a
+    package that doesn't exist. "brokendeps", explicitly 'optional',
+    exists but has broken dependencies. "no-such-package", explicitly
+    'optional', does not exist.
+    broken_group_2 lists two packages, one explicitly mandatory and
+    one explicitly default, neither of which exists.
+    missing_name_group is a group which has no name. The package it
+    contains exists and is not broken.
+    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
-    https://bugzilla.redhat.com/show_bug.cgi?id=1461539 for some of
-    the background on this.
+    https://bugzilla.redhat.com/show_bug.cgi?id=1427365,
+    https://bugzilla.redhat.com/show_bug.cgi?id=1461539 and
+    https://github.com/rpm-software-management/dnf/pull/1848
+    for some of the background on this.
     """
 
     REPOS = ['main', 'broken_group']
     COMPS = True
     COMPS_SEED_PERSISTOR = True
 
+    def test_group_install_missing(self):
+        """Here we will test installing a group with two packages,
+        one mandatory and one default, both non-existent. We should
+        raise an error with both packages in it.
+        """
+        comps_group = self.base.comps.group_by_pattern('Broken Group 2')
+        swdb_group = self.history.group.get(comps_group.id)
+        self.assertIsNone(swdb_group)
+
+        cnt = self.base.group_install(comps_group.id, ('mandatory', 'default', 'optional'))
+        self.assertEqual(cnt, 2)
+
+        self._swdb_commit()
+        with self.assertRaises(dnf.exceptions.MarkingErrors) as exccm:
+            self.base.resolve()
+        missing = set(exccm.exception.no_match_pkg_specs)
+        expected = set(['no-such-package', 'no-such-package-2'])
+        self.assertEqual(missing, expected)
+
     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.
+        packages. We expect this to succeed. It shouldn't log any
+        warnings either, 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)
@@ -228,11 +254,8 @@ def test_group_install_broken_mandatory(self):
         cnt = self.base.group_install(comps_group.id, ('mandatory',))
         self._swdb_commit()
         self.base.resolve()
-        # this counts packages *listed* in the group, so 2
-        self.assertEqual(cnt, 2)
-
+        self.assertEqual(cnt, 1)
         inst, removed = self.installed_removed(self.base)
-        # the above should work, but only 'lotus' actually installed
         self.assertLength(inst, 1)
         self.assertEmpty(removed)
 
@@ -251,8 +274,8 @@ def test_group_install_broken_default(self):
         cnt = self.base.group_install(comps_group.id, ('mandatory', 'default'))
         self._swdb_commit()
         self.base.resolve()
-        # this counts packages *listed* in the group, so 3
-        self.assertEqual(cnt, 3)
+        # 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
@@ -278,7 +301,8 @@ def test_group_install_broken_optional(self):
     def test_group_install_broken_optional_nonstrict(self):
         """Here we test installing the group with optional packages
         included, but with strict=False. We expect this to succeed,
-        skipping the package with broken dependencies.
+        skipping the package with broken dependencies, and the package
+        that doesn't exist.
         """
         comps_group = self.base.comps.group_by_pattern('Broken Group')
         swdb_group = self.history.group.get(comps_group.id)