Code review comment for ~jibel/curtin/+git/add-layer-images-support:master

>
> It was very interesting to see how the implementation works; and I want to
> document the expectations between "main-layers" and other images. There are
> some interesting side-effects that I think we must document. In particular,
> only main images seem to have peers at the same level, for example showing
> nested main layers; that is we cannot put a set of main layers in a sublayer.
> Pointing at any of the sub-images in the tree below only gets the sub-image,
> no peers nor parents, is that expected?
>

It's a very good point and not expected. We've fixed the behaviour to load a set of main layers in a sublayer. The only constraint is that the names must start with 2 digits. If they don't start with 2 digits it'll try again with fs in the parent directory until it reaches the uppermost level.

Here is the change that we will push when all the other comments are addressed

diff --git a/curtin/commands/extract.py b/curtin/commands/extract.py
index 1ab8f767..5e14f24d 100644
--- a/curtin/commands/extract.py
+++ b/curtin/commands/extract.py
@@ -148,7 +148,7 @@ def _get_image_paths(path):
     images = [path]

     m = re.match(r"([0-9]{2})-.*", imgname)
- # Main layers
+ # Directory with peer fs images
     if m:
         i = int(m.group(1)) - 1
         match = "{:02d}-*{}".format(i, fileext)
@@ -160,11 +160,11 @@ def _get_image_paths(path):
         if candidate:
             images.extend(
                 _get_image_paths(os.path.join(dirname, candidate[-1])))
- return images
- else:
- # Climbing up from uppermost squashfes sublayers
- images.extend(_get_image_paths(dirname + fileext))
- return images
+ return images
+
+ # Climbing up from uppermost squashfes sublayers
+ images.extend(_get_image_paths(dirname + fileext))
+ return images

 def copy_to_target(source, target):
diff --git a/tests/unittests/test_commands_extract.py b/tests/unittests/test_commands_extract.py
index b85f3a6c..b90ad8e2 100644
--- a/tests/unittests/test_commands_extract.py
+++ b/tests/unittests/test_commands_extract.py
@@ -123,6 +123,26 @@ class TestGetImagesPaths(CiTestCase):
             _get_image_paths(
                 os.path.join(tdir, "02-main2/sublayer2/sublayera.fs")))

+ def test_leading_digits_mounts_peer_images(self):
+ """_get_image_paths mounts peer images in a folder when starting with leading digits"""
+ tdir = self._create_temporary_fs_struct([
+ "main1.fs",
+ "main2.fs",
+ "main2/01-sublayer1.fs",
+ "main2/02-sublayer2.fs",
+ "main2/02-sublayer2/sublayera.fs",
+ "main2/02-sublayer2/sublayerb.fs",
+ "main3.fs",
+ ])
+ expected_base = ["main2/02-sublayer2/sublayerb.fs",
+ "main2/02-sublayer2.fs",
+ "main2/01-sublayer1.fs",
+ "main2.fs"]
+ self.assertEqual(
+ [os.path.join(tdir, p) for p in expected_base],
+ _get_image_paths(
+ os.path.join(tdir, "main2/02-sublayer2/sublayerb.fs")))
+
     def test_missing_file(self):
         """_get_image_paths returns an empty tuple"""
         self.assertEqual([], _get_image_paths("unexisting.fs"))