From 8f1beeb54b7b86172a2736c63c85799bcce5aa8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Thu, 5 Apr 2018 15:16:06 +0200 Subject: [PATCH] init: Always filter comps file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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ář --- pungi/phases/gather/sources/source_comps.py | 12 +--- pungi/phases/init.py | 32 ++------- tests/test_initphase.py | 75 +++++++++++++++------ 3 files changed, 61 insertions(+), 58 deletions(-) diff --git a/pungi/phases/gather/sources/source_comps.py b/pungi/phases/gather/sources/source_comps.py index 0b4a87d5..b60885c8 100644 --- a/pungi/phases/gather/sources/source_comps.py +++ b/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)) - - 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) + comps = CompsWrapper(self.compose.paths.work.comps(arch=arch, variant=variant)) for i in comps.get_comps_groups(): groups.add(i) diff --git a/pungi/phases/init.py b/pungi/phases/init.py index a01168a9..5d2b042a 100644 --- a/pungi/phases/init.py +++ b/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) + write_variant_comps(self.compose, arch, variant) # download variants.xml / product.xml? @@ -128,22 +115,17 @@ def write_variant_comps(compose, arch, variant): "--output=%s" % comps_file, compose.paths.work.comps(arch="global")]) comps = CompsWrapper(comps_file) - unmatched = comps.filter_groups(variant.groups) - for grp in unmatched: - compose.log_warning(UNMATCHED_GROUP_MSG % (variant.uid, arch, grp)) + 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)) if compose.conf["comps_filter_environments"]: comps.filter_environments(variant.environments) 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"] diff --git a/tests/test_initphase.py b/tests/test_initphase.py index ae6c4e52..d4a1e0bc 100644 --- a/tests/test_initphase.py +++ b/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'])]) - self.assertItemsEqual(copy_comps.mock_calls, - [mock.call(compose, 'x86_64', compose.variants['Everything']), + mock.call(compose, 'amd64', compose.variants['Client']), + 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):