Merge #83 Fix recently discovered bugs

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

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/data/dummy-comps.xml

@@ -45,7 +45,7 @@
     <default>false</default>
     <uservisible>false</uservisible>
     <packagelist>
-      <packagereq>dummy-firefox</packagereq>
+      <packagereq>Dummy-firefox</packagereq>
       <packagereq>dummy-icedtea-web</packagereq>
     </packagelist>
   </group>

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
+BuildRequires:  Dummy-xulrunner
-Requires:       dummy-xulrunner
+Requires:       Dummy-xulrunner
 
 %description
 A dummy firefox package

tests/data/specs/Dummy-xulrunner.spec

@@ -1,4 +1,4 @@
-Name:           dummy-xulrunner
+Name:           Dummy-xulrunner
 Version:        16.0.1
 Release:        1
 License:        LGPLv2

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()