public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
From: "nicusor.huhulea@siemens.com" <nicusor.huhulea@siemens.com>
To: Uladzimir Bely <ubely@ilbers.de>,
	"quirin.gylstorff@siemens.com" <quirin.gylstorff@siemens.com>,
	Nicusor Liviu Huhulea <nicusor_huhulea@mentor.com>,
	"isar-users@googlegroups.com" <isar-users@googlegroups.com>
Subject: Re: [PATCH v2 1/1] copy dtbs into a subdirectory based on image name
Date: Wed, 17 Apr 2024 09:30:24 +0000	[thread overview]
Message-ID: <DB3PR10MB690874D22E815251D4C02A76E60F2@DB3PR10MB6908.EURPRD10.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <83c763aaf967f29b0e47ae05267df0402a5247e1.camel@ilbers.de>

The idea of the kernel's recipe being responsible for the deployment of the dtbs was explored but it just did not seem something straight forward.

Since this problem appeared, several possible solutions have been exposed by different users in different threads, and I just chose one that I believe is the best and has the least interference:
1. changing the bitbake variable DEPLOY_DIR_IMAGE
2. no copy of the dtbs of they are the same
3. change the dtb filename based on the image name
4. move the dtbs into a subdirectory based on image name
5. kernel's recipe should handle this
...(there can be more)

So, each has advantages and disadvantages. What I have implemented just keeps in line with what is currently present. The disadvantages are those dtbs path changes. A case that Jan mentioned about a dirty deploy dir with a different config for the same machine/distro and a need for the down layers to modify the dtb paths as Quirin mentioned.
Having the same dtbs in different folder I don't consider it a disadvantage as there can be cases where the same dtb name implement different functionality.

Kernel's recipe solution seems much feasible but I don't know the implications on it. We can consider this as a transitory solution until a better one is found

Nicu













________________________________________
From: Uladzimir Bely <ubely@ilbers.de>
Sent: Wednesday, April 17, 2024 10:59 AM
To: Gylstorff, Quirin (T CED OES-DE); Nicusor Liviu Huhulea; isar-users@googlegroups.com
Cc: Huhulea, Nicusor Liviu (DI CTO FDS CES LX SVCS)
Subject: Re: [PATCH v2 1/1] copy dtbs into a subdirectory based on image name

On Tue, 2024-04-16 at 17:36 +0200, 'Gylstorff Quirin' via isar-users
wrote:
>
>
> On 4/16/24 12:07 PM, Nicusor Liviu Huhulea wrote:
> > From: Nicusor Huhulea <nicusor.huhulea@siemens.com>
> >
> > There are cases when multiple images are build and because the same
> > dtbs
> > are copied in deploydir we have a build failure. The build error is
> > related
> > to files that are installed into a shared area where those files
> > already exists.
> >
> > To solve this situation we will copy each dtb in a subdirectory
> > based on
> > image name thus avoiding the conflicts. Therefore the other areas
> > needs to be
> > updated on the dtbs paths.
> >
>
> This is a special use case, which breaks all downstream layers which
> currently use devicetrees. Coping to the directory dtbs should be
> disabled by default as for most uses it is unnecessary.
>

As an idea - could it be possible to move dtb file deployment to the
kernel recipe?

We have the conflict in deployment when, e.g., "debug" and "base"
images are built at the same time. But both them use same DTB files
that kernel recipe built and put into /usr/lib/linux-image-${krel}. Why
not make the kernel recipe responcible for deployment of DTB files
then?

On the other hand, this case won't work if we use prebuilt kernel/dtb
packages.

> Quirin
> >
> > Signed-off-by: Nicusor Huhulea <nicusor.huhulea@siemens.com>
> > ---
> >   meta/classes/image.bbclass                    | 21 +++++++++++---
> > -----
> >   meta/classes/imagetypes_wic.bbclass           |  2 +-
> >   .../wic/plugins/source/bootimg-efi-isar.py    |  8 +++++--
> >   scripts/lib/wic/plugins/source/bootimg-efi.py |  8 +++++--
> >   .../wic/plugins/source/bootimg-partition.py   | 10 ++++++++-
> >   5 files changed, 34 insertions(+), 15 deletions(-)
> >
> > diff --git a/meta/classes/image.bbclass
> > b/meta/classes/image.bbclass
> > index 98741da0..e90f8fde 100644
> > --- a/meta/classes/image.bbclass
> > +++ b/meta/classes/image.bbclass
> > @@ -364,16 +364,19 @@ do_copy_boot_files() {
> >           fi
> >       fi
> >
> > -    for file in ${DTB_FILES}; do
> > -        dtb="$(find '${IMAGE_ROOTFS}/usr/lib' -type f \
> > -                    -iwholename '*linux-image-*/'${file} | head -
> > 1)"
> > -
> > -        if [ -z "$dtb" -o ! -e "$dtb" ]; then
> > -            die "${file} not found"
> > -        fi
> > +    if [ -n "${DTB_FILES}" ]; then
> > +        mkdir -p "${DEPLOYDIR}/${IMAGE_FULLNAME}.dtbs"
> > +        for file in ${DTB_FILES}; do
> > +            dtb="$(find '${IMAGE_ROOTFS}/usr/lib' -type f \
> > +                        -iwholename '*linux-image-*/'${file} |
> > head -1)"
> > +
> > +            if [ -z "$dtb" -o ! -e "$dtb" ]; then
> > +                die "${file} not found"
> > +            fi
> >
> > -        cp -f "$dtb" "${DEPLOYDIR}/"
> > -    done
> > +            cp -f "$dtb" "${DEPLOYDIR}/${IMAGE_FULLNAME}.dtbs"
> > +        done
> > +    fi
> >   }
> >   addtask copy_boot_files before do_rootfs_postprocess after
> > do_rootfs_install
> >
> > diff --git a/meta/classes/imagetypes_wic.bbclass
> > b/meta/classes/imagetypes_wic.bbclass
> > index bce881ed..adbde400 100644
> > --- a/meta/classes/imagetypes_wic.bbclass
> > +++ b/meta/classes/imagetypes_wic.bbclass
> > @@ -107,7 +107,7 @@ WICVARS += "\
> >              ROOTFS_SIZE STAGING_DATADIR STAGING_DIR STAGING_LIBDIR
> > TARGET_SYS TRANSLATED_TARGET_ARCH"
> >
> >   # Isar specific vars used in our plugins
> > -WICVARS += "DISTRO DISTRO_ARCH"
> > +WICVARS += "DISTRO DISTRO_ARCH IMAGE_FULLNAME"
> >
> >   python do_rootfs_wicenv () {
> >       wicvars = d.getVar('WICVARS')
> > diff --git a/meta/scripts/lib/wic/plugins/source/bootimg-efi-
> > isar.py b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
> > index 4bfb70a0..218a7fe7 100644
> > --- a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
> > +++ b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
> > @@ -57,7 +57,9 @@ class BootimgEFIPlugin(SourcePlugin):
> >           if dtb:
> >               if ';' in dtb:
> >                   raise WicError("Only one DTB supported, exiting")
> > -            cp_cmd = "cp %s/%s %s" % (bootimg_dir, dtb, hdddir)
> > +            image_fullname = get_bitbake_var("IMAGE_FULLNAME")
> > +            dtbs_dir = os.path.join(bootimg_dir + "/" +
> > image_fullname + ".dtbs")
> > +            cp_cmd = "cp %s/%s %s" % (dtbs_dir, dtb, hdddir)
> >               exec_cmd(cp_cmd, True)
> >
> >       @classmethod
> > @@ -353,8 +355,10 @@ class BootimgEFIPlugin(SourcePlugin):
> >                   if dtb:
> >                       if ';' in dtb:
> >                           raise WicError("Only one DTB supported,
> > exiting")
> > +                    image_fullname =
> > get_bitbake_var("IMAGE_FULLNAME")
> > +                    dtbs_dir = os.path.join(deploy_dir + "/" +
> > image_fullname + ".dtbs")
> >                       dtb_params = '--add-section .dtb=%s/%s --
> > change-section-vma .dtb=0x40000' % \
> > -                        (deploy_dir, dtb)
> > +                        (dtbs_dir, dtb)
> >                   else:
> >                       dtb_params = ''
> >
> > diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py
> > b/scripts/lib/wic/plugins/source/bootimg-efi.py
> > index 634a808d..07b177df 100644
> > --- a/scripts/lib/wic/plugins/source/bootimg-efi.py
> > +++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
> > @@ -51,7 +51,9 @@ class BootimgEFIPlugin(SourcePlugin):
> >           if dtb:
> >               if ';' in dtb:
> >                   raise WicError("Only one DTB supported, exiting")
> > -            cp_cmd = "cp %s/%s %s" % (bootimg_dir, dtb, hdddir)
> > +            image_fullname = get_bitbake_var("IMAGE_FULLNAME")
> > +            dtbs_dir = os.path.join(bootimg_dir + "/" +
> > image_fullname + ".dtbs")
> > +            cp_cmd = "cp %s/%s %s" % (dtbs_dir, dtb, hdddir)
> >               exec_cmd(cp_cmd, True)
> >
> >       @classmethod
> > @@ -334,8 +336,10 @@ class BootimgEFIPlugin(SourcePlugin):
> >                   if dtb:
> >                       if ';' in dtb:
> >                           raise WicError("Only one DTB supported,
> > exiting")
> > +                    image_fullname =
> > get_bitbake_var("IMAGE_FULLNAME")
> > +                    dtbs_dir = os.path.join(deploy_dir + "/" +
> > image_fullname + ".dtbs")
> >                       dtb_params = '--add-section .dtb=%s/%s --
> > change-section-vma .dtb=0x40000' % \
> > -                        (deploy_dir, dtb)
> > +                        (dtbs_dir, dtb)
> >                   else:
> >                       dtb_params = ''
> >
> > diff --git a/scripts/lib/wic/plugins/source/bootimg-partition.py
> > b/scripts/lib/wic/plugins/source/bootimg-partition.py
> > index 5dbe2558..1ae6216f 100644
> > --- a/scripts/lib/wic/plugins/source/bootimg-partition.py
> > +++ b/scripts/lib/wic/plugins/source/bootimg-partition.py
> > @@ -180,10 +180,18 @@ class BootimgPartitionPlugin(SourcePlugin):
> >
> >           logger.debug('Kernel dir: %s', bootimg_dir)
> >
> > +        image_fullname = get_bitbake_var("IMAGE_FULLNAME")
> > +        dtbs_dir = os.path.join(kernel_dir + "/" + image_fullname
> > + ".dtbs/")
> >
> >           for task in cls.install_task:
> >               src_path, dst_path = task
> > -            logger.debug('Install %s as %s', src_path, dst_path)
> > +
> > +            dtb_file = os.path.join(dtbs_dir +
> > os.path.basename(src_path))
> > +
> > +            if os.path.exists(dtb_file):
> > +                src_path = os.path.join(dtbs_dir + src_path)
> > +
> > +            logger.debug('Install %s as %s', (src_path, dst_path))
> >               install_cmd = "install -m 0644 -D %s %s" \
> >                             % (os.path.join(kernel_dir, src_path),
> >                                os.path.join(hdddir, dst_path))
>


  reply	other threads:[~2024-04-17  9:30 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-16 10:07 Nicusor Liviu Huhulea
2024-04-16 15:36 ` Gylstorff Quirin
2024-04-17  7:59   ` Uladzimir Bely
2024-04-17  9:30     ` nicusor.huhulea [this message]
2024-04-17  8:25   ` Baurzhan Ismagulov
2024-04-19  9:45     ` Gylstorff Quirin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DB3PR10MB690874D22E815251D4C02A76E60F2@DB3PR10MB6908.EURPRD10.PROD.OUTLOOK.COM \
    --to=nicusor.huhulea@siemens.com \
    --cc=isar-users@googlegroups.com \
    --cc=nicusor_huhulea@mentor.com \
    --cc=quirin.gylstorff@siemens.com \
    --cc=ubely@ilbers.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox