Fix generating checksum files

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ář <lsedlar@redhat.com>

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
 ========================
 

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))
+            f.write('%s (%s) = %s\n' % (alg.upper(), file, checksum))
-
-
-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)))

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_checksums)
-                                         dump_individual)
 
 
 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, dump, cc, exists):
-    def test_checksum_save_individuals(self, indiv_dump, dump, cc, exists):
         compose = _DummyCompose({
             'media_checksums': ['md5', 'sha256'],
         })
@@ -79,14 +87,64 @@ class TestImageChecksumPhase(unittest.TestCase):
 
         phase.run()
 
-        indiv_dump.assert_has_calls(
+        dump.assert_has_calls(
-            [mock.call('/a/b/Client/i386/iso/image.iso', 'cafebabe', 'md5'),
+            [mock.call('/a/b/Client/i386/iso', 'md5', {'image.iso': 'cafebabe'}, 'image.iso.MD5SUM'),
-             mock.call('/a/b/Client/i386/iso/image.iso', 'deadbeef', 'sha256')],
+             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', 'md5', {'image.iso': 'cafebabe'}, 'image.iso.MD5SUM'),
-             mock.call('/a/b/Client/i386/iso', {'image.iso': 'deadbeef'}, 'SHA256SUM')],
+             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',
+                'MD5 (file1.iso) = abcdef',
-                'cafebabe *file2.iso',
+                '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()