diff --git a/dnf/base.py b/dnf/base.py index 154eb4e37b..03469c5dfe 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 @@ -1695,6 +1696,7 @@ def trans_remove(query, remove_query, comps_pkg): (trans.upgrade, trans_upgrade), (trans.remove, trans_remove)) + missing_fatals = [] for (attr, fn) in attr_fn: for comps_pkg in attr: query_args = {'name': comps_pkg.name} @@ -1706,13 +1708,21 @@ def trans_remove(query, remove_query, comps_pkg): package_string = comps_pkg.name if comps_pkg.basearchonly: package_string += '.' + basearch - logger.warning(_('No match for group package "{}"').format(package_string)) + if comps_pkg.type == libcomps.PACKAGE_TYPE_OPTIONAL: + logger.warning(_('No match for group package "{}"').format(package_string)) + else: + missing_fatals.append(package_string) continue remove_query = fn(q, remove_query, comps_pkg) self._goal.group_members.add(comps_pkg.name) + # we do this before raising the exception for missing packages + # so apps can catch the exception and proceed if desired self._remove_if_unneeded(remove_query) + if missing_fatals: + raise dnf.exceptions.MarkingErrors(no_match_pkg_specs=missing_fatals) + def _build_comps_solver(self): def reason_fn(pkgname): q = self.sack.query().installed().filterm(name=pkgname) diff --git a/tests/repos/main_comps.xml b/tests/repos/main_comps.xml index 584bb25b3a..33fa6aa189 100644 --- a/tests/repos/main_comps.xml +++ b/tests/repos/main_comps.xml @@ -43,17 +43,25 @@ broken-group Broken Group - meaning-of-life lotus librita brokendeps + no-such-package + + + + broken-group-2 + Broken Group 2 + + no-such-package + no-such-package-2 missing-name-group - meaning-of-life + lotus diff --git a/tests/support.py b/tests/support.py index e50684ef5b..c408d228db 100644 --- a/tests/support.py +++ b/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 diff --git a/tests/test_comps.py b/tests/test_comps.py index 763218587f..6ea4abeabc 100644 --- a/tests/test_comps.py +++ b/tests/test_comps.py @@ -107,7 +107,7 @@ 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()) @@ -115,7 +115,7 @@ def test_iteration(self): 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) diff --git a/tests/test_groups.py b/tests/test_groups.py index 407cd91408..cdaf633ffa 100644 --- a/tests/test_groups.py +++ b/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)