Re: [OE-core] [PATCH v6 01/21] devicetree: minor improvements

[Bruce Ashfield <bruce.ashfield@gmail.com>]

In message: [OE-core] [PATCH v6 01/21] devicetree: minor improvements
on 02/06/2025 Adrian Freihofer via lists.openembedded.org wrote:

> From: Adrian Freihofer <adrian.freihofer@siemens.com>
> 
> - Do not use the ${} bitbake syntax for shell internal variables
> - Fix shellcheck SC2045 warning:
>   Iterating over ls output is fragile. Use globs.
> - Improve error handling for dtc. Print the output, not only the exit
>   value.
> 
> Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
> ---
>  meta/classes-recipe/devicetree.bbclass | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/meta/classes-recipe/devicetree.bbclass b/meta/classes-recipe/devicetree.bbclass
> index 1806cb62cbf..0d0583c13a0 100644
> --- a/meta/classes-recipe/devicetree.bbclass
> +++ b/meta/classes-recipe/devicetree.bbclass
> @@ -109,7 +109,11 @@ def devicetree_compile(dtspath, includes, d):
>          ppargs.append("-I{0}".format(i))
>      ppargs += ["-o", "{0}.pp".format(dts), dtspath]
>      bb.note("Running {0}".format(" ".join(ppargs)))
> -    subprocess.run(ppargs, check = True)
> +    try:
> +        subprocess.run(ppargs, check=True, capture_output=True)
> +    except subprocess.CalledProcessError as e:
> +        bb.fatal(f"Command '{' '.join(ppargs)}' failed with return code {e.returncode}\nstdout: {e.stdout.decode()}\nstderr: {e.stderr.decode()}\ndtspath: {os.path.abspath(dtspath)}")
> +
>  
>      # determine if the file is an overlay or not (using the preprocessed file)
>      isoverlay = devicetree_source_is_overlay("{0}.pp".format(dts))
> @@ -125,7 +129,11 @@ def devicetree_compile(dtspath, includes, d):
>      dtcargs += ["-o", "{0}.{1}".format(dtname, "dtbo" if isoverlay else "dtb")]
>      dtcargs += ["-I", "dts", "-O", "dtb", "{0}.pp".format(dts)]
>      bb.note("Running {0}".format(" ".join(dtcargs)))
> -    subprocess.run(dtcargs, check = True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
> +    try:
> +        subprocess.run(dtcargs, check=True, capture_output=True)
> +    except subprocess.CalledProcessError as e:
> +        bb.fatal(f"Command '{' '.join(dtcargs)}' failed with return code {e.returncode}\nstdout: {e.stdout.decode()}\nstderr: {e.stderr.decode()}\ndtname: {dtname}")
> +
>  
>  python devicetree_do_compile() {
>      import re
> @@ -144,14 +152,14 @@ python devicetree_do_compile() {
>  }
>  
>  devicetree_do_install() {
> -    for DTB_FILE in `ls *.dtb *.dtbo`; do
> -        install -Dm 0644 ${B}/${DTB_FILE} ${D}/boot/devicetree/${DTB_FILE}
> +    for dtb_file in *.dtb *.dtbo; do
> +        install -Dm 0644 "${B}/$dtb_file" "${D}/boot/devicetree/$dtb_file"
>      done
>  }
>  
>  devicetree_do_deploy() {
> -    for DTB_FILE in `ls *.dtb *.dtbo`; do
> -        install -Dm 0644 ${B}/${DTB_FILE} ${DEPLOYDIR}/devicetree/${DTB_FILE}
> +    for dtb_file in *.dtb *.dtbo; do

The behaviour on no dtb and dtbo files is different between
the two.

Is that a situation we should detect and throw a useful error
message ? It may end up not being possible to happen, but since
this patch is about improving the handling of the other commands,
I thought it might be relevant.

Bruce

> +        install -Dm 0644 "${B}/$dtb_file" "${DEPLOYDIR}/devicetree/$dtb_file"
>      done
>  }
>  addtask deploy before do_build after do_install
> -- 
> 2.49.0
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#217689): https://lists.openembedded.org/g/openembedded-core/message/217689
> Mute This Topic: https://lists.openembedded.org/mt/113424423/1050810
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>