[Buildroot] [PATCH 1/2] core.br2-external: fix reporting errors

[Romain Naour]

Le 22/05/2020 à 23:40, Yann E. MORIN a écrit :
> When a br2-external tree has an issue, e.g. a missing file, or does not
> have a name, or the name uses invalid cahrs, we report that condition by
> setting the variable BR2_EXTERNAL_ERROR.
> 
> That variable is defined in the script support/scripts/br2-external,
> which outputs it on stdout, and checked by the Makefile.
> 
> Before d027cd75d09, stdout was explicitly redirected to the generated
> .mk file, with   exec >"${ofile}"   as the Makefile and Kconfig
> fragments were generated each with their own call to the script, and o
> the validation phase would emit the BR2_EXTERNAL_ERROR variable in the
> Makefile fragment.
> 
> But with d027cd75d09, both the Makefile and Kconfig fragmetns were now
> generated with a single call to the script, and as such the semantics of
> the scripts changed, and only each of the actual generators, do_mk and
> do_kconfig, ahd their out put redirected. Which left do_validate with
> the default stdout. Which would emit BR2_EXTERNAL_ERROR on stdout.
> 
> In turn, the stdout of the script would be interpreted by as part of the
> Makefile. But this does not end up very well when a br2-external tree
> indeed has an error:
> 
>     Makefile:184: *** multiple target patterns.  Stop.

When external.desc exist but empty, I get another error (I'm currently on master)

$ echo "" > support/testing/tests/download/br2-external/git-hash/external.desc
$ make BR2_EXTERNAL=$PWD/support/testing/tests/download/br2-external/git-hash/
menuconfig

Config.in:22: can't open file "output/.br2-external.in.paths"

But Indeed, I get the same error when external.desc is missing.


Here is the error message displayed with the series applied:

case: missing external.desc

  does not have an 'external.desc'. See
https://buildroot.org/manual.html#br2-external-converting.

case: empty external.desc

  does not define the name.

While at it you can use MANUAL_URL in the error message when the external name
is missing.

With that fixed(improved :p):
Reviewed-by: Romain Naour <romain.naour at gmail.com>
Tested-by: Romain Naour <romain.naour at gmail.com>

Also, it would be great to have some tests in the testsuite for such cases. It's
easy to make a mistake while adding a br2_external tree.

Best regards,
Romain


> 
> So we must redirect the output of the validation step to the
> Makefile fragment.
> 
> Note that we don't need to append in do_mk, and we can do an overwrite
> redirection: if we go so far as to call do_mk, it means there was no
> error, and thus the fragment is empty.
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998 at free.fr>
> ---
>  support/scripts/br2-external | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/support/scripts/br2-external b/support/scripts/br2-external
> index 171526f8c8..fdea5aa251 100755
> --- a/support/scripts/br2-external
> +++ b/support/scripts/br2-external
> @@ -33,9 +33,8 @@ main() {
>      # Trap any unexpected error to generate a meaningful error message
>      trap "error 'unexpected error while generating ${ofile}\n'" ERR
>  
> -    do_validate ${@//:/ }
> -
>      mkdir -p "${outputdir}"
> +    do_validate "${outputdir}" ${@//:/ }
>      do_mk "${outputdir}"
>      do_kconfig "${outputdir}"
>  }
> @@ -51,7 +50,9 @@ main() {
>  # snippet means that there were no error.
>  #
>  do_validate() {
> +    local outputdir="${1}"
>      local br2_ext
> +    shift
>  
>      if [ ${#} -eq 0 ]; then
>          # No br2-external tree is valid
> @@ -60,7 +61,7 @@ do_validate() {
>  
>      for br2_ext in "${@}"; do
>          do_validate_one "${br2_ext}"
> -    done
> +    done >"${outputdir}/.br2-external.mk"
>  }
>  
>  do_validate_one() {
>