init: Always filter comps file

Even for Everything we want to filter the comps file to make sure we
remove the stuff that is not compatible with current arch. All groups
are still preserved in that case.

This allows us to do the filtering once in init phase than just use the
prepared file in comps source.

Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>

pungi/phases/gather/sources/source_comps.py

@@ -37,17 +37,7 @@ class GatherSourceComps(pungi.phases.gather.source.GatherSourceBase):
         if not self.compose.conf.get('comps_file'):
             return set(), set()
 
-        comps = CompsWrapper(self.compose.paths.work.comps(arch=arch))
+        comps = CompsWrapper(self.compose.paths.work.comps(arch=arch, variant=variant))
-
-        is_modular = variant and not variant.groups and variant.modules is not None
-        if variant is not None and (variant.groups or variant.type != 'variant' or is_modular):
-            # Get packages for a particular variant. We want to skip the
-            # filtering if the variant is top-level and has no groups (to use
-            # all of them).
-            # For optional we always want to filter (and parent groups will be
-            # added later), for addons and layered products it makes no sense
-            # for the groups to be empty in the first place.
-            comps.filter_groups(variant.groups)
 
         for i in comps.get_comps_groups():
             groups.add(i)

pungi/phases/init.py

@@ -45,21 +45,8 @@ class InitPhase(PhaseBase):
 
             # write variant comps
             for variant in self.compose.get_variants():
-                is_modular = not variant.groups and variant.modules
                 for arch in variant.arches:
-                    if variant.groups or variant.type == 'optional' or is_modular:
-                        # The variant lists only some groups, run filter. Other
-                        # option is that it's optional variant, in which case
-                        # we want to filter everything (unless there was
-                        # explicit list in which case it will be used).
-                        # For fully modular variant (one without groups but
-                        # with modules) we also want to filter (effectively
-                        # producing empty comps).
                     write_variant_comps(self.compose, arch, variant)
-                    else:
-                        # The variant does not mention any groups, copy
-                        # original file.
-                        copy_variant_comps(self.compose, arch, variant)
 
         # download variants.xml / product.xml?
 
@@ -128,6 +115,9 @@ def write_variant_comps(compose, arch, variant):
          "--output=%s" % comps_file, compose.paths.work.comps(arch="global")])
 
     comps = CompsWrapper(comps_file)
+    if variant.groups or variant.modules is not None or variant.type != 'variant':
+        # Filter groups if the variant has some, or it's a modular variant, or
+        # is not a base variant.
         unmatched = comps.filter_groups(variant.groups)
         for grp in unmatched:
             compose.log_warning(UNMATCHED_GROUP_MSG % (variant.uid, arch, grp))
@@ -136,14 +126,6 @@ def write_variant_comps(compose, arch, variant):
     comps.write_comps()
 
 
-def copy_variant_comps(compose, arch, variant):
-    global_comps = compose.paths.work.comps(arch="global")
-    comps_file = compose.paths.work.comps(arch=arch, variant=variant)
-    msg = "Copying original comps file (arch: %s, variant: %s): %s" % (arch, variant, comps_file)
-    compose.log_debug(msg)
-    shutil.copy(global_comps, comps_file)
-
-
 def create_comps_repo(compose, arch):
     createrepo_c = compose.conf["createrepo_c"]
     createrepo_checksum = compose.conf["createrepo_checksum"]

tests/test_initphase.py

@@ -42,14 +42,13 @@ class TestInitPhase(PungiTestCase):
                                mock.call(compose, 'amd64', compose.variants['Everything']),
                                mock.call(compose, 'x86_64', compose.all_variants['Server-optional'])])
 
-    @mock.patch('pungi.phases.init.copy_variant_comps')
     @mock.patch('pungi.phases.init.write_global_comps')
     @mock.patch('pungi.phases.init.write_arch_comps')
     @mock.patch('pungi.phases.init.create_comps_repo')
     @mock.patch('pungi.phases.init.write_variant_comps')
     @mock.patch('pungi.phases.init.write_prepopulate_file')
     def test_run_with_preserve(self, write_prepopulate, write_variant, create_comps,
-                               write_arch, write_global, copy_comps):
+                               write_arch, write_global):
         compose = DummyCompose(self.topdir, {})
         compose.has_comps = True
         compose.variants['Everything'].groups = []
@@ -66,19 +65,17 @@ class TestInitPhase(PungiTestCase):
         self.assertItemsEqual(write_variant.mock_calls,
                               [mock.call(compose, 'x86_64', compose.variants['Server']),
                                mock.call(compose, 'amd64', compose.variants['Server']),
-                               mock.call(compose, 'amd64', compose.variants['Client'])])
+                               mock.call(compose, 'amd64', compose.variants['Client']),
-        self.assertItemsEqual(copy_comps.mock_calls,
+                               mock.call(compose, 'x86_64', compose.variants['Everything']),
-                              [mock.call(compose, 'x86_64', compose.variants['Everything']),
                                mock.call(compose, 'amd64', compose.variants['Everything'])])
 
-    @mock.patch('pungi.phases.init.copy_variant_comps')
     @mock.patch('pungi.phases.init.write_global_comps')
     @mock.patch('pungi.phases.init.write_arch_comps')
     @mock.patch('pungi.phases.init.create_comps_repo')
     @mock.patch('pungi.phases.init.write_variant_comps')
     @mock.patch('pungi.phases.init.write_prepopulate_file')
     def test_run_without_comps(self, write_prepopulate, write_variant, create_comps,
-                               write_arch, write_global, copy_comps):
+                               write_arch, write_global):
         compose = DummyCompose(self.topdir, {})
         compose.has_comps = False
         phase = init.InitPhase(compose)
@@ -89,21 +86,6 @@ class TestInitPhase(PungiTestCase):
         self.assertItemsEqual(write_arch.mock_calls, [])
         self.assertItemsEqual(create_comps.mock_calls, [])
         self.assertItemsEqual(write_variant.mock_calls, [])
-        self.assertItemsEqual(copy_comps.mock_calls, [])
-
-
-class TestCopyVariantComps(PungiTestCase):
-
-    @mock.patch('shutil.copy')
-    def test_run(self, copy):
-        compose = DummyCompose(self.topdir, {})
-        variant = compose.variants['Server']
-
-        init.copy_variant_comps(compose, 'x86_64', variant)
-
-        self.assertEqual(copy.mock_calls,
-                         [mock.call(self.topdir + '/work/global/comps/comps-global.xml',
-                                    self.topdir + '/work/x86_64/comps/comps-Server.x86_64.xml')])
 
 
 class TestWriteArchComps(PungiTestCase):
@@ -219,6 +201,55 @@ class TestWriteVariantComps(PungiTestCase):
                          [mock.call(variant.environments)])
         self.assertEqual(comps.write_comps.mock_calls, [mock.call()])
 
+    @mock.patch('pungi.phases.init.run')
+    @mock.patch('pungi.phases.init.CompsWrapper')
+    def test_run_no_filter_without_groups(self, CompsWrapper, run):
+        compose = DummyCompose(self.topdir, {})
+        compose.DEBUG = False
+        variant = compose.variants['Server']
+        variant.groups = []
+        comps = CompsWrapper.return_value
+        comps.filter_groups.return_value = []
+
+        init.write_variant_comps(compose, 'x86_64', variant)
+
+        self.assertEqual(run.mock_calls,
+                         [mock.call(['comps_filter', '--arch=x86_64', '--keep-empty-group=conflicts',
+                                     '--keep-empty-group=conflicts-server',
+                                     '--output=%s/work/x86_64/comps/comps-Server.x86_64.xml' % self.topdir,
+                                     self.topdir + '/work/global/comps/comps-global.xml'])])
+        self.assertEqual(CompsWrapper.call_args_list,
+                         [mock.call(self.topdir + '/work/x86_64/comps/comps-Server.x86_64.xml')])
+        self.assertEqual(comps.filter_groups.call_args_list, [])
+        self.assertEqual(comps.filter_environments.mock_calls,
+                         [mock.call(variant.environments)])
+        self.assertEqual(comps.write_comps.mock_calls, [mock.call()])
+
+    @mock.patch('pungi.phases.init.run')
+    @mock.patch('pungi.phases.init.CompsWrapper')
+    def test_run_filter_for_modular(self, CompsWrapper, run):
+        compose = DummyCompose(self.topdir, {})
+        compose.DEBUG = False
+        variant = compose.variants['Server']
+        variant.groups = []
+        variant.modules = ['testmodule:2.0']
+        comps = CompsWrapper.return_value
+        comps.filter_groups.return_value = []
+
+        init.write_variant_comps(compose, 'x86_64', variant)
+
+        self.assertEqual(run.mock_calls,
+                         [mock.call(['comps_filter', '--arch=x86_64', '--keep-empty-group=conflicts',
+                                     '--keep-empty-group=conflicts-server',
+                                     '--output=%s/work/x86_64/comps/comps-Server.x86_64.xml' % self.topdir,
+                                     self.topdir + '/work/global/comps/comps-global.xml'])])
+        self.assertEqual(CompsWrapper.call_args_list,
+                         [mock.call(self.topdir + '/work/x86_64/comps/comps-Server.x86_64.xml')])
+        self.assertEqual(comps.filter_groups.call_args_list, [mock.call([])])
+        self.assertEqual(comps.filter_environments.mock_calls,
+                         [mock.call(variant.environments)])
+        self.assertEqual(comps.write_comps.mock_calls, [mock.call()])
+
     @mock.patch('pungi.phases.init.run')
     @mock.patch('pungi.phases.init.CompsWrapper')
     def test_run_report_unmatched(self, CompsWrapper, run):