Merge #125 `[image-build] Stop creating wrong arch dirs`

pungi/paths.py

@@ -515,7 +515,7 @@ class ComposePaths(object):
 
         return os.path.join(path, filename)
 
-    def image_dir(self, variant, symlink_to=None, create_dir=True, relative=False):
+    def image_dir(self, variant, symlink_to=None, relative=False):
         """
         The arch is listed as literal '%(arch)s'
         Examples:
@@ -523,21 +523,18 @@ class ComposePaths(object):
             None
         @param variant
         @param symlink_to=None
-        @param create_dir=True
         @param relative=False
         """
         # skip optional and addons
         if variant.type != "variant":
             return None
 
-        path = os.path.join(self.topdir('%(arch)s', variant, create_dir=create_dir, relative=relative),
+        path = os.path.join(self.topdir('%(arch)s', variant, create_dir=False, relative=relative),
                             "images")
         if symlink_to:
             topdir = self.compose.topdir.rstrip("/") + "/"
             relative_dir = path[len(topdir):]
             target_dir = os.path.join(symlink_to, self.compose.compose_id, relative_dir)
-            if create_dir and not relative:
-                makedirs(target_dir)
             try:
                 os.symlink(target_dir, path)
             except OSError as ex:
@@ -548,9 +545,6 @@ class ComposePaths(object):
                     raise RuntimeError(msg)
                 if os.path.abspath(os.readlink(path)) != target_dir:
                     raise RuntimeError(msg)
-        else:
-            if create_dir and not relative:
-                makedirs(path)
         return path
 
     def jigdo_dir(self, arch, variant, create_dir=True, relative=False):

pungi/phases/image_build.py

@@ -5,7 +5,7 @@ import os
 import time
 from kobo import shortcuts
 
-from pungi.util import get_variant_data, resolve_git_url
+from pungi.util import get_variant_data, resolve_git_url, makedirs
 from pungi.phases.base import PhaseBase
 from pungi.linker import Linker
 from pungi.paths import translate_path
@@ -44,7 +44,7 @@ class ImageBuildPhase(PhaseBase):
                 % (install_tree_from, variant.uid))
         return translate_path(
             self.compose,
-            self.compose.paths.compose.os_tree('$arch', install_tree_source)
+            self.compose.paths.compose.os_tree('$arch', install_tree_source, create_dir=False)
         )
 
     def _get_repo(self, image_conf, variant):
@@ -66,8 +66,9 @@ class ImageBuildPhase(PhaseBase):
                 raise RuntimeError(
                     'There is no variant %s to get repo from when building image for %s.'
                     % (extra, variant.uid))
-            repo.append(translate_path(self.compose,
-                                       self.compose.paths.compose.os_tree('$arch', v)))
+            repo.append(translate_path(
+                self.compose,
+                self.compose.paths.compose.os_tree('$arch', v, create_dir=False)))
 
         return ",".join(repo)
 
@@ -117,7 +118,7 @@ class ImageBuildPhase(PhaseBase):
                     ),
                     "image_dir": self.compose.paths.compose.image_dir(variant),
                     "relative_image_dir": self.compose.paths.compose.image_dir(
-                        variant, create_dir=False, relative=True
+                        variant, relative=True
                     ),
                     "link_type": self.compose.conf.get("link_type", "hardlink-or-copy")
                 }
@@ -204,6 +205,7 @@ class CreateImageBuildThread(WorkerThread):
         linker = Linker(logger=compose._logger)
         for image_info in image_infos:
             image_dir = cmd["image_dir"] % {"arch": image_info['arch']}
+            makedirs(image_dir)
             relative_image_dir = cmd["relative_image_dir"] % {"arch": image_info['arch']}
 
             # let's not change filename of koji outputs

tests/test_imagebuildphase.py

@@ -31,10 +31,10 @@ class _DummyCompose(object):
             compose=mock.Mock(
                 topdir=mock.Mock(return_value='/a/b'),
                 os_tree=mock.Mock(
-                    side_effect=lambda arch, variant: os.path.join('/ostree', arch, variant.uid)
+                    side_effect=lambda arch, variant, create_dir=False: os.path.join('/ostree', arch, variant.uid)
                 ),
                 image_dir=mock.Mock(
-                    side_effect=lambda variant, create_dir=False, relative=False: os.path.join(
+                    side_effect=lambda variant, relative=False: os.path.join(
                         '' if relative else '/', 'image_dir', variant.uid, '%(arch)s'
                     )
                 )
@@ -287,7 +287,8 @@ class TestCreateImageBuildThread(unittest.TestCase):
 
     @mock.patch('pungi.phases.image_build.KojiWrapper')
     @mock.patch('pungi.phases.image_build.Linker')
-    def test_process(self, Linker, KojiWrapper):
+    @mock.patch('pungi.phases.image_build.makedirs')
+    def test_process(self, makedirs, Linker, KojiWrapper):
         compose = _DummyCompose({
             'koji_profile': 'koji'
         })
@@ -393,6 +394,12 @@ class TestCreateImageBuildThread(unittest.TestCase):
             self.assertEqual(data['format'], image.format)
             self.assertEqual(data['type'], image.type)
 
+        self.assertItemsEqual(makedirs.mock_calls,
+                              [mock.call('/image_dir/Client/amd64'),
+                               mock.call('/image_dir/Client/amd64'),
+                               mock.call('/image_dir/Client/x86_64'),
+                               mock.call('/image_dir/Client/x86_64')])
+
     @mock.patch('pungi.phases.image_build.KojiWrapper')
     @mock.patch('pungi.phases.image_build.Linker')
     def test_process_handle_fail(self, Linker, KojiWrapper):