From 8e4b1e278c13e3f3e255e850a0191485ebe0ff8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Thu, 12 May 2016 14:19:14 +0200 Subject: [PATCH] [extra-files] Only copy files when there is a config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of running the copy function for all variant.arch pairs unconditionally, only do it if there is something to do. This makes the log more understandable. Signed-off-by: Lubomír Sedlář --- pungi/phases/extra_files.py | 11 +++-- tests/test_extra_files_phase.py | 86 +++++++++++++++------------------ 2 files changed, 47 insertions(+), 50 deletions(-) diff --git a/pungi/phases/extra_files.py b/pungi/phases/extra_files.py index ec3ccfaa..252d90a1 100644 --- a/pungi/phases/extra_files.py +++ b/pungi/phases/extra_files.py @@ -45,10 +45,15 @@ class ExtraFilesPhase(ConfigGuardedPhase): def run(self): for arch in self.compose.get_arches() + ["src"]: for variant in self.compose.get_variants(arch=arch): - copy_extra_files(self.compose, arch, variant, self.pkgset_phase.package_sets) + cfg = get_arch_variant_data(self.compose.conf, self.name, arch, variant) + if cfg: + copy_extra_files(self.compose, cfg, arch, variant, self.pkgset_phase.package_sets) + else: + self.compose.log_info('[SKIP ] No extra files (arch: %s, variant: %s)' + % (arch, variant.uid)) -def copy_extra_files(compose, arch, variant, package_sets): +def copy_extra_files(compose, cfg, arch, variant, package_sets): var_dict = { "arch": arch, "variant_id": variant.id, @@ -63,7 +68,7 @@ def copy_extra_files(compose, arch, variant, package_sets): os_tree = compose.paths.compose.os_tree(arch, variant) extra_files_dir = compose.paths.work.extra_files_dir(arch, variant) - for scm_dict in get_arch_variant_data(compose.conf, "extra_files", arch, variant): + for scm_dict in cfg: scm_dict = copy.deepcopy(scm_dict) # if scm is "rpm" and repo contains only a package name, find the # package(s) in package set diff --git a/tests/test_extra_files_phase.py b/tests/test_extra_files_phase.py index f8652f50..e29c57e8 100755 --- a/tests/test_extra_files_phase.py +++ b/tests/test_extra_files_phase.py @@ -28,7 +28,7 @@ class TestExtraFilePhase(helpers.PungiTestCase): pkgset_phase = mock.Mock() compose = helpers.DummyCompose(self.topdir, { 'extra_files': [ - ('^.+$', {'*': cfg}) + ('^.+$', {'x86_64': [cfg]}) ] }) @@ -37,11 +37,10 @@ class TestExtraFilePhase(helpers.PungiTestCase): self.assertItemsEqual( copy_extra_files.call_args_list, - [mock.call(compose, 'x86_64', compose.variants['Server'], pkgset_phase.package_sets), - mock.call(compose, 'amd64', compose.variants['Server'], pkgset_phase.package_sets), - mock.call(compose, 'amd64', compose.variants['Client'], pkgset_phase.package_sets), - mock.call(compose, 'x86_64', compose.variants['Everything'], pkgset_phase.package_sets), - mock.call(compose, 'amd64', compose.variants['Everything'], pkgset_phase.package_sets)] + [mock.call(compose, [cfg], 'x86_64', compose.variants['Server'], + pkgset_phase.package_sets), + mock.call(compose, [cfg], 'x86_64', compose.variants['Everything'], + pkgset_phase.package_sets)] ) @@ -50,36 +49,41 @@ class TestCopyFiles(helpers.PungiTestCase): def test_copy_local_file(self): tgt = os.path.join(self.topdir, 'file') helpers.touch(tgt) - compose = helpers.DummyCompose(self.topdir, { - 'extra_files': [ - ('^.+$', {'*': { - 'scm': 'file', - 'file': tgt, - 'repo': None, - }}) - ] - }) + compose = helpers.DummyCompose(self.topdir, {}) + cfg = {'scm': 'file', 'file': tgt, 'repo': None} - extra_files.copy_extra_files(compose, 'x86_64', compose.variants['Server'], mock.Mock()) + extra_files.copy_extra_files(compose, [cfg], 'x86_64', + compose.variants['Server'], mock.Mock()) self.assertTrue(os.path.isfile(os.path.join( self.topdir, 'compose', 'Server', 'x86_64', 'os', 'file'))) + def test_copy_multiple_sources(self): + tgt1 = os.path.join(self.topdir, 'file') + tgt2 = os.path.join(self.topdir, 'gpl') + helpers.touch(tgt1) + helpers.touch(tgt2) + compose = helpers.DummyCompose(self.topdir, {}) + cfg1 = {'scm': 'file', 'file': tgt1, 'repo': None} + cfg2 = {'scm': 'file', 'file': tgt2, 'repo': None, 'target': 'license'} + + extra_files.copy_extra_files(compose, [cfg1, cfg2], 'x86_64', + compose.variants['Server'], mock.Mock()) + + self.assertTrue(os.path.isfile(os.path.join( + self.topdir, 'compose', 'Server', 'x86_64', 'os', 'file'))) + self.assertTrue(os.path.isfile(os.path.join( + self.topdir, 'compose', 'Server', 'x86_64', 'os', 'license', 'gpl'))) + def test_copy_local_dir(self): helpers.touch(os.path.join(self.topdir, 'src', 'file')) helpers.touch(os.path.join(self.topdir, 'src', 'another')) - compose = helpers.DummyCompose(self.topdir, { - 'extra_files': [ - ('^.+$', {'*': { - 'scm': 'file', - 'dir': os.path.join(self.topdir, 'src'), - 'repo': None, - 'target': 'subdir', - }}) - ] - }) + compose = helpers.DummyCompose(self.topdir, {}) + cfg = {'scm': 'file', 'dir': os.path.join(self.topdir, 'src'), + 'repo': None, 'target': 'subdir'} - extra_files.copy_extra_files(compose, 'x86_64', compose.variants['Server'], mock.Mock()) + extra_files.copy_extra_files(compose, [cfg], 'x86_64', + compose.variants['Server'], mock.Mock()) self.assertTrue(os.path.isfile(os.path.join( self.topdir, 'compose', 'Server', 'x86_64', 'os', 'subdir', 'file'))) @@ -89,19 +93,13 @@ class TestCopyFiles(helpers.PungiTestCase): @mock.patch('pungi.phases.extra_files.get_file_from_scm') @mock.patch('pungi.phases.extra_files.get_dir_from_scm') def test_copy_from_external_rpm(self, get_dir_from_scm, get_file_from_scm): - compose = helpers.DummyCompose(self.topdir, { - 'extra_files': [ - ('^.+$', {'*': { - 'scm': 'rpm', - 'file': 'file.txt', - 'repo': 'http://example.com/package.rpm', - }}) - ] - }) + compose = helpers.DummyCompose(self.topdir, {}) + cfg = {'scm': 'rpm', 'file': 'file.txt', 'repo': 'http://example.com/package.rpm'} get_file_from_scm.side_effect = self.fake_get_file - extra_files.copy_extra_files(compose, 'x86_64', compose.variants['Server'], mock.Mock()) + extra_files.copy_extra_files(compose, [cfg], 'x86_64', + compose.variants['Server'], mock.Mock()) self.assertEqual(len(get_file_from_scm.call_args_list), 1) self.assertEqual(get_dir_from_scm.call_args_list, []) @@ -114,15 +112,8 @@ class TestCopyFiles(helpers.PungiTestCase): @mock.patch('pungi.phases.extra_files.get_file_from_scm') @mock.patch('pungi.phases.extra_files.get_dir_from_scm') def test_copy_from_rpm_in_compose(self, get_dir_from_scm, get_file_from_scm): - compose = helpers.DummyCompose(self.topdir, { - 'extra_files': [ - ('^.+$', {'*': { - 'scm': 'rpm', - 'file': 'file.txt', - 'repo': '%(variant_uid)s-data*', - }}) - ] - }) + compose = helpers.DummyCompose(self.topdir, {}) + cfg = {'scm': 'rpm', 'file': 'file.txt', 'repo': '%(variant_uid)s-data*'} server_po, client_po, src_po = mock.Mock(), mock.Mock(), mock.Mock() server_po.configure_mock(name='Server-data-1.1-1.fc24.x86_64.rpm', file_path='/server/location', @@ -141,7 +132,8 @@ class TestCopyFiles(helpers.PungiTestCase): get_file_from_scm.side_effect = self.fake_get_file - extra_files.copy_extra_files(compose, 'x86_64', compose.variants['Server'], package_sets) + extra_files.copy_extra_files(compose, [cfg], 'x86_64', + compose.variants['Server'], package_sets) self.assertEqual(len(get_file_from_scm.call_args_list), 1) self.assertEqual(get_dir_from_scm.call_args_list, [])