From 2d9dd0e535ce075ecbf7c72765ef3e54f1a5ee10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Thu, 10 Mar 2016 10:44:21 +0100 Subject: [PATCH] [checks] Relax check for isohybrid command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is used only in createiso and productimg phases. For productimg, it needs to be present only when the compose is bootable and productimg phase is explicitly enabled. For createiso, it is only needed if runroot is not enabled. Additionally, if we detect pungi running on arch for which syslinux is not available, a warning is printed, but the compose is allowed to continue (and possibly crash later). Signed-off-by: Lubomír Sedlář --- pungi/checks.py | 21 ++++++++++++- tests/test_checks.py | 72 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 81 insertions(+), 12 deletions(-) diff --git a/pungi/checks.py b/pungi/checks.py index c3555762..d874ace0 100644 --- a/pungi/checks.py +++ b/pungi/checks.py @@ -16,12 +16,31 @@ import os.path +import platform def is_jigdo_needed(conf): return conf.get('create_jigdo', True) +def is_isohybrid_needed(conf): + """The isohybrid command is needed locally only for productimg phase and + createiso phase without runroot. If that is not going to run, we don't need + to check for it. Additionally, the syslinux package is only available on + x86_64 and i386. + """ + runroot = conf.get('runroot', False) + will_do_productimg = conf.get('productimg', False) and conf.get('bootable', False) + if runroot and not will_do_productimg: + return False + if platform.machine() not in ('x86_64', 'i386'): + msg = ('Not checking for /usr/bin/isohybrid due to current architecture. ' + 'Expect failures in productimg phase.') + print msg + return False + return True + + # The first element in the tuple is package name expected to have the # executable (2nd element of the tuple). The last element is an optional # function that should determine if the tool is required based on @@ -32,7 +51,7 @@ tools = [ ("jigdo", "/usr/bin/jigdo-lite", is_jigdo_needed), ("genisoimage", "/usr/bin/genisoimage", None), ("gettext", "/usr/bin/msgfmt", None), - ("syslinux", "/usr/bin/isohybrid", None), + ("syslinux", "/usr/bin/isohybrid", is_isohybrid_needed), ("yum-utils", "/usr/bin/createrepo", None), ("yum-utils", "/usr/bin/mergerepo", None), ("yum-utils", "/usr/bin/repoquery", None), diff --git a/tests/test_checks.py b/tests/test_checks.py index 61e741ea..a957ea33 100755 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -13,6 +13,10 @@ from pungi import checks class CheckDependenciesTestCase(unittest.TestCase): + + def dont_find(self, paths): + return lambda path: path not in paths + def test_all_deps_missing(self): def custom_exists(path): return False @@ -22,16 +26,13 @@ class CheckDependenciesTestCase(unittest.TestCase): exists.side_effect = custom_exists result = checks.check({}) - self.assertEqual(12, len(out.getvalue().strip().split('\n'))) + self.assertGreater(len(out.getvalue().strip().split('\n')), 1) self.assertFalse(result) def test_all_deps_ok(self): - def custom_exists(path): - return True - with mock.patch('sys.stdout', new_callable=StringIO.StringIO) as out: with mock.patch('os.path.exists') as exists: - exists.side_effect = custom_exists + exists.side_effect = self.dont_find([]) result = checks.check({}) self.assertEqual('', out.getvalue()) @@ -42,17 +43,66 @@ class CheckDependenciesTestCase(unittest.TestCase): 'create_jigdo': False } - def custom_exists(path): - if path == '/usr/bin/jigdo-lite': - return False - return True - with mock.patch('os.path.exists') as exists: - exists.side_effect = custom_exists + exists.side_effect = self.dont_find(['/usr/bin/jigdo-lite']) result = checks.check(conf) self.assertTrue(result) + def test_isohybrid_not_required_without_productimg_phase(self): + conf = { + 'bootable': True, + 'productimg': False, + 'runroot': True, + } + + with mock.patch('os.path.exists') as exists: + exists.side_effect = self.dont_find(['/usr/bin/isohybrid']) + result = checks.check(conf) + + self.assertTrue(result) + + def test_isohybrid_not_required_on_not_bootable(self): + conf = { + 'bootable': False, + 'runroot': True, + } + + with mock.patch('os.path.exists') as exists: + exists.side_effect = self.dont_find(['/usr/bin/isohybrid']) + result = checks.check(conf) + + self.assertTrue(result) + + def test_isohybrid_not_required_on_arm(self): + conf = { + 'bootable': True, + 'productimg': True, + 'runroot': True, + } + + with mock.patch('sys.stdout', new_callable=StringIO.StringIO) as out: + with mock.patch('platform.machine') as machine: + machine.return_value = 'armhfp' + with mock.patch('os.path.exists') as exists: + exists.side_effect = self.dont_find(['/usr/bin/isohybrid']) + result = checks.check(conf) + + self.assertRegexpMatches(out.getvalue(), r'^Not checking.*Expect failures.*$') + self.assertTrue(result) + + def test_isohybrid_not_needed_in_runroot(self): + conf = { + 'runroot': True, + } + + with mock.patch('platform.machine') as machine: + machine.return_value = 'armhfp' + with mock.patch('os.path.exists') as exists: + exists.side_effect = self.dont_find(['/usr/bin/isohybrid']) + result = checks.check(conf) + + self.assertTrue(result) if __name__ == "__main__": unittest.main()