[U-Boot] [PATCH v4 01/10] moveconfig: Always run savedefconfig on the moved config

[Masahiro Yamada]

Hi Joe,

2015-05-16 6:40 GMT+09:00 Joe Hershberger <joe.hershberger at ni.com>:
> This will ensure that the order of the defconfig entries will always
> match that of the Kconfig files. After one slightly painful (but
> still early in the process) pass over all boards, this should keep
> the defconfigs clean from here on.
>
> Users must edit the Kconfig first to add the menu entries and then run
> moveconfig.py to update the defconfig files and the include configs.
>
> As such, moveconfig.py cannot compare against the '.config' contents.
>
> Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>


This feature expects the defconfigs are all clean.
Otherwise, savedefconfig would make "git diff" noisier.

It is safer to use "make menuconfig && make savedefconfig"
for adding new options, but some people still try to
edit the defconfig directly...

Perhaps, should do the global cleanup periodically?


> ---
> This is based on https://patchwork.ozlabs.org/patch/472591/
>
> Changes in v4:
> -Rebased series on Masahiro's v2
>
> Changes in v3: None
> Changes in v2: None
>
>  tools/moveconfig.py | 35 ++++++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
> index c39ea95..544f6af 100755
> --- a/tools/moveconfig.py
> +++ b/tools/moveconfig.py
> @@ -46,6 +46,9 @@ should look like this:
>    CONFIG_CMD_USB bool n
>    CONFIG_SYS_TEXT_BASE hex 0x00000000
>
> +Next you must edit the Kconfig to add the menu entries for the configs
> +you are moving.
> +
>  And then run this tool giving the file name of the recipe

Uh, I was doing in a different work-flow.
(I edit the Kconfig after I move CONFIGs to defconfigs).

But, the Kconfig must be edited beforehand
to do savedefconfig.

So, I am OK with this change.



>    $ tools/moveconfig.py recipe
> @@ -192,6 +195,7 @@ CROSS_COMPILE = {
>  STATE_IDLE = 0
>  STATE_DEFCONFIG = 1
>  STATE_AUTOCONF = 2
> +STATE_SAVEDEFCONFIG = 3
>
>  ACTION_MOVE = 0
>  ACTION_DEFAULT_VALUE = 1
> @@ -390,8 +394,7 @@ class KconfigParser:
>
>          return CROSS_COMPILE.get(arch, '')
>
> -    def parse_one_config(self, config_attr, defconfig_lines,
> -                         dotconfig_lines, autoconf_lines):
> +    def parse_one_config(self, config_attr, defconfig_lines, autoconf_lines):
>          """Parse .config, defconfig, include/autoconf.mk for one config.
>
>          This function looks for the config options in the lines from
> @@ -402,7 +405,6 @@ class KconfigParser:
>            config_attr: A dictionary including the name, the type,
>                         and the default value of the target config.
>            defconfig_lines: lines from the original defconfig file.
> -          dotconfig_lines: lines from the .config file.
>            autoconf_lines: lines from the include/autoconf.mk file.
>
>          Returns:
> @@ -418,7 +420,7 @@ class KconfigParser:
>          else:
>              default = config + '=' + config_attr['default']
>
> -        for line in defconfig_lines + dotconfig_lines:
> +        for line in defconfig_lines:
>              line = line.rstrip()
>              if line.startswith(config + '=') or line == not_set:
>                  return (ACTION_ALREADY_EXIST, line)
> @@ -463,15 +465,12 @@ class KconfigParser:
>          with open(defconfig_path) as f:
>              defconfig_lines = f.readlines()
>
> -        with open(dotconfig_path) as f:
> -            dotconfig_lines = f.readlines()
> -
>          with open(autoconf_path) as f:
>              autoconf_lines = f.readlines()
>
>          for config_attr in self.config_attrs:
>              result = self.parse_one_config(config_attr, defconfig_lines,
> -                                           dotconfig_lines, autoconf_lines)
> +                                           autoconf_lines)
>              results.append(result)
>
>          log = ''


With the change of the work-flow above,
we need not parse the .config, so this seems OK.



> @@ -499,7 +498,7 @@ class KconfigParser:
>          print log,
>
>          if not self.options.dry_run:
> -            with open(defconfig_path, 'a') as f:
> +            with open(dotconfig_path, 'a') as f:
>                  for (action, value) in results:
>                      if action == ACTION_MOVE:
>                          f.write(value + '\n')
> @@ -608,6 +607,24 @@ class Slot:
>
>          if self.state == STATE_AUTOCONF:
>              self.parser.update_defconfig(self.defconfig)
> +
> +            """Save off the defconfig in a consistent way"""
> +            cmd = list(self.make_cmd)
> +            cmd.append('savedefconfig')
> +            self.ps = subprocess.Popen(cmd, stdout=self.devnull,
> +                                       stderr=self.devnull)
> +            self.state = STATE_SAVEDEFCONFIG
> +            return False
> +
> +        if self.state == STATE_SAVEDEFCONFIG:
> +            defconfig_path = os.path.join(self.build_dir, 'defconfig')
> +            if not os.path.exists(defconfig_path):
> +                print >> sys.stderr, log_msg(self.options.color,
> +                                             COLOR_LIGHT_RED,
> +                                             self.defconfig,
> +                                             'The defconfig was not updated')

Is this warning message reachable?

The missing defconfig means "make savedefconfig" failed.

That case has been already caught by 'Failed to process.' above, I think.


> +            shutil.move(defconfig_path,
> +                        os.path.join('configs', self.defconfig))
>              self.state = STATE_IDLE
>              return True





-- 
Best Regards
Masahiro Yamada