From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2761FC27C6E for ; Sun, 16 Jun 2024 04:26:55 +0000 (UTC) Subject: Re: [PATCH v4 2/2] image-bootfiles.bbclass: new class, copy boot files to root filesystem To: openembedded-core@lists.openembedded.org From: "Konrad Weihmann" X-Originating-Location: Berlin, Land Berlin, DE (89.12.182.18) X-Originating-Platform: Linux Chrome 124 User-Agent: GROUPS.IO Web Poster MIME-Version: 1.0 Date: Sat, 15 Jun 2024 21:26:49 -0700 References: <20240530095314.407638-3-marcus.folkesson@gmail.com> In-Reply-To: <20240530095314.407638-3-marcus.folkesson@gmail.com> Message-ID: <20106.1718512009844562919@lists.openembedded.org> Content-Type: multipart/alternative; boundary="VLRZVXlNL9C7H4mMTSOA" List-Id: X-Webhook-Received: from li982-79.members.linode.com [45.33.32.79] by aws-us-west-2-korg-lkml-1.web.codeaurora.org with HTTPS for ; Sun, 16 Jun 2024 04:26:55 -0000 X-Groupsio-URL: https://lists.openembedded.org/g/openembedded-core/message/200772 --VLRZVXlNL9C7H4mMTSOA Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Looking at the combination of the patches and the following + boot_files =3D d.getVar("IMAGE_BOOT_FILES") + if boot_files is None: + return + + install_files =3D get_boot_files(deploy_image_dir, boot_files) + if install_files is None: + bb.warn("Could not find any boot files to install even though IMAGE_BOOT_= FILES is not empty") + return get_boot_files only returns None if boot_files is None, which is caught alr= eady earlier on here, so the bb.warn clause likely will not be reached Personally a check like `if install_files:` would make more sense IMAGE_PREPROCESS_COMMAND +=3D "bootfiles_populate;" I'm not sure if that wouldn't raise issues if any of the packages would ins= tall a file by the same path/name as also set here. In my opinion moving it to a POSTPROCESS function with some checks that it = won't overwrite any files/folders might be the better option --VLRZVXlNL9C7H4mMTSOA Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable

Looking at the combination of the patches and the following

 

+ boot_files =3D d.getVar("IMAGE_BOOT_FILES")
+ if boot_files is None:
+ return
= +
+ install_files = =3D get_boot_files(deploy_image_dir, boot_files)
+ if install_files is None:
+ bb.warn("Could not find any boot files to install ev= en though IMAGE_BOOT_FILES is not empty")
+ return

 

get_boot_files only returns None if boot_files is No= ne, which is caught already earlier on here, so the bb.warn clause likely w= ill not be reached

Personally a check like `if install_files:` would ma= ke more sense

 

IMAGE_PREPROCESS_COMMAND +=3D "bootfiles_p= opulate;"

 

I'm not sure if that wouldn't raise issues if any of the packages would = install a file by the same path/name as also set here.

In my opinion moving it to a POSTPROCESS function with some checks that = it won't overwrite any files/folders might be the better option

--VLRZVXlNL9C7H4mMTSOA--