From 3c758a4ea670fab1f4b55fa878ebf2b2ff4b678b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Hr=C3=A1zk=C3=BD?= <lhrazky@redhat.com>
Date: Tue, 28 Apr 2020 09:08:05 +0200
Subject: [PATCH] Handle empty comps group name (RhBug:1826198)
Don't crash on empty comps group/environment name. In outputs, use the
"<name-unset>" placeholder instead of the name.
https://bugzilla.redhat.com/show_bug.cgi?id=1826198
---
dnf/cli/commands/group.py | 4 ++--
dnf/cli/output.py | 16 ++++++++++------
dnf/comps.py | 11 ++++++++++-
dnf/db/group.py | 12 ++++++++----
tests/repos/main_comps.xml | 7 +++++++
tests/support.py | 2 +-
tests/test_comps.py | 6 +++---
tests/test_groups.py | 9 +++++++++
8 files changed, 50 insertions(+), 17 deletions(-)
diff --git a/dnf/cli/commands/group.py b/dnf/cli/commands/group.py
index f535a50980..4ffd3b89c8 100644
--- a/dnf/cli/commands/group.py
+++ b/dnf/cli/commands/group.py
@@ -177,7 +177,7 @@ def _list(self, userlist):
def _out_grp(sect, group):
if not done:
print(sect)
- msg = ' %s' % group.ui_name
+ msg = ' %s' % (group.ui_name if group.ui_name is not None else _("<name-unset>"))
if print_ids:
msg += ' (%s)' % group.id
if group.lang_only:
@@ -188,7 +188,7 @@ def _out_env(sect, envs):
if envs:
print(sect)
for e in envs:
- msg = ' %s' % e.ui_name
+ msg = ' %s' % (e.ui_name if e.ui_name is not None else _("<name-unset>"))
if print_ids:
msg += ' (%s)' % e.id
print(msg)
diff --git a/dnf/cli/output.py b/dnf/cli/output.py
index 67eab80b19..2585a5c773 100644
--- a/dnf/cli/output.py
+++ b/dnf/cli/output.py
@@ -1221,47 +1221,51 @@ def _add_line(lines, data, a_wid, po, obsoletes=[]):
lines.append((name, "", "", "", "", "", ""))
pkglist_lines.append((action, lines))
if self.base._history:
+ def format_line(group):
+ name = group.getName()
+ return (name if name else _("<name-unset>"), "", "", "", "", "", "")
+
install_env_group = self.base._history.env._installed
if install_env_group:
action = _("Installing Environment Groups")
lines = []
for group in install_env_group.values():
- lines.append((group.getName(), "", "", "", "", "", ""))
+ lines.append(format_line(group))
pkglist_lines.append((action, lines))
upgrade_env_group = self.base._history.env._upgraded
if upgrade_env_group:
action = _("Upgrading Environment Groups")
lines = []
for group in upgrade_env_group.values():
- lines.append((group.getName(), "", "", "", "", "", ""))
+ lines.append(format_line(group))
pkglist_lines.append((action, lines))
remove_env_group = self.base._history.env._removed
if remove_env_group:
action = _("Removing Environment Groups")
lines = []
for group in remove_env_group.values():
- lines.append((group.getName(), "", "", "", "", "", ""))
+ lines.append(format_line(group))
pkglist_lines.append((action, lines))
install_group = self.base._history.group._installed
if install_group:
action = _("Installing Groups")
lines = []
for group in install_group.values():
- lines.append((group.getName(), "", "", "", "", "", ""))
+ lines.append(format_line(group))
pkglist_lines.append((action, lines))
upgrade_group = self.base._history.group._upgraded
if upgrade_group:
action = _("Upgrading Groups")
lines = []
for group in upgrade_group.values():
- lines.append((group.getName(), "", "", "", "", "", ""))
+ lines.append(format_line(group))
pkglist_lines.append((action, lines))
remove_group = self.base._history.group._removed
if remove_group:
action = _("Removing Groups")
lines = []
for group in remove_group.values():
- lines.append((group.getName(), "", "", "", "", "", ""))
+ lines.append(format_line(group))
pkglist_lines.append((action, lines))
# show skipped conflicting packages
if not self.conf.best and self.base._goal.actions & forward_actions:
diff --git a/dnf/comps.py b/dnf/comps.py
index 316d647087..4ca15b1e07 100644
--- a/dnf/comps.py
+++ b/dnf/comps.py
@@ -75,7 +75,16 @@ def _by_pattern(pattern, case_sensitive, sqn):
else:
match = re.compile(fnmatch.translate(pattern), flags=re.I).match
- return {g for g in sqn if match(g.name) or match(g.id) or match(g.ui_name)}
+ ret = set()
+ for g in sqn:
+ if match(g.id):
+ ret.add(g)
+ elif g.name is not None and match(g.name):
+ ret.add(g)
+ elif g.ui_name is not None and match(g.ui_name):
+ ret.add(g)
+
+ return ret
def _fn_display_order(group):
diff --git a/dnf/db/group.py b/dnf/db/group.py
index e3a087760b..5d7e18d1a8 100644
--- a/dnf/db/group.py
+++ b/dnf/db/group.py
@@ -78,8 +78,10 @@ def _get_obj_id(self, obj):
def new(self, obj_id, name, translated_name, pkg_types):
swdb_group = self.history.swdb.createCompsGroupItem()
swdb_group.setGroupId(obj_id)
- swdb_group.setName(name)
- swdb_group.setTranslatedName(translated_name)
+ if name is not None:
+ swdb_group.setName(name)
+ if translated_name is not None:
+ swdb_group.setTranslatedName(translated_name)
swdb_group.setPackageTypes(pkg_types)
return swdb_group
@@ -136,8 +138,10 @@ def _get_obj_id(self, obj):
def new(self, obj_id, name, translated_name, pkg_types):
swdb_env = self.history.swdb.createCompsEnvironmentItem()
swdb_env.setEnvironmentId(obj_id)
- swdb_env.setName(name)
- swdb_env.setTranslatedName(translated_name)
+ if name is not None:
+ swdb_env.setName(name)
+ if translated_name is not None:
+ swdb_env.setTranslatedName(translated_name)
swdb_env.setPackageTypes(pkg_types)
return swdb_env
diff --git a/tests/repos/main_comps.xml b/tests/repos/main_comps.xml
index 9e694d13a5..584bb25b3a 100644
--- a/tests/repos/main_comps.xml
+++ b/tests/repos/main_comps.xml
@@ -49,6 +49,13 @@
<packagereq type="optional">brokendeps</packagereq>
</packagelist>
</group>
+ <group>
+ <id>missing-name-group</id>
+ <name></name>
+ <packagelist>
+ <packagereq type="mandatory">meaning-of-life</packagereq>
+ </packagelist>
+ </group>
<category>
<id>base-system</id>
<display_order>99</display_order>
diff --git a/tests/support.py b/tests/support.py
index e549ba5b95..a7d6a8542c 100644
--- a/tests/support.py
+++ b/tests/support.py
@@ -94,7 +94,7 @@ def mock_open(mock=None, data=None):
MAIN_NSOLVABLES = 9
UPDATES_NSOLVABLES = 4
AVAILABLE_NSOLVABLES = MAIN_NSOLVABLES + UPDATES_NSOLVABLES
-TOTAL_GROUPS = 4
+TOTAL_GROUPS = 5
TOTAL_NSOLVABLES = SYSTEM_NSOLVABLES + AVAILABLE_NSOLVABLES
diff --git a/tests/test_comps.py b/tests/test_comps.py
index 30d468e3af..763218587f 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"])
+ ['Base', 'Solid Ground', "Pepper's", "Broken Group", 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'])
+ ["Pepper's", 'Base', 'Solid Ground', 'Broken Group', 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, 6)
+ self.assertLength(comps, 7)
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 fe388f96c0..8972da687e 100644
--- a/tests/test_groups.py
+++ b/tests/test_groups.py
@@ -295,6 +295,15 @@ def test_group_install_broken_optional_nonstrict(self):
self.assertLength(inst, 1)
self.assertEmpty(removed)
+ def test_group_install_missing_name(self):
+ comps_group = self.base.comps.group_by_pattern('missing-name-group')
+
+ cnt = self.base.group_install(comps_group.id, ('mandatory', 'default', 'optional'),
+ strict=False)
+ self._swdb_commit()
+ self.base.resolve()
+ self.assertEqual(cnt, 1)
+
class EnvironmentInstallTest(tests.support.ResultTestCase):
"""Set up a test where sugar is considered not installed."""