diff --git a/pungi/metadata.py b/pungi/metadata.py index 34b9a2be..2bdae802 100644 --- a/pungi/metadata.py +++ b/pungi/metadata.py @@ -17,7 +17,6 @@ import copy import os import time -import json import productmd.composeinfo import productmd.treeinfo @@ -346,43 +345,34 @@ def write_tree_info(compose, arch, variant, timestamp=None, bi=None): ti.dump(path) -def write_extra_files(tree_path, files, checksum_type='sha256', logger=None): +def populate_extra_files_metadata( + metadata, variant, arch, topdir, files, checksum_types, relative_root=None +): """ - Write the metadata for all extra files added to the compose. - - :param tree_path: - Root of the tree to write the ``extra_files.json`` metadata file for. - - :param files: - A list of files that should be included in the metadata file. These - should be paths that are relative to ``tree_path``. - - :return: - Path to the metadata file written. + :param metadata: an instance of productmd.extra_files.ExtraFiles to + populate with the current files + :param Variant variant: under which variant should the files be listed + :param str arch: under which arch should the files be listed + :param topdir: directory where files are located + :param files: list of file paths relative to topdir + :param checksum_types: list of checksums to compute + :param relative_root: ancestor directory of topdir, this will be removed + from paths written to local metadata file """ - metadata_path = os.path.join(tree_path, 'extra_files.json') - if logger: - logger.info('Calculating content of {metadata}'.format(metadata=metadata_path)) - metadata = {'header': {'version': '1.0'}, 'data': []} - for f in files: - if logger: - logger.debug('Processing {file}'.format(file=f)) - path = os.path.join(tree_path, f) + for copied_file in files: + full_path = os.path.join(topdir, copied_file) + size = os.path.getsize(full_path) try: - checksum = compute_file_checksums(path, checksum_type) + checksums = compute_file_checksums(full_path, checksum_types) except IOError as exc: - file = os.path.relpath(exc.filename, '/'.join(tree_path.split('/')[:-3])) - raise RuntimeError('Failed to calculate checksum for %s: %s' % (file, exc.strerror)) - entry = { - 'file': f, - 'checksums': checksum, - 'size': os.path.getsize(path), - } - metadata['data'].append(entry) + raise RuntimeError( + "Failed to calculate checksum for %s: %s" % (full_path, exc) + ) - if logger: - logger.info('Writing {metadata}'.format(metadata=metadata_path)) + if relative_root: + copied_file = os.path.relpath(full_path, relative_root) + metadata.add(variant.uid, arch, copied_file, size, checksums) - with open(metadata_path, 'w') as fd: - json.dump(metadata, fd, sort_keys=True, indent=4, separators=(',', ': ')) - return metadata_path + strip_prefix = (os.path.relpath(topdir, relative_root) + "/") if relative_root else "" + with open(os.path.join(topdir, "extra_files.json"), "w") as f: + metadata.dump_for_tree(f, variant.uid, arch, strip_prefix) diff --git a/pungi/phases/extra_files.py b/pungi/phases/extra_files.py index 38e32d59..efe67945 100644 --- a/pungi/phases/extra_files.py +++ b/pungi/phases/extra_files.py @@ -18,11 +18,13 @@ import os import copy import fnmatch +from productmd.extra_files import ExtraFiles + from pungi.util import get_arch_variant_data, pkg_is_rpm, copy_all from pungi.arch import split_name_arch -from pungi import metadata from pungi.wrappers.scm import get_file_from_scm, get_dir_from_scm from pungi.phases.base import ConfigGuardedPhase +from pungi import metadata class ExtraFilesPhase(ConfigGuardedPhase): @@ -33,6 +35,12 @@ class ExtraFilesPhase(ConfigGuardedPhase): super(ExtraFilesPhase, self).__init__(compose) # pkgset_phase provides package_sets self.pkgset_phase = pkgset_phase + # Prepare metadata + self.metadata = ExtraFiles() + self.metadata.compose.id = self.compose.compose_id + self.metadata.compose.type = self.compose.compose_type + self.metadata.compose.date = self.compose.compose_date + self.metadata.compose.respin = self.compose.compose_respin def run(self): for variant in self.compose.get_variants(): @@ -41,13 +49,26 @@ class ExtraFilesPhase(ConfigGuardedPhase): for arch in variant.arches + ["src"]: 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) + copy_extra_files( + self.compose, + cfg, + arch, + variant, + self.pkgset_phase.package_sets, + self.metadata, + ) else: self.compose.log_info('[SKIP ] No extra files (arch: %s, variant: %s)' % (arch, variant.uid)) + metadata_path = self.compose.paths.compose.metadata("extra_files.json") + self.compose.log_info("Writing global extra files metadata: %s" % metadata_path) + self.metadata.dump(metadata_path) -def copy_extra_files(compose, cfg, arch, variant, package_sets, checksum_type=None): + +def copy_extra_files( + compose, cfg, arch, variant, package_sets, extra_metadata, checksum_type=None +): checksum_type = checksum_type or compose.conf['media_checksums'] var_dict = { "arch": arch, @@ -85,8 +106,15 @@ def copy_extra_files(compose, cfg, arch, variant, package_sets, checksum_type=No getter(scm_dict, target_path, compose=compose) if os.listdir(extra_files_dir): - files_copied = copy_all(extra_files_dir, os_tree) - metadata.write_extra_files(os_tree, files_copied, checksum_type, compose._logger) + metadata.populate_extra_files_metadata( + extra_metadata, + variant, + arch, + os_tree, + copy_all(extra_files_dir, os_tree), + compose.conf["media_checksums"], + relative_root=compose.paths.compose.topdir(), + ) compose.log_info("[DONE ] %s" % msg) diff --git a/pungi/phases/extra_isos.py b/pungi/phases/extra_isos.py index 5ea95793..0a138015 100644 --- a/pungi/phases/extra_isos.py +++ b/pungi/phases/extra_isos.py @@ -18,6 +18,7 @@ import os from kobo.shortcuts import force_list from kobo.threads import ThreadPool, WorkerThread import productmd.treeinfo +from productmd.extra_files import ExtraFiles from pungi import createiso from pungi import metadata @@ -155,11 +156,13 @@ def get_extra_files(compose, variant, arch, extra_files): ) if filelist: - metadata.write_extra_files( + metadata.populate_extra_files_metadata( + ExtraFiles(), + variant, + arch, extra_files_dir, filelist, compose.conf["media_checksums"], - logger=compose._logger, ) diff --git a/tests/test_extra_files_phase.py b/tests/test_extra_files_phase.py index 796dda33..081f3cf1 100644 --- a/tests/test_extra_files_phase.py +++ b/tests/test_extra_files_phase.py @@ -4,6 +4,8 @@ import mock import os import sys +from productmd.extra_files import ExtraFiles + import six sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) @@ -38,23 +40,44 @@ class TestExtraFilePhase(helpers.PungiTestCase): six.assertCountEqual( self, copy_extra_files.call_args_list, - [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)] + [ + mock.call( + compose, + [cfg], + "x86_64", + compose.variants["Server"], + pkgset_phase.package_sets, + phase.metadata, + ), + mock.call( + compose, + [cfg], + "x86_64", + compose.variants["Everything"], + pkgset_phase.package_sets, + phase.metadata, + ), + ], ) + self.assertTrue(isinstance(phase.metadata, ExtraFiles)) class TestCopyFiles(helpers.PungiTestCase): + def setUp(self): + super(TestCopyFiles, self).setUp() + self.metadata = ExtraFiles() + self.compose = helpers.DummyCompose(self.topdir, {}) + self.variant = self.compose.variants["Server"] + def test_copy_local_file(self): tgt = os.path.join(self.topdir, 'file') helpers.touch(tgt) - compose = helpers.DummyCompose(self.topdir, {}) cfg = {'scm': 'file', 'file': tgt, 'repo': None} - extra_files.copy_extra_files(compose, [cfg], 'x86_64', - compose.variants['Server'], mock.Mock()) + extra_files.copy_extra_files( + self.compose, [cfg], "x86_64", self.variant, mock.Mock(), self.metadata + ) self.assertTrue(os.path.isfile(os.path.join( self.topdir, 'compose', 'Server', 'x86_64', 'os', 'file'))) @@ -64,12 +87,17 @@ class TestCopyFiles(helpers.PungiTestCase): 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()) + extra_files.copy_extra_files( + self.compose, + [cfg1, cfg2], + "x86_64", + self.variant, + mock.Mock(), + self.metadata + ) self.assertTrue(os.path.isfile(os.path.join( self.topdir, 'compose', 'Server', 'x86_64', 'os', 'file'))) @@ -79,11 +107,11 @@ class TestCopyFiles(helpers.PungiTestCase): 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, {}) cfg = {'scm': 'file', 'dir': os.path.join(self.topdir, 'src'), 'repo': None, 'target': 'subdir'} - extra_files.copy_extra_files(compose, [cfg], 'x86_64', - compose.variants['Server'], mock.Mock()) + extra_files.copy_extra_files( + self.compose, [cfg], "x86_64", self.variant, mock.Mock(), self.metadata + ) self.assertTrue(os.path.isfile(os.path.join( self.topdir, 'compose', 'Server', 'x86_64', 'os', 'subdir', 'file'))) @@ -93,13 +121,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, {}) 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, [cfg], 'x86_64', - compose.variants['Server'], mock.Mock()) + extra_files.copy_extra_files( + self.compose, [cfg], "x86_64", self.variant, mock.Mock(), self.metadata + ) self.assertEqual(len(get_file_from_scm.call_args_list), 1) self.assertEqual(get_dir_from_scm.call_args_list, []) @@ -112,7 +140,6 @@ 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, {}) 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', @@ -136,8 +163,9 @@ class TestCopyFiles(helpers.PungiTestCase): get_file_from_scm.side_effect = self.fake_get_file - extra_files.copy_extra_files(compose, [cfg], 'x86_64', - compose.variants['Server'], package_sets) + extra_files.copy_extra_files( + self.compose, [cfg], "x86_64", self.variant, package_sets, self.metadata + ) self.assertEqual(len(get_file_from_scm.call_args_list), 1) self.assertEqual(get_dir_from_scm.call_args_list, []) @@ -156,13 +184,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_non_existing_rpm_in_compose(self, get_dir_from_scm, get_file_from_scm): - compose = helpers.DummyCompose(self.topdir, {}) cfg = {'scm': 'rpm', 'file': 'file.txt', 'repo': 'bad-%(variant_uid_lower)s*'} package_sets = [{"x86_64": {}}] with self.assertRaises(RuntimeError) as ctx: extra_files.copy_extra_files( - compose, [cfg], 'x86_64', compose.variants['Server'], package_sets) + self.compose, [cfg], "x86_64", self.variant, package_sets, self.metadata + ) self.assertRegexpMatches( str(ctx.exception), r'No.*package.*matching bad-server\*.*' diff --git a/tests/test_extra_isos_phase.py b/tests/test_extra_isos_phase.py index e6ecc6fd..a59fadd1 100644 --- a/tests/test_extra_isos_phase.py +++ b/tests/test_extra_isos_phase.py @@ -424,7 +424,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): self.assertEqual(aitm.call_args_list, []) -@mock.patch("pungi.metadata.write_extra_files") +@mock.patch("pungi.metadata.populate_extra_files_metadata") @mock.patch('pungi.phases.extra_isos.get_file_from_scm') @mock.patch('pungi.phases.extra_isos.get_dir_from_scm') class GetExtraFilesTest(helpers.PungiTestCase): @@ -438,14 +438,14 @@ class GetExtraFilesTest(helpers.PungiTestCase): self.topdir, "work", self.arch, self.variant.uid, "extra-iso-extra-files" ) - def test_no_config(self, get_dir, get_file, write_extra): + def test_no_config(self, get_dir, get_file, populate_md): extra_isos.get_extra_files(self.compose, self.variant, self.arch, []) self.assertEqual(get_dir.call_args_list, []) self.assertEqual(get_file.call_args_list, []) - self.assertEqual(write_extra.call_args_list, []) + self.assertEqual(populate_md.call_args_list, []) - def test_get_file(self, get_dir, get_file, write_extra): + def test_get_file(self, get_dir, get_file, populate_md): get_file.return_value = ["GPL"] cfg = { 'scm': 'git', @@ -465,18 +465,20 @@ class GetExtraFilesTest(helpers.PungiTestCase): ], ) self.assertEqual( - write_extra.call_args_list, + populate_md.call_args_list, [ mock.call( + mock.ANY, + self.variant, + self.arch, self.dir, ["legalese/GPL"], self.compose.conf["media_checksums"], - logger=self.compose._logger, ) ], ) - def test_get_dir(self, get_dir, get_file, write_extra): + def test_get_dir(self, get_dir, get_file, populate_md): get_dir.return_value = ["a", "b"] cfg = { 'scm': 'git', @@ -489,25 +491,23 @@ class GetExtraFilesTest(helpers.PungiTestCase): self.assertEqual(get_file.call_args_list, []) self.assertEqual( get_dir.call_args_list, - [ - mock.call( - cfg, os.path.join(self.dir, "foo"), compose=self.compose - ) - ], + [mock.call(cfg, os.path.join(self.dir, "foo"), compose=self.compose)], ) self.assertEqual( - write_extra.call_args_list, + populate_md.call_args_list, [ mock.call( + mock.ANY, + self.variant, + self.arch, self.dir, ["foo/a", "foo/b"], self.compose.conf["media_checksums"], - logger=self.compose._logger, - ) + ), ], ) - def test_get_multiple_files(self, get_dir, get_file, write_extra): + def test_get_multiple_files(self, get_dir, get_file, populate_md): get_file.side_effect = [["GPL"], ["setup.py"]] cfg1 = { 'scm': 'git', @@ -531,14 +531,16 @@ class GetExtraFilesTest(helpers.PungiTestCase): ], ) self.assertEqual( - write_extra.call_args_list, + populate_md.call_args_list, [ mock.call( + mock.ANY, + self.variant, + self.arch, self.dir, ["legalese/GPL", "setup.py"], self.compose.conf["media_checksums"], - logger=self.compose._logger, - ) + ), ], ) diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 08d82626..e2f5bed0 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -1,4 +1,3 @@ -import json import mock import os import sys @@ -153,101 +152,67 @@ class MediaRepoTestCase(helpers.PungiTestCase): self.assertFalse(os.path.isfile(self.path)) -class TestWriteExtraFiles(helpers.PungiTestCase): +FOO_MD5 = {"md5": "acbd18db4cc2f85cedef654fccc4a4d8"} +BAR_MD5 = {"md5": "37b51d194a7513e45b56f6524f2d51f2"} + + +class TestPopulateExtraFiles(helpers.PungiTestCase): def setUp(self): - super(TestWriteExtraFiles, self).setUp() - self.compose = helpers.DummyCompose(self.topdir, {}) + super(TestPopulateExtraFiles, self).setUp() + self.variant = mock.Mock(uid="Server") + self.metadata = mock.Mock() - def test_write_extra_files(self): - """Assert metadata is written to the proper location with valid data""" - mock_logger = mock.Mock() - files = ['file1', 'file2', 'subdir/file3'] - expected_metadata = { - u'header': {u'version': u'1.0'}, - u'data': [ - { - u'file': u'file1', - u'checksums': {u'sha256': u'ecdc5536f73bdae8816f0ea40726ef5e9b810d914493075903bb90623d97b1d8'}, - u'size': 6, - }, - { - u'file': u'file2', - u'checksums': {u'sha256': u'67ee5478eaadb034ba59944eb977797b49ca6aa8d3574587f36ebcbeeb65f70e'}, - u'size': 6, - }, - { - u'file': u'subdir/file3', - u'checksums': {u'sha256': u'52f9f0e467e33da811330cad085fdb4eaa7abcb9ebfe6001e0f5910da678be51'}, - u'size': 13, - }, - ] - } - tree_dir = os.path.join(self.topdir, 'compose', 'Server', 'x86_64', 'os') - for f in files: - helpers.touch(os.path.join(tree_dir, f), f + '\n') + def test_with_relative_root(self): + helpers.touch( + os.path.join(self.topdir, "compose/Server/x86_64/os/foo"), content="foo" + ) + helpers.touch( + os.path.join(self.topdir, "compose/Server/x86_64/os/bar"), content="bar" + ) - metadata_file = metadata.write_extra_files(tree_dir, files, logger=mock_logger) - with open(metadata_file) as metadata_fd: - actual_metadata = json.load(metadata_fd) + metadata.populate_extra_files_metadata( + self.metadata, + self.variant, + "x86_64", + os.path.join(self.topdir, "compose/Server/x86_64/os"), + ["foo", "bar"], + ["md5"], + relative_root=os.path.join(self.topdir, "compose"), + ) - self.assertEqual(expected_metadata['header'], actual_metadata['header']) - self.assertEqual(expected_metadata['data'], actual_metadata['data']) - - def test_write_extra_files_multiple_checksums(self): - """Assert metadata is written to the proper location with valid data""" self.maxDiff = None - mock_logger = mock.Mock() - files = ['file1', 'file2', 'subdir/file3'] - expected_metadata = { - u'header': {u'version': u'1.0'}, - u'data': [ - { - u'file': u'file1', - u'checksums': { - u'md5': u'5149d403009a139c7e085405ef762e1a', - u'sha256': u'ecdc5536f73bdae8816f0ea40726ef5e9b810d914493075903bb90623d97b1d8' - }, - u'size': 6, - }, - { - u'file': u'file2', - u'checksums': { - u'md5': u'3d709e89c8ce201e3c928eb917989aef', - u'sha256': u'67ee5478eaadb034ba59944eb977797b49ca6aa8d3574587f36ebcbeeb65f70e' - }, - u'size': 6, - }, - { - u'file': u'subdir/file3', - u'checksums': { - u'md5': u'1ed02b5cf7fd8626f854e9ef3fee8694', - u'sha256': u'52f9f0e467e33da811330cad085fdb4eaa7abcb9ebfe6001e0f5910da678be51' - }, - u'size': 13, - }, - ] - } - tree_dir = os.path.join(self.topdir, 'compose', 'Server', 'x86_64', 'os') - for f in files: - helpers.touch(os.path.join(tree_dir, f), f + '\n') - metadata_file = metadata.write_extra_files(tree_dir, files, - checksum_type=['md5', 'sha256'], - logger=mock_logger) - with open(metadata_file) as metadata_fd: - actual_metadata = json.load(metadata_fd) + six.assertCountEqual( + self, + self.metadata.mock_calls, + [ + mock.call.add( + "Server", "x86_64", "Server/x86_64/os/foo", 3, FOO_MD5 + ), + mock.call.add( + "Server", "x86_64", "Server/x86_64/os/bar", 3, BAR_MD5 + ), + mock.call.dump_for_tree( + mock.ANY, "Server", "x86_64", "Server/x86_64/os/" + ), + ], + ) - self.assertEqual(expected_metadata['header'], actual_metadata['header']) - self.assertEqual(expected_metadata['data'], actual_metadata['data']) + def test_without_relative_root(self): + helpers.touch(os.path.join(self.topdir, "foo"), content="foo") + helpers.touch(os.path.join(self.topdir, "bar"), content="bar") - def test_write_extra_files_missing_file(self): - """Assert metadata is written to the proper location with valid data""" - mock_logger = mock.Mock() - files = ['file1', 'file2', 'subdir/file3'] - tree_dir = os.path.join(self.topdir, 'compose', 'Server', 'x86_64', 'os') - for f in files: - helpers.touch(os.path.join(tree_dir, f), f + '\n') - files.append('missing_file') + metadata.populate_extra_files_metadata( + self.metadata, self.variant, "x86_64", self.topdir, ["foo", "bar"], ["md5"] + ) - self.assertRaises(RuntimeError, metadata.write_extra_files, tree_dir, files, 'sha256', mock_logger) + six.assertCountEqual( + self, + self.metadata.mock_calls, + [ + mock.call.add("Server", "x86_64", "foo", 3, FOO_MD5), + mock.call.add("Server", "x86_64", "bar", 3, BAR_MD5), + mock.call.dump_for_tree(mock.ANY, "Server", "x86_64", ""), + ], + )