[Buildroot] [PATCH 0/3 v4] support/br2-external: fix shellcheck and better document BR2_EXTERNAL (branch yem/misc)

[Buildroot] [PATCH 0/3 v4] support/br2-external: fix shellcheck and better document BR2_EXTERNAL (branch yem/misc)

[Yann E. MORIN via buildroot]

Hello All!

This small series is a respin of an old patch [0] that has been
lingering for quite some time now...

Baiscally, the case is to only document the fact that BR2_EXTERNAL is a
space-separated list, like all other lists in Buildroot, and not a
colon-separated one (but still support colon-separated lists for legacy
purposes).

As a bonus from the previous iterations, the series now cleans up
shelcheck errors in the script.

[0] https://patchwork.ozlabs.org/project/buildroot/patch/20240904205354.504964-1-yann.morin.1998@free.fr/

Changes v3 -> v4:
  - fix pre-existing shellcheck errors
  - extend commit log with more data

Changes v2 -> v3:
  - add comment in support/scripts/br2-external  (Fiona)

Changes v1 -> v2:
  - fix typoes

Regards,
Yann E. MORIN.


----------------------------------------------------------------
Yann E. MORIN (3):
      support/br2-external: remove leftover trap
      support/br2-external: fix remaining shellcheck errors
      docs/manual: use space-separated list for BR2_EXTERNAL

 .checkpackageignore                   |  1 -
 docs/manual/customize-outside-br.adoc |  5 +++--
 support/scripts/br2-external          | 20 ++++++++++++--------
 3 files changed, 15 insertions(+), 11 deletions(-)

--
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 1/3 v4] support/br2-external: remove leftover trap

[Yann E. MORIN via buildroot]

The trap was initially introduced in c5fa9308ea4e (core/br2-external:
properly report unexpected errors), in 2017, to catch all unexpected
errors, back when a single file was generated, and errors emitted to
stderr.

Since commit d027cd75d098 (core: generate all br2-external files in
one go), in 2019 the single output file 'ofile' is no longer created,
as multiple output files were then introduced, while messages for
*expected errors* were redirected to a Makefile variable assignment
emitted on stdout, at which point the script just exits (in error);
expected failures only occur in do_validate().

Unexpected errors can only occur on failure to create, or write to,
output files, either '.br2-external.mk' in do_validate() or do_mk(),
or any of the kconfig fragments in do_kconfig(). Cause for failure to
create those can only be a no-space-left-on-device condition, as they
are created in a directory that was just created by the script earlier
in main(), and thus has the necessary mode; failure to create that
directory is now caught explicitly.

A trap on ERR is not called when the shell exits explicitly with a call
to 'exit', thus, only failures to create or write to output file would
be caught. In that case, we are better off not trying to write to those
files anyway: failure to create the file would already be reported by
the shell on stderr, while disk-full would not allow to store the output
anyway...

In any case, the script exits in error, which is going to be caught by
the caller, which will terminate.

So, drop the trap altogether.

As a side effect, that squelches a shellcheck error.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

---
Note: remaining shellcheck errors will be addressed separately in the
following patch.
---
 support/scripts/br2-external | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/support/scripts/br2-external b/support/scripts/br2-external
index 8aea479d20..6d4fc1a791 100755
--- a/support/scripts/br2-external
+++ b/support/scripts/br2-external
@@ -30,10 +30,10 @@ main() {
         error "no output directory specified (-d)\n"
     fi
 
-    # Trap any unexpected error to generate a meaningful error message
-    trap "error 'unexpected error while generating ${ofile}\n'" ERR
+    if ! mkdir -p "${outputdir}"; then
+        error "Cannot create output directory '%s'\n" "${outputdir}"
+    fi
 
-    mkdir -p "${outputdir}"
     do_validate "${outputdir}" ${@//:/ }
     do_mk "${outputdir}"
     do_kconfig "${outputdir}"
-- 
2.52.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 2/3 v4] support/br2-external: fix remaining shellcheck errors

[Yann E. MORIN via buildroot]

Boring changes: either do what shellcheck suggested, or comment why we
don't want to fix the code.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 .checkpackageignore          |  1 -
 support/scripts/br2-external | 11 ++++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/.checkpackageignore b/.checkpackageignore
index 16a0d5b511..67e531512e 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -1085,7 +1085,6 @@ support/libtool/buildroot-libtool-v2.4.4.patch lib_patch.ApplyOrder lib_patch.Up
 support/libtool/buildroot-libtool-v2.4.patch lib_patch.ApplyOrder lib_patch.Sob lib_patch.Upstream
 support/misc/relocate-sdk.sh Shellcheck
 support/scripts/apply-patches.sh Shellcheck
-support/scripts/br2-external Shellcheck
 support/scripts/check-bin-arch Shellcheck
 support/scripts/check-host-rpath Shellcheck
 support/scripts/expunge-gconv-modules Shellcheck
diff --git a/support/scripts/br2-external b/support/scripts/br2-external
index 6d4fc1a791..474feaded7 100755
--- a/support/scripts/br2-external
+++ b/support/scripts/br2-external
@@ -34,6 +34,7 @@ main() {
         error "Cannot create output directory '%s'\n" "${outputdir}"
     fi
 
+    # shellcheck disable=SC2068  # We do want to split on spaces
     do_validate "${outputdir}" ${@//:/ }
     do_mk "${outputdir}"
     do_kconfig "${outputdir}"
@@ -71,7 +72,7 @@ do_validate_one() {
     if [ ! -d "${br2_ext}" ]; then
         error "'%s': no such file or directory\n" "${br2_ext}"
     fi
-    if [ ! -r "${br2_ext}" -o ! -x "${br2_ext}" ]; then
+    if [ ! -r "${br2_ext}" ] || [ ! -x "${br2_ext}" ]; then
         error "'%s': permission denied\n" "${br2_ext}"
     fi
     if [ ! -f "${br2_ext}/external.desc" ]; then
@@ -106,9 +107,9 @@ do_validate_one() {
     br2_ext="$( cd "${br2_ext}"; pwd )"
     br2_ver="$( support/scripts/setlocalversion "${br2_ext}" )"
     BR2_EXT_NAMES+=( "${br2_name}" )
-    eval BR2_EXT_PATHS_${br2_name}="\"\${br2_ext}\""
-    eval BR2_EXT_VERS_${br2_name}="\"\${br2_ver}\""
-    eval BR2_EXT_DESCS_${br2_name}="\"\${br2_desc:-\${br2_name}}\""
+    eval "BR2_EXT_PATHS_${br2_name}=\"\${br2_ext}\""
+    eval "BR2_EXT_VERS_${br2_name}=\"\${br2_ver}\""
+    eval "BR2_EXT_DESCS_${br2_name}=\"\${br2_desc:-\${br2_name}}\""
 }
 
 # Generate the .mk snippet that defines makefile variables
@@ -272,7 +273,7 @@ do_kconfig() {
     printf 'endmenu\n' >>"${outputdir}/.br2-external.in.menus"
 }
 
+# shellcheck disable=SC2059  # fmt *is* a format
 error() { local fmt="${1}"; shift; printf "BR2_EXTERNAL_ERROR = ${fmt}" "${@}"; exit 1; }
 
-my_name="${0##*/}"
 main "${@}"
-- 
2.52.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 3/3 v4] docs/manual: use space-separated list for BR2_EXTERNAL

[Yann E. MORIN via buildroot]

Specifying a list of br2-external trees is poorly documented, and the
only example uses a colon to separate the br2-external paths.

Adding the support for colon-separated list is the biggest mistake that
was made when introducing support for multiple br2-external [0]. Indeed,
both space and colon can be used to separate entries in the list, and it
is also possible to mix the two. However, internally, the list is stored
as a space-separated list, and all the code will split on spaces.

Besides, all other lists in Buildroot are a space-separated:
    BR2_ROOTFS_DEVICE_TABLE
    BR2_ROOTFS_STATIC_DEVICE_TABLE
    BR2_TARGET_TZ_ZONELIST
    BR2_ROOTFS_USERS_TABLES
    BR2_ROOTFS_OVERLAY
    BR2_ROOTFS_PRE_BUILD_SCRIPT
    BR2_ROOTFS_POST_BUILD_SCRIPT
    BR2_ROOTFS_POST_FAKEROOT_SCRIPT
    BR2_ROOTFS_POST_IMAGE_SCRIPT
    ...

So, using colons is odd.

The fact that BR2_EXTERNAL is pqssed on the command line rather than
being a Kconfig item is not a reason enough to justify that it be
colon-separated.

Change the documentation to only mention using a space-separated list.

Of course, for backward compatibility, we keep the code as-is to accept
a colon-separated list, but we just do not advertise it.

Note that keeping the split on colons means that colons are not accepted
in pathnames of br2-external trees; in practice, this is not a new
restriction, or one that could lift as usign colons in Makefiles are
problematic anyway.

[0] in 20cd49738781 core: add support for multiple br2-external trees

Reported-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
Reported-by: Brandon Maier <Brandon.Maier@collins.com>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

---
Note: support/scripts/br2-external has shellcheck issues; this commit
does not try to address those on purpose, as it only adds a comment.

---
Changes v3 -> v4:
  - add an extract of all other list variables from [1]
  - sumarise and include Arnout's comments from [2]
  - fix typo

[1] https://patchwork.ozlabs.org/project/buildroot/patch/20240904205354.504964-1-yann.morin.1998@free.fr/#3375145
[2] https://patchwork.ozlabs.org/project/buildroot/patch/20240904205354.504964-1-yann.morin.1998@free.fr/#3375416

Changes v2 -> v3:
  - add comment in support/scripts/br2-external  (Fiona)

Changes v1 -> v2:
  - fix typoes
---
 docs/manual/customize-outside-br.adoc | 5 +++--
 support/scripts/br2-external          | 3 +++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/docs/manual/customize-outside-br.adoc b/docs/manual/customize-outside-br.adoc
index 78065489d4..8a76f3a998 100644
--- a/docs/manual/customize-outside-br.adoc
+++ b/docs/manual/customize-outside-br.adoc
@@ -58,10 +58,11 @@ We can switch to another br2-external tree at any time:
 buildroot/ $ make BR2_EXTERNAL=/where/we/have/bar xconfig
 ----
 
-We can also use multiple br2-external trees:
+We can also use multiple br2-external trees, by specifying a space-separated
+list of paths to use:
 
 ----
-buildroot/ $ make BR2_EXTERNAL=/path/to/foo:/where/we/have/bar menuconfig
+buildroot/ $ make BR2_EXTERNAL="/path/to/foo /where/we/have/bar" menuconfig
 ----
 
 Or disable the usage of any br2-external tree:
diff --git a/support/scripts/br2-external b/support/scripts/br2-external
index 474feaded7..641f8e18ae 100755
--- a/support/scripts/br2-external
+++ b/support/scripts/br2-external
@@ -34,6 +34,9 @@ main() {
         error "Cannot create output directory '%s'\n" "${outputdir}"
     fi
 
+    # Historically, BR2_EXTERNAL could also be colon-separated, so for
+    # backward compatibility, keep splitting on colons (in addition to
+    # spaces).
     # shellcheck disable=SC2068  # We do want to split on spaces
     do_validate "${outputdir}" ${@//:/ }
     do_mk "${outputdir}"
-- 
2.52.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/3 v4] support/br2-external: remove leftover trap

[Romain Naour via buildroot]

Le 03/02/2026 à 22:54, Yann E. MORIN via buildroot a écrit :
> The trap was initially introduced in c5fa9308ea4e (core/br2-external:
> properly report unexpected errors), in 2017, to catch all unexpected
> errors, back when a single file was generated, and errors emitted to
> stderr.
> 
> Since commit d027cd75d098 (core: generate all br2-external files in
> one go), in 2019 the single output file 'ofile' is no longer created,
> as multiple output files were then introduced, while messages for
> *expected errors* were redirected to a Makefile variable assignment
> emitted on stdout, at which point the script just exits (in error);
> expected failures only occur in do_validate().
> 
> Unexpected errors can only occur on failure to create, or write to,
> output files, either '.br2-external.mk' in do_validate() or do_mk(),
> or any of the kconfig fragments in do_kconfig(). Cause for failure to
> create those can only be a no-space-left-on-device condition, as they
> are created in a directory that was just created by the script earlier
> in main(), and thus has the necessary mode; failure to create that
> directory is now caught explicitly.
> 
> A trap on ERR is not called when the shell exits explicitly with a call
> to 'exit', thus, only failures to create or write to output file would
> be caught. In that case, we are better off not trying to write to those
> files anyway: failure to create the file would already be reported by
> the shell on stderr, while disk-full would not allow to store the output
> anyway...
> 
> In any case, the script exits in error, which is going to be caught by
> the caller, which will terminate.
> 
> So, drop the trap altogether.
> 
> As a side effect, that squelches a shellcheck error.
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

Applied to master, thanks.

Best regards,
Romain


> 
> ---
> Note: remaining shellcheck errors will be addressed separately in the
> following patch.
> ---
>  support/scripts/br2-external | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/support/scripts/br2-external b/support/scripts/br2-external
> index 8aea479d20..6d4fc1a791 100755
> --- a/support/scripts/br2-external
> +++ b/support/scripts/br2-external
> @@ -30,10 +30,10 @@ main() {
>          error "no output directory specified (-d)\n"
>      fi
>  
> -    # Trap any unexpected error to generate a meaningful error message
> -    trap "error 'unexpected error while generating ${ofile}\n'" ERR
> +    if ! mkdir -p "${outputdir}"; then
> +        error "Cannot create output directory '%s'\n" "${outputdir}"
> +    fi
>  
> -    mkdir -p "${outputdir}"
>      do_validate "${outputdir}" ${@//:/ }
>      do_mk "${outputdir}"
>      do_kconfig "${outputdir}"

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 3/3 v4] docs/manual: use space-separated list for BR2_EXTERNAL

[Thomas Perale via buildroot]

In reply of:
> Specifying a list of br2-external trees is poorly documented, and the
> only example uses a colon to separate the br2-external paths.
> 
> Adding the support for colon-separated list is the biggest mistake that
> was made when introducing support for multiple br2-external [0]. Indeed,
> both space and colon can be used to separate entries in the list, and it
> is also possible to mix the two. However, internally, the list is stored
> as a space-separated list, and all the code will split on spaces.
> 
> Besides, all other lists in Buildroot are a space-separated:
>     BR2_ROOTFS_DEVICE_TABLE
>     BR2_ROOTFS_STATIC_DEVICE_TABLE
>     BR2_TARGET_TZ_ZONELIST
>     BR2_ROOTFS_USERS_TABLES
>     BR2_ROOTFS_OVERLAY
>     BR2_ROOTFS_PRE_BUILD_SCRIPT
>     BR2_ROOTFS_POST_BUILD_SCRIPT
>     BR2_ROOTFS_POST_FAKEROOT_SCRIPT
>     BR2_ROOTFS_POST_IMAGE_SCRIPT
>     ...
> 
> So, using colons is odd.
> 
> The fact that BR2_EXTERNAL is pqssed on the command line rather than
> being a Kconfig item is not a reason enough to justify that it be
> colon-separated.
> 
> Change the documentation to only mention using a space-separated list.
> 
> Of course, for backward compatibility, we keep the code as-is to accept
> a colon-separated list, but we just do not advertise it.
> 
> Note that keeping the split on colons means that colons are not accepted
> in pathnames of br2-external trees; in practice, this is not a new
> restriction, or one that could lift as usign colons in Makefiles are
> problematic anyway.
> 
> [0] in 20cd49738781 core: add support for multiple br2-external trees
> 
> Reported-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
> Reported-by: Brandon Maier <Brandon.Maier@collins.com>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 

Applied to 2025.02.x & 2026.02.x. Thanks

> ---
> Note: support/scripts/br2-external has shellcheck issues; this commit
> does not try to address those on purpose, as it only adds a comment.
> 
> ---
> Changes v3 -> v4:
>   - add an extract of all other list variables from [1]
>   - sumarise and include Arnout's comments from [2]
>   - fix typo
> 
> [1] https://patchwork.ozlabs.org/project/buildroot/patch/20240904205354.504964-1-yann.morin.1998@free.fr/#3375145
> [2] https://patchwork.ozlabs.org/project/buildroot/patch/20240904205354.504964-1-yann.morin.1998@free.fr/#3375416
> 
> Changes v2 -> v3:
>   - add comment in support/scripts/br2-external  (Fiona)
> 
> Changes v1 -> v2:
>   - fix typoes
> ---
>  docs/manual/customize-outside-br.adoc | 5 +++--
>  support/scripts/br2-external          | 3 +++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/manual/customize-outside-br.adoc b/docs/manual/customize-outside-br.adoc
> index 78065489d4..8a76f3a998 100644
> --- a/docs/manual/customize-outside-br.adoc
> +++ b/docs/manual/customize-outside-br.adoc
> @@ -58,10 +58,11 @@ We can switch to another br2-external tree at any time:
>  buildroot/ $ make BR2_EXTERNAL=/where/we/have/bar xconfig
>  ----
>  
> -We can also use multiple br2-external trees:
> +We can also use multiple br2-external trees, by specifying a space-separated
> +list of paths to use:
>  
>  ----
> -buildroot/ $ make BR2_EXTERNAL=/path/to/foo:/where/we/have/bar menuconfig
> +buildroot/ $ make BR2_EXTERNAL="/path/to/foo /where/we/have/bar" menuconfig
>  ----
>  
>  Or disable the usage of any br2-external tree:
> diff --git a/support/scripts/br2-external b/support/scripts/br2-external
> index 474feaded7..641f8e18ae 100755
> --- a/support/scripts/br2-external
> +++ b/support/scripts/br2-external
> @@ -34,6 +34,9 @@ main() {
>          error "Cannot create output directory '%s'\n" "${outputdir}"
>      fi
>  
> +    # Historically, BR2_EXTERNAL could also be colon-separated, so for
> +    # backward compatibility, keep splitting on colons (in addition to
> +    # spaces).
>      # shellcheck disable=SC2068  # We do want to split on spaces
>      do_validate "${outputdir}" ${@//:/ }
>      do_mk "${outputdir}"
> -- 
> 2.52.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 2/3 v4] support/br2-external: fix remaining shellcheck errors

[Thomas Perale via buildroot]

In reply of:
> Boring changes: either do what shellcheck suggested, or comment why we
> don't want to fix the code.
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

Applied to 2025.02.x & 2026.02.x. Thanks

> ---
>  .checkpackageignore          |  1 -
>  support/scripts/br2-external | 11 ++++++-----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/.checkpackageignore b/.checkpackageignore
> index 16a0d5b511..67e531512e 100644
> --- a/.checkpackageignore
> +++ b/.checkpackageignore
> @@ -1085,7 +1085,6 @@ support/libtool/buildroot-libtool-v2.4.4.patch lib_patch.ApplyOrder lib_patch.Up
>  support/libtool/buildroot-libtool-v2.4.patch lib_patch.ApplyOrder lib_patch.Sob lib_patch.Upstream
>  support/misc/relocate-sdk.sh Shellcheck
>  support/scripts/apply-patches.sh Shellcheck
> -support/scripts/br2-external Shellcheck
>  support/scripts/check-bin-arch Shellcheck
>  support/scripts/check-host-rpath Shellcheck
>  support/scripts/expunge-gconv-modules Shellcheck
> diff --git a/support/scripts/br2-external b/support/scripts/br2-external
> index 6d4fc1a791..474feaded7 100755
> --- a/support/scripts/br2-external
> +++ b/support/scripts/br2-external
> @@ -34,6 +34,7 @@ main() {
>          error "Cannot create output directory '%s'\n" "${outputdir}"
>      fi
>  
> +    # shellcheck disable=SC2068  # We do want to split on spaces
>      do_validate "${outputdir}" ${@//:/ }
>      do_mk "${outputdir}"
>      do_kconfig "${outputdir}"
> @@ -71,7 +72,7 @@ do_validate_one() {
>      if [ ! -d "${br2_ext}" ]; then
>          error "'%s': no such file or directory\n" "${br2_ext}"
>      fi
> -    if [ ! -r "${br2_ext}" -o ! -x "${br2_ext}" ]; then
> +    if [ ! -r "${br2_ext}" ] || [ ! -x "${br2_ext}" ]; then
>          error "'%s': permission denied\n" "${br2_ext}"
>      fi
>      if [ ! -f "${br2_ext}/external.desc" ]; then
> @@ -106,9 +107,9 @@ do_validate_one() {
>      br2_ext="$( cd "${br2_ext}"; pwd )"
>      br2_ver="$( support/scripts/setlocalversion "${br2_ext}" )"
>      BR2_EXT_NAMES+=( "${br2_name}" )
> -    eval BR2_EXT_PATHS_${br2_name}="\"\${br2_ext}\""
> -    eval BR2_EXT_VERS_${br2_name}="\"\${br2_ver}\""
> -    eval BR2_EXT_DESCS_${br2_name}="\"\${br2_desc:-\${br2_name}}\""
> +    eval "BR2_EXT_PATHS_${br2_name}=\"\${br2_ext}\""
> +    eval "BR2_EXT_VERS_${br2_name}=\"\${br2_ver}\""
> +    eval "BR2_EXT_DESCS_${br2_name}=\"\${br2_desc:-\${br2_name}}\""
>  }
>  
>  # Generate the .mk snippet that defines makefile variables
> @@ -272,7 +273,7 @@ do_kconfig() {
>      printf 'endmenu\n' >>"${outputdir}/.br2-external.in.menus"
>  }
>  
> +# shellcheck disable=SC2059  # fmt *is* a format
>  error() { local fmt="${1}"; shift; printf "BR2_EXTERNAL_ERROR = ${fmt}" "${@}"; exit 1; }
>  
> -my_name="${0##*/}"
>  main "${@}"
> -- 
> 2.52.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/3 v4] support/br2-external: remove leftover trap

[Thomas Perale via buildroot]

In reply of:
> The trap was initially introduced in c5fa9308ea4e (core/br2-external:
> properly report unexpected errors), in 2017, to catch all unexpected
> errors, back when a single file was generated, and errors emitted to
> stderr.
> 
> Since commit d027cd75d098 (core: generate all br2-external files in
> one go), in 2019 the single output file 'ofile' is no longer created,
> as multiple output files were then introduced, while messages for
> *expected errors* were redirected to a Makefile variable assignment
> emitted on stdout, at which point the script just exits (in error);
> expected failures only occur in do_validate().
> 
> Unexpected errors can only occur on failure to create, or write to,
> output files, either '.br2-external.mk' in do_validate() or do_mk(),
> or any of the kconfig fragments in do_kconfig(). Cause for failure to
> create those can only be a no-space-left-on-device condition, as they
> are created in a directory that was just created by the script earlier
> in main(), and thus has the necessary mode; failure to create that
> directory is now caught explicitly.
> 
> A trap on ERR is not called when the shell exits explicitly with a call
> to 'exit', thus, only failures to create or write to output file would
> be caught. In that case, we are better off not trying to write to those
> files anyway: failure to create the file would already be reported by
> the shell on stderr, while disk-full would not allow to store the output
> anyway...
> 
> In any case, the script exits in error, which is going to be caught by
> the caller, which will terminate.
> 
> So, drop the trap altogether.
> 
> As a side effect, that squelches a shellcheck error.
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> 

Applied to 2025.02.x & 2026.02.x. Thanks

> ---
> Note: remaining shellcheck errors will be addressed separately in the
> following patch.
> ---
>  support/scripts/br2-external | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/support/scripts/br2-external b/support/scripts/br2-external
> index 8aea479d20..6d4fc1a791 100755
> --- a/support/scripts/br2-external
> +++ b/support/scripts/br2-external
> @@ -30,10 +30,10 @@ main() {
>          error "no output directory specified (-d)\n"
>      fi
>  
> -    # Trap any unexpected error to generate a meaningful error message
> -    trap "error 'unexpected error while generating ${ofile}\n'" ERR
> +    if ! mkdir -p "${outputdir}"; then
> +        error "Cannot create output directory '%s'\n" "${outputdir}"
> +    fi
>  
> -    mkdir -p "${outputdir}"
>      do_validate "${outputdir}" ${@//:/ }
>      do_mk "${outputdir}"
>      do_kconfig "${outputdir}"
> -- 
> 2.52.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot