PR#188: [buildinstall] Allow hardlinking images - pungi

pungi

#188 [buildinstall] Allow hardlinking images

Merged 9 years ago by ausil. Opened 9 years ago by lsedlar.

lsedlar/pungi hardlink-buildinstall into master

[buildinstall] Hardlink boot isos

Lubomír Sedlář • 9 years ago

636ac79

pungi/phases/buildinstall.py

file modified

+14 -7

		`@@ -24,11 +24,12 @@`
		`import re`

		`from kobo.threads import ThreadPool, WorkerThread`
		`- from kobo.shortcuts import run, relative_path`
		`+ from kobo.shortcuts import run`
		`from productmd.images import Image`

		`from pungi.arch import get_valid_arches`
		`from pungi.util import get_buildroot_rpms, get_volid, get_arch_variant_data`
		`+ from pungi.util import get_file_size, get_mtime`
		`from pungi.wrappers.lorax import LoraxWrapper`
		`from pungi.wrappers.kojiwrapper import KojiWrapper`
		`from pungi.wrappers.iso import IsoWrapper`
		`@@ -72,6 +73,11 @@`
		`"expected_types": [str],`
		`"optional": True,`
		`},`
		`+ {`
		`+ "name": "buildinstall_symlink",`
		`+ "expected_types": [bool],`
		`+ "optional": True,`
		`+ },`
		`)`

		`def __init__(self, compose):`
		`@@ -332,9 +338,11 @@`
		`return`

		`compose.log_info("[BEGIN] %s" % msg)`
		`- # can't make a hardlink - possible cross-device link due to 'symlink_to' argument`
		`- symlink_target = relative_path(boot_iso_path, new_boot_iso_path)`
		`- os.symlink(symlink_target, new_boot_iso_path)`
		`+ # Try to hardlink, and copy if that fails`
		`+ try:`
		`+ os.link(boot_iso_path, new_boot_iso_path)`
		`+ except OSError:`
		`+ shutil.copy2(boot_iso_path, new_boot_iso_path)`

		`iso = IsoWrapper()`
		`implant_md5 = iso.get_implanted_md5(new_boot_iso_path)`
		`@@ -345,10 +353,9 @@`
		`run(iso.get_manifest_cmd(iso_name), workdir=iso_dir)`

		`img = Image(compose.im)`
		`- img.implant_md5 = iso.get_implanted_md5(new_boot_iso_path)`
		`img.path = new_boot_iso_relative_path`
		`- img.mtime = int(os.stat(new_boot_iso_path).st_mtime)`
		`- img.size = os.path.getsize(new_boot_iso_path)`
		`+ img.mtime = get_mtime(new_boot_iso_path)`
		`+ img.size = get_file_size(new_boot_iso_path)`
		`img.arch = arch`
		`img.type = "boot"`
		`img.format = "iso"`

tests/helpers.py

file modified

+13

		`@@ -1,6 +1,7 @@`
		`# -- coding: utf-8 --`

		`import mock`
		`+ import os`
		`import unittest`
		`import tempfile`
		`import shutil`
		`@@ -43,6 +44,7 @@`
		`'Everything': mock.Mock(uid='Everything', arches=['x86_64', 'amd64'],`
		`type='variant', is_empty=False),`
		`}`
		`+ self.log_info = mock.Mock()`
		`self.log_error = mock.Mock()`
		`self.log_debug = mock.Mock()`
		`self.get_image_name = mock.Mock(return_value='image-name')`
		`@@ -61,3 +63,14 @@`
		`for variant in self.variants.itervalues():`
		`result \|= set(variant.arches)`
		`return result`
		`+`
		`+`
		`+ def touch(path):`
		`+ """Helper utility that creates an dummy file in given location. Directories`
		`+ will be created."""`
		`+ try:`
		`+ os.makedirs(os.path.dirname(path))`
		`+ except OSError:`
		`+ pass`
		`+ with open(path, 'w') as f:`
		`+ f.write(path + '\n')`

tests/test_buildinstall.py

file modified

+58 -2

		`@@ -10,8 +10,8 @@`

		`sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))`

		`- from pungi.phases.buildinstall import BuildinstallPhase, BuildinstallThread`
		`- from tests.helpers import DummyCompose, PungiTestCase`
		`+ from pungi.phases.buildinstall import BuildinstallPhase, BuildinstallThread, symlink_boot_iso`
		`+ from tests.helpers import DummyCompose, PungiTestCase, touch`


		`class BuildInstallCompose(DummyCompose):`
		`@@ -534,5 +534,61 @@`
		`])`


		`+ class TestSymlinkIso(PungiTestCase):`
		`+`
		`+ def setUp(self):`
		`+ super(TestSymlinkIso, self).setUp()`
		`+ self.compose = BuildInstallCompose(self.topdir, {})`
		`+ os_tree = self.compose.paths.compose.os_tree('x86_64', self.compose.variants['Server'])`
		`+ self.boot_iso_path = os.path.join(os_tree, "images", "boot.iso")`
		`+ touch(self.boot_iso_path)`
		`+`
		`+ @mock.patch('pungi.phases.buildinstall.Image')`
		`+ @mock.patch('pungi.phases.buildinstall.get_mtime')`
		`+ @mock.patch('pungi.phases.buildinstall.get_file_size')`
		`+ @mock.patch('pungi.phases.buildinstall.IsoWrapper')`
		`+ @mock.patch('pungi.phases.buildinstall.run')`
		`+ def test_hardlink(self, run, IsoWrapperCls, get_file_size, get_mtime, ImageCls):`
		`+ self.compose.conf = {'buildinstall_symlink': False}`
		`+ IsoWrapper = IsoWrapperCls.return_value`
		`+ get_file_size.return_value = 1024`
		`+ get_mtime.return_value = 13579`
		`+`
		`+ symlink_boot_iso(self.compose, 'x86_64', self.compose.variants['Server'])`
		`+`
		`+ tgt = self.topdir + '/compose/Server/x86_64/iso/image-name'`
		`+ self.assertTrue(os.path.isfile(tgt))`
		`+ self.assertEqual(os.stat(tgt).st_ino,`
		`+ os.stat(self.topdir + '/compose/Server/x86_64/os/images/boot.iso').st_ino)`
		`+`
		`+ self.assertItemsEqual(`
		`+ self.compose.get_image_name.mock_calls,`
		`+ [mock.call('x86_64', self.compose.variants['Server'],`
		`+ disc_type='boot', disc_num=None, suffix='.iso')])`
		`+ self.assertItemsEqual(IsoWrapper.get_implanted_md5.mock_calls,`
		`+ [mock.call(tgt)])`
		`+ self.assertItemsEqual(IsoWrapper.get_manifest_cmd.mock_calls,`
		`+ [mock.call('image-name')])`
		`+ self.assertItemsEqual(IsoWrapper.get_volume_id.mock_calls,`
		`+ [mock.call(tgt)])`
		`+ self.assertItemsEqual(run.mock_calls,`
		`+ [mock.call(IsoWrapper.get_manifest_cmd.return_value,`
		`+ workdir=self.topdir + '/compose/Server/x86_64/iso')])`
		`+`
		`+ image = ImageCls.return_value`
		`+ self.assertEqual(image.path, 'Server/x86_64/iso/image-name')`
		`+ self.assertEqual(image.mtime, 13579)`
		`+ self.assertEqual(image.size, 1024)`
		`+ self.assertEqual(image.arch, 'x86_64')`
		`+ self.assertEqual(image.type, "boot")`
		`+ self.assertEqual(image.format, "iso")`
		`+ self.assertEqual(image.disc_number, 1)`
		`+ self.assertEqual(image.disc_count, 1)`
		`+ self.assertEqual(image.bootable, True)`
		`+ self.assertEqual(image.implant_md5, IsoWrapper.get_implanted_md5.return_value)`
		`+ self.assertEqual(self.compose.im.add.mock_calls,`
		`+ [mock.call('Server', 'x86_64', image)])`
		`+`
		`+`
		`if __name__ == "__main__":`
		`unittest.main()`

no initial comment

Pull-Request has been rebased

9 years ago

lkocman commented 9 years ago

The symlink behavior existed due having iso files on a separate filesystem and symlinked to the compose.

This feature was probably only in old distill and we don't have it (or use it) any more ... if there is still some code remaining ... it should be removed.

Please remove the symlink behavior and use only hardlinks instead. This is a perfectly valid use case.

Lubos

Pull-Request has been rebased

9 years ago

lsedlar commented 9 years ago

Cool, thanks. Updated to only hardlink.