From 539736a11e9a5837b9b2d48724f55b2776f6bb74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Thu, 3 Dec 2015 08:39:33 +0100 Subject: [PATCH 1/2] Use lowercase hashed directories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On case-insensitive filesystems it is not such a good idea to have directories that only differ in case. Packages should be always split into lowercased directories. The test data is modified to include some packages starting with uppercase letters. The example in code can be verified by running `nosetests --with-doctest`. Signed-off-by: Lubomír Sedlář --- pungi/phases/gather/link.py | 12 +++++++++++- tests/data/dummy-comps.xml | 2 +- .../specs/{dummy-firefox.spec => Dummy-firefox.spec} | 6 +++--- .../{dummy-xulrunner.spec => Dummy-xulrunner.spec} | 2 +- 4 files changed, 16 insertions(+), 6 deletions(-) rename tests/data/specs/{dummy-firefox.spec => Dummy-firefox.spec} (85%) rename tests/data/specs/{dummy-xulrunner.spec => Dummy-xulrunner.spec} (94%) diff --git a/pungi/phases/gather/link.py b/pungi/phases/gather/link.py index d2af6779..4eba3c12 100644 --- a/pungi/phases/gather/link.py +++ b/pungi/phases/gather/link.py @@ -39,8 +39,18 @@ def _get_src_nevra(compose, pkg_obj, srpm_map): def get_package_path(filename, hashed_directory=False): + """Get path for filename. If ``hashed_directory`` is ``True``, the path + will include a prefix based on the initial letter. + + >>> get_package_path('my-package.rpm') + 'my-package.rpm' + >>> get_package_path('my-package.rpm', True) + 'm/my-package.rpm' + >>> get_package_path('My-Package.rpm', True) + 'm/My-Package.rpm' + """ if hashed_directory: - prefix = filename[0] + prefix = filename[0].lower() return os.path.join(prefix, filename) return filename diff --git a/tests/data/dummy-comps.xml b/tests/data/dummy-comps.xml index 9bb65950..179635e7 100644 --- a/tests/data/dummy-comps.xml +++ b/tests/data/dummy-comps.xml @@ -45,7 +45,7 @@ false false - dummy-firefox + Dummy-firefox dummy-icedtea-web diff --git a/tests/data/specs/dummy-firefox.spec b/tests/data/specs/Dummy-firefox.spec similarity index 85% rename from tests/data/specs/dummy-firefox.spec rename to tests/data/specs/Dummy-firefox.spec index 8c161696..29457ec7 100644 --- a/tests/data/specs/dummy-firefox.spec +++ b/tests/data/specs/Dummy-firefox.spec @@ -1,11 +1,11 @@ -Name: dummy-firefox +Name: Dummy-firefox Version: 16.0.1 Release: 1 License: LGPLv2 Summary: A dummy firefox package BuildRequires: dummy-krb5-devel -BuildRequires: dummy-xulrunner -Requires: dummy-xulrunner +BuildRequires: Dummy-xulrunner +Requires: Dummy-xulrunner %description A dummy firefox package diff --git a/tests/data/specs/dummy-xulrunner.spec b/tests/data/specs/Dummy-xulrunner.spec similarity index 94% rename from tests/data/specs/dummy-xulrunner.spec rename to tests/data/specs/Dummy-xulrunner.spec index 3a7f0fab..d5c5a762 100644 --- a/tests/data/specs/dummy-xulrunner.spec +++ b/tests/data/specs/Dummy-xulrunner.spec @@ -1,4 +1,4 @@ -Name: dummy-xulrunner +Name: Dummy-xulrunner Version: 16.0.1 Release: 1 License: LGPLv2 From b6d9b5632ee4f0f5e9058de987e06966508c287d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Thu, 3 Dec 2015 09:03:50 +0100 Subject: [PATCH 2/2] Fix generating checksum files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch modifies how checksums are stored - it uses BSD-style checksums. The filename with the checksum can now be customized depending on actual compose run and metadata. This required adding another option to the checksumming phase. Documentation is updated and includes example for creating names used in Fedora. Signed-off-by: Lubomír Sedlář --- doc/configuration.rst | 14 +++++ pungi/phases/image_checksum.py | 54 +++++++++++------- tests/test_imagechecksumphase.py | 95 +++++++++++++++++++++++++------- 3 files changed, 123 insertions(+), 40 deletions(-) diff --git a/doc/configuration.rst b/doc/configuration.rst index 414cee3c..facf40ad 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -535,6 +535,20 @@ Media Checksums Settings directory; this option requires ``media_checksums`` to only specify one type +**media_checksum_base_filename** + (*str*) -- when not set, all checksums will be save to a file named either + ``CHECKSUM`` or based on the digest type; this option allows adding any + prefix to that name + + It is possible to use format strings that will be replace by actual values. + The allowed keys are ``%(release_showrt)s``, ``%(release_short)s``, + ``%(release_id)s``, ``%(variant)s``, ``%(version)s``, ``%(date)s``, + ``%(type_suffix)s`` and ``%(respin)s`` + + For example, for Fedora the prefix should be + ``%(release_short)s-%(variant)s-%(version)s-%(date)s%(type_suffix)s.%(respin)s``. + + Translate Paths Settings ======================== diff --git a/pungi/phases/image_checksum.py b/pungi/phases/image_checksum.py index 2efe4bc4..4c8735d9 100644 --- a/pungi/phases/image_checksum.py +++ b/pungi/phases/image_checksum.py @@ -29,6 +29,11 @@ class ImageChecksumPhase(PhaseBase): "name": "media_checksum_one_file", "expected_types": [bool], "optional": True, + }, + { + "name": "media_checksum_base_filename", + "expected_types": [str], + "optional": True, } ) @@ -61,12 +66,28 @@ class ImageChecksumPhase(PhaseBase): for arch in self.compose.im.images[variant]: for image in self.compose.im.images[variant][arch]: path = os.path.dirname(os.path.join(top_dir, image.path)) - images.setdefault(path, set()).add(image) + images.setdefault((variant, path), set()).add(image) return images + def _get_base_filename(self, variant): + base_checksum_name = self.compose.conf.get('media_checksum_base_filename', '') + if base_checksum_name: + base_checksum_name = base_checksum_name % { + 'release_short': self.compose.ci_base.release.short, + 'release_id': self.compose.ci_base.release_id, + 'variant': variant, + 'version': self.compose.ci_base.release.version, + 'date': self.compose.compose_date, + 'type_suffix': self.compose.compose_type_suffix, + 'respin': self.compose.compose_respin, + } + base_checksum_name += '-' + return base_checksum_name + def run(self): - for path, images in self._get_images().iteritems(): + for (variant, path), images in self._get_images().iteritems(): checksums = {} + base_checksum_name = self._get_base_filename(variant) for image in images: filename = os.path.basename(image.path) full_path = os.path.join(path, filename) @@ -78,37 +99,32 @@ class ImageChecksumPhase(PhaseBase): checksums.setdefault(checksum, {})[filename] = digest image.add_checksum(None, checksum, digest) if not self.one_file: - dump_individual(full_path, digest, checksum) + dump_checksums(path, checksum, + {filename: digest}, + '%s.%sSUM' % (filename, checksum.upper())) if not checksums: continue if self.one_file: - dump_checksums(path, checksums[self.checksums[0]]) + dump_checksums(path, self.checksums[0], + checksums[self.checksums[0]], + base_checksum_name + 'CHECKSUM') else: for checksum in self.checksums: - dump_checksums(path, checksums[checksum], '%sSUM' % checksum.upper()) + dump_checksums(path, checksum, + checksums[checksum], + '%s%sSUM' % (base_checksum_name, checksum.upper())) -def dump_checksums(dir, checksums, filename='CHECKSUM'): +def dump_checksums(dir, alg, checksums, filename): """Create file with checksums. :param dir: where to put the file + :param alg: which method was used :param checksums: mapping from filenames to checksums :param filename: what to call the file """ with open(os.path.join(dir, filename), 'w') as f: for file, checksum in checksums.iteritems(): - f.write('{} *{}\n'.format(checksum, file)) - - -def dump_individual(path, checksum, ext): - """Create a file with a single checksum, saved into a file with an extra - extension. - - :param path: path to the checksummed file - :param checksum: the actual digest value - :param ext: what extension to add to the checksum file - """ - with open('%s.%sSUM' % (path, ext.upper()), 'w') as f: - f.write('{} *{}\n'.format(checksum, os.path.basename(path))) + f.write('%s (%s) = %s\n' % (alg.upper(), file, checksum)) diff --git a/tests/test_imagechecksumphase.py b/tests/test_imagechecksumphase.py index b98222c3..a4482c24 100755 --- a/tests/test_imagechecksumphase.py +++ b/tests/test_imagechecksumphase.py @@ -13,12 +13,21 @@ import shutil sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) from pungi.phases.image_checksum import (ImageChecksumPhase, - dump_checksums, - dump_individual) + dump_checksums) class _DummyCompose(object): def __init__(self, config): + self.compose_date = '20151203' + self.compose_type_suffix = '.t' + self.compose_respin = 0 + self.ci_base = mock.Mock( + release_id='Test-1.0', + release=mock.Mock( + short='test', + version='1.0', + ), + ) self.conf = config self.paths = mock.Mock( compose=mock.Mock( @@ -59,15 +68,14 @@ class TestImageChecksumPhase(unittest.TestCase): phase.run() - dump.assert_called_once_with('/a/b/Client/i386/iso', {'image.iso': 'cafebabe'}) + dump.assert_called_once_with('/a/b/Client/i386/iso', 'sha256', {'image.iso': 'cafebabe'}, 'CHECKSUM') cc.assert_called_once_with('/a/b/Client/i386/iso/image.iso', ['sha256']) compose.image.add_checksum.assert_called_once_with(None, 'sha256', 'cafebabe') @mock.patch('os.path.exists') @mock.patch('kobo.shortcuts.compute_file_checksums') @mock.patch('pungi.phases.image_checksum.dump_checksums') - @mock.patch('pungi.phases.image_checksum.dump_individual') - def test_checksum_save_individuals(self, indiv_dump, dump, cc, exists): + def test_checksum_save_individuals(self, dump, cc, exists): compose = _DummyCompose({ 'media_checksums': ['md5', 'sha256'], }) @@ -79,14 +87,64 @@ class TestImageChecksumPhase(unittest.TestCase): phase.run() - indiv_dump.assert_has_calls( - [mock.call('/a/b/Client/i386/iso/image.iso', 'cafebabe', 'md5'), - mock.call('/a/b/Client/i386/iso/image.iso', 'deadbeef', 'sha256')], + dump.assert_has_calls( + [mock.call('/a/b/Client/i386/iso', 'md5', {'image.iso': 'cafebabe'}, 'image.iso.MD5SUM'), + mock.call('/a/b/Client/i386/iso', 'sha256', {'image.iso': 'deadbeef'}, 'image.iso.SHA256SUM'), + mock.call('/a/b/Client/i386/iso', 'md5', {'image.iso': 'cafebabe'}, 'MD5SUM'), + mock.call('/a/b/Client/i386/iso', 'sha256', {'image.iso': 'deadbeef'}, 'SHA256SUM')], any_order=True ) + cc.assert_called_once_with('/a/b/Client/i386/iso/image.iso', ['md5', 'sha256']) + compose.image.add_checksum.assert_has_calls([mock.call(None, 'sha256', 'deadbeef'), + mock.call(None, 'md5', 'cafebabe')], + any_order=True) + + @mock.patch('os.path.exists') + @mock.patch('kobo.shortcuts.compute_file_checksums') + @mock.patch('pungi.phases.image_checksum.dump_checksums') + def test_checksum_one_file_custom_name(self, dump, cc, exists): + compose = _DummyCompose({ + 'media_checksums': ['sha256'], + 'media_checksum_one_file': True, + 'media_checksum_base_filename': '%(release_short)s-%(variant)s-%(version)s-%(date)s%(type_suffix)s.%(respin)s' + }) + + phase = ImageChecksumPhase(compose) + + exists.return_value = True + cc.return_value = {'sha256': 'cafebabe'} + + phase.run() + + dump.assert_called_once_with('/a/b/Client/i386/iso', 'sha256', + {'image.iso': 'cafebabe'}, + 'test-Client-1.0-20151203.t.0-CHECKSUM') + cc.assert_called_once_with('/a/b/Client/i386/iso/image.iso', ['sha256']) + compose.image.add_checksum.assert_called_once_with(None, 'sha256', 'cafebabe') + + @mock.patch('os.path.exists') + @mock.patch('kobo.shortcuts.compute_file_checksums') + @mock.patch('pungi.phases.image_checksum.dump_checksums') + def test_checksum_save_individuals_custom_name(self, dump, cc, exists): + compose = _DummyCompose({ + 'media_checksums': ['md5', 'sha256'], + 'media_checksum_base_filename': '%(release_short)s-%(variant)s-%(version)s-%(date)s%(type_suffix)s.%(respin)s' + }) + + phase = ImageChecksumPhase(compose) + + exists.return_value = True + cc.return_value = {'md5': 'cafebabe', 'sha256': 'deadbeef'} + + phase.run() + dump.assert_has_calls( - [mock.call('/a/b/Client/i386/iso', {'image.iso': 'cafebabe'}, 'MD5SUM'), - mock.call('/a/b/Client/i386/iso', {'image.iso': 'deadbeef'}, 'SHA256SUM')], + [mock.call('/a/b/Client/i386/iso', 'md5', {'image.iso': 'cafebabe'}, 'image.iso.MD5SUM'), + mock.call('/a/b/Client/i386/iso', 'sha256', {'image.iso': 'deadbeef'}, 'image.iso.SHA256SUM'), + mock.call('/a/b/Client/i386/iso', 'md5', {'image.iso': 'cafebabe'}, + 'test-Client-1.0-20151203.t.0-MD5SUM'), + mock.call('/a/b/Client/i386/iso', 'sha256', {'image.iso': 'deadbeef'}, + 'test-Client-1.0-20151203.t.0-SHA256SUM')], any_order=True ) cc.assert_called_once_with('/a/b/Client/i386/iso/image.iso', ['md5', 'sha256']) @@ -104,23 +162,18 @@ class TestChecksums(unittest.TestCase): shutil.rmtree(self.tmp_dir) def test_dump_checksums(self): - dump_checksums(self.tmp_dir, {'file1.iso': 'abcdef', 'file2.iso': 'cafebabe'}) + dump_checksums(self.tmp_dir, + 'md5', + {'file1.iso': 'abcdef', 'file2.iso': 'cafebabe'}, + 'CHECKSUM') with open(os.path.join(self.tmp_dir, 'CHECKSUM'), 'r') as f: data = f.read().rstrip().split('\n') expected = [ - 'abcdef *file1.iso', - 'cafebabe *file2.iso', + 'MD5 (file1.iso) = abcdef', + 'MD5 (file2.iso) = cafebabe', ] self.assertItemsEqual(expected, data) - def test_dump_individual(self): - base_path = os.path.join(self.tmp_dir, 'file.iso') - dump_individual(base_path, 'cafebabe', 'md5') - - with open(base_path + '.MD5SUM', 'r') as f: - data = f.read() - self.assertEqual('cafebabe *file.iso\n', data) - if __name__ == "__main__": unittest.main()