| Message ID | 20230925203128.73512-1-pkj@axis.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCHv2] bitbake-getvar: Add an option to specify a default value | expand |
Hi Peter, On Mon, 2023-09-25 at 22:31 +0200, Peter Kjellerstedt wrote: > The default value is returned in case the specified variable or variable > flag is not defined. > > In case no default option has been specified, bitbake-getvar fails with > an error instead. > > Suggested-by: Mario Domenech Goulart <mario@parenteses.org> > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> > --- > > PATCHv2: New solution based on the discussion after the previous patch. > > bitbake/bin/bitbake-getvar | 36 +++++++++++++++++++++++++++--------- > 1 file changed, 27 insertions(+), 9 deletions(-) I've been thinking about this and I did poll a few others. The logic for implementing the "default" is pretty ugly, I understand why that was necessary but I'm not keen on what it does to the code, or the potential confusion on the output on the normal case without -- value. All things considered I think I'd prefer to add the --ignore-undefined option, with None being translated to "". That is at least really clear at the caller sites. Cheers, Richard
> -----Original Message----- > From: bitbake-devel@lists.openembedded.org <bitbake-devel@lists.openembedded.org> On Behalf Of Richard Purdie > Sent: den 27 september 2023 13:42 > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; bitbake-devel@lists.openembedded.org > Subject: Re: [bitbake-devel] [PATCHv2] bitbake-getvar: Add an option to specify a default value > > Hi Peter, > > On Mon, 2023-09-25 at 22:31 +0200, Peter Kjellerstedt wrote: > > The default value is returned in case the specified variable or variable > > flag is not defined. > > > > In case no default option has been specified, bitbake-getvar fails with > > an error instead. > > > > Suggested-by: Mario Domenech Goulart <mario@parenteses.org> > > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> > > --- > > > > PATCHv2: New solution based on the discussion after the previous patch. > > > > bitbake/bin/bitbake-getvar | 36 +++++++++++++++++++++++++++--------- > > 1 file changed, 27 insertions(+), 9 deletions(-) > > I've been thinking about this and I did poll a few others. > > The logic for implementing the "default" is pretty ugly, I understand > why that was necessary but I'm not keen on what it does to the code, or > the potential confusion on the output on the normal case without -- > value. Ok, then let's drop that idea. While I liked the idea together with --value, the need to inject the value to get the correct output when --value is not used felt weird. > All things considered I think I'd prefer to add the --ignore-undefined > option, with None being translated to "". That is at least really clear > at the caller sites. Ok, I will do that. > Cheers, > > Richard One thing I would ask you to reconsider though is your wish to only support a long option for --ignore-undefined. I believe the choice should be with the user of the tool. Typically when I write scripts that call other tools, I tend to be more expletive and use long options (at least for commands that are not as common as, e.g., `rm -rf`). However, when typing the commands on the command line I much prefer to be able to use short options to save on typing (especially for commands where I do not have any Zsh completion support). And on that note, I would like to add a short option for --value as well since I found it odd that it was missing when I started using bitbake-getvar. The obvious choice is -v... Now, since I expect the above to be potentially controversial (even though to me it is just natural to have short options for all long options), I have split the change into four commits, one for the new errors, one to change the output for undefined variables, and two for the short options. So while I of course would like to see all of them integrated, this leaves the decision with you. //Peter
diff --git a/bitbake/bin/bitbake-getvar b/bitbake/bin/bitbake-getvar index 4a9eb4f311..1c670e7b96 100755 --- a/bitbake/bin/bitbake-getvar +++ b/bitbake/bin/bitbake-getvar @@ -26,15 +26,16 @@ if __name__ == "__main__": parser.add_argument('-f', '--flag', help='Specify a variable flag to query (with --value)', default=None) parser.add_argument('--value', help='Only report the value, no history and no variable name', action="store_true") parser.add_argument('-q', '--quiet', help='Silence bitbake server logging', action="store_true") + parser.add_argument('-d', '--default', help='Default value returned in case variable is not defined', + default=None, const=True, required=False, nargs='?') args = parser.parse_args() - if args.unexpand and not args.value: - print("--unexpand only makes sense with --value") - sys.exit(1) + if not args.value: + if args.unexpand: + sys.exit("--unexpand only makes sense with --value") - if args.flag and not args.value: - print("--flag only makes sense with --value") - sys.exit(1) + if args.flag: + sys.exit("--flag only makes sense with --value") with bb.tinfoil.Tinfoil(tracking=True, setup_logging=not args.quiet) as tinfoil: if args.recipe: @@ -43,9 +44,26 @@ 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)))) - elif args.value: - print(str(d.getVar(args.variable, expand=(not args.unexpand)))) + value = d.getVarFlag(args.variable, args.flag, expand=(not args.unexpand)) + if value is None: + if args.default is None: + sys.exit(f"The flag '{args.flag}' is not defined for variable '{args.variable}'") + elif isinstance(args.default, str): + value = args.default + else: + value = d.getVar(args.variable, expand=(not args.unexpand)) + if value is None: + if args.default is None: + sys.exit(f"The variable '{args.variable}' is not defined") + elif isinstance(args.default, str): + value = args.default + # Inject the default value, in case it is needed by bb.data.emit_var() below. + d.setVar(args.variable, value); + if args.value: + if value is not None: + print(str(value)) else: bb.data.emit_var(args.variable, d=d, all=True)
The default value is returned in case the specified variable or variable flag is not defined. In case no default option has been specified, bitbake-getvar fails with an error instead. Suggested-by: Mario Domenech Goulart <mario@parenteses.org> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> --- PATCHv2: New solution based on the discussion after the previous patch. bitbake/bin/bitbake-getvar | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)