[PATCH 3/3] patman: fix project-defaults not propagating into parsers

[Simon Glass]

Hi Philipp,

On Wed, 25 Nov 2020 at 12:41, Philipp Tomsich <philipp.tomsich at vrull.eu> wrote:
>
> Simon,
>
> On Wed, 25 Nov 2020 at 16:30, Simon Glass <sjg at chromium.org> wrote:
> > Here is a pointer to the docs I saw:
> >
> > https://docs.python.org/3/library/argparse.html#argparse.ArgumentParser.set_defaults
> >
> > "Parser-level defaults can be particularly useful when working with
> > multiple parsers. See the add_subparsers() method for an example of
> > this type."
>
> I had looked at the same documentation and needed to carefully read the example
> to infer the original author's intended messaging...
>
> The only mention of this in the subparser-docs was:
>>
>> One particularly effective way of handling sub-commands is to combine the use of the add_subparsers() method with calls to set_defaults() so that each subparser knows which Python function it should execute.
>
> This example just demonstrated that two different subparsers could create a different
> default for the same argument name.  I couldn't find any example of a set_defaults()
> on a higher-level parser propagating and the argparse-source doesn't seem to try to
> propagate defaults either.
>
> I am starting to think that the correct fix for this would be more along the lines of:
>>
>> diff --git a/tools/patman/settings.py b/tools/patman/settings.py
>> index bb3f868..525c69e 100644
>> --- a/tools/patman/settings.py
>> +++ b/tools/patman/settings.py
>> @@ -266,7 +266,11 @@ def _UpdateDefaults(main_parser, config):
>>              print("WARNING: Unknown setting %s" % name)
>>
>>      # Set all the defaults (this propagates through all subparsers)
>> -    main_parser.set_defaults(**defaults)
>> +    for parser in parsers:
>> +        for name, val in defaults.items():
>> +            if parser.get_default(name) and val:
>> +                parser.set_defaults(name=val)
>>
>>  def _ReadAliasFile(fname):
>>      """Read in the U-Boot git alias file if it exists.
>
> than of what I sent out earlier.

Can you check u-boot/next for this? I have been testing it a bit and
from what I can tell the code there does propagate things down to
parsers.

>
> An interesting aside: it looks as if the double-parsing of the args leads to 'defaults'
> being installed for all arguments that are passed in the first cycle (e.g. count,
> project and patchwork_url suddenly have the values loaded from the config files
> and parsed from the args populated with 'default' values).

Yes, it isn't very elegant. If you have a better way, it would be nice
to improve it.

Regards,
Simon