| Message ID | 20230925032246.1316597-1-pkj@axis.com |
|---|---|
| State | New |
| Headers | show |
| Series | bitbake-getvar: Do not output anything with --value and undefined variable | expand |
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
> -----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
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
> -----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
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
> -----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
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
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
> -----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
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))
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(-)