diff mbox series

ltp: fix warning in remove_broken_musl_sources

Message ID 20260319132904.1308436-1-joaomarcos.costa@bootlin.com
State Changes Requested
Headers show
Series ltp: fix warning in remove_broken_musl_sources | expand

Commit Message

Joao Marcos Costa March 19, 2026, 1:29 p.m. UTC
If this was originally intended to be an actual warning, it was not
behaving as such: echo only prints to log.do_patch, so the message ends
up hidden there.

Replace 'echo' by bbwarn to display the warning message correctly.

Signed-off-by: João Marcos Costa <joaomarcos.costa@bootlin.com>
---
 meta/recipes-extended/ltp/ltp_20260130.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mathieu Dubois-Briand March 19, 2026, 5:38 p.m. UTC | #1
On Thu Mar 19, 2026 at 2:29 PM CET, Joao Marcos Costa via lists.openembedded.org wrote:
> If this was originally intended to be an actual warning, it was not
> behaving as such: echo only prints to log.do_patch, so the message ends
> up hidden there.
>
> Replace 'echo' by bbwarn to display the warning message correctly.
>
> Signed-off-by: João Marcos Costa <joaomarcos.costa@bootlin.com>
> ---

Hi João,

Thanks for your patch.

>  meta/recipes-extended/ltp/ltp_20260130.bb | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/recipes-extended/ltp/ltp_20260130.bb b/meta/recipes-extended/ltp/ltp_20260130.bb
> index 75c5b8b7bd..36432c15b2 100644
> --- a/meta/recipes-extended/ltp/ltp_20260130.bb
> +++ b/meta/recipes-extended/ltp/ltp_20260130.bb
> @@ -126,7 +126,7 @@ remove_broken_musl_sources() {
>  	[ "${TCLIBC}" = "musl" ] || return 0
>  
>  	cd ${S}
> -	echo "WARNING: remove unsupported tests (until they're fixed)"
> +	bbwarn "remove unsupported tests (until they're fixed)"

So I believe this will add a WARNING on all musl builds, right? I'm not
sure we can really accept that.

Thanks,
Mathieu
Joao Marcos Costa March 19, 2026, 5:55 p.m. UTC | #2
Hello,

On 3/19/26 18:38, Mathieu Dubois-Briand wrote:
> On Thu Mar 19, 2026 at 2:29 PM CET, Joao Marcos Costa via lists.openembedded.org wrote:
>> If this was originally intended to be an actual warning, it was not
>> behaving as such: echo only prints to log.do_patch, so the message ends
>> up hidden there.
>>
>> Replace 'echo' by bbwarn to display the warning message correctly.
>>
>> Signed-off-by: João Marcos Costa <joaomarcos.costa@bootlin.com>
>> ---
> 
> Hi João,
> 
> Thanks for your patch.
> 
>>   meta/recipes-extended/ltp/ltp_20260130.bb | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/meta/recipes-extended/ltp/ltp_20260130.bb b/meta/recipes-extended/ltp/ltp_20260130.bb
>> index 75c5b8b7bd..36432c15b2 100644
>> --- a/meta/recipes-extended/ltp/ltp_20260130.bb
>> +++ b/meta/recipes-extended/ltp/ltp_20260130.bb
>> @@ -126,7 +126,7 @@ remove_broken_musl_sources() {
>>   	[ "${TCLIBC}" = "musl" ] || return 0
>>   
>>   	cd ${S}
>> -	echo "WARNING: remove unsupported tests (until they're fixed)"
>> +	bbwarn "remove unsupported tests (until they're fixed)"
> 
> So I believe this will add a WARNING on all musl builds, right? I'm not
> sure we can really accept that.
>
Yep, it will. Would an INFO/NOTE be acceptable instead?

Thanks!
Mathieu Dubois-Briand March 19, 2026, 6:17 p.m. UTC | #3
On Thu Mar 19, 2026 at 6:55 PM CET, Joao Marcos Costa wrote:
> Hello,
>
> On 3/19/26 18:38, Mathieu Dubois-Briand wrote:
>> On Thu Mar 19, 2026 at 2:29 PM CET, Joao Marcos Costa via lists.openembedded.org wrote:
>>> If this was originally intended to be an actual warning, it was not
>>> behaving as such: echo only prints to log.do_patch, so the message ends
>>> up hidden there.
>>>
>>> Replace 'echo' by bbwarn to display the warning message correctly.
>>>
>>> Signed-off-by: João Marcos Costa <joaomarcos.costa@bootlin.com>
>>> ---
>> 
>> Hi João,
>> 
>> Thanks for your patch.
>> 
>>>   meta/recipes-extended/ltp/ltp_20260130.bb | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/meta/recipes-extended/ltp/ltp_20260130.bb b/meta/recipes-extended/ltp/ltp_20260130.bb
>>> index 75c5b8b7bd..36432c15b2 100644
>>> --- a/meta/recipes-extended/ltp/ltp_20260130.bb
>>> +++ b/meta/recipes-extended/ltp/ltp_20260130.bb
>>> @@ -126,7 +126,7 @@ remove_broken_musl_sources() {
>>>   	[ "${TCLIBC}" = "musl" ] || return 0
>>>   
>>>   	cd ${S}
>>> -	echo "WARNING: remove unsupported tests (until they're fixed)"
>>> +	bbwarn "remove unsupported tests (until they're fixed)"
>> 
>> So I believe this will add a WARNING on all musl builds, right? I'm not
>> sure we can really accept that.
>>
> Yep, it will. Would an INFO/NOTE be acceptable instead?
>
> Thanks!

Yes, I believe info or note are acceptable.

Thanks,
Mathieu
Joao Marcos Costa March 20, 2026, 9:16 a.m. UTC | #4
Hello,

On 3/19/26 19:17, Mathieu Dubois-Briand wrote:
> On Thu Mar 19, 2026 at 6:55 PM CET, Joao Marcos Costa wrote:
>> Hello,
>>
>> On 3/19/26 18:38, Mathieu Dubois-Briand wrote:
>>> On Thu Mar 19, 2026 at 2:29 PM CET, Joao Marcos Costa via lists.openembedded.org wrote:
>>>> If this was originally intended to be an actual warning, it was not
>>>> behaving as such: echo only prints to log.do_patch, so the message ends
>>>> up hidden there.
>>>>
>>>> Replace 'echo' by bbwarn to display the warning message correctly.
>>>>
>>>> Signed-off-by: João Marcos Costa <joaomarcos.costa@bootlin.com>
>>>> ---
>>>
(...)>>>
>> Yep, it will. Would an INFO/NOTE be acceptable instead?
>>
>> Thanks!
> 
> Yes, I believe info or note are acceptable.
> 
> Thanks,
> Mathieu

I sent a v2 to fix that.

One more thing, though: this same function uses an 'rm -rfv' and the 
"-f" option will not raise an error if the file is not there.

Do you think this should be addressed? I would send this in a different 
patch, of course.

Thanks!
Richard Purdie March 20, 2026, 9:26 a.m. UTC | #5
On Fri, 2026-03-20 at 10:16 +0100, Joao Marcos Costa via lists.openembedded.org wrote:
> Hello,
> 
> On 3/19/26 19:17, Mathieu Dubois-Briand wrote:
> > On Thu Mar 19, 2026 at 6:55 PM CET, Joao Marcos Costa wrote:
> > > Hello,
> > > 
> > > On 3/19/26 18:38, Mathieu Dubois-Briand wrote:
> > > > On Thu Mar 19, 2026 at 2:29 PM CET, Joao Marcos Costa via lists.openembedded.org wrote:
> > > > > If this was originally intended to be an actual warning, it was not
> > > > > behaving as such: echo only prints to log.do_patch, so the message ends
> > > > > up hidden there.
> > > > > 
> > > > > Replace 'echo' by bbwarn to display the warning message correctly.
> > > > > 
> > > > > Signed-off-by: João Marcos Costa <joaomarcos.costa@bootlin.com>
> > > > > ---
> > > > 
> (...)>>>
> > > Yep, it will. Would an INFO/NOTE be acceptable instead?
> > > 
> > > Thanks!
> > 
> > Yes, I believe info or note are acceptable.
> > 
> > Thanks,
> > Mathieu
> 
> I sent a v2 to fix that.
> 
> One more thing, though: this same function uses an 'rm -rfv' and the 
> "-f" option will not raise an error if the file is not there.
> 
> Do you think this should be addressed? I would send this in a different 
> patch, of course.

Tasks need to be idempotent, we can run then multiple times with the
same result. I didn't check where this is being called but the -f is
probably fine.

Cheers,

Richard
diff mbox series

Patch

diff --git a/meta/recipes-extended/ltp/ltp_20260130.bb b/meta/recipes-extended/ltp/ltp_20260130.bb
index 75c5b8b7bd..36432c15b2 100644
--- a/meta/recipes-extended/ltp/ltp_20260130.bb
+++ b/meta/recipes-extended/ltp/ltp_20260130.bb
@@ -126,7 +126,7 @@  remove_broken_musl_sources() {
 	[ "${TCLIBC}" = "musl" ] || return 0
 
 	cd ${S}
-	echo "WARNING: remove unsupported tests (until they're fixed)"
+	bbwarn "remove unsupported tests (until they're fixed)"
 
 	# sync with upstream
 	# https://github.com/linux-test-project/ltp/blob/master/ci/alpine.sh#L33