[PATCH v6 0/2] image-bootfiles: new class

[PATCH v6 0/2] image-bootfiles: new class

[Marcus Folkesson]

The image-bootfiles class is used to put all files listed in
IMAGE_BOOT_FILES into the root filesystem.

IMAGE_BOOT_FILES is used by the bootimg-partition wic plugin to put the
files into a boot partition. 
Be able to list files as "boot files" in e.g. your .conf or image files
instead of install those in every recipe is a good thing.

It is not always desired to have a separate boot partition for boot
files. Sometimes it could be good to have them as a part of the root
filesystem.

For example, if a double copy strategy is used for update the system,
then you probably want to update both the boot files and root filesystem
at the same time as there may be dependencies.

v2:
    - Removed the documentation from the patch series (will be submitted later)
    - Break out the parts in bootimg-partition that is used by
      image-bootfiles to a common library
    - Make the destination directory in root filesystem configurable
v3:
    - See changelog in patches

v4:
    - See changelog in patches

v5:
    - See changelog in patches

v6:
    - See changelog in patches

Marcus Folkesson (2):
  bootimg-partition: break out code to a common library.
  image-bootfiles.bbclass: new class, copy boot files to root filesystem

 meta/classes/image-bootfiles.bbclass          | 41 +++++++++++++
 meta/lib/oe/bootfiles.py                      | 57 +++++++++++++++++++
 .../wic/plugins/source/bootimg-partition.py   | 39 +------------
 3 files changed, 100 insertions(+), 37 deletions(-)
 create mode 100644 meta/classes/image-bootfiles.bbclass
 create mode 100644 meta/lib/oe/bootfiles.py

-- 
2.45.1

[PATCH v6 1/2] bootimg-partition: break out code to a common library.

[Marcus Folkesson]

Break out the code that parse IMAGE_BOOT_FILES to a common library.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
Reviewed-by: Konrad Weihmann <kweihmann@outlook.com>
---

Notes:
    v3:
       - Removed unnecessary "import glob"
       - Explicitely tell that bootfiles_populate() returns a tuple
    v4:
       - Remove the right "import glob" :-)
    v5:
       - No changes
    v6:
        - Collect tags to make it easier to apply

 meta/lib/oe/bootfiles.py                      | 57 +++++++++++++++++++
 .../wic/plugins/source/bootimg-partition.py   | 39 +------------
 2 files changed, 59 insertions(+), 37 deletions(-)
 create mode 100644 meta/lib/oe/bootfiles.py

diff --git a/meta/lib/oe/bootfiles.py b/meta/lib/oe/bootfiles.py
new file mode 100644
index 0000000000..155fe742db
--- /dev/null
+++ b/meta/lib/oe/bootfiles.py
@@ -0,0 +1,57 @@
+#
+# SPDX-License-Identifier: MIT
+#
+# Copyright (C) 2024 Marcus Folkesson
+# Author: Marcus Folkesson <marcus.folkesson@gmail.com>
+#
+# Utility functions handling boot files
+#
+# Look into deploy_dir and search for boot_files.
+# Returns a list of tuples with (original filepath relative to
+# deploy_dir, desired filepath renaming)
+#
+# Heavily inspired of bootimg-partition.py
+#
+def get_boot_files(deploy_dir, boot_files):
+    import re
+    import os
+    from glob import glob
+
+    if boot_files is None:
+        return None
+
+    # list of tuples (src_name, dst_name)
+    deploy_files = []
+    for src_entry in re.findall(r'[\w;\-\./\*]+', boot_files):
+        if ';' in src_entry:
+            dst_entry = tuple(src_entry.split(';'))
+            if not dst_entry[0] or not dst_entry[1]:
+                raise ValueError('Malformed boot file entry: %s' % src_entry)
+        else:
+            dst_entry = (src_entry, src_entry)
+
+        deploy_files.append(dst_entry)
+
+    install_files = []
+    for deploy_entry in deploy_files:
+        src, dst = deploy_entry
+        if '*' in src:
+            # by default install files under their basename
+            entry_name_fn = os.path.basename
+            if dst != src:
+                # unless a target name was given, then treat name
+                # as a directory and append a basename
+                entry_name_fn = lambda name: \
+                                os.path.join(dst,
+                                             os.path.basename(name))
+
+            srcs = glob(os.path.join(deploy_dir, src))
+
+            for entry in srcs:
+                src = os.path.relpath(entry, deploy_dir)
+                entry_dst_name = entry_name_fn(entry)
+                install_files.append((src, entry_dst_name))
+        else:
+            install_files.append((src, dst))
+
+    return install_files
diff --git a/scripts/lib/wic/plugins/source/bootimg-partition.py b/scripts/lib/wic/plugins/source/bootimg-partition.py
index 1071d1af3f..589853a439 100644
--- a/scripts/lib/wic/plugins/source/bootimg-partition.py
+++ b/scripts/lib/wic/plugins/source/bootimg-partition.py
@@ -16,7 +16,7 @@ import logging
 import os
 import re
 
-from glob import glob
+from oe.bootfiles import get_boot_files
 
 from wic import WicError
 from wic.engine import get_custom_config
@@ -66,42 +66,7 @@ class BootimgPartitionPlugin(SourcePlugin):
 
         logger.debug('Boot files: %s', boot_files)
 
-        # list of tuples (src_name, dst_name)
-        deploy_files = []
-        for src_entry in re.findall(r'[\w;\-\./\*]+', boot_files):
-            if ';' in src_entry:
-                dst_entry = tuple(src_entry.split(';'))
-                if not dst_entry[0] or not dst_entry[1]:
-                    raise WicError('Malformed boot file entry: %s' % src_entry)
-            else:
-                dst_entry = (src_entry, src_entry)
-
-            logger.debug('Destination entry: %r', dst_entry)
-            deploy_files.append(dst_entry)
-
-        cls.install_task = [];
-        for deploy_entry in deploy_files:
-            src, dst = deploy_entry
-            if '*' in src:
-                # by default install files under their basename
-                entry_name_fn = os.path.basename
-                if dst != src:
-                    # unless a target name was given, then treat name
-                    # as a directory and append a basename
-                    entry_name_fn = lambda name: \
-                                    os.path.join(dst,
-                                                 os.path.basename(name))
-
-                srcs = glob(os.path.join(kernel_dir, src))
-
-                logger.debug('Globbed sources: %s', ', '.join(srcs))
-                for entry in srcs:
-                    src = os.path.relpath(entry, kernel_dir)
-                    entry_dst_name = entry_name_fn(entry)
-                    cls.install_task.append((src, entry_dst_name))
-            else:
-                cls.install_task.append((src, dst))
-
+        cls.install_task = get_boot_files(kernel_dir, boot_files)
         if source_params.get('loader') != "u-boot":
             return
 
-- 
2.45.1

[PATCH v6 2/2] image-bootfiles.bbclass: new class, copy boot files to root filesystem

[Marcus Folkesson]

image-bootfiles class copy files listed in IMAGE_BOOT_FILES
to the IMAGE_BOOT_FILES_DIR directory of the root filesystem.

This is useful when there is no explicit boot partition but all boot
files should instead reside inside the root filesystem.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
Reviewed-by: Konrad Weihmann <kweihmann@outlook.com>
---

Notes:
    v3:
            - Skip the intermediate bootfiles() function
            - Rename variable names to be consistent
            - Various python optimizations
    v4:
            - Correct usage description
            - Create image_dst before copy
    v5:
            - Check if install_files is [] rather than None
            - Print warning if overwriting files
            - run as ROOTFS_POSTPROCESS_COMMAND instead
    v6:
            - Collect tags to make it easier to apply

 meta/classes/image-bootfiles.bbclass | 41 ++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 meta/classes/image-bootfiles.bbclass

diff --git a/meta/classes/image-bootfiles.bbclass b/meta/classes/image-bootfiles.bbclass
new file mode 100644
index 0000000000..a17c92f66b
--- /dev/null
+++ b/meta/classes/image-bootfiles.bbclass
@@ -0,0 +1,41 @@
+#
+# SPDX-License-Identifier: MIT
+#
+# Copyright (C) 2024 Marcus Folkesson
+# Author: Marcus Folkesson <marcus.folkesson@gmail.com>
+#
+# Writes IMAGE_BOOT_FILES to the IMAGE_BOOT_FILES_DIR directory.
+#
+# Usage: add "inherit image-bootfiles" to your image.
+#
+
+IMAGE_BOOT_FILES_DIR ?= "boot"
+
+python bootfiles_populate() {
+    import shutil
+    from oe.bootfiles import get_boot_files
+
+    deploy_image_dir = d.getVar("DEPLOY_DIR_IMAGE")
+    boot_dir = os.path.join(d.getVar("IMAGE_ROOTFS"), d.getVar("IMAGE_BOOT_FILES_DIR"))
+
+    boot_files = d.getVar("IMAGE_BOOT_FILES")
+    if boot_files is None:
+        return
+
+    install_files = get_boot_files(deploy_image_dir, boot_files)
+    if not install_files:
+        bb.warn("Could not find any boot files to install even though IMAGE_BOOT_FILES is not empty")
+        return
+
+    os.makedirs(boot_dir, exist_ok=True)
+    for src, dst  in install_files:
+        image_src = os.path.join(deploy_image_dir, src)
+        image_dst = os.path.join(boot_dir, dst)
+        if os.path.exists(image_dst):
+            bb.warn("%s does already exist and will be overwritten" % image_dst)
+
+        os.makedirs(os.path.dirname(image_dst), exist_ok=True)
+        shutil.copyfile(image_src, image_dst)
+}
+
+ROOTFS_POSTPROCESS_COMMAND += "bootfiles_populate;"
-- 
2.45.1

Re: [PATCH v6 0/2] image-bootfiles: new class

[Marcus Folkesson]

[-- Attachment #1: Type: text/plain, Size: 2059 bytes --]

Hi,

On Wed, Jul 10, 2024 at 10:53:08AM +0200, Marcus Folkesson wrote:
> The image-bootfiles class is used to put all files listed in
> IMAGE_BOOT_FILES into the root filesystem.
> 
> IMAGE_BOOT_FILES is used by the bootimg-partition wic plugin to put the
> files into a boot partition. 
> Be able to list files as "boot files" in e.g. your .conf or image files
> instead of install those in every recipe is a good thing.
> 
> It is not always desired to have a separate boot partition for boot
> files. Sometimes it could be good to have them as a part of the root
> filesystem.
> 
> For example, if a double copy strategy is used for update the system,
> then you probably want to update both the boot files and root filesystem
> at the same time as there may be dependencies.
> 
> v2:
>     - Removed the documentation from the patch series (will be submitted later)
>     - Break out the parts in bootimg-partition that is used by
>       image-bootfiles to a common library
>     - Make the destination directory in root filesystem configurable
> v3:
>     - See changelog in patches
> 
> v4:
>     - See changelog in patches
> 
> v5:
>     - See changelog in patches
> 
> v6:
>     - See changelog in patches
> 
> Marcus Folkesson (2):
>   bootimg-partition: break out code to a common library.
>   image-bootfiles.bbclass: new class, copy boot files to root filesystem
> 
>  meta/classes/image-bootfiles.bbclass          | 41 +++++++++++++
>  meta/lib/oe/bootfiles.py                      | 57 +++++++++++++++++++
>  .../wic/plugins/source/bootimg-partition.py   | 39 +------------
>  3 files changed, 100 insertions(+), 37 deletions(-)
>  create mode 100644 meta/classes/image-bootfiles.bbclass
>  create mode 100644 meta/lib/oe/bootfiles.py
> 
> -- 
> 2.45.1
> 

I think this is ready to be merged unless someone has any objections.
It has been under review since June 19, v6 that I sent out just under a
week ago just adds Reviewed-by tags to the commit messages.

Thanks,
Marcus Folkesson


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [OE-core] [PATCH v6 0/2] image-bootfiles: new class

[Alexandre Belloni]

On 15/07/2024 12:00:07+0200, Marcus Folkesson wrote:
> 
> I think this is ready to be merged unless someone has any objections.
> It has been under review since June 19, v6 that I sent out just under a
> week ago just adds Reviewed-by tags to the commit messages.
> 

I didn't carry this in my branch yet so it has not been tested but I'll
do this this week.




-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [OE-core] [PATCH v6 0/2] image-bootfiles: new class

[Marcus Folkesson]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

On Mon, Jul 15, 2024 at 02:26:12PM +0200, Alexandre Belloni wrote:
> On 15/07/2024 12:00:07+0200, Marcus Folkesson wrote:
> > 
> > I think this is ready to be merged unless someone has any objections.
> > It has been under review since June 19, v6 that I sent out just under a
> > week ago just adds Reviewed-by tags to the commit messages.
> > 
> 
> I didn't carry this in my branch yet so it has not been tested but I'll
> do this this week.

How did it go?

/Marcus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [OE-core] [PATCH v6 0/2] image-bootfiles: new class

[Richard Purdie]

On Wed, 2024-07-10 at 10:53 +0200, Marcus Folkesson via lists.openembedded.org wrote:
> The image-bootfiles class is used to put all files listed in
> IMAGE_BOOT_FILES into the root filesystem.
> 
> IMAGE_BOOT_FILES is used by the bootimg-partition wic plugin to put the
> files into a boot partition. 
> Be able to list files as "boot files" in e.g. your .conf or image files
> instead of install those in every recipe is a good thing.
> 
> It is not always desired to have a separate boot partition for boot
> files. Sometimes it could be good to have them as a part of the root
> filesystem.
> 
> For example, if a double copy strategy is used for update the system,
> then you probably want to update both the boot files and root filesystem
> at the same time as there may be dependencies.
> 
> v2:
>     - Removed the documentation from the patch series (will be submitted later)
>     - Break out the parts in bootimg-partition that is used by
>       image-bootfiles to a common library
>     - Make the destination directory in root filesystem configurable
> v3:
>     - See changelog in patches
> 
> v4:
>     - See changelog in patches
> 
> v5:
>     - See changelog in patches
> 
> v6:
>     - See changelog in patches
> 
> Marcus Folkesson (2):
>   bootimg-partition: break out code to a common library.
>   image-bootfiles.bbclass: new class, copy boot files to root filesystem

During review we've been in two minds about these changes. It is a
fairly specific use case. In general if you wanted these files in the
rootfs, you'd never have set the variable in the first place. Setting
the variable, then using a class to put them back where they were
originally seems a strange thing to want to do and I'm not sure we want
core support for it.

That said, I understand how someone can get into that situation.

We have therefore merged the first patch which allows a standalone
class but we don't really want to take the class itself. This is partly
also partly as there wouldn't usage/testing in core.

If there becomes a large number of users of it over time, we can
reconsider that decision but right now, I don't think it make sense.

Cheers,

Richard