[checks] Relax check for isohybrid command

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ář <lsedlar@redhat.com>

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

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