Merge #433 createiso: Do not split bootable media

pungi/phases/createiso.py

@@ -84,7 +84,9 @@ class CreateisoPhase(PhaseBase):
                                              % (variant.uid, arch))
                     continue
 
-                split_iso_data = split_iso(self.compose, arch, variant)
+                bootable = self._is_bootable(variant, arch)
+
+                split_iso_data = split_iso(self.compose, arch, variant, no_split=bootable)
                 disc_count = len(split_iso_data)
 
                 for disc_num, iso_data in enumerate(split_iso_data):
@@ -103,8 +105,6 @@ class CreateisoPhase(PhaseBase):
                                                disc_num=disc_num, disc_count=disc_count,
                                                split_iso_data=iso_data)
 
-                    bootable = self._is_bootable(variant, arch)
-
                     cmd = {
                         "iso_path": iso_path,
                         "bootable": bootable,
@@ -281,17 +281,23 @@ class CreateIsoThread(WorkerThread):
                                   variant=str(variant))
 
 
-def split_iso(compose, arch, variant):
+def split_iso(compose, arch, variant, no_split=False):
     """
     Split contents of the os/ directory for given tree into chunks fitting on ISO.
 
     All files from the directory are taken except for possible boot.iso image.
     Files added in extra_files phase are put on all disks.
+
+    If `no_split` is set, we will pretend that the media is practically
+    infinite so that everything goes on single disc. A warning is printed if
+    the size is bigger than configured.
     """
     media_size = compose.conf['iso_size']
     media_reserve = compose.conf['split_iso_reserve']
+    split_size = convert_media_size(media_size) - convert_media_size(media_reserve)
+    real_size = 10**20 if no_split else split_size
 
-    ms = MediaSplitter(convert_media_size(media_size) - convert_media_size(media_reserve), compose)
+    ms = MediaSplitter(real_size, compose)
 
     os_tree = compose.paths.compose.os_tree(arch, variant)
     extra_files_dir = compose.paths.work.extra_files_dir(arch, variant)
@@ -336,7 +342,14 @@ def split_iso(compose, arch, variant):
     for path, size, sticky in all_files + packages:
         ms.add_file(path, size, sticky)
 
-    return ms.split()
+    result = ms.split()
+    if no_split and result[0]['size'] > split_size:
+        compose.log_warning('ISO for %s.%s does not fit on single media! '
+                            'It is %s bytes too big. (Total size: %s B)'
+                            % (variant.uid, arch,
+                               result[0]['size'] - split_size,
+                               result[0]['size']))
+    return result
 
 
 def prepare_iso(compose, arch, variant, disc_num=1, disc_count=None, split_iso_data=None):

tests/test_createiso_phase.py

@@ -93,7 +93,7 @@ class CreateisoPhaseTest(helpers.PungiTestCase):
                          [mock.call(compose, 'x86_64', compose.variants['Server'],
                                     disc_count=1, disc_num=1, split_iso_data=disc_data)])
         self.assertEqual(split_iso.call_args_list,
-                         [mock.call(compose, 'x86_64', compose.variants['Server'])])
+                         [mock.call(compose, 'x86_64', compose.variants['Server'], no_split=False)])
         self.assertEqual(len(pool.add.call_args_list), 1)
         self.maxDiff = None
         self.assertItemsEqual(
@@ -160,8 +160,8 @@ class CreateisoPhaseTest(helpers.PungiTestCase):
                        disc_count=1, disc_num=1, split_iso_data=disc_data)])
         self.assertItemsEqual(
             split_iso.call_args_list,
-            [mock.call(compose, 'x86_64', compose.variants['Server']),
+            [mock.call(compose, 'x86_64', compose.variants['Server'], no_split=True),
-             mock.call(compose, 'src', compose.variants['Server'])])
+             mock.call(compose, 'src', compose.variants['Server'], no_split=False)])
         self.assertEqual(len(pool.add.call_args_list), 2)
         self.maxDiff = None
         self.assertItemsEqual(
@@ -690,6 +690,39 @@ class SplitIsoTest(helpers.PungiTestCase):
                                      os.path.join(base_path, 'Packages/b/bash.rpm')],
                            'size': 3242196992}])
 
+    def test_no_split_when_requested(self):
+        compose = helpers.DummyCompose(self.topdir, {})
+        helpers.touch(os.path.join(self.topdir, 'compose/Server/x86_64/os/.treeinfo'),
+                      TREEINFO)
+        helpers.touch(os.path.join(self.topdir, 'work/x86_64/Server/extra-files/GPL'))
+        helpers.touch(os.path.join(self.topdir, 'compose/Server/x86_64/os/GPL'))
+        helpers.touch(os.path.join(self.topdir, 'compose/Server/x86_64/os/repodata/repomd.xml'))
+        helpers.touch(os.path.join(self.topdir, 'compose/Server/x86_64/os/Packages/b/bash.rpm'))
+        helpers.touch(os.path.join(self.topdir, 'compose/Server/x86_64/os/n/media.repo'))
+
+        M = 1024 ** 2
+        G = 1024 ** 3
+
+        with mock.patch('os.path.getsize',
+                        DummySize({'GPL': 20 * M, 'bash': 3 * G,
+                                   'media': 2 * G, 'treeinfo': 10 * M})):
+            data = createiso.split_iso(compose, 'x86_64', compose.variants['Server'], no_split=True)
+
+        base_path = os.path.join(self.topdir, 'compose/Server/x86_64/os')
+        # GPL is the only sticky file, it should be first at all times.
+        # Files are searched top-down, so nested ones are after top level ones.
+        self.assertEqual(data,
+                         [{'files': [os.path.join(base_path, 'GPL'),
+                                     os.path.join(base_path, '.treeinfo'),
+                                     os.path.join(base_path, 'n/media.repo'),
+                                     os.path.join(base_path, 'Packages/b/bash.rpm')],
+                           'size': 5400166400}])
+        self.assertEqual(
+            compose.log_warning.call_args_list,
+            [mock.call('ISO for Server.x86_64 does not fit on single media! '
+                       'It is 710652160 bytes too big. (Total size: 5400166400 B)')]
+        )
+
     def test_keeps_reserve(self):
         compose = helpers.DummyCompose(self.topdir, {})
         helpers.touch(os.path.join(self.topdir, 'compose/Server/x86_64/os/.treeinfo'),