richardphibel / rpms / dnf

Forked from rpms/dnf 2 years ago
Clone
Blob Blame History Raw
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."""