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

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

[Peter Kjellerstedt]

Rather than outputting the string "None" when an unknown variable is
used together with --value, output nothing. This is consistent with the
no --value case where only a comment stating that the variable is not
known is output.

This also means it is now possible to differentiate between an undefined
variable and an empty variable since the empty variable will be output
as a linefeed.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 bitbake/bin/bitbake-getvar | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/bitbake/bin/bitbake-getvar b/bitbake/bin/bitbake-getvar
index 4a9eb4f311..806bbd8e4e 100755
--- a/bitbake/bin/bitbake-getvar
+++ b/bitbake/bin/bitbake-getvar
@@ -43,9 +43,12 @@ if __name__ == "__main__":
         else:
             tinfoil.prepare(quiet=2, config_only=True)
             d = tinfoil.config_data
+        value = None
         if args.flag:
-            print(str(d.getVarFlag(args.variable, args.flag, expand=(not args.unexpand))))
+            value = d.getVarFlag(args.variable, args.flag, expand=(not args.unexpand))
         elif args.value:
-            print(str(d.getVar(args.variable, expand=(not args.unexpand))))
+            value = d.getVar(args.variable, expand=(not args.unexpand))
         else:
             bb.data.emit_var(args.variable, d=d, all=True)
+        if value is not None:
+            print(str(value))

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

[Richard Purdie]

On Mon, 2023-09-25 at 05:22 +0200, Peter Kjellerstedt wrote:
> Rather than outputting the string "None" when an unknown variable is
> used together with --value, output nothing. This is consistent with the
> no --value case where only a comment stating that the variable is not
> known is output.
> 
> This also means it is now possible to differentiate between an undefined
> variable and an empty variable since the empty variable will be output
> as a linefeed.
> 
> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> ---
>  bitbake/bin/bitbake-getvar | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/bitbake/bin/bitbake-getvar b/bitbake/bin/bitbake-getvar
> index 4a9eb4f311..806bbd8e4e 100755
> --- a/bitbake/bin/bitbake-getvar
> +++ b/bitbake/bin/bitbake-getvar
> @@ -43,9 +43,12 @@ if __name__ == "__main__":
>          else:
>              tinfoil.prepare(quiet=2, config_only=True)
>              d = tinfoil.config_data
> +        value = None
>          if args.flag:
> -            print(str(d.getVarFlag(args.variable, args.flag, expand=(not args.unexpand))))
> +            value = d.getVarFlag(args.variable, args.flag, expand=(not args.unexpand))
>          elif args.value:
> -            print(str(d.getVar(args.variable, expand=(not args.unexpand))))
> +            value = d.getVar(args.variable, expand=(not args.unexpand))
>          else:
>              bb.data.emit_var(args.variable, d=d, all=True)
> +        if value is not None:
> +            print(str(value))

Whilst I can see where you're going with this, I'm worried users will
be confused by it.

getVar within bitbake returns None for things which do not exist. Not
many users will notice the difference between linefeed output and no
linefeed. It might be better to do something more obvious like an error
exit code but that also has challenges. Variables not existing when the
user expects them to is something we need to make really clear.

It was issues like this which stopped us implementing bitbake-getvar
for a lot of years too :/.

Cheers,

Richard

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

[Peter Kjellerstedt]

> -----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
> 
> On Mon, 2023-09-25 at 05:22 +0200, Peter Kjellerstedt wrote:
> > Rather than outputting the string "None" when an unknown variable is
> > used together with --value, output nothing. This is consistent with the
> > no --value case where only a comment stating that the variable is not
> > known is output.
> >
> > This also means it is now possible to differentiate between an undefined
> > variable and an empty variable since the empty variable will be output
> > as a linefeed.
> >
> > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > ---
> >  bitbake/bin/bitbake-getvar | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/bitbake/bin/bitbake-getvar b/bitbake/bin/bitbake-getvar
> > index 4a9eb4f311..806bbd8e4e 100755
> > --- a/bitbake/bin/bitbake-getvar
> > +++ b/bitbake/bin/bitbake-getvar
> > @@ -43,9 +43,12 @@ if __name__ == "__main__":
> >          else:
> >              tinfoil.prepare(quiet=2, config_only=True)
> >              d = tinfoil.config_data
> > +        value = None
> >          if args.flag:
> > -            print(str(d.getVarFlag(args.variable, args.flag, expand=(not args.unexpand))))
> > +            value = d.getVarFlag(args.variable, args.flag, expand=(not args.unexpand))
> >          elif args.value:
> > -            print(str(d.getVar(args.variable, expand=(not args.unexpand))))
> > +            value = d.getVar(args.variable, expand=(not args.unexpand))
> >          else:
> >              bb.data.emit_var(args.variable, d=d, all=True)
> > +        if value is not None:
> > +            print(str(value))
> 
> Whilst I can see where you're going with this, I'm worried users will
> be confused by it.

Too be honest, I expect them to be much more confused if they get the 
string "None" back when asking for the value of a variable, which 
happens to not be defined.

> 
> getVar within bitbake returns None for things which do not exist. Not
> many users will notice the difference between linefeed output and no
> linefeed. It might be better to do something more obvious like an error
> exit code but that also has challenges. Variables not existing when the
> user expects them to is something we need to make really clear.
> 
> It was issues like this which stopped us implementing bitbake-getvar
> for a lot of years too :/.
> 
> Cheers,
> 
> Richard

My goal here was not really to be able to determine if the variable is 
defined or not, but to not get the string "None" back as a result of 
asking for an undefined variable, because that is really problematic 
when `bitbake-getvar --value` is used from a script or similar. This 
is because there is no way to determine (from the value) whether the 
variable is undefined, or if it is defined and has the actual value 
"None".

My assumption is that for most people the difference will not matter and 
the reason for that is that since if you use `bitbake-getvar --value` in 
a script or similar, you typically strip the linefeed that is output after 
the variable so you get the actual value, and then there is no difference 
in the result whether there was a linefeed (if the variable was defined, 
but empty) or there was no linefeed (if the variable was not defined). In 
both cases the result will be an empty string.

The reason I stumbled upon this was that I was working on a script that 
uses `bitbake-getvar --value` to get a variable with a path to a file, 
and then my script is expected to read that file. And I was a bit surprised 
when I tested it and I got an error stating that it failed to open the 
file "None"...

Now of course I can write something like:

	myfile=$(bitbake-getvar --value MYFILE)
	if [ "$myfile" ] && [ "$myfile" != None ]; then

but it feels much more natural (shell wise) to be able to write:

	myfile=$(bitbake-getvar --value MYFILE)
	if [ "$myfile" ]; then

as that matches the expectations (that at least I have) for all CLI 
commands that are supposed to return a value that may not exist.

//Peter

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

[Richard Purdie]

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
> > 
> > On Mon, 2023-09-25 at 05:22 +0200, Peter Kjellerstedt wrote:
> > > Rather than outputting the string "None" when an unknown variable is
> > > used together with --value, output nothing. This is consistent with the
> > > no --value case where only a comment stating that the variable is not
> > > known is output.
> > > 
> > > This also means it is now possible to differentiate between an undefined
> > > variable and an empty variable since the empty variable will be output
> > > as a linefeed.
> > > 
> > > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > > ---
> > >  bitbake/bin/bitbake-getvar | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/bitbake/bin/bitbake-getvar b/bitbake/bin/bitbake-getvar
> > > index 4a9eb4f311..806bbd8e4e 100755
> > > --- a/bitbake/bin/bitbake-getvar
> > > +++ b/bitbake/bin/bitbake-getvar
> > > @@ -43,9 +43,12 @@ if __name__ == "__main__":
> > >          else:
> > >              tinfoil.prepare(quiet=2, config_only=True)
> > >              d = tinfoil.config_data
> > > +        value = None
> > >          if args.flag:
> > > -            print(str(d.getVarFlag(args.variable, args.flag, expand=(not args.unexpand))))
> > > +            value = d.getVarFlag(args.variable, args.flag, expand=(not args.unexpand))
> > >          elif args.value:
> > > -            print(str(d.getVar(args.variable, expand=(not args.unexpand))))
> > > +            value = d.getVar(args.variable, expand=(not args.unexpand))
> > >          else:
> > >              bb.data.emit_var(args.variable, d=d, all=True)
> > > +        if value is not None:
> > > +            print(str(value))
> > 
> > Whilst I can see where you're going with this, I'm worried users will
> > be confused by it.
> 
> Too be honest, I expect them to be much more confused if they get the
> string "None" back when asking for the value of a variable, which 
> happens to not be defined.
> 
> > 
> > getVar within bitbake returns None for things which do not exist. Not
> > many users will notice the difference between linefeed output and no
> > linefeed. It might be better to do something more obvious like an error
> > exit code but that also has challenges. Variables not existing when the
> > user expects them to is something we need to make really clear.
> > 
> > It was issues like this which stopped us implementing bitbake-getvar
> > for a lot of years too :/.
> > 
> > Cheers,
> > 
> > Richard
> 
> 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.

> , but to not get the string "None" back as a result of 
> asking for an undefined variable, because that is really problematic 
> when `bitbake-getvar --value` is used from a script or similar.

I do agree on that. I just don't agree we should hide "undefined" as
we're replacing one bad problem with equally bad one.

>  This 
> is because there is no way to determine (from the value) whether the 
> variable is undefined, or if it is defined and has the actual value 
> "None".

FWIW I'm not aware of any variable having "None" as a valid value.

> My assumption is that for most people the difference will not matter and 
> the reason for that is that since if you use `bitbake-getvar --value` in 
> a script or similar, you typically strip the linefeed that is output after 
> the variable so you get the actual value, and then there is no difference 
> in the result whether there was a linefeed (if the variable was defined, 
> but empty) or there was no linefeed (if the variable was not defined). In 
> both cases the result will be an empty string.

This is a problem as users likely should care if the variable is not
set at all, it suggests bigger problems.

Cheers,

Richard

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

[Peter Kjellerstedt]

> -----Original Message-----
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> Sent: den 25 september 2023 12:30
> 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 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
> > >
> > > On Mon, 2023-09-25 at 05:22 +0200, Peter Kjellerstedt wrote:
> > > > Rather than outputting the string "None" when an unknown variable is
> > > > used together with --value, output nothing. This is consistent with the
> > > > no --value case where only a comment stating that the variable is not
> > > > known is output.
> > > >
> > > > This also means it is now possible to differentiate between an undefined
> > > > variable and an empty variable since the empty variable will be output
> > > > as a linefeed.
> > > >
> > > > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > > > ---
> > > >  bitbake/bin/bitbake-getvar | 7 +++++--
> > > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/bitbake/bin/bitbake-getvar b/bitbake/bin/bitbake-getvar
> > > > index 4a9eb4f311..806bbd8e4e 100755
> > > > --- a/bitbake/bin/bitbake-getvar
> > > > +++ b/bitbake/bin/bitbake-getvar
> > > > @@ -43,9 +43,12 @@ if __name__ == "__main__":
> > > >          else:
> > > >              tinfoil.prepare(quiet=2, config_only=True)
> > > >              d = tinfoil.config_data
> > > > +        value = None
> > > >          if args.flag:
> > > > -            print(str(d.getVarFlag(args.variable, args.flag, expand=(not args.unexpand))))
> > > > +            value = d.getVarFlag(args.variable, args.flag, expand=(not args.unexpand))
> > > >          elif args.value:
> > > > -            print(str(d.getVar(args.variable, expand=(not args.unexpand))))
> > > > +            value = d.getVar(args.variable, expand=(not args.unexpand))
> > > >          else:
> > > >              bb.data.emit_var(args.variable, d=d, all=True)
> > > > +        if value is not None:
> > > > +            print(str(value))
> > >
> > > Whilst I can see where you're going with this, I'm worried users will
> > > be confused by it.
> >
> > Too be honest, I expect them to be much more confused if they get the
> > string "None" back when asking for the value of a variable, which
> > happens to not be defined.
> >
> > >
> > > getVar within bitbake returns None for things which do not exist. Not
> > > many users will notice the difference between linefeed output and no
> > > linefeed. It might be better to do something more obvious like an error
> > > exit code but that also has challenges. Variables not existing when the
> > > user expects them to is something we need to make really clear.
> > >
> > > It was issues like this which stopped us implementing bitbake-getvar
> > > for a lot of years too :/.
> > >
> > > Cheers,
> > >
> > > Richard
> >
> > 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.

And for the common case where you just want the value of a variable and 
do not care if it is not defined or if it is defined as the empty string, 
things just work. Compare the behavior when you write something like 
`echo "$VAR"` in a shell script, where most of the time you do not make 
any distinction on whether the variable VAR is defined or not. This is 
the same expectation I have for a command line tool that exposes an API 
to return the value of a variable. I.e., it should return the value and 
nothing else (and definitely not logs as my other patch series took care 
of).

> 
> > , but to not get the string "None" back as a result of
> > asking for an undefined variable, because that is really problematic
> > when `bitbake-getvar --value` is used from a script or similar.
> 
> I do agree on that. I just don't agree we should hide "undefined" as
> we're replacing one bad problem with equally bad one.

Would an update to the help output from `bitbake-getvar --help` to make 
the distinction documented make you more comfortable about this change?

> 
> >  This
> > is because there is no way to determine (from the value) whether the
> > variable is undefined, or if it is defined and has the actual value
> > "None".
> 
> FWIW I'm not aware of any variable having "None" as a valid value.

Neither do I. But having to explicitly test for it everywhere you use 
`bitbake-getvar --value` is awkward. 

> 
> > My assumption is that for most people the difference will not matter and
> > the reason for that is that since if you use `bitbake-getvar --value` in
> > a script or similar, you typically strip the linefeed that is output after
> > the variable so you get the actual value, and then there is no difference
> > in the result whether there was a linefeed (if the variable was defined,
> > but empty) or there was no linefeed (if the variable was not defined). In
> > both cases the result will be an empty string.
> 
> This is a problem as users likely should care if the variable is not
> set at all, it suggests bigger problems.

Well, in my experience, in most cases it is not necessary to make the 
distinction. Compare to how shell variables, and, in most cases, bitbake 
variables are used. Yes, there can definitely be cases where you need to 
make a distinction, and then you want it to be definitive, i.e., you do 
not want to assume that a value of "None" actually means that the variable 
is not defined, while technically it would be a legal value for the variable.

> 
> Cheers,
> 
> Richard

//Peter

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

[Richard Purdie]

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.

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.

I'm not taking this patch as it stands.

Cheers,

Richard

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

[Peter Kjellerstedt]

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

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 would also prefer if it is possible 
to silence the error for the case where you do not care if the 
variable is undefined.

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'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]

If -s or --no-messages (borrowed from `grep`) is also specified, then 
no error message is output and the exit code is 0.

> I'm not taking this patch as it stands.
> 
> Cheers,
> 
> Richard

//Peter

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

[Richard Purdie]

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.

Cheers,

Richard

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

[Mario Domenech Goulart]

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

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

[Peter Kjellerstedt]

> -----Original Message-----
> From: Mario Domenech Goulart <mario@parenteses.org>
> Sent: den 25 september 2023 20:39
> To: Richard Purdie <richard.purdie@linuxfoundation.org>
> Cc: 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
> 
> 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.

Sorry. I was probably still in "why doesn't he see it my way" mode 
when I wrote that, before I turned to "ok, we can find a solution 
for this" mode.

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

I liked this idea and I have implemented it (patch to follow shortly). 

There is one caveat though. For the case when --value is not used 
and the requested variable is undefined, the code will actually 
have to inject the default value using d.setVar() to make sure the 
output is consistent.

And just in case RP does not like this solution, I have an 
implementation of the original proposal with --ignore-undefined as 
well. But let's try this one first.

//Peter