[PATCH v2 1/6] patman: Fix updating argument defaults from settings

[Sean Anderson]

Hi Doug,

On 7/1/22 4:23 PM, Douglas Anderson wrote:
> Ever since commit 4600767d294d ("patman: Refactor how the default
> subcommand works"), when I use patman on the Linux tree I get grumbles
> about unknown tags. This is because the Linux default making
> process_tags be False wasn't working anymore.
> 
> It appears that the comment claiming that the defaults propagates
> through all subparsers no longer works for some reason.
> 
> We're already looking at all the subparsers anyway. Let's just update
> each one.
> 
> Fixes: 4600767d294d ("patman: Refactor how the default subcommand works")
> Signed-off-by: Douglas Anderson <dianders at chromium.org>
> Tested-by: Brian Norris <briannorris at chromium.org>
> Reviewed-by: Brian Norris <briannorris at chromium.org>
> ---
> 
> (no changes since v1)
> 
>  tools/patman/settings.py | 41 +++++++++++++++++++++-------------------
>  1 file changed, 22 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/patman/settings.py b/tools/patman/settings.py
> index 7c2b5c196c06..5eefe3d1f55e 100644
> --- a/tools/patman/settings.py
> +++ b/tools/patman/settings.py
> @@ -244,28 +244,31 @@ def _UpdateDefaults(main_parser, config):
>                    if isinstance(action, argparse._SubParsersAction)
>                    for _, subparser in action.choices.items()]
>  
> +    unknown_settings = set(name for name, val in config.items('settings'))
> +
>      # Collect the defaults from each parser
> -    defaults = {}
>      for parser in parsers:
>          pdefs = parser.parse_known_args()[0]
> -        defaults.update(vars(pdefs))
> -
> -    # Go through the settings and collect defaults
> -    for name, val in config.items('settings'):
> -        if name in defaults:
> -            default_val = defaults[name]
> -            if isinstance(default_val, bool):
> -                val = config.getboolean('settings', name)
> -            elif isinstance(default_val, int):
> -                val = config.getint('settings', name)
> -            elif isinstance(default_val, str):
> -                val = config.get('settings', name)
> -            defaults[name] = val
> -        else:
> -            print("WARNING: Unknown setting %s" % name)
> -
> -    # Set all the defaults (this propagates through all subparsers)
> -    main_parser.set_defaults(**defaults)
> +        defaults = dict(vars(pdefs))
> +
> +        # Go through the settings and collect defaults
> +        for name, val in config.items('settings'):
> +            if name in defaults:
> +                default_val = defaults[name]
> +                if isinstance(default_val, bool):
> +                    val = config.getboolean('settings', name)
> +                elif isinstance(default_val, int):
> +                    val = config.getint('settings', name)
> +                elif isinstance(default_val, str):
> +                    val = config.get('settings', name)
> +                defaults[name] = val
> +                unknown_settings.discard(name)
> +
> +        # Set all the defaults
> +        parser.set_defaults(**defaults)
> +
> +    for name in sorted(unknown_settings):
> +        print("WARNING: Unknown setting %s" % name)

Can you see if 4780f7d8a6b ("patman: Fix defaults not propagating to
subparsers") [1] addresses this problem? The implementation is different,
but I believe these should have the same effect.

--Sean

[1] https://lore.kernel.org/u-boot/20220429145334.2497202-1-sean.anderson@seco.com/