Re: [EXTERNAL] RE: [OE-core][PATCH] kernel-devicetree: allow specification of dtb directory

[Randolph Sapp <rs@ti.com>]

On 5/8/23 01:13, Peter Kjellerstedt wrote:
> That is most certainly a good alternative.
> 
> //Peter
> 
> *From:*Martin Jansa <martin.jansa@gmail.com>
> *Sent:* den 8 maj 2023 00:20
> *To:* Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> *Cc:* rs@ti.com; afd@ti.com; detheridge@ti.com; reatmon@ti.com; 
> denis@denix.org; alexandre.belloni@bootlin.com; 
> openembedded-core@lists.openembedded.org
> *Subject:* Re: [OE-core][PATCH] kernel-devicetree: allow specification 
> of dtb directory
> 
> or
> 
> if "${@'true' if oe.types.boolean(d.getVar('KERNEL_DTBVENDORED')) else 
> 'false'} 
> <mailto:$%7b@'true'%20if%20oe.types.boolean(d.getVar('KERNEL_DTBVENDORED'))%20else%20'false'%7d>"; then
> 
> to avoid test as well (like systemd recipes is using) and I did here as 
> well in:
> 
> https://git.openembedded.org/openembedded-core-contrib/commit/?h=jansa/master&id=e2a6da5202a6671113758f9746ddbd8141a75757 <https://git.openembedded.org/openembedded-core-contrib/commit/?h=jansa/master&id=e2a6da5202a6671113758f9746ddbd8141a75757>
> 
> On Sun, May 7, 2023 at 10:49 PM Peter Kjellerstedt 
> <peter.kjellerstedt@axis.com <mailto:peter.kjellerstedt@axis.com>> wrote:
> 
>     [ Outlook does not support commenting inline for HTML mails, thus
>     I’m top posting…]
> 
>     The problem is `==`, which is a bashism. POSIX shells (like dash)
>     only support `=`. When it comes to quoting, you typically want to
>     quote shell variables in tests in case they are empty. OTOH, static
>     strings without whitespace or other special characters do not need
>     quoting. So the correct way to write the if statements is:
> 
>                     if [ "${KERNEL_DTBVENDORED}" = false ]; then
> 
>     (In this case it is actually a bitbake variable being quoted, but
>     unless you can guarantee it is not empty, the same rule applies.)
> 
>     //Peter
> 
>     *From:*openembedded-core@lists.openembedded.org
>     <mailto:openembedded-core@lists.openembedded.org>
>     <openembedded-core@lists.openembedded.org
>     <mailto:openembedded-core@lists.openembedded.org>> *On Behalf Of
>     *Martin Jansa
>     *Sent:* den 6 maj 2023 11:10
>     *To:* rs@ti.com <mailto:rs@ti.com>
>     *Cc:* afd@ti.com <mailto:afd@ti.com>; detheridge@ti.com
>     <mailto:detheridge@ti.com>; reatmon@ti.com <mailto:reatmon@ti.com>;
>     denis@denix.org <mailto:denis@denix.org>;
>     alexandre.belloni@bootlin.com
>     <mailto:alexandre.belloni@bootlin.com>;
>     openembedded-core@lists.openembedded.org
>     <mailto:openembedded-core@lists.openembedded.org>
>     *Subject:* Re: [OE-core][PATCH] kernel-devicetree: allow
>     specification of dtb directory
> 
>     On Fri, May 5, 2023 at 6:38 PM Randolph Sapp via
>     lists.openembedded.org <http://lists.openembedded.org>
>     <rs=ti.com@lists.openembedded.org
>     <mailto:ti.com@lists.openembedded.org>> wrote:
> 
>         From: Randolph Sapp <rs@ti.com <mailto:rs@ti.com>>
> 
>         Fedora/Redhat and Arch are somewhat standardized on their dtb
>         directory
>         structure. Let's add some flags to configure yocto to mimic that
>         behavior.
> 
>         Add the following variables to the kernel class:
>                  - KERNEL_DTBDEST (controls the destination directory
>         for dtbs)
>                  - KERNEL_DTBVENDORED (controls if vendor subdirectories
>         are to
>                    be respected)
> 
>         Currently KERNEL_DTBDEST is expected to be a subdir of
>         KERNEL_IMAGEDEST
>         and KERNEL_DTBVENDORED is expected to be "true"/"false". This only
>         applies to the package directory structure. The deploydir
>         structure is
>         purposely left untouched for compatibility with existing recipes.
> 
>         By default this is configured to behave the same as the current
>         recipe
>         and produce a flat dtb directory at KERNEL_IMAGEDEST.
> 
>         Signed-off-by: Randolph Sapp <rs@ti.com <mailto:rs@ti.com>>
>         ---
> 
>         Well, suppose I was breaking things by submitting this to kirkstone
>         first. This is just the master version of the following patchset:
>         https://lists.openembedded.org/g/openembedded-core/message/180754 <https://lists.openembedded.org/g/openembedded-core/message/180754>
> 
>         I'd love to get that series merged as well if this patch is
>         acceptable.
> 
>           meta/classes-recipe/kernel-devicetree.bbclass | 22
>         ++++++++++++++-----
>           meta/classes-recipe/kernel.bbclass            |  2 ++
>           2 files changed, 19 insertions(+), 5 deletions(-)
> 
>         diff --git a/meta/classes-recipe/kernel-devicetree.bbclass
>         b/meta/classes-recipe/kernel-devicetree.bbclass
>         index 4d0ecb1032..a6c6c5f227 100644
>         --- a/meta/classes-recipe/kernel-devicetree.bbclass
>         +++ b/meta/classes-recipe/kernel-devicetree.bbclass
>         ...
> 
>         -               dtb_base_name=`basename $dtb .$dtb_ext`
>                          dtb_path=`get_real_dtb_path_in_kernel "$dtb"`
>         -               install -m 0644 $dtb_path
>         ${D}/${KERNEL_IMAGEDEST}/$dtb_base_name.$dtb_ext
>         +               if [ ${KERNEL_DTBVENDORED} == "false" ]; then
> 
>     dash doesn't like this:
> 
>       /bin/dash -c "if [ false == "false" ]; then echo foo; fi"
> 
>     add quotes or use single '='.
> 
>         +                       dtb_ext=${dtb##*.}
>         +                       dtb_base_name=`basename $dtb .$dtb_ext`
>         +                       dtb=$dtb_base_name.$dtb_ext
>         +               fi
>         +               install -Dm 0644 $dtb_path
>         ${D}/${KERNEL_DTBDEST}/$dtb
>                  done
>           }
> 
>         @@ -88,7 +97,10 @@ do_deploy:append() {
>                          dtb_ext=${dtb##*.}
>                          dtb_base_name=`basename $dtb .$dtb_ext`
>                          install -d $deployDir
>         -               install -m 0644
>         ${D}/${KERNEL_IMAGEDEST}/$dtb_base_name.$dtb_ext
>         $deployDir/$dtb_base_name-${KERNEL_DTB_NAME}.$dtb_ext
>         +               if [ ${KERNEL_DTBVENDORED} == "false" ]; then
> 
>     Same here
> 
>         +                       dtb=$dtb_base_name.$dtb_ext
>         +               fi
>         +               install -m 0644 ${D}/${KERNEL_DTBDEST}/$dtb
>         $deployDir/$dtb_base_name-${KERNEL_DTB_NAME}.$dtb_ext
>                          if [ "${KERNEL_IMAGETYPE_SYMLINK}" = "1" ] ; then
>                                  ln -sf
>         $dtb_base_name-${KERNEL_DTB_NAME}.$dtb_ext
>         $deployDir/$dtb_base_name.$dtb_ext
>                          fi
> 

Ah, my bad. I need to figure out a way to get shellcheck to run on bash 
blocks in bb recipes. Would save everyone some time. I'll submit a V2 
with your modifications Martin!