From bf7fec5f781281abe8b09998756ae7c805de9a1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Tue, 26 Apr 2022 21:04:39 +0200 Subject: [PATCH] Fixed scope of setup_isolinux_boot_path There is a method called setup_isolinux_boot_path which is encoded in the Iso class. The method allows to change the boot path in the isolinux binary and makes sense when the bootloader is selected to be isolinux. However, the method was called in the scope of the FileSystemIsoFs class which responsibility is to create an ISO filesystem. The creation of an ISO filesystem has no direct connection to a bootloader. Thus calling this method in the scope of the FileSystemIsoFs implementation is wrong and can lead to unexpected side effects. This commit moves the call of the method to the places where isolinux as a bootloader can still be used. This Fixes #2117 --- kiwi/builder/install.py | 3 +++ kiwi/builder/live.py | 2 ++ kiwi/filesystem/isofs.py | 7 ------- test/unit/builder/install_test.py | 4 +++- test/unit/builder/live_test.py | 6 ++++-- test/unit/filesystem/isofs_test.py | 14 ++------------ 6 files changed, 14 insertions(+), 22 deletions(-) diff --git a/kiwi/builder/install.py b/kiwi/builder/install.py index 959cb11b..596ae56f 100644 --- a/kiwi/builder/install.py +++ b/kiwi/builder/install.py @@ -42,6 +42,7 @@ from kiwi.archive.tar import ArchiveTar from kiwi.system.setup import SystemSetup from kiwi.iso_tools.base import IsoToolsBase from kiwi.xml_state import XMLState +from kiwi.iso_tools.iso import Iso from kiwi.exceptions import ( @@ -231,6 +232,8 @@ class InstallImageBuilder: self.boot_image_task.boot_root_directory, self.media_dir.name, bootloader_config.get_boot_theme() ) + if not self.firmware.efi_mode(): + Iso(self.media_dir.name).setup_isolinux_boot_path() bootloader_config.write_meta_data() bootloader_config.setup_install_image_config( mbrid=self.mbrid diff --git a/kiwi/builder/live.py b/kiwi/builder/live.py index 17d660f9..a5258f52 100644 --- a/kiwi/builder/live.py +++ b/kiwi/builder/live.py @@ -167,6 +167,8 @@ class LiveImageBuilder: self.boot_image.boot_root_directory, self.media_dir.name, bootloader_config.get_boot_theme() ) + if not self.firmware.efi_mode(): + Iso(self.media_dir.name).setup_isolinux_boot_path() bootloader_config.write_meta_data() bootloader_config.setup_live_image_config( mbrid=self.mbrid diff --git a/kiwi/filesystem/isofs.py b/kiwi/filesystem/isofs.py index c1c8f313..96df8e43 100644 --- a/kiwi/filesystem/isofs.py +++ b/kiwi/filesystem/isofs.py @@ -20,7 +20,6 @@ from typing import List # project from kiwi.filesystem.base import FileSystemBase -from kiwi.iso_tools.iso import Iso from kiwi.iso_tools import IsoTools log = logging.getLogger('kiwi') @@ -45,15 +44,9 @@ class FileSystemIsoFs(FileSystemBase): """ self.filename = filename meta_data = self.custom_args['meta_data'] - efi_mode = meta_data.get('efi_mode') - ofw_mode = meta_data.get('ofw_mode') efi_loader = meta_data.get('efi_loader') iso_tool = IsoTools.new(self.root_dir) - iso = Iso(self.root_dir) - if not efi_mode and not ofw_mode: - iso.setup_isolinux_boot_path() - iso_tool.init_iso_creation_parameters(meta_data) if efi_loader: diff --git a/test/unit/builder/install_test.py b/test/unit/builder/install_test.py index 74cab536..32102cb6 100644 --- a/test/unit/builder/install_test.py +++ b/test/unit/builder/install_test.py @@ -131,8 +131,9 @@ class TestInstallImageBuilder: @patch('kiwi.builder.install.Temporary') @patch('kiwi.builder.install.Command.run') @patch('kiwi.builder.install.Defaults.get_grub_boot_directory_name') + @patch('kiwi.builder.install.Iso') def test_create_install_iso( - self, mock_grub_dir, mock_command, mock_Temporary, mock_copy, + self, mock_Iso, mock_grub_dir, mock_command, mock_Temporary, mock_copy, mock_setup_media_loader_directory, mock_BootLoaderConfig, mock_DeviceProvider ): @@ -264,6 +265,7 @@ class TestInstallImageBuilder: with patch('builtins.open', m_open, create=True): self.install_image.create_install_iso() + mock_Iso.return_value.setup_isolinux_boot_path.assert_called_once_with() mock_BootLoaderConfig.assert_called_once_with( 'isolinux', self.xml_state, root_dir='root_dir', boot_dir='temp_media_dir' diff --git a/test/unit/builder/live_test.py b/test/unit/builder/live_test.py index 0112f2e1..37ca3043 100644 --- a/test/unit/builder/live_test.py +++ b/test/unit/builder/live_test.py @@ -138,6 +138,7 @@ class TestLiveImageBuilder: @patch('kiwi.builder.live.Temporary') @patch('kiwi.builder.live.shutil') @patch('kiwi.builder.live.Iso.set_media_tag') + @patch('kiwi.builder.live.Iso') @patch('kiwi.builder.live.FileSystemIsoFs') @patch('kiwi.builder.live.FileSystem.new') @patch('kiwi.builder.live.SystemSize') @@ -145,8 +146,8 @@ class TestLiveImageBuilder: @patch('os.path.exists') def test_create_overlay_structure( self, mock_exists, mock_grub_dir, mock_size, mock_filesystem, - mock_isofs, mock_tag, mock_shutil, mock_Temporary, - mock_setup_media_loader_directory, mock_DeviceProvider + mock_isofs, mock_Iso, mock_tag, mock_shutil, + mock_Temporary, mock_setup_media_loader_directory, mock_DeviceProvider ): mock_exists.return_value = True mock_grub_dir.return_value = 'grub2' @@ -329,6 +330,7 @@ class TestLiveImageBuilder: tmpdir_name = [temp_squashfs, temp_media_dir] kiwi.builder.live.BootLoaderConfig.new.reset_mock() self.live_image.create() + mock_Iso.return_value.setup_isolinux_boot_path.assert_called_once_with() kiwi.builder.live.BootLoaderConfig.new.assert_called_once_with( 'isolinux', self.xml_state, root_dir='root_dir', boot_dir='temp_media_dir' diff --git a/test/unit/filesystem/isofs_test.py b/test/unit/filesystem/isofs_test.py index 6dadc42d..2b549a09 100644 --- a/test/unit/filesystem/isofs_test.py +++ b/test/unit/filesystem/isofs_test.py @@ -27,8 +27,7 @@ class TestFileSystemIsoFs: assert self.isofs.custom_args['some_args'] == 'data' @patch('kiwi.filesystem.isofs.IsoTools') - @patch('kiwi.filesystem.isofs.Iso') - def test_create_on_file(self, mock_iso, mock_IsoTools): + def test_create_on_file(self, mock_IsoTools): iso_tool = mock.Mock() iso_tool.has_iso_hybrid_capability = mock.Mock( return_value=True @@ -37,20 +36,14 @@ class TestFileSystemIsoFs: return_value='/usr/bin/xorriso' ) mock_IsoTools.new.return_value = iso_tool - iso = mock.Mock() - iso.header_end_name = 'header_end' - mock_iso.return_value = iso self.isofs.create_on_file('myimage', None) - iso.setup_isolinux_boot_path.assert_called_once_with() - iso_tool.init_iso_creation_parameters.assert_called_once_with({}) iso_tool.create_iso.assert_called_once_with('myimage') @patch('kiwi.filesystem.isofs.IsoTools') - @patch('kiwi.filesystem.isofs.Iso') - def test_create_on_file_EFI_enabled(self, mock_iso, mock_IsoTools): + def test_create_on_file_EFI_enabled(self, mock_IsoTools): iso_tool = mock.Mock() iso_tool.has_iso_hybrid_capability = mock.Mock( return_value=True @@ -59,9 +52,6 @@ class TestFileSystemIsoFs: return_value='/usr/bin/xorriso' ) mock_IsoTools.new.return_value = iso_tool - iso = mock.Mock() - iso.header_end_name = 'header_end' - mock_iso.return_value = iso self.isofs.custom_args['meta_data']['efi_mode'] = 'uefi' self.isofs.custom_args['meta_data']['efi_loader'] = 'esp-image-file' with self._caplog.at_level(logging.WARNING):