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

[Marcus Folkesson <marcus.folkesson@gmail.com>]

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

Hi Quentin,

On Tue, May 28, 2024 at 01:01:29PM +0200, Quentin Schulz wrote:
> Hi Marcus,
> 
> On 5/28/24 11:08 AM, Marcus Folkesson wrote:
> > 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>
> > ---
> > 
> > Notes:
> >      v3:
> >         - Removed unnecessary "import glob"
> 
> Ah, there was a misunderstanding here, the "wrong" import glob got removed
> :)

Ah, of course.

> 
> >         - Explicitely tell that bootfiles_populate() returns a tuple
> > 
> >   meta/lib/oe/bootfiles.py                      | 56 +++++++++++++++++++
> >   .../wic/plugins/source/bootimg-partition.py   | 39 +------------
> >   2 files changed, 59 insertions(+), 36 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..666141df4e
> > --- /dev/null
> > +++ b/meta/lib/oe/bootfiles.py
> > @@ -0,0 +1,56 @@
> > +#
> > +# 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
> > +
> 
> We need from glob import glob here........
> 
> > +    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))
> > +
> 
> .... otherwise this won't work (it'll I believe but only if the python
> module that calls it imports glob itself).

Yep, it did, that was why my testing did not catch it.

> 
> > +            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..b22a448b65 100644
> > --- a/scripts/lib/wic/plugins/source/bootimg-partition.py
> > +++ b/scripts/lib/wic/plugins/source/bootimg-partition.py
> > @@ -18,6 +18,8 @@ import re
> >   from glob import glob
> 
> It's this glob we don't need anymore, because it would then be imported by
> the python lib added in this very patch.
> 
> Cheers,
> Quentin


Thanks again Quentin.
I will delay v4 a day or two to catch more comments.


Best regards,
Marcus Folkesson

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