All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 0/2] core/br2-external: fix reporting errors (branch yem/br2-ext-fixes)
@ 2020-05-22 21:40 Yann E. MORIN
  2020-05-22 21:40 ` [Buildroot] [PATCH 1/2] core.br2-external: fix reporting errors Yann E. MORIN
  2020-05-22 21:40 ` [Buildroot] [PATCH 2/2] core/br2-external: report better error messages Yann E. MORIN
  0 siblings, 2 replies; 10+ messages in thread
From: Yann E. MORIN @ 2020-05-22 21:40 UTC (permalink / raw)
  To: buildroot

Hello All!

Following a few reports on IRC, we discovered an issue with how our
br2-external support was reporting errors. Fix that.


Regards,
Yann E. MORIN.


The following changes since commit a00db9f80819982348e64834f76048c2c8381f40

  DEVELOPERS: remove python-pycrypto (2020-05-21 17:05:20 +0200)


are available in the git repository at:

  git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to f7b374aa76093cdf59fd788d7c8cc39b86415eb6

  core/br2-external: report better error messages (2020-05-22 23:39:05 +0200)


----------------------------------------------------------------
Yann E. MORIN (2):
      core.br2-external: fix reporting errors
      core/br2-external: report better error messages

 support/scripts/br2-external | 9 +++++----
 1 file changed, 5 insertions(+), 4 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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 1/2] core.br2-external: fix reporting errors
  2020-05-22 21:40 [Buildroot] [PATCH 0/2] core/br2-external: fix reporting errors (branch yem/br2-ext-fixes) Yann E. MORIN
@ 2020-05-22 21:40 ` Yann E. MORIN
  2020-06-14 15:33   ` Romain Naour
  2020-07-15 19:29   ` Peter Korsgaard
  2020-05-22 21:40 ` [Buildroot] [PATCH 2/2] core/br2-external: report better error messages Yann E. MORIN
  1 sibling, 2 replies; 10+ messages in thread
From: Yann E. MORIN @ 2020-05-22 21:40 UTC (permalink / raw)
  To: buildroot

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.

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@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() {
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 2/2] core/br2-external: report better error messages
  2020-05-22 21:40 [Buildroot] [PATCH 0/2] core/br2-external: fix reporting errors (branch yem/br2-ext-fixes) Yann E. MORIN
  2020-05-22 21:40 ` [Buildroot] [PATCH 1/2] core.br2-external: fix reporting errors Yann E. MORIN
@ 2020-05-22 21:40 ` Yann E. MORIN
  2020-06-14 15:33   ` Romain Naour
  2020-07-15 19:29   ` Peter Korsgaard
  1 sibling, 2 replies; 10+ messages in thread
From: Yann E. MORIN @ 2020-05-22 21:40 UTC (permalink / raw)
  To: buildroot

The error is misleading: it reports that no name was provided,
when in fact the external.desc file is missing.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>p
---
 support/scripts/br2-external | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/scripts/br2-external b/support/scripts/br2-external
index fdea5aa251..ededd2d900 100755
--- a/support/scripts/br2-external
+++ b/support/scripts/br2-external
@@ -75,7 +75,7 @@ do_validate_one() {
         error "'%s': permission denied\n" "${br2_ext}"
     fi
     if [ ! -f "${br2_ext}/external.desc" ]; then
-        error "'%s': does not have a name (in 'external.desc'). See %s\n" \
+        error "'%s': does not have an 'external.desc'. See %s\n" \
             "${br2_ext}" "${MANUAL_URL}"
     fi
     br2_name="$(sed -r -e '/^name: +(.*)$/!d; s//\1/' "${br2_ext}/external.desc")"
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 1/2] core.br2-external: fix reporting errors
  2020-05-22 21:40 ` [Buildroot] [PATCH 1/2] core.br2-external: fix reporting errors Yann E. MORIN
@ 2020-06-14 15:33   ` Romain Naour
  2020-06-14 15:55     ` Yann E. MORIN
  2020-07-15 19:29   ` Peter Korsgaard
  1 sibling, 1 reply; 10+ messages in thread
From: Romain Naour @ 2020-06-14 15:33 UTC (permalink / raw)
  To: buildroot

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@gmail.com>
Tested-by: Romain Naour <romain.naour@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@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() {
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 2/2] core/br2-external: report better error messages
  2020-05-22 21:40 ` [Buildroot] [PATCH 2/2] core/br2-external: report better error messages Yann E. MORIN
@ 2020-06-14 15:33   ` Romain Naour
  2020-07-15 19:29   ` Peter Korsgaard
  1 sibling, 0 replies; 10+ messages in thread
From: Romain Naour @ 2020-06-14 15:33 UTC (permalink / raw)
  To: buildroot

Le 22/05/2020 ? 23:40, Yann E. MORIN a ?crit?:
> The error is misleading: it reports that no name was provided,
> when in fact the external.desc file is missing.


Reviewed-by: Romain Naour <romain.naour@gmail.com>

Best regards,
Romain

> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>p
> ---
>  support/scripts/br2-external | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/support/scripts/br2-external b/support/scripts/br2-external
> index fdea5aa251..ededd2d900 100755
> --- a/support/scripts/br2-external
> +++ b/support/scripts/br2-external
> @@ -75,7 +75,7 @@ do_validate_one() {
>          error "'%s': permission denied\n" "${br2_ext}"
>      fi
>      if [ ! -f "${br2_ext}/external.desc" ]; then
> -        error "'%s': does not have a name (in 'external.desc'). See %s\n" \
> +        error "'%s': does not have an 'external.desc'. See %s\n" \
>              "${br2_ext}" "${MANUAL_URL}"
>      fi
>      br2_name="$(sed -r -e '/^name: +(.*)$/!d; s//\1/' "${br2_ext}/external.desc")"
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 1/2] core.br2-external: fix reporting errors
  2020-06-14 15:33   ` Romain Naour
@ 2020-06-14 15:55     ` Yann E. MORIN
  2020-06-14 16:47       ` Romain Naour
  0 siblings, 1 reply; 10+ messages in thread
From: Yann E. MORIN @ 2020-06-14 15:55 UTC (permalink / raw)
  To: buildroot

Romain, All,

On 2020-06-14 17:33 +0200, Romain Naour spake thusly:
> 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"

Ah yeah, this one is also fixed! ;-)

> 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.

I disagree. The first test is for testing that an old br2-external, from
before the multi-br2-external support, in which case we want to point
the user to the manual for the conversion step.

However, when external.desc exists, this is not a conversion, and the
user is already aware of the requirements for external.desc; they just
made a mistake and should just refer to the manual.

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

Does that still stand with the patch as-is?

> 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.

As already shared on IRC, I hae a set of test-cases that I wrote back in
the days I was working on multi-br2-external support:

    https://git.buildroot.org/~ymorin/git/br2-external-tests/

Feel free to grab those and turn them into actual tests in our testing
infra! ;-)

Regards,
Yann E. MORIN.

> 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@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() {
> > 
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 1/2] core.br2-external: fix reporting errors
  2020-06-14 15:55     ` Yann E. MORIN
@ 2020-06-14 16:47       ` Romain Naour
  2020-06-15  9:03         ` Yann E. MORIN
  0 siblings, 1 reply; 10+ messages in thread
From: Romain Naour @ 2020-06-14 16:47 UTC (permalink / raw)
  To: buildroot

Yann,

Le 14/06/2020 ? 17:55, Yann E. MORIN a ?crit?:
> Romain, All,
> 
> On 2020-06-14 17:33 +0200, Romain Naour spake thusly:
>> 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"
> 
> Ah yeah, this one is also fixed! ;-)
> 
>> 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.
> 
> I disagree. The first test is for testing that an old br2-external, from
> before the multi-br2-external support, in which case we want to point
> the user to the manual for the conversion step.
> 
> However, when external.desc exists, this is not a conversion, and the
> user is already aware of the requirements for external.desc; they just
> made a mistake and should just refer to the manual.
> 
>> With that fixed(improved :p):
>> Reviewed-by: Romain Naour <romain.naour@gmail.com>
>> Tested-by: Romain Naour <romain.naour@gmail.com>
> 
> Does that still stand with the patch as-is?

Ok as-is.

> 
>> 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.
> 
> As already shared on IRC, I hae a set of test-cases that I wrote back in
> the days I was working on multi-br2-external support:
> 
>     https://git.buildroot.org/~ymorin/git/br2-external-tests/
> 
> Feel free to grab those and turn them into actual tests in our testing
> infra! ;-)

Best regards,
Romain

> 
> Regards,
> Yann E. MORIN.
> 
>> 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@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() {
>>>
>>
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 1/2] core.br2-external: fix reporting errors
  2020-06-14 16:47       ` Romain Naour
@ 2020-06-15  9:03         ` Yann E. MORIN
  0 siblings, 0 replies; 10+ messages in thread
From: Yann E. MORIN @ 2020-06-15  9:03 UTC (permalink / raw)
  To: buildroot

Romain, All,

On 2020-06-14 18:47 +0200, Romain Naour spake thusly:
> Le 14/06/2020 ? 17:55, Yann E. MORIN a ?crit?:
> > On 2020-06-14 17:33 +0200, Romain Naour spake thusly:
[--SNIP--]
> >> With that fixed(improved :p):
> >> Reviewed-by: Romain Naour <romain.naour@gmail.com>
> >> Tested-by: Romain Naour <romain.naour@gmail.com>
> > Does that still stand with the patch as-is?
> Ok as-is.

Applied to master; thanks for the review.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 1/2] core.br2-external: fix reporting errors
  2020-05-22 21:40 ` [Buildroot] [PATCH 1/2] core.br2-external: fix reporting errors Yann E. MORIN
  2020-06-14 15:33   ` Romain Naour
@ 2020-07-15 19:29   ` Peter Korsgaard
  1 sibling, 0 replies; 10+ messages in thread
From: Peter Korsgaard @ 2020-07-15 19:29 UTC (permalink / raw)
  To: buildroot

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > 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.

 > 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@free.fr>

Committed to 2020.02.x and 2020.05.x, thanks.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 2/2] core/br2-external: report better error messages
  2020-05-22 21:40 ` [Buildroot] [PATCH 2/2] core/br2-external: report better error messages Yann E. MORIN
  2020-06-14 15:33   ` Romain Naour
@ 2020-07-15 19:29   ` Peter Korsgaard
  1 sibling, 0 replies; 10+ messages in thread
From: Peter Korsgaard @ 2020-07-15 19:29 UTC (permalink / raw)
  To: buildroot

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > The error is misleading: it reports that no name was provided,
 > when in fact the external.desc file is missing.

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

Committed to 2020.02.x and 2020.05.x, thanks.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-07-15 19:29 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-22 21:40 [Buildroot] [PATCH 0/2] core/br2-external: fix reporting errors (branch yem/br2-ext-fixes) Yann E. MORIN
2020-05-22 21:40 ` [Buildroot] [PATCH 1/2] core.br2-external: fix reporting errors Yann E. MORIN
2020-06-14 15:33   ` Romain Naour
2020-06-14 15:55     ` Yann E. MORIN
2020-06-14 16:47       ` Romain Naour
2020-06-15  9:03         ` Yann E. MORIN
2020-07-15 19:29   ` Peter Korsgaard
2020-05-22 21:40 ` [Buildroot] [PATCH 2/2] core/br2-external: report better error messages Yann E. MORIN
2020-06-14 15:33   ` Romain Naour
2020-07-15 19:29   ` Peter Korsgaard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.