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

[Joe Hershberger]

Hi Masahiro-san,

On Mon, May 18, 2015 at 8:58 PM, Masahiro Yamada
<yamada.masahiro at socionext.com> wrote:
> 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.

OK... dropping that warning.

>> +            shutil.move(defconfig_path,
>> +                        os.path.join('configs', self.defconfig))
>>              self.state = STATE_IDLE
>>              return True
>
>
>
>
>
> --
> Best Regards
> Masahiro Yamada
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot