Re: [bitbake-devel] [PATCH] bitbake-getvar: Do not output anything with --value and undefined variable

[Mario Domenech Goulart <mario@parenteses.org>]

Hi,

On Mon, 25 Sep 2023 17:17:53 +0100 "Richard Purdie" <richard.purdie@linuxfoundation.org> wrote:

> On Mon, 2023-09-25 at 16:00 +0000, Peter Kjellerstedt wrote:
>> > -----Original Message-----
>> > From: Richard Purdie <richard.purdie@linuxfoundation.org>
>> > Sent: den 25 september 2023 16:07
>> > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; bitbake-
>> > devel@lists.openembedded.org
>> > Subject: Re: [bitbake-devel] [PATCH] bitbake-getvar: Do not output
>> > anything with --value and undefined variable
>> > 
>> > On Mon, 2023-09-25 at 13:55 +0000, Peter Kjellerstedt wrote:
>> > > > From: Richard Purdie <richard.purdie@linuxfoundation.org>
>> > > > On Mon, 2023-09-25 at 10:16 +0000, Peter Kjellerstedt wrote:
>> > > > > > -----Original Message-----
>> > > > > > From: Richard Purdie <richard.purdie@linuxfoundation.org>
>> > > > > > Sent: den 25 september 2023 11:22
>> > > > > > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>;
>> > > > > > bitbake-devel@lists.openembedded.org
>> > > > > > Subject: Re: [bitbake-devel] [PATCH] bitbake-getvar: Do not
>> > > > > > output anything with --value and undefined variable
>> > > > > > 
>> > > > > > 
>> > > > > 
>> > > > > My goal here was not really to be able to determine if the
>> > > > > variable is
>> > > > > defined or not
>> > > > 
>> > > > I know, but that is an important detail and whilst it may not
>> > > > matter to
>> > > > you, it does matter a lot.
>> > > 
>> > > Well, as I see it, with my change it _is_ now possible to make
>> > > the
>> > > distinction if you really care, where before you could only
>> > > assume
>> > > that a returned value of "None" actually meant that the variable
>> > > is
>> > > not defined.
>> > 
>> > As things currently stand it is clear something went wrong through
>> > the
>> > process you yourself mentioned as how noticed. Once your proposed
>> > change merges, it becomes very unclear what is happening.
>> 
>> The documentation (i.e., `bitbake-getvar -h`) for --value states 
>> "Only report the value, no history and no variable name". When I 
>> read that, I instinctly assume that it will not output anything if 
>> I ask for a variable that does not exist, not that it will output 
>> something that looks like a valid value (i.e., "None"), but isn't. 
>
> Please stop trying to convince me the current behaviour is wrong. I've
> said multiple times, I agree. What I don't agree with is how you're
> fixing it.
>
>> I would be fine with an error message to stderr, but then I would 
>> assume the same result if bitbake-getvar is called without --value 
>> for an undefined variable.
>
> I agree with that.
>
>>  I would also prefer if it is possible to silence the error for the
>> case where you do not care if the variable is undefined.
>
> I only agree with this if the user specifically asks for that behaviour
> as I think it is a perfect way to end up with obscure bugs otherwise.
>
>> Whether bitbake-getvar should actually fail with an error in case 
>> the variable does not exist is more debatable. I can see use cases 
>> both ways.
>
> I can see the use case both ways but I think the user has to
> specifically ask for undefined to result in "".
>
>> > I'm not disagreeing that it needs to be fixed somehow we agree on
>> > that
>> > and I've already said as much. What we disagree on is how to fix
>> > it. I
>> > disagree on this way of addressing it.
>> 
>> Based on my comments above, here is my suggestion: 
>> 
>> When an undefined variable is requested, give an error message, e.g.:
>> 
>> $ bitbake-getvar UNDEFINED_VAR
>> The variable 'UNDEFINED_VAR' is not defined! [on stderr]
>> 
>> and set the exit code to 1. And the same result with --value:
>> 
>> $ bitbake-getvar --value UNDEFINED_VAR
>> The variable 'UNDEFINED_VAR' is not defined! [on stderr]
>
> That seems reasonable to me, without the "!" though.
>
>> If -s or --no-messages (borrowed from `grep`) is also specified, then
>> no error message is output and the exit code is 0.
>
> I'd suggest only a long option --ignore-undefined. That way when you
> see it in a shell script, you know exactly what risk is being taken.

How about providing a --default parameter?  The behavior of
bitbake-getvar would then be:

* If the requested variable is defined, print its value and exit zero
  (in this case --default is irrelevant).

* If the requested variable is not defined and --default <value> is
  provided, print <value> and exit zero.
  
* If the requested variable is not defined and --default is not
  provided, exit non-zero.

Rationale:

* The proposed behavior/API allows for:

  * Detecting undefined variables.

  * Controlling when errors should be raised.

  * Using custom fallback values for undefined variables.

All the best.
Mario
-- 
http://parenteses.org/mario