u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] moveconfig: Improve the pylist score a little
@ 2021-12-18 21:54 Simon Glass
  2021-12-18 21:54 ` [PATCH 1/6] moveconfig: Use single quotes Simon Glass
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Simon Glass @ 2021-12-18 21:54 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Masahiro Yamada, Simon Glass, Trevor Woerner

There are about 240 warnings in this file.

This series refactors file reading and writing, which fixes about 40. It
also includes a patch to make a start on the rest, reducing the count to
about 160.

It also tidies up quoting.

There are still about 160 warnings left.


Simon Glass (6):
  moveconfig: Use single quotes
  moveconfig: Convert to ArgumentParser
  moveconfig: Drop check for old Python
  moveconfig: Use a function to write files
  moveconfig: Use a function to read files
  moveconfig: Fix some pylint errors

 tools/moveconfig.py | 645 +++++++++++++++++++++++---------------------
 1 file changed, 336 insertions(+), 309 deletions(-)

-- 
2.34.1.173.g76aa8bc2d0-goog


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/6] moveconfig: Use single quotes
  2021-12-18 21:54 [PATCH 0/6] moveconfig: Improve the pylist score a little Simon Glass
@ 2021-12-18 21:54 ` Simon Glass
  2021-12-18 22:16   ` Heinrich Schuchardt
  2022-01-25 15:57   ` Tom Rini
  2021-12-18 21:54 ` [PATCH 2/6] moveconfig: Convert to ArgumentParser Simon Glass
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Simon Glass @ 2021-12-18 21:54 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Masahiro Yamada, Simon Glass, Trevor Woerner

Quite a few places use double quotes. Fix this to be consistent with
other Python code in U-Boot.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/moveconfig.py | 72 ++++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index a86c07caa6e..ab2d4905db5 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -71,23 +71,23 @@ CONFIG_DATABASE = 'moveconfig.db'
 CONFIG_LEN = len('CONFIG_')
 
 SIZES = {
-    "SZ_1":    0x00000001, "SZ_2":    0x00000002,
-    "SZ_4":    0x00000004, "SZ_8":    0x00000008,
-    "SZ_16":   0x00000010, "SZ_32":   0x00000020,
-    "SZ_64":   0x00000040, "SZ_128":  0x00000080,
-    "SZ_256":  0x00000100, "SZ_512":  0x00000200,
-    "SZ_1K":   0x00000400, "SZ_2K":   0x00000800,
-    "SZ_4K":   0x00001000, "SZ_8K":   0x00002000,
-    "SZ_16K":  0x00004000, "SZ_32K":  0x00008000,
-    "SZ_64K":  0x00010000, "SZ_128K": 0x00020000,
-    "SZ_256K": 0x00040000, "SZ_512K": 0x00080000,
-    "SZ_1M":   0x00100000, "SZ_2M":   0x00200000,
-    "SZ_4M":   0x00400000, "SZ_8M":   0x00800000,
-    "SZ_16M":  0x01000000, "SZ_32M":  0x02000000,
-    "SZ_64M":  0x04000000, "SZ_128M": 0x08000000,
-    "SZ_256M": 0x10000000, "SZ_512M": 0x20000000,
-    "SZ_1G":   0x40000000, "SZ_2G":   0x80000000,
-    "SZ_4G":  0x100000000
+    'SZ_1':    0x00000001, 'SZ_2':    0x00000002,
+    'SZ_4':    0x00000004, 'SZ_8':    0x00000008,
+    'SZ_16':   0x00000010, 'SZ_32':   0x00000020,
+    'SZ_64':   0x00000040, 'SZ_128':  0x00000080,
+    'SZ_256':  0x00000100, 'SZ_512':  0x00000200,
+    'SZ_1K':   0x00000400, 'SZ_2K':   0x00000800,
+    'SZ_4K':   0x00001000, 'SZ_8K':   0x00002000,
+    'SZ_16K':  0x00004000, 'SZ_32K':  0x00008000,
+    'SZ_64K':  0x00010000, 'SZ_128K': 0x00020000,
+    'SZ_256K': 0x00040000, 'SZ_512K': 0x00080000,
+    'SZ_1M':   0x00100000, 'SZ_2M':   0x00200000,
+    'SZ_4M':   0x00400000, 'SZ_8M':   0x00800000,
+    'SZ_16M':  0x01000000, 'SZ_32M':  0x02000000,
+    'SZ_64M':  0x04000000, 'SZ_128M': 0x08000000,
+    'SZ_256M': 0x10000000, 'SZ_512M': 0x20000000,
+    'SZ_1G':   0x40000000, 'SZ_2G':   0x80000000,
+    'SZ_4G':  0x100000000
 }
 
 ### helper functions ###
@@ -536,12 +536,12 @@ def try_expand(line):
         aeval = asteval.Interpreter( usersyms=SIZES, minimal=True )
         cfg, val = re.split("=", line)
         val= val.strip('\"')
-        if re.search("[*+-/]|<<|SZ_+|\(([^\)]+)\)", val):
+        if re.search(r'[*+-/]|<<|SZ_+|\(([^\)]+)\)', val):
             newval = hex(aeval(val))
-            print("\tExpanded expression %s to %s" % (val, newval))
+            print('\tExpanded expression %s to %s' % (val, newval))
             return cfg+'='+newval
     except:
-        print("\tFailed to expand expression in %s" % line)
+        print('\tFailed to expand expression in %s' % line)
 
     return line
 
@@ -735,10 +735,10 @@ class KconfigParser:
                 actlog = "Move '%s'" % value
                 log_color = COLOR_LIGHT_GREEN
             elif action == ACTION_NO_ENTRY:
-                actlog = "%s is not defined in Kconfig.  Do nothing." % value
+                actlog = '%s is not defined in Kconfig.  Do nothing.' % value
                 log_color = COLOR_LIGHT_BLUE
             elif action == ACTION_NO_ENTRY_WARN:
-                actlog = "%s is not defined in Kconfig (suspicious).  Do nothing." % value
+                actlog = '%s is not defined in Kconfig (suspicious).  Do nothing.' % value
                 log_color = COLOR_YELLOW
                 suspicious = True
             elif action == ACTION_NO_CHANGE:
@@ -746,10 +746,10 @@ class KconfigParser:
                          % value
                 log_color = COLOR_LIGHT_PURPLE
             elif action == ACTION_SPL_NOT_EXIST:
-                actlog = "SPL is not enabled for this defconfig.  Skip."
+                actlog = 'SPL is not enabled for this defconfig.  Skip.'
                 log_color = COLOR_PURPLE
             else:
-                sys.exit("Internal Error. This should not happen.")
+                sys.exit('Internal Error. This should not happen.')
 
             log += color_text(self.options.color, log_color, actlog) + '\n'
 
@@ -930,7 +930,7 @@ class Slot:
         elif self.state == STATE_SAVEDEFCONFIG:
             self.update_defconfig()
         else:
-            sys.exit("Internal Error. This should not happen.")
+            sys.exit('Internal Error. This should not happen.')
 
         return True if self.state == STATE_IDLE else False
 
@@ -938,7 +938,7 @@ class Slot:
         """Handle error cases."""
 
         self.log += color_text(self.options.color, COLOR_LIGHT_RED,
-                               "Failed to process.\n")
+                               'Failed to process.\n')
         if self.options.verbose:
             self.log += color_text(self.options.color, COLOR_LIGHT_CYAN,
                                    self.ps.stderr.read().decode())
@@ -999,9 +999,9 @@ class Slot:
             return
         if updated:
             self.log += color_text(self.options.color, COLOR_LIGHT_GREEN,
-                                   "Syncing by savedefconfig...\n")
+                                   'Syncing by savedefconfig...\n')
         else:
-            self.log += "Syncing by savedefconfig (forced by option)...\n"
+            self.log += 'Syncing by savedefconfig (forced by option)...\n'
 
         cmd = list(self.make_cmd)
         cmd.append('savedefconfig')
@@ -1022,7 +1022,7 @@ class Slot:
 
         if updated:
             self.log += color_text(self.options.color, COLOR_LIGHT_BLUE,
-                                   "defconfig was updated.\n")
+                                   'defconfig was updated.\n')
 
         if not self.options.dry_run and updated:
             shutil.move(new_defconfig, orig_defconfig)
@@ -1045,7 +1045,7 @@ class Slot:
 
         if not success:
             if self.options.exit_on_error:
-                sys.exit("Exit on error.")
+                sys.exit('Exit on error.')
             # If --exit-on-error flag is not set, skip this board and continue.
             # Record the failed board.
             self.failed_boards.add(self.defconfig)
@@ -1137,9 +1137,9 @@ class Slots:
 
         if boards:
             boards = '\n'.join(boards) + '\n'
-            msg = "The following boards were not processed due to error:\n"
+            msg = 'The following boards were not processed due to error:\n'
             msg += boards
-            msg += "(the list has been saved in %s)\n" % output_file
+            msg += '(the list has been saved in %s)\n' % output_file
             print(color_text(self.options.color, COLOR_LIGHT_RED,
                                             msg), file=sys.stderr)
 
@@ -1156,10 +1156,10 @@ class Slots:
 
         if boards:
             boards = '\n'.join(boards) + '\n'
-            msg = "The following boards might have been converted incorrectly.\n"
-            msg += "It is highly recommended to check them manually:\n"
+            msg = 'The following boards might have been converted incorrectly.\n'
+            msg += 'It is highly recommended to check them manually:\n'
             msg += boards
-            msg += "(the list has been saved in %s)\n" % output_file
+            msg += '(the list has been saved in %s)\n' % output_file
             print(color_text(self.options.color, COLOR_YELLOW,
                                             msg), file=sys.stderr)
 
@@ -1177,7 +1177,7 @@ class ReferenceSource:
           commit: commit to git-clone
         """
         self.src_dir = tempfile.mkdtemp()
-        print("Cloning git repo to a separate work directory...")
+        print('Cloning git repo to a separate work directory...')
         subprocess.check_output(['git', 'clone', os.getcwd(), '.'],
                                 cwd=self.src_dir)
         print("Checkout '%s' to build the original autoconf.mk." % \
-- 
2.34.1.173.g76aa8bc2d0-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/6] moveconfig: Convert to ArgumentParser
  2021-12-18 21:54 [PATCH 0/6] moveconfig: Improve the pylist score a little Simon Glass
  2021-12-18 21:54 ` [PATCH 1/6] moveconfig: Use single quotes Simon Glass
@ 2021-12-18 21:54 ` Simon Glass
  2021-12-18 22:32   ` Heinrich Schuchardt
  2022-01-25 15:57   ` Tom Rini
  2021-12-18 21:54 ` [PATCH 3/6] moveconfig: Drop check for old Python Simon Glass
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Simon Glass @ 2021-12-18 21:54 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Masahiro Yamada, Simon Glass, Trevor Woerner

This is a newer library and is now preferred for Python scripts. Update
the code to use it instead of optparse

Use 'args' instead of 'options' throughout, since this is the term used
in that module. Also it helps to avoid confusion with CONFIG options, a
term that is used in this file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/moveconfig.py | 221 ++++++++++++++++++++++----------------------
 1 file changed, 112 insertions(+), 109 deletions(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index ab2d4905db5..521297f7d58 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -10,6 +10,7 @@ Move config options from headers to defconfig files.
 See doc/develop/moveconfig.rst for documentation.
 """
 
+from argparse import ArgumentParser
 import asteval
 import collections
 import copy
@@ -19,7 +20,6 @@ import filecmp
 import fnmatch
 import glob
 import multiprocessing
-import optparse
 import os
 import queue
 import re
@@ -267,8 +267,8 @@ def extend_matched_lines(lines, matched, pre_patterns, post_patterns, extend_pre
     matched += extended_matched
     matched.sort()
 
-def confirm(options, prompt):
-    if not options.yes:
+def confirm(args, prompt):
+    if not args.yes:
         while True:
             choice = input('{} [y/n]: '.format(prompt))
             choice = choice.lower()
@@ -281,12 +281,12 @@ def confirm(options, prompt):
 
     return True
 
-def cleanup_empty_blocks(header_path, options):
+def cleanup_empty_blocks(header_path, args):
     """Clean up empty conditional blocks
 
     Arguments:
       header_path: path to the cleaned file.
-      options: option flags.
+      args: program arguments
     """
     pattern = re.compile(r'^\s*#\s*if.*$\n^\s*#\s*endif.*$\n*', flags=re.M)
     with open(header_path) as f:
@@ -299,22 +299,22 @@ def cleanup_empty_blocks(header_path, options):
     new_data = pattern.sub('\n', data)
 
     show_diff(data.splitlines(True), new_data.splitlines(True), header_path,
-              options.color)
+              args.color)
 
-    if options.dry_run:
+    if args.dry_run:
         return
 
     with open(header_path, 'w') as f:
         f.write(new_data)
 
-def cleanup_one_header(header_path, patterns, options):
+def cleanup_one_header(header_path, patterns, args):
     """Clean regex-matched lines away from a file.
 
     Arguments:
       header_path: path to the cleaned file.
       patterns: list of regex patterns.  Any lines matching to these
                 patterns are deleted.
-      options: option flags.
+      args: program arguments
     """
     with open(header_path) as f:
         try:
@@ -362,23 +362,23 @@ def cleanup_one_header(header_path, patterns, options):
     for i in reversed(matched):
         tolines.pop(i)
 
-    show_diff(lines, tolines, header_path, options.color)
+    show_diff(lines, tolines, header_path, args.color)
 
-    if options.dry_run:
+    if args.dry_run:
         return
 
     with open(header_path, 'w') as f:
         for line in tolines:
             f.write(line)
 
-def cleanup_headers(configs, options):
+def cleanup_headers(configs, args):
     """Delete config defines from board headers.
 
     Arguments:
       configs: A list of CONFIGs to remove.
-      options: option flags.
+      args: program arguments
     """
-    if not confirm(options, 'Clean up headers?'):
+    if not confirm(args, 'Clean up headers?'):
         return
 
     patterns = []
@@ -397,16 +397,16 @@ def cleanup_headers(configs, options):
                     # This file contains UTF-16 data and no CONFIG symbols
                     if header_path == 'include/video_font_data.h':
                         continue
-                    cleanup_one_header(header_path, patterns, options)
-                    cleanup_empty_blocks(header_path, options)
+                    cleanup_one_header(header_path, patterns, args)
+                    cleanup_empty_blocks(header_path, args)
 
-def cleanup_one_extra_option(defconfig_path, configs, options):
+def cleanup_one_extra_option(defconfig_path, configs, args):
     """Delete config defines in CONFIG_SYS_EXTRA_OPTIONS in one defconfig file.
 
     Arguments:
       defconfig_path: path to the cleaned defconfig file.
       configs: A list of CONFIGs to remove.
-      options: option flags.
+      args: program arguments
     """
 
     start = 'CONFIG_SYS_EXTRA_OPTIONS="'
@@ -440,23 +440,23 @@ def cleanup_one_extra_option(defconfig_path, configs, options):
     else:
         tolines.pop(i)
 
-    show_diff(lines, tolines, defconfig_path, options.color)
+    show_diff(lines, tolines, defconfig_path, args.color)
 
-    if options.dry_run:
+    if args.dry_run:
         return
 
     with open(defconfig_path, 'w') as f:
         for line in tolines:
             f.write(line)
 
-def cleanup_extra_options(configs, options):
+def cleanup_extra_options(configs, args):
     """Delete config defines in CONFIG_SYS_EXTRA_OPTIONS in defconfig files.
 
     Arguments:
       configs: A list of CONFIGs to remove.
-      options: option flags.
+      args: program arguments
     """
-    if not confirm(options, 'Clean up CONFIG_SYS_EXTRA_OPTIONS?'):
+    if not confirm(args, 'Clean up CONFIG_SYS_EXTRA_OPTIONS?'):
         return
 
     configs = [ config[len('CONFIG_'):] for config in configs ]
@@ -465,16 +465,16 @@ def cleanup_extra_options(configs, options):
 
     for defconfig in defconfigs:
         cleanup_one_extra_option(os.path.join('configs', defconfig), configs,
-                                 options)
+                                 args)
 
-def cleanup_whitelist(configs, options):
+def cleanup_whitelist(configs, args):
     """Delete config whitelist entries
 
     Arguments:
       configs: A list of CONFIGs to remove.
-      options: option flags.
+      args: program arguments
     """
-    if not confirm(options, 'Clean up whitelist entries?'):
+    if not confirm(args, 'Clean up whitelist entries?'):
         return
 
     with open(os.path.join('scripts', 'config_whitelist.txt')) as f:
@@ -491,14 +491,14 @@ def find_matching(patterns, line):
             return True
     return False
 
-def cleanup_readme(configs, options):
+def cleanup_readme(configs, args):
     """Delete config description in README
 
     Arguments:
       configs: A list of CONFIGs to remove.
-      options: option flags.
+      args: program arguments
     """
-    if not confirm(options, 'Clean up README?'):
+    if not confirm(args, 'Clean up README?'):
         return
 
     patterns = []
@@ -590,16 +590,16 @@ class KconfigParser:
     re_arch = re.compile(r'CONFIG_SYS_ARCH="(.*)"')
     re_cpu = re.compile(r'CONFIG_SYS_CPU="(.*)"')
 
-    def __init__(self, configs, options, build_dir):
+    def __init__(self, configs, args, build_dir):
         """Create a new parser.
 
         Arguments:
           configs: A list of CONFIGs to move.
-          options: option flags.
+          args: program arguments
           build_dir: Build directory.
         """
         self.configs = configs
-        self.options = options
+        self.args = args
         self.dotconfig = os.path.join(build_dir, '.config')
         self.autoconf = os.path.join(build_dir, 'include', 'autoconf.mk')
         self.spl_autoconf = os.path.join(build_dir, 'spl', 'include',
@@ -704,7 +704,7 @@ class KconfigParser:
         suspicious = False
         rm_files = [self.config_autoconf, self.autoconf]
 
-        if self.options.spl:
+        if self.args.spl:
             if os.path.exists(self.spl_autoconf):
                 autoconf_path = self.spl_autoconf
                 rm_files.append(self.spl_autoconf)
@@ -712,7 +712,7 @@ class KconfigParser:
                 for f in rm_files:
                     os.remove(f)
                 return (updated, suspicious,
-                        color_text(self.options.color, COLOR_BROWN,
+                        color_text(self.args.color, COLOR_BROWN,
                                    "SPL is not enabled.  Skipped.") + '\n')
         else:
             autoconf_path = self.autoconf
@@ -751,7 +751,7 @@ class KconfigParser:
             else:
                 sys.exit('Internal Error. This should not happen.')
 
-            log += color_text(self.options.color, log_color, actlog) + '\n'
+            log += color_text(self.args.color, log_color, actlog) + '\n'
 
         with open(self.dotconfig, 'a') as f:
             for (action, value) in results:
@@ -782,7 +782,7 @@ class KconfigParser:
             if action != ACTION_MOVE:
                 continue
             if not value + '\n' in defconfig_lines:
-                log += color_text(self.options.color, COLOR_YELLOW,
+                log += color_text(self.args.color, COLOR_YELLOW,
                                   "'%s' was removed by savedefconfig.\n" %
                                   value)
 
@@ -825,14 +825,14 @@ class Slot:
     for faster processing.
     """
 
-    def __init__(self, toolchains, configs, options, progress, devnull,
+    def __init__(self, toolchains, configs, args, progress, devnull,
 		 make_cmd, reference_src_dir, db_queue):
         """Create a new process slot.
 
         Arguments:
           toolchains: Toolchains object containing toolchains.
           configs: A list of CONFIGs to move.
-          options: option flags.
+          args: Program arguments
           progress: A progress indicator.
           devnull: A file object of '/dev/null'.
           make_cmd: command name of GNU Make.
@@ -841,14 +841,14 @@ class Slot:
           db_queue: output queue to write config info for the database
         """
         self.toolchains = toolchains
-        self.options = options
+        self.args = args
         self.progress = progress
         self.build_dir = tempfile.mkdtemp()
         self.devnull = devnull
         self.make_cmd = (make_cmd, 'O=' + self.build_dir)
         self.reference_src_dir = reference_src_dir
         self.db_queue = db_queue
-        self.parser = KconfigParser(configs, options, self.build_dir)
+        self.parser = KconfigParser(configs, args, self.build_dir)
         self.state = STATE_IDLE
         self.failed_boards = set()
         self.suspicious_boards = set()
@@ -923,7 +923,7 @@ class Slot:
             if self.current_src_dir:
                 self.current_src_dir = None
                 self.do_defconfig()
-            elif self.options.build_db:
+            elif self.args.build_db:
                 self.do_build_db()
             else:
                 self.do_savedefconfig()
@@ -937,10 +937,10 @@ class Slot:
     def handle_error(self):
         """Handle error cases."""
 
-        self.log += color_text(self.options.color, COLOR_LIGHT_RED,
+        self.log += color_text(self.args.color, COLOR_LIGHT_RED,
                                'Failed to process.\n')
-        if self.options.verbose:
-            self.log += color_text(self.options.color, COLOR_LIGHT_CYAN,
+        if self.args.verbose:
+            self.log += color_text(self.args.color, COLOR_LIGHT_CYAN,
                                    self.ps.stderr.read().decode())
         self.finish(False)
 
@@ -961,7 +961,7 @@ class Slot:
         try:
             toolchain = self.toolchains.Select(arch)
         except ValueError:
-            self.log += color_text(self.options.color, COLOR_YELLOW,
+            self.log += color_text(self.args.color, COLOR_YELLOW,
                     "Tool chain for '%s' is missing.  Do nothing.\n" % arch)
             self.finish(False)
             return
@@ -994,11 +994,11 @@ class Slot:
             self.suspicious_boards.add(self.defconfig)
         self.log += log
 
-        if not self.options.force_sync and not updated:
+        if not self.args.force_sync and not updated:
             self.finish(True)
             return
         if updated:
-            self.log += color_text(self.options.color, COLOR_LIGHT_GREEN,
+            self.log += color_text(self.args.color, COLOR_LIGHT_GREEN,
                                    'Syncing by savedefconfig...\n')
         else:
             self.log += 'Syncing by savedefconfig (forced by option)...\n'
@@ -1021,10 +1021,10 @@ class Slot:
         updated = not filecmp.cmp(orig_defconfig, new_defconfig)
 
         if updated:
-            self.log += color_text(self.options.color, COLOR_LIGHT_BLUE,
+            self.log += color_text(self.args.color, COLOR_LIGHT_BLUE,
                                    'defconfig was updated.\n')
 
-        if not self.options.dry_run and updated:
+        if not self.args.dry_run and updated:
             shutil.move(new_defconfig, orig_defconfig)
         self.finish(True)
 
@@ -1044,7 +1044,7 @@ class Slot:
         print(log, file=(sys.stdout if success else sys.stderr))
 
         if not success:
-            if self.options.exit_on_error:
+            if self.args.exit_on_error:
                 sys.exit('Exit on error.')
             # If --exit-on-error flag is not set, skip this board and continue.
             # Record the failed board.
@@ -1068,25 +1068,25 @@ class Slots:
 
     """Controller of the array of subprocess slots."""
 
-    def __init__(self, toolchains, configs, options, progress,
+    def __init__(self, toolchains, configs, args, progress,
 		 reference_src_dir, db_queue):
         """Create a new slots controller.
 
         Arguments:
           toolchains: Toolchains object containing toolchains.
           configs: A list of CONFIGs to move.
-          options: option flags.
+          args: Program arguments
           progress: A progress indicator.
           reference_src_dir: Determine the true starting config state from this
                              source tree.
           db_queue: output queue to write config info for the database
         """
-        self.options = options
+        self.args = args
         self.slots = []
         devnull = get_devnull()
         make_cmd = get_make_cmd()
-        for i in range(options.jobs):
-            self.slots.append(Slot(toolchains, configs, options, progress,
+        for i in range(args.jobs):
+            self.slots.append(Slot(toolchains, configs, args, progress,
 				   devnull, make_cmd, reference_src_dir,
 				   db_queue))
 
@@ -1140,7 +1140,7 @@ class Slots:
             msg = 'The following boards were not processed due to error:\n'
             msg += boards
             msg += '(the list has been saved in %s)\n' % output_file
-            print(color_text(self.options.color, COLOR_LIGHT_RED,
+            print(color_text(self.args.color, COLOR_LIGHT_RED,
                                             msg), file=sys.stderr)
 
             with open(output_file, 'w') as f:
@@ -1160,7 +1160,7 @@ class Slots:
             msg += 'It is highly recommended to check them manually:\n'
             msg += boards
             msg += '(the list has been saved in %s)\n' % output_file
-            print(color_text(self.options.color, COLOR_YELLOW,
+            print(color_text(self.args.color, COLOR_YELLOW,
                                             msg), file=sys.stderr)
 
             with open(output_file, 'w') as f:
@@ -1200,37 +1200,37 @@ class ReferenceSource:
 
         return self.src_dir
 
-def move_config(toolchains, configs, options, db_queue):
+def move_config(toolchains, configs, args, db_queue):
     """Move config options to defconfig files.
 
     Arguments:
       configs: A list of CONFIGs to move.
-      options: option flags
+      args: Program arguments
     """
     if len(configs) == 0:
-        if options.force_sync:
+        if args.force_sync:
             print('No CONFIG is specified. You are probably syncing defconfigs.', end=' ')
-        elif options.build_db:
+        elif args.build_db:
             print('Building %s database' % CONFIG_DATABASE)
         else:
             print('Neither CONFIG nor --force-sync is specified. Nothing will happen.', end=' ')
     else:
         print('Move ' + ', '.join(configs), end=' ')
-    print('(jobs: %d)\n' % options.jobs)
+    print('(jobs: %d)\n' % args.jobs)
 
-    if options.git_ref:
-        reference_src = ReferenceSource(options.git_ref)
+    if args.git_ref:
+        reference_src = ReferenceSource(args.git_ref)
         reference_src_dir = reference_src.get_dir()
     else:
         reference_src_dir = None
 
-    if options.defconfigs:
-        defconfigs = get_matched_defconfigs(options.defconfigs)
+    if args.defconfigs:
+        defconfigs = get_matched_defconfigs(args.defconfigs)
     else:
         defconfigs = get_all_defconfigs()
 
     progress = Progress(len(defconfigs))
-    slots = Slots(toolchains, configs, options, progress, reference_src_dir,
+    slots = Slots(toolchains, configs, args, progress, reference_src_dir,
 		  db_queue)
 
     # Main loop to process defconfig files:
@@ -1648,65 +1648,69 @@ def main():
     except NotImplementedError:
         cpu_count = 1
 
-    parser = optparse.OptionParser()
-    # Add options here
-    parser.add_option('-a', '--add-imply', type='string', default='',
+    epilog = '''Move config options from headers to defconfig files. See
+doc/develop/moveconfig.rst for documentation.'''
+
+    parser = ArgumentParser(epilog=epilog)
+    # Add arguments here
+    parser.add_argument('-a', '--add-imply', type=str, default='',
                       help='comma-separated list of CONFIG options to add '
                       "an 'imply' statement to for the CONFIG in -i")
-    parser.add_option('-A', '--skip-added', action='store_true', default=False,
+    parser.add_argument('-A', '--skip-added', action='store_true', default=False,
                       help="don't show options which are already marked as "
                       'implying others')
-    parser.add_option('-b', '--build-db', action='store_true', default=False,
+    parser.add_argument('-b', '--build-db', action='store_true', default=False,
                       help='build a CONFIG database')
-    parser.add_option('-c', '--color', action='store_true', default=False,
+    parser.add_argument('-c', '--color', action='store_true', default=False,
                       help='display the log in color')
-    parser.add_option('-C', '--commit', action='store_true', default=False,
+    parser.add_argument('-C', '--commit', action='store_true', default=False,
                       help='Create a git commit for the operation')
-    parser.add_option('-d', '--defconfigs', type='string',
+    parser.add_argument('-d', '--defconfigs', type=str,
                       help='a file containing a list of defconfigs to move, '
                       "one per line (for example 'snow_defconfig') "
                       "or '-' to read from stdin")
-    parser.add_option('-e', '--exit-on-error', action='store_true',
+    parser.add_argument('-e', '--exit-on-error', action='store_true',
                       default=False,
                       help='exit immediately on any error')
-    parser.add_option('-f', '--find', action='store_true', default=False,
+    parser.add_argument('-f', '--find', action='store_true', default=False,
                       help='Find boards with a given config combination')
-    parser.add_option('-H', '--headers-only', dest='cleanup_headers_only',
+    parser.add_argument('-H', '--headers-only', dest='cleanup_headers_only',
                       action='store_true', default=False,
                       help='only cleanup the headers')
-    parser.add_option('-i', '--imply', action='store_true', default=False,
+    parser.add_argument('-i', '--imply', action='store_true', default=False,
                       help='find options which imply others')
-    parser.add_option('-I', '--imply-flags', type='string', default='',
+    parser.add_argument('-I', '--imply-flags', type=str, default='',
                       help="control the -i option ('help' for help")
-    parser.add_option('-j', '--jobs', type='int', default=cpu_count,
+    parser.add_argument('-j', '--jobs', type=int, default=cpu_count,
                       help='the number of jobs to run simultaneously')
-    parser.add_option('-n', '--dry-run', action='store_true', default=False,
+    parser.add_argument('-n', '--dry-run', action='store_true', default=False,
                       help='perform a trial run (show log with no changes)')
-    parser.add_option('-r', '--git-ref', type='string',
+    parser.add_argument('-r', '--git-ref', type=str,
                       help='the git ref to clone for building the autoconf.mk')
-    parser.add_option('-s', '--force-sync', action='store_true', default=False,
+    parser.add_argument('-s', '--force-sync', action='store_true', default=False,
                       help='force sync by savedefconfig')
-    parser.add_option('-S', '--spl', action='store_true', default=False,
+    parser.add_argument('-S', '--spl', action='store_true', default=False,
                       help='parse config options defined for SPL build')
-    parser.add_option('-t', '--test', action='store_true', default=False,
+    parser.add_argument('-t', '--test', action='store_true', default=False,
                       help='run unit tests')
-    parser.add_option('-y', '--yes', action='store_true', default=False,
+    parser.add_argument('-y', '--yes', action='store_true', default=False,
                       help="respond 'yes' to any prompts")
-    parser.add_option('-v', '--verbose', action='store_true', default=False,
+    parser.add_argument('-v', '--verbose', action='store_true', default=False,
                       help='show any build errors as boards are built')
-    parser.usage += ' CONFIG ...'
+    parser.add_argument('configs', nargs='*')
 
-    (options, configs) = parser.parse_args()
+    args = parser.parse_args()
+    configs = args.configs
 
-    if options.test:
+    if args.test:
         sys.argv = [sys.argv[0]]
         fail, count = doctest.testmod()
         if fail:
             return 1
         unittest.main()
 
-    if len(configs) == 0 and not any((options.force_sync, options.build_db,
-                                      options.imply, options.find)):
+    if not any((len(configs), args.force_sync, args.build_db, args.imply,
+                args.find)):
         parser.print_usage()
         sys.exit(1)
 
@@ -1715,13 +1719,13 @@ def main():
 
     check_top_directory()
 
-    if options.imply:
+    if args.imply:
         imply_flags = 0
-        if options.imply_flags == 'all':
+        if args.imply_flags == 'all':
             imply_flags = -1
 
-        elif options.imply_flags:
-            for flag in options.imply_flags.split(','):
+        elif args.imply_flags:
+            for flag in args.imply_flags.split(','):
                 bad = flag not in IMPLY_FLAGS
                 if bad:
                     print("Invalid flag '%s'" % flag)
@@ -1733,11 +1737,10 @@ def main():
                     sys.exit(1)
                 imply_flags |= IMPLY_FLAGS[flag][0]
 
-        do_imply_config(configs, options.add_imply, imply_flags,
-                        options.skip_added)
+        do_imply_config(configs, args.add_imply, imply_flags, args.skip_added)
         return
 
-    if options.find:
+    if args.find:
         do_find_config(configs)
         return
 
@@ -1747,22 +1750,22 @@ def main():
     t.setDaemon(True)
     t.start()
 
-    if not options.cleanup_headers_only:
+    if not args.cleanup_headers_only:
         check_clean_directory()
         bsettings.Setup('')
         toolchains = toolchain.Toolchains()
         toolchains.GetSettings()
         toolchains.Scan(verbose=False)
-        move_config(toolchains, configs, options, db_queue)
+        move_config(toolchains, configs, args, db_queue)
         db_queue.join()
 
     if configs:
-        cleanup_headers(configs, options)
-        cleanup_extra_options(configs, options)
-        cleanup_whitelist(configs, options)
-        cleanup_readme(configs, options)
+        cleanup_headers(configs, args)
+        cleanup_extra_options(configs, args)
+        cleanup_whitelist(configs, args)
+        cleanup_readme(configs, args)
 
-    if options.commit:
+    if args.commit:
         subprocess.call(['git', 'add', '-u'])
         if configs:
             msg = 'Convert %s %sto Kconfig' % (configs[0],
@@ -1774,7 +1777,7 @@ def main():
             msg += '\n\nRsync all defconfig files using moveconfig.py'
         subprocess.call(['git', 'commit', '-s', '-m', msg])
 
-    if options.build_db:
+    if args.build_db:
         with open(CONFIG_DATABASE, 'w') as fd:
             for defconfig, configs in config_db.items():
                 fd.write('%s\n' % defconfig)
-- 
2.34.1.173.g76aa8bc2d0-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/6] moveconfig: Drop check for old Python
  2021-12-18 21:54 [PATCH 0/6] moveconfig: Improve the pylist score a little Simon Glass
  2021-12-18 21:54 ` [PATCH 1/6] moveconfig: Use single quotes Simon Glass
  2021-12-18 21:54 ` [PATCH 2/6] moveconfig: Convert to ArgumentParser Simon Glass
@ 2021-12-18 21:54 ` Simon Glass
  2021-12-18 22:27   ` Heinrich Schuchardt
  2022-01-25 15:58   ` Tom Rini
  2021-12-18 21:54 ` [PATCH 4/6] moveconfig: Use a function to write files Simon Glass
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Simon Glass @ 2021-12-18 21:54 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Masahiro Yamada, Simon Glass, Trevor Woerner

Python 2 is not supported anymore and Python 3 has had subprocess.DEVNULL
since version 3.3 which was released in 2012. Drop the unnecessary check.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/moveconfig.py | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index 521297f7d58..0b33f3190e3 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -91,14 +91,6 @@ SIZES = {
 }
 
 ### helper functions ###
-def get_devnull():
-    """Get the file object of '/dev/null' device."""
-    try:
-        devnull = subprocess.DEVNULL # py3k
-    except AttributeError:
-        devnull = open(os.devnull, 'wb')
-    return devnull
-
 def check_top_directory():
     """Exit if we are not at the top of source directory."""
     for f in ('README', 'Licenses'):
@@ -1083,7 +1075,7 @@ class Slots:
         """
         self.args = args
         self.slots = []
-        devnull = get_devnull()
+        devnull = subprocess.DEVNULL
         make_cmd = get_make_cmd()
         for i in range(args.jobs):
             self.slots.append(Slot(toolchains, configs, args, progress,
-- 
2.34.1.173.g76aa8bc2d0-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/6] moveconfig: Use a function to write files
  2021-12-18 21:54 [PATCH 0/6] moveconfig: Improve the pylist score a little Simon Glass
                   ` (2 preceding siblings ...)
  2021-12-18 21:54 ` [PATCH 3/6] moveconfig: Drop check for old Python Simon Glass
@ 2021-12-18 21:54 ` Simon Glass
  2022-01-25 15:58   ` Tom Rini
  2021-12-18 21:54 ` [PATCH 5/6] moveconfig: Use a function to read files Simon Glass
  2021-12-18 21:54 ` [PATCH 6/6] moveconfig: Fix some pylint errors Simon Glass
  5 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-12-18 21:54 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Masahiro Yamada, Simon Glass, Trevor Woerner

At present there is quite a bit of ad-hoc code writing to files. The
treatment of newlines is different in some of them. Put it in a function
and set the unicode encoding correctly.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/moveconfig.py | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index 0b33f3190e3..4932bd9b86f 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -273,6 +273,21 @@ def confirm(args, prompt):
 
     return True
 
+def write_file(fname, data):
+    """Write data to a file
+
+    Args:
+        fname (str): Filename to write to
+        data (list of str): Lines to write (with or without trailing newline);
+            or str to write
+    """
+    with open(fname, 'w', encoding='utf-8') as out:
+        if isinstance(data, list):
+            for line in data:
+                print(line.rstrip('\n'), file=out)
+        else:
+            out.write(data)
+
 def cleanup_empty_blocks(header_path, args):
     """Clean up empty conditional blocks
 
@@ -296,8 +311,7 @@ def cleanup_empty_blocks(header_path, args):
     if args.dry_run:
         return
 
-    with open(header_path, 'w') as f:
-        f.write(new_data)
+    write_file(header_path, new_data)
 
 def cleanup_one_header(header_path, patterns, args):
     """Clean regex-matched lines away from a file.
@@ -359,9 +373,7 @@ def cleanup_one_header(header_path, patterns, args):
     if args.dry_run:
         return
 
-    with open(header_path, 'w') as f:
-        for line in tolines:
-            f.write(line)
+    write_file(header_path, tolines)
 
 def cleanup_headers(configs, args):
     """Delete config defines from board headers.
@@ -437,9 +449,7 @@ def cleanup_one_extra_option(defconfig_path, configs, args):
     if args.dry_run:
         return
 
-    with open(defconfig_path, 'w') as f:
-        for line in tolines:
-            f.write(line)
+    write_file(defconfig_path, tolines)
 
 def cleanup_extra_options(configs, args):
     """Delete config defines in CONFIG_SYS_EXTRA_OPTIONS in defconfig files.
@@ -474,8 +484,7 @@ def cleanup_whitelist(configs, args):
 
     lines = [x for x in lines if x.strip() not in configs]
 
-    with open(os.path.join('scripts', 'config_whitelist.txt'), 'w') as f:
-        f.write(''.join(lines))
+    write_file(os.path.join('scripts', 'config_whitelist.txt'), lines)
 
 def find_matching(patterns, line):
     for pat in patterns:
@@ -514,8 +523,7 @@ def cleanup_readme(configs, args):
         if not found:
             newlines.append(line)
 
-    with open('README', 'w') as f:
-        f.write(''.join(newlines))
+    write_file('README', newlines)
 
 def try_expand(line):
     """If value looks like an expression, try expanding it
@@ -1135,8 +1143,7 @@ class Slots:
             print(color_text(self.args.color, COLOR_LIGHT_RED,
                                             msg), file=sys.stderr)
 
-            with open(output_file, 'w') as f:
-                f.write(boards)
+            write_file(output_file, boards)
 
     def show_suspicious_boards(self):
         """Display all boards (defconfigs) with possible misconversion."""
@@ -1155,8 +1162,7 @@ class Slots:
             print(color_text(self.args.color, COLOR_YELLOW,
                                             msg), file=sys.stderr)
 
-            with open(output_file, 'w') as f:
-                f.write(boards)
+            write_file(output_file, boards)
 
 class ReferenceSource:
 
@@ -1315,8 +1321,7 @@ def add_imply_rule(config, fname, linenum):
     for offset, line in enumerate(data[linenum:]):
         if line.strip().startswith('help') or not line:
             data.insert(linenum + offset, '\timply %s' % config)
-            with open(fname, 'w') as fd:
-                fd.write('\n'.join(data) + '\n')
+            write_file(fname, data)
             return 'added%s' % file_line
 
     return 'could not insert%s'
-- 
2.34.1.173.g76aa8bc2d0-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/6] moveconfig: Use a function to read files
  2021-12-18 21:54 [PATCH 0/6] moveconfig: Improve the pylist score a little Simon Glass
                   ` (3 preceding siblings ...)
  2021-12-18 21:54 ` [PATCH 4/6] moveconfig: Use a function to write files Simon Glass
@ 2021-12-18 21:54 ` Simon Glass
  2022-01-25 15:58   ` Tom Rini
  2021-12-18 21:54 ` [PATCH 6/6] moveconfig: Fix some pylint errors Simon Glass
  5 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-12-18 21:54 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Masahiro Yamada, Simon Glass, Trevor Woerner

At present there is quite a bit of ad-hoc code reading from files. The
most common case is to read the file as lines. Put it in a function and
set the unicode encoding correctly.

Avoid writing back to a file when there are obviously no changes as this
speeds things up slightly.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/moveconfig.py | 113 ++++++++++++++++++++++++--------------------
 1 file changed, 63 insertions(+), 50 deletions(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index 4932bd9b86f..c41aa4ee283 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -288,6 +288,34 @@ def write_file(fname, data):
         else:
             out.write(data)
 
+def read_file(fname, as_lines=True, skip_unicode=False):
+    """Read a file and return the contents
+
+    Args:
+        fname (str): Filename to read from
+        as_lines: Return file contents as a list of lines
+        skip_unicode (bool): True to report unicode errors and continue
+
+    Returns:
+        iter of str: List of ;ines from the file with newline removed; str if
+            as_lines is False with newlines intact; or None if a unicode error
+            occurred
+
+    Raises:
+        UnicodeDecodeError: Unicode error occurred when reading
+    """
+    with open(fname, encoding='utf-8') as inf:
+        try:
+            if as_lines:
+                return [line.rstrip('\n') for line in inf.readlines()]
+            else:
+                return inf.read()
+        except UnicodeDecodeError as e:
+            if not skip_unicode:
+                raises
+            print("Failed on file %s': %s" % (fname, e))
+            return None
+
 def cleanup_empty_blocks(header_path, args):
     """Clean up empty conditional blocks
 
@@ -296,12 +324,9 @@ def cleanup_empty_blocks(header_path, args):
       args: program arguments
     """
     pattern = re.compile(r'^\s*#\s*if.*$\n^\s*#\s*endif.*$\n*', flags=re.M)
-    with open(header_path) as f:
-        try:
-            data = f.read()
-        except UnicodeDecodeError as e:
-            print("Failed on file %s': %s" % (header_path, e))
-            return
+    data = read_file(header_path, as_lines=False, skip_unicode=True)
+    if data is None:
+        return
 
     new_data = pattern.sub('\n', data)
 
@@ -311,7 +336,8 @@ def cleanup_empty_blocks(header_path, args):
     if args.dry_run:
         return
 
-    write_file(header_path, new_data)
+    if new_data != data:
+        write_file(header_path, new_data)
 
 def cleanup_one_header(header_path, patterns, args):
     """Clean regex-matched lines away from a file.
@@ -322,12 +348,9 @@ def cleanup_one_header(header_path, patterns, args):
                 patterns are deleted.
       args: program arguments
     """
-    with open(header_path) as f:
-        try:
-            lines = f.readlines()
-        except UnicodeDecodeError as e:
-            print("Failed on file %s': %s" % (header_path, e))
-            return
+    lines = read_file(header_path, skip_unicode=True)
+    if lines is None:
+        return
 
     matched = []
     for i, line in enumerate(lines):
@@ -416,8 +439,7 @@ def cleanup_one_extra_option(defconfig_path, configs, args):
     start = 'CONFIG_SYS_EXTRA_OPTIONS="'
     end = '"\n'
 
-    with open(defconfig_path) as f:
-        lines = f.readlines()
+    lines = read_file(defconfig_path)
 
     for i, line in enumerate(lines):
         if line.startswith(start) and line.endswith(end):
@@ -479,8 +501,7 @@ def cleanup_whitelist(configs, args):
     if not confirm(args, 'Clean up whitelist entries?'):
         return
 
-    with open(os.path.join('scripts', 'config_whitelist.txt')) as f:
-        lines = f.readlines()
+    lines = read_file(os.path.join('scripts', 'config_whitelist.txt'))
 
     lines = [x for x in lines if x.strip() not in configs]
 
@@ -506,8 +527,7 @@ def cleanup_readme(configs, args):
     for config in configs:
         patterns.append(re.compile(r'^\s+%s' % config))
 
-    with open('README') as f:
-        lines = f.readlines()
+    lines = read_file('README')
 
     found = False
     newlines = []
@@ -615,7 +635,7 @@ class KconfigParser:
         """
         arch = ''
         cpu = ''
-        for line in open(self.dotconfig):
+        for line in read_file(self.dotconfig):
             m = self.re_arch.match(line)
             if m:
                 arch = m.group(1)
@@ -717,11 +737,9 @@ class KconfigParser:
         else:
             autoconf_path = self.autoconf
 
-        with open(self.dotconfig) as f:
-            dotconfig_lines = f.readlines()
+        dotconfig_lines = read_file(self.dotconfig)
 
-        with open(autoconf_path) as f:
-            autoconf_lines = f.readlines()
+        autoconf_lines = read_file(autoconf_path)
 
         for config in self.configs:
             result = self.parse_one_config(config, dotconfig_lines,
@@ -775,8 +793,7 @@ class KconfigParser:
 
         log = ''
 
-        with open(self.defconfig) as f:
-            defconfig_lines = f.readlines()
+        defconfig_lines = read_file(self.defconfig)
 
         for (action, value) in self.results:
             if action != ACTION_MOVE:
@@ -978,11 +995,10 @@ class Slot:
     def do_build_db(self):
         """Add the board to the database"""
         configs = {}
-        with open(os.path.join(self.build_dir, AUTO_CONF_PATH)) as fd:
-            for line in fd.readlines():
-                if line.startswith('CONFIG'):
-                    config, value = line.split('=', 1)
-                    configs[config] = value.rstrip()
+        for line in read_file(os.path.join(self.build_dir, AUTO_CONF_PATH)):
+            if line.startswith('CONFIG'):
+                config, value = line.split('=', 1)
+                configs[config] = value.rstrip()
         self.db_queue.put([self.defconfig, configs])
         self.finish(True)
 
@@ -1297,8 +1313,7 @@ def check_imply_rule(kconf, config, imply_config):
     if cwd and fname.startswith(cwd):
         fname = fname[len(cwd) + 1:]
     file_line = ' at %s:%d' % (fname, linenum)
-    with open(fname) as fd:
-        data = fd.read().splitlines()
+    data = read_file(fname)
     if data[linenum - 1] != 'config %s' % imply_config:
         return None, 0, 'bad sym format %s%s' % (data[linenum], file_line)
     return fname, linenum, 'adding%s' % file_line
@@ -1315,7 +1330,7 @@ def add_imply_rule(config, fname, linenum):
         Message indicating the result
     """
     file_line = ' at %s:%d' % (fname, linenum)
-    data = open(fname).read().splitlines()
+    data = read_file(fname)
     linenum -= 1
 
     for offset, line in enumerate(data[linenum:]):
@@ -1368,20 +1383,19 @@ def read_database():
     all_defconfigs = set()
 
     defconfig_db = collections.defaultdict(set)
-    with open(CONFIG_DATABASE) as fd:
-        for line in fd.readlines():
-            line = line.rstrip()
-            if not line:  # Separator between defconfigs
-                config_db[defconfig] = configs
-                all_defconfigs.add(defconfig)
-                configs = {}
-            elif line[0] == ' ':  # CONFIG line
-                config, value = line.strip().split('=', 1)
-                configs[config] = value
-                defconfig_db[config].add(defconfig)
-                all_configs.add(config)
-            else:  # New defconfig
-                defconfig = line
+    for line in read_file(CONFIG_DATABASE):
+        line = line.rstrip()
+        if not line:  # Separator between defconfigs
+            config_db[defconfig] = configs
+            all_defconfigs.add(defconfig)
+            configs = {}
+        elif line[0] == ' ':  # CONFIG line
+            config, value = line.strip().split('=', 1)
+            configs[config] = value
+            defconfig_db[config].add(defconfig)
+            all_configs.add(config)
+        else:  # New defconfig
+            defconfig = line
 
     return all_configs, all_defconfigs, config_db, defconfig_db
 
@@ -1578,8 +1592,7 @@ def do_find_config(config_list):
     all_configs, all_defconfigs, config_db, defconfig_db = read_database()
 
     # Get the whitelist
-    with open('scripts/config_whitelist.txt') as inf:
-        adhoc_configs = set(inf.read().splitlines())
+    adhoc_configs = set(read_file('scripts/config_whitelist.txt'))
 
     # Start with all defconfigs
     out = all_defconfigs
-- 
2.34.1.173.g76aa8bc2d0-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/6] moveconfig: Fix some pylint errors
  2021-12-18 21:54 [PATCH 0/6] moveconfig: Improve the pylist score a little Simon Glass
                   ` (4 preceding siblings ...)
  2021-12-18 21:54 ` [PATCH 5/6] moveconfig: Use a function to read files Simon Glass
@ 2021-12-18 21:54 ` Simon Glass
  2022-01-25 15:58   ` Tom Rini
  5 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-12-18 21:54 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Masahiro Yamada, Simon Glass, Trevor Woerner

There are over 200 errors in this file. Fix some of them, starting at the
beginning of the file. Future work can continue this effort.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/moveconfig.py | 206 +++++++++++++++++++++++---------------------
 1 file changed, 110 insertions(+), 96 deletions(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index c41aa4ee283..35fe6710d70 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -13,6 +13,7 @@ See doc/develop/moveconfig.rst for documentation.
 from argparse import ArgumentParser
 import asteval
 import collections
+from contextlib import ExitStack
 import copy
 import difflib
 import doctest
@@ -93,14 +94,14 @@ SIZES = {
 ### helper functions ###
 def check_top_directory():
     """Exit if we are not at the top of source directory."""
-    for f in ('README', 'Licenses'):
-        if not os.path.exists(f):
+    for fname in 'README', 'Licenses':
+        if not os.path.exists(fname):
             sys.exit('Please run at the top of source directory.')
 
 def check_clean_directory():
     """Exit if the source tree is not clean."""
-    for f in ('.config', 'include/config'):
-        if os.path.exists(f):
+    for fname in '.config', 'include/config':
+        if os.path.exists(fname):
             sys.exit("source tree is not clean, please run 'make mrproper'")
 
 def get_make_cmd():
@@ -110,22 +111,22 @@ def get_make_cmd():
     necessarily "make". (for example, "gmake" on FreeBSD).
     Returns the most appropriate command name on your system.
     """
-    process = subprocess.Popen([SHOW_GNU_MAKE], stdout=subprocess.PIPE)
-    ret = process.communicate()
-    if process.returncode:
-        sys.exit('GNU Make not found')
+    with subprocess.Popen([SHOW_GNU_MAKE], stdout=subprocess.PIPE) as proc:
+        ret = proc.communicate()
+        if proc.returncode:
+            sys.exit('GNU Make not found')
     return ret[0].rstrip()
 
 def get_matched_defconfig(line):
     """Get the defconfig files that match a pattern
 
     Args:
-        line: Path or filename to match, e.g. 'configs/snow_defconfig' or
+        line (str): Path or filename to match, e.g. 'configs/snow_defconfig' or
             'k2*_defconfig'. If no directory is provided, 'configs/' is
             prepended
 
     Returns:
-        a list of matching defconfig files
+        list of str: a list of matching defconfig files
     """
     dirname = os.path.dirname(line)
     if dirname:
@@ -138,38 +139,43 @@ def get_matched_defconfigs(defconfigs_file):
     """Get all the defconfig files that match the patterns in a file.
 
     Args:
-        defconfigs_file: File containing a list of defconfigs to process, or
-            '-' to read the list from stdin
+        defconfigs_file (str): File containing a list of defconfigs to process,
+            or '-' to read the list from stdin
 
     Returns:
-        A list of paths to defconfig files, with no duplicates
+        list of str: A list of paths to defconfig files, with no duplicates
     """
     defconfigs = []
-    if defconfigs_file == '-':
-        fd = sys.stdin
-        defconfigs_file = 'stdin'
-    else:
-        fd = open(defconfigs_file)
-    for i, line in enumerate(fd):
-        line = line.strip()
-        if not line:
-            continue # skip blank lines silently
-        if ' ' in line:
-            line = line.split(' ')[0]  # handle 'git log' input
-        matched = get_matched_defconfig(line)
-        if not matched:
-            print("warning: %s:%d: no defconfig matched '%s'" % \
-                                                 (defconfigs_file, i + 1, line), file=sys.stderr)
-
-        defconfigs += matched
+    with ExitStack() as stack:
+        if defconfigs_file == '-':
+            inf = sys.stdin
+            defconfigs_file = 'stdin'
+        else:
+            inf = stack.enter_context(open(defconfigs_file, encoding='utf-8'))
+        for i, line in enumerate(inf):
+            line = line.strip()
+            if not line:
+                continue # skip blank lines silently
+            if ' ' in line:
+                line = line.split(' ')[0]  # handle 'git log' input
+            matched = get_matched_defconfig(line)
+            if not matched:
+                print(f"warning: {defconfigs_file}:{i + 1}: no defconfig matched '{line}'",
+                      file=sys.stderr)
+
+            defconfigs += matched
 
     # use set() to drop multiple matching
-    return [ defconfig[len('configs') + 1:]  for defconfig in set(defconfigs) ]
+    return [defconfig[len('configs') + 1:]  for defconfig in set(defconfigs)]
 
 def get_all_defconfigs():
-    """Get all the defconfig files under the configs/ directory."""
+    """Get all the defconfig files under the configs/ directory.
+
+    Returns:
+        list of str: List of paths to defconfig files
+    """
     defconfigs = []
-    for (dirpath, dirnames, filenames) in os.walk('configs'):
+    for (dirpath, _, filenames) in os.walk('configs'):
         dirpath = dirpath[len('configs') + 1:]
         for filename in fnmatch.filter(filenames, '*_defconfig'):
             defconfigs.append(os.path.join(dirpath, filename))
@@ -183,20 +189,18 @@ def color_text(color_enabled, color, string):
         # Otherwise, additional whitespace or line-feed might be printed.
         return '\n'.join([ '\033[' + color + 'm' + s + '\033[0m' if s else ''
                            for s in string.split('\n') ])
-    else:
-        return string
+    return string
 
-def show_diff(a, b, file_path, color_enabled):
+def show_diff(alines, blines, file_path, color_enabled):
     """Show unidified diff.
 
-    Arguments:
-      a: A list of lines (before)
-      b: A list of lines (after)
-      file_path: Path to the file
-      color_enabled: Display the diff in color
+    Args:
+       alines (list of str): A list of lines (before)
+       blines (list of str): A list of lines (after)
+       file_path (str): Path to the file
+       color_enabled (bool): Display the diff in color
     """
-
-    diff = difflib.unified_diff(a, b,
+    diff = difflib.unified_diff(alines, blines,
                                 fromfile=os.path.join('a', file_path),
                                 tofile=os.path.join('b', file_path))
 
@@ -208,21 +212,23 @@ def show_diff(a, b, file_path, color_enabled):
         else:
             print(line, end=' ')
 
-def extend_matched_lines(lines, matched, pre_patterns, post_patterns, extend_pre,
-                         extend_post):
+def extend_matched_lines(lines, matched, pre_patterns, post_patterns,
+                         extend_pre, extend_post):
     """Extend matched lines if desired patterns are found before/after already
     matched lines.
 
-    Arguments:
-      lines: A list of lines handled.
-      matched: A list of line numbers that have been already matched.
-               (will be updated by this function)
-      pre_patterns: A list of regular expression that should be matched as
-                    preamble.
-      post_patterns: A list of regular expression that should be matched as
-                     postamble.
-      extend_pre: Add the line number of matched preamble to the matched list.
-      extend_post: Add the line number of matched postamble to the matched list.
+    Args:
+      lines (list of str): list of lines handled.
+      matched (list of int): list of line numbers that have been already
+          matched (will be updated by this function)
+      pre_patterns (list of re.Pattern): list of regular expression that should
+          be matched as preamble
+      post_patterns (list of re.Pattern): list of regular expression that should
+          be matched as postamble
+      extend_pre (bool): Add the line number of matched preamble to the matched
+          list
+      extend_post (bool): Add the line number of matched postamble to the
+          matched list
     """
     extended_matched = []
 
@@ -237,15 +243,15 @@ def extend_matched_lines(lines, matched, pre_patterns, post_patterns, extend_pre
         if j >= len(lines):
             break
 
-        for p in pre_patterns:
-            if p.search(lines[i - 1]):
+        for pat in pre_patterns:
+            if pat.search(lines[i - 1]):
                 break
         else:
             # not matched
             continue
 
-        for p in post_patterns:
-            if p.search(lines[j]):
+        for pat in post_patterns:
+            if pat.search(lines[j]):
                 break
         else:
             # not matched
@@ -260,12 +266,20 @@ def extend_matched_lines(lines, matched, pre_patterns, post_patterns, extend_pre
     matched.sort()
 
 def confirm(args, prompt):
+    """Ask the user to confirm something
+
+    Args:
+        args (Namespace ): program arguments
+
+    Returns:
+        bool: True to confirm, False to cancel/stop
+    """
     if not args.yes:
         while True:
-            choice = input('{} [y/n]: '.format(prompt))
+            choice = input(f'{prompt} [y/n]: ')
             choice = choice.lower()
             print(choice)
-            if choice == 'y' or choice == 'n':
+            if choice in ('y', 'n'):
                 break
 
         if choice == 'n':
@@ -319,9 +333,9 @@ def read_file(fname, as_lines=True, skip_unicode=False):
 def cleanup_empty_blocks(header_path, args):
     """Clean up empty conditional blocks
 
-    Arguments:
-      header_path: path to the cleaned file.
-      args: program arguments
+    Args:
+      header_path (str): path to the cleaned file.
+      args (Namespace): program arguments
     """
     pattern = re.compile(r'^\s*#\s*if.*$\n^\s*#\s*endif.*$\n*', flags=re.M)
     data = read_file(header_path, as_lines=False, skip_unicode=True)
@@ -342,11 +356,11 @@ def cleanup_empty_blocks(header_path, args):
 def cleanup_one_header(header_path, patterns, args):
     """Clean regex-matched lines away from a file.
 
-    Arguments:
+    Args:
       header_path: path to the cleaned file.
       patterns: list of regex patterns.  Any lines matching to these
                 patterns are deleted.
-      args: program arguments
+      args (Namespace): program arguments
     """
     lines = read_file(header_path, skip_unicode=True)
     if lines is None:
@@ -401,9 +415,9 @@ def cleanup_one_header(header_path, patterns, args):
 def cleanup_headers(configs, args):
     """Delete config defines from board headers.
 
-    Arguments:
+    Args:
       configs: A list of CONFIGs to remove.
-      args: program arguments
+      args (Namespace): program arguments
     """
     if not confirm(args, 'Clean up headers?'):
         return
@@ -430,10 +444,10 @@ def cleanup_headers(configs, args):
 def cleanup_one_extra_option(defconfig_path, configs, args):
     """Delete config defines in CONFIG_SYS_EXTRA_OPTIONS in one defconfig file.
 
-    Arguments:
+    Args:
       defconfig_path: path to the cleaned defconfig file.
       configs: A list of CONFIGs to remove.
-      args: program arguments
+      args (Namespace): program arguments
     """
 
     start = 'CONFIG_SYS_EXTRA_OPTIONS="'
@@ -476,9 +490,9 @@ def cleanup_one_extra_option(defconfig_path, configs, args):
 def cleanup_extra_options(configs, args):
     """Delete config defines in CONFIG_SYS_EXTRA_OPTIONS in defconfig files.
 
-    Arguments:
+    Args:
       configs: A list of CONFIGs to remove.
-      args: program arguments
+      args (Namespace): program arguments
     """
     if not confirm(args, 'Clean up CONFIG_SYS_EXTRA_OPTIONS?'):
         return
@@ -494,9 +508,9 @@ def cleanup_extra_options(configs, args):
 def cleanup_whitelist(configs, args):
     """Delete config whitelist entries
 
-    Arguments:
+    Args:
       configs: A list of CONFIGs to remove.
-      args: program arguments
+      args (Namespace): program arguments
     """
     if not confirm(args, 'Clean up whitelist entries?'):
         return
@@ -516,9 +530,9 @@ def find_matching(patterns, line):
 def cleanup_readme(configs, args):
     """Delete config description in README
 
-    Arguments:
+    Args:
       configs: A list of CONFIGs to remove.
-      args: program arguments
+      args (Namespace): program arguments
     """
     if not confirm(args, 'Clean up README?'):
         return
@@ -574,7 +588,7 @@ class Progress:
     def __init__(self, total):
         """Create a new progress indicator.
 
-        Arguments:
+        Args:
           total: A number of defconfig files to process.
         """
         self.current = 0
@@ -613,9 +627,9 @@ class KconfigParser:
     def __init__(self, configs, args, build_dir):
         """Create a new parser.
 
-        Arguments:
+        Args:
           configs: A list of CONFIGs to move.
-          args: program arguments
+          args (Namespace): program arguments
           build_dir: Build directory.
         """
         self.configs = configs
@@ -660,7 +674,7 @@ class KconfigParser:
         defconfig, .config, and include/autoconf.mk in order to decide
         which action should be taken for this defconfig.
 
-        Arguments:
+        Args:
           config: CONFIG name to parse.
           dotconfig_lines: lines from the .config file.
           autoconf_lines: lines from the include/autoconf.mk file.
@@ -710,7 +724,7 @@ class KconfigParser:
         searching the target options.
         Move the config option(s) to the .config as needed.
 
-        Arguments:
+        Args:
           defconfig: defconfig name.
 
         Returns:
@@ -771,10 +785,10 @@ class KconfigParser:
 
             log += color_text(self.args.color, log_color, actlog) + '\n'
 
-        with open(self.dotconfig, 'a') as f:
+        with open(self.dotconfig, 'a', encoding='utf-8') as out:
             for (action, value) in results:
                 if action == ACTION_MOVE:
-                    f.write(value + '\n')
+                    out.write(value + '\n')
                     updated = True
 
         self.results = results
@@ -846,7 +860,7 @@ class Slot:
 		 make_cmd, reference_src_dir, db_queue):
         """Create a new process slot.
 
-        Arguments:
+        Args:
           toolchains: Toolchains object containing toolchains.
           configs: A list of CONFIGs to move.
           args: Program arguments
@@ -892,7 +906,7 @@ class Slot:
         given defconfig and add it to the slot.  Just returns False if
         the slot is occupied (i.e. the current subprocess is still running).
 
-        Arguments:
+        Args:
           defconfig: defconfig name.
 
         Returns:
@@ -1047,7 +1061,7 @@ class Slot:
     def finish(self, success):
         """Display log along with progress and go to the idle state.
 
-        Arguments:
+        Args:
           success: Should be True when the defconfig was processed
                    successfully, or False when it fails.
         """
@@ -1088,7 +1102,7 @@ class Slots:
 		 reference_src_dir, db_queue):
         """Create a new slots controller.
 
-        Arguments:
+        Args:
           toolchains: Toolchains object containing toolchains.
           configs: A list of CONFIGs to move.
           args: Program arguments
@@ -1109,7 +1123,7 @@ class Slots:
     def add(self, defconfig):
         """Add a new subprocess if a vacant slot is found.
 
-        Arguments:
+        Args:
           defconfig: defconfig name to be put into.
 
         Returns:
@@ -1187,7 +1201,7 @@ class ReferenceSource:
     def __init__(self, commit):
         """Create a reference source directory based on a specified commit.
 
-        Arguments:
+        Args:
           commit: commit to git-clone
         """
         self.src_dir = tempfile.mkdtemp()
@@ -1217,7 +1231,7 @@ class ReferenceSource:
 def move_config(toolchains, configs, args, db_queue):
     """Move config options to defconfig files.
 
-    Arguments:
+    Args:
       configs: A list of CONFIGs to move.
       args: Program arguments
     """
@@ -1245,7 +1259,7 @@ def move_config(toolchains, configs, args, db_queue):
 
     progress = Progress(len(defconfigs))
     slots = Slots(toolchains, configs, args, progress, reference_src_dir,
-		  db_queue)
+                  db_queue)
 
     # Main loop to process defconfig files:
     #  Add a new subprocess into a vacant slot.
@@ -1351,7 +1365,7 @@ IMPLY_FLAGS = {
     'non-arch-board': [
         IMPLY_NON_ARCH_BOARD,
         'Allow Kconfig options outside arch/ and /board/ to imply'],
-};
+}
 
 
 def read_database():
@@ -1470,10 +1484,10 @@ def do_imply_config(config_list, add_imply, imply_flags, skip_added,
         for imply_config in rest_configs:
             if 'ERRATUM' in imply_config:
                 continue
-            if not (imply_flags & IMPLY_CMD):
+            if not imply_flags & IMPLY_CMD:
                 if 'CONFIG_CMD' in imply_config:
                     continue
-            if not (imply_flags & IMPLY_TARGET):
+            if not imply_flags & IMPLY_TARGET:
                 if 'CONFIG_TARGET' in imply_config:
                     continue
 
@@ -1556,7 +1570,7 @@ def do_imply_config(config_list, add_imply, imply_flags, skip_added,
                     in_arch_board = not sym or (fname.startswith('arch') or
                                                 fname.startswith('board'))
                     if (not in_arch_board and
-                        not (imply_flags & IMPLY_NON_ARCH_BOARD)):
+                        not imply_flags & IMPLY_NON_ARCH_BOARD):
                         continue
 
                     if add_imply and (add_imply == 'all' or
@@ -1788,7 +1802,7 @@ doc/develop/moveconfig.rst for documentation.'''
         subprocess.call(['git', 'commit', '-s', '-m', msg])
 
     if args.build_db:
-        with open(CONFIG_DATABASE, 'w') as fd:
+        with open(CONFIG_DATABASE, 'w', encoding='utf-8') as fd:
             for defconfig, configs in config_db.items():
                 fd.write('%s\n' % defconfig)
                 for config in sorted(configs.keys()):
-- 
2.34.1.173.g76aa8bc2d0-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/6] moveconfig: Use single quotes
  2021-12-18 21:54 ` [PATCH 1/6] moveconfig: Use single quotes Simon Glass
@ 2021-12-18 22:16   ` Heinrich Schuchardt
  2021-12-28  8:33     ` Simon Glass
  2022-01-25 15:57   ` Tom Rini
  1 sibling, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2021-12-18 22:16 UTC (permalink / raw)
  To: Simon Glass; +Cc: Masahiro Yamada, Trevor Woerner, U-Boot Mailing List

On 12/18/21 22:54, Simon Glass wrote:
> Quite a few places use double quotes. Fix this to be consistent with
> other Python code in U-Boot.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   tools/moveconfig.py | 72 ++++++++++++++++++++++-----------------------
>   1 file changed, 36 insertions(+), 36 deletions(-)
>
> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
> index a86c07caa6e..ab2d4905db5 100755
> --- a/tools/moveconfig.py
> +++ b/tools/moveconfig.py
> @@ -71,23 +71,23 @@ CONFIG_DATABASE = 'moveconfig.db'
>   CONFIG_LEN = len('CONFIG_')
>
>   SIZES = {
> -    "SZ_1":    0x00000001, "SZ_2":    0x00000002,
> -    "SZ_4":    0x00000004, "SZ_8":    0x00000008,
> -    "SZ_16":   0x00000010, "SZ_32":   0x00000020,
> -    "SZ_64":   0x00000040, "SZ_128":  0x00000080,
> -    "SZ_256":  0x00000100, "SZ_512":  0x00000200,
> -    "SZ_1K":   0x00000400, "SZ_2K":   0x00000800,
> -    "SZ_4K":   0x00001000, "SZ_8K":   0x00002000,
> -    "SZ_16K":  0x00004000, "SZ_32K":  0x00008000,
> -    "SZ_64K":  0x00010000, "SZ_128K": 0x00020000,
> -    "SZ_256K": 0x00040000, "SZ_512K": 0x00080000,
> -    "SZ_1M":   0x00100000, "SZ_2M":   0x00200000,
> -    "SZ_4M":   0x00400000, "SZ_8M":   0x00800000,
> -    "SZ_16M":  0x01000000, "SZ_32M":  0x02000000,
> -    "SZ_64M":  0x04000000, "SZ_128M": 0x08000000,
> -    "SZ_256M": 0x10000000, "SZ_512M": 0x20000000,
> -    "SZ_1G":   0x40000000, "SZ_2G":   0x80000000,
> -    "SZ_4G":  0x100000000
> +    'SZ_1':    0x00000001, 'SZ_2':    0x00000002,
> +    'SZ_4':    0x00000004, 'SZ_8':    0x00000008,
> +    'SZ_16':   0x00000010, 'SZ_32':   0x00000020,
> +    'SZ_64':   0x00000040, 'SZ_128':  0x00000080,
> +    'SZ_256':  0x00000100, 'SZ_512':  0x00000200,
> +    'SZ_1K':   0x00000400, 'SZ_2K':   0x00000800,
> +    'SZ_4K':   0x00001000, 'SZ_8K':   0x00002000,
> +    'SZ_16K':  0x00004000, 'SZ_32K':  0x00008000,
> +    'SZ_64K':  0x00010000, 'SZ_128K': 0x00020000,
> +    'SZ_256K': 0x00040000, 'SZ_512K': 0x00080000,
> +    'SZ_1M':   0x00100000, 'SZ_2M':   0x00200000,
> +    'SZ_4M':   0x00400000, 'SZ_8M':   0x00800000,
> +    'SZ_16M':  0x01000000, 'SZ_32M':  0x02000000,
> +    'SZ_64M':  0x04000000, 'SZ_128M': 0x08000000,
> +    'SZ_256M': 0x10000000, 'SZ_512M': 0x20000000,
> +    'SZ_1G':   0x40000000, 'SZ_2G':   0x80000000,
> +    'SZ_4G':  0x100000000
>   }
>
>   ### helper functions ###
> @@ -536,12 +536,12 @@ def try_expand(line):
>           aeval = asteval.Interpreter( usersyms=SIZES, minimal=True )
>           cfg, val = re.split("=", line)
>           val= val.strip('\"')
> -        if re.search("[*+-/]|<<|SZ_+|\(([^\)]+)\)", val):
> +        if re.search(r'[*+-/]|<<|SZ_+|\(([^\)]+)\)', val):

In the commit message you could have mentioned that you mark a string as
regular expression to fix a pylint warning.

>               newval = hex(aeval(val))
> -            print("\tExpanded expression %s to %s" % (val, newval))
> +            print('\tExpanded expression %s to %s' % (val, newval))
>               return cfg+'='+newval
>       except:
> -        print("\tFailed to expand expression in %s" % line)
> +        print('\tFailed to expand expression in %s' % line)
>
>       return line
>
> @@ -735,10 +735,10 @@ class KconfigParser:
>                   actlog = "Move '%s'" % value
>                   log_color = COLOR_LIGHT_GREEN
>               elif action == ACTION_NO_ENTRY:
> -                actlog = "%s is not defined in Kconfig.  Do nothing." % value
> +                actlog = '%s is not defined in Kconfig.  Do nothing.' % value
>                   log_color = COLOR_LIGHT_BLUE
>               elif action == ACTION_NO_ENTRY_WARN:
> -                actlog = "%s is not defined in Kconfig (suspicious).  Do nothing." % value
> +                actlog = '%s is not defined in Kconfig (suspicious).  Do nothing.' % value
>                   log_color = COLOR_YELLOW
>                   suspicious = True
>               elif action == ACTION_NO_CHANGE:
> @@ -746,10 +746,10 @@ class KconfigParser:
>                            % value
>                   log_color = COLOR_LIGHT_PURPLE
>               elif action == ACTION_SPL_NOT_EXIST:
> -                actlog = "SPL is not enabled for this defconfig.  Skip."
> +                actlog = 'SPL is not enabled for this defconfig.  Skip.'
>                   log_color = COLOR_PURPLE
>               else:
> -                sys.exit("Internal Error. This should not happen.")
> +                sys.exit('Internal Error. This should not happen.')
>
>               log += color_text(self.options.color, log_color, actlog) + '\n'
>
> @@ -930,7 +930,7 @@ class Slot:
>           elif self.state == STATE_SAVEDEFCONFIG:
>               self.update_defconfig()
>           else:
> -            sys.exit("Internal Error. This should not happen.")
> +            sys.exit('Internal Error. This should not happen.')
>
>           return True if self.state == STATE_IDLE else False
>
> @@ -938,7 +938,7 @@ class Slot:
>           """Handle error cases."""
>
>           self.log += color_text(self.options.color, COLOR_LIGHT_RED,
> -                               "Failed to process.\n")
> +                               'Failed to process.\n')
>           if self.options.verbose:
>               self.log += color_text(self.options.color, COLOR_LIGHT_CYAN,
>                                      self.ps.stderr.read().decode())
> @@ -999,9 +999,9 @@ class Slot:
>               return
>           if updated:
>               self.log += color_text(self.options.color, COLOR_LIGHT_GREEN,
> -                                   "Syncing by savedefconfig...\n")
> +                                   'Syncing by savedefconfig...\n')
>           else:
> -            self.log += "Syncing by savedefconfig (forced by option)...\n"
> +            self.log += 'Syncing by savedefconfig (forced by option)...\n'
>
>           cmd = list(self.make_cmd)
>           cmd.append('savedefconfig')
> @@ -1022,7 +1022,7 @@ class Slot:
>
>           if updated:
>               self.log += color_text(self.options.color, COLOR_LIGHT_BLUE,
> -                                   "defconfig was updated.\n")
> +                                   'defconfig was updated.\n')
>
>           if not self.options.dry_run and updated:
>               shutil.move(new_defconfig, orig_defconfig)
> @@ -1045,7 +1045,7 @@ class Slot:
>
>           if not success:
>               if self.options.exit_on_error:
> -                sys.exit("Exit on error.")
> +                sys.exit('Exit on error.')
>               # If --exit-on-error flag is not set, skip this board and continue.
>               # Record the failed board.
>               self.failed_boards.add(self.defconfig)
> @@ -1137,9 +1137,9 @@ class Slots:
>
>           if boards:
>               boards = '\n'.join(boards) + '\n'
> -            msg = "The following boards were not processed due to error:\n"
> +            msg = 'The following boards were not processed due to error:\n'
>               msg += boards
> -            msg += "(the list has been saved in %s)\n" % output_file
> +            msg += '(the list has been saved in %s)\n' % output_file
>               print(color_text(self.options.color, COLOR_LIGHT_RED,
>                                               msg), file=sys.stderr)
>
> @@ -1156,10 +1156,10 @@ class Slots:
>
>           if boards:
>               boards = '\n'.join(boards) + '\n'
> -            msg = "The following boards might have been converted incorrectly.\n"
> -            msg += "It is highly recommended to check them manually:\n"
> +            msg = 'The following boards might have been converted incorrectly.\n'
> +            msg += 'It is highly recommended to check them manually:\n'
>               msg += boards
> -            msg += "(the list has been saved in %s)\n" % output_file
> +            msg += '(the list has been saved in %s)\n' % output_file
>               print(color_text(self.options.color, COLOR_YELLOW,
>                                               msg), file=sys.stderr)
>
> @@ -1177,7 +1177,7 @@ class ReferenceSource:
>             commit: commit to git-clone
>           """
>           self.src_dir = tempfile.mkdtemp()
> -        print("Cloning git repo to a separate work directory...")
> +        print('Cloning git repo to a separate work directory...')
>           subprocess.check_output(['git', 'clone', os.getcwd(), '.'],
>                                   cwd=self.src_dir)
>           print("Checkout '%s' to build the original autoconf.mk." % \

After the patch a lot of string relates pylint warnings remain:

tools/moveconfig.py:167:18: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:268:0: C0116: Missing function or method docstring
(missing-function-docstring)
tools/moveconfig.py:271:27: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:294:18: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:321:18: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:384:35: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:385:35: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:486:0: C0116: Missing function or method docstring
(missing-function-docstring)
tools/moveconfig.py:504:35: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:539:18: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:542:14: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:568:14: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:650:18: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:733:25: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:736:25: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:739:25: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:743:25: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:784:34: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:963:20: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1140:19: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1160:19: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1181:14: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1212:18: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1217:10: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1293:15: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1298:16: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1301:28: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1302:24: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1303:27: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1316:16: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1322:42: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1325:19: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1421:18: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1428:14: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1510:39: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1536:22: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1621:26: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1625:30: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1658:18: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1670:25: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)
tools/moveconfig.py:1672:29: C0209: Formatting a regular string which
could be a f-string (consider-using-f-string)

This could be fixed in a separate patch.

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/6] moveconfig: Drop check for old Python
  2021-12-18 21:54 ` [PATCH 3/6] moveconfig: Drop check for old Python Simon Glass
@ 2021-12-18 22:27   ` Heinrich Schuchardt
  2022-01-25 15:58   ` Tom Rini
  1 sibling, 0 replies; 18+ messages in thread
From: Heinrich Schuchardt @ 2021-12-18 22:27 UTC (permalink / raw)
  To: Simon Glass; +Cc: Masahiro Yamada, Trevor Woerner, U-Boot Mailing List

On 12/18/21 22:54, Simon Glass wrote:
> Python 2 is not supported anymore and Python 3 has had subprocess.DEVNULL
> since version 3.3 which was released in 2012. Drop the unnecessary check.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

> ---
>
>   tools/moveconfig.py | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
> index 521297f7d58..0b33f3190e3 100755
> --- a/tools/moveconfig.py
> +++ b/tools/moveconfig.py
> @@ -91,14 +91,6 @@ SIZES = {
>   }
>
>   ### helper functions ###
> -def get_devnull():
> -    """Get the file object of '/dev/null' device."""
> -    try:
> -        devnull = subprocess.DEVNULL # py3k
> -    except AttributeError:
> -        devnull = open(os.devnull, 'wb')
> -    return devnull
> -
>   def check_top_directory():
>       """Exit if we are not at the top of source directory."""
>       for f in ('README', 'Licenses'):
> @@ -1083,7 +1075,7 @@ class Slots:
>           """
>           self.args = args
>           self.slots = []
> -        devnull = get_devnull()
> +        devnull = subprocess.DEVNULL
>           make_cmd = get_make_cmd()
>           for i in range(args.jobs):
>               self.slots.append(Slot(toolchains, configs, args, progress,


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] moveconfig: Convert to ArgumentParser
  2021-12-18 21:54 ` [PATCH 2/6] moveconfig: Convert to ArgumentParser Simon Glass
@ 2021-12-18 22:32   ` Heinrich Schuchardt
  2021-12-18 23:33     ` Simon Glass
  2022-01-25 15:57   ` Tom Rini
  1 sibling, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2021-12-18 22:32 UTC (permalink / raw)
  To: Simon Glass, U-Boot Mailing List; +Cc: Masahiro Yamada, Trevor Woerner

On 12/18/21 22:54, Simon Glass wrote:
> This is a newer library and is now preferred for Python scripts. Update
> the code to use it instead of optparse
>
> Use 'args' instead of 'options' throughout, since this is the term used
> in that module. Also it helps to avoid confusion with CONFIG options, a
> term that is used in this file.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

The patch is not applicable to origin/master:

$ git am 02.patch
Applying: moveconfig: Convert to ArgumentParser
error: patch failed: tools/moveconfig.py:1648
error: tools/moveconfig.py: patch does not apply
Patch failed at 0001 moveconfig: Convert to ArgumentParser

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] moveconfig: Convert to ArgumentParser
  2021-12-18 22:32   ` Heinrich Schuchardt
@ 2021-12-18 23:33     ` Simon Glass
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Glass @ 2021-12-18 23:33 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: U-Boot Mailing List, Masahiro Yamada, Trevor Woerner

Hi Heinrich,

On Sat, 18 Dec 2021 at 15:32, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 12/18/21 22:54, Simon Glass wrote:
> > This is a newer library and is now preferred for Python scripts. Update
> > the code to use it instead of optparse
> >
> > Use 'args' instead of 'options' throughout, since this is the term used
> > in that module. Also it helps to avoid confusion with CONFIG options, a
> > term that is used in this file.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
>
> The patch is not applicable to origin/master:
>
> $ git am 02.patch
> Applying: moveconfig: Convert to ArgumentParser
> error: patch failed: tools/moveconfig.py:1648
> error: tools/moveconfig.py: patch does not apply
> Patch failed at 0001 moveconfig: Convert to ArgumentParser
>

Sorry, this patch is on top of the previous series which is based on
-next. Please see u-boot-dm/move-working for the full tree.

Regards,
Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/6] moveconfig: Use single quotes
  2021-12-18 22:16   ` Heinrich Schuchardt
@ 2021-12-28  8:33     ` Simon Glass
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Glass @ 2021-12-28  8:33 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: Masahiro Yamada, Trevor Woerner, U-Boot Mailing List

Hi Heinrich,

On Sat, 18 Dec 2021 at 15:16, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 12/18/21 22:54, Simon Glass wrote:
> > Quite a few places use double quotes. Fix this to be consistent with
> > other Python code in U-Boot.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >   tools/moveconfig.py | 72 ++++++++++++++++++++++-----------------------
> >   1 file changed, 36 insertions(+), 36 deletions(-)
> >
> > diff --git a/tools/moveconfig.py b/tools/moveconfig.py
> > index a86c07caa6e..ab2d4905db5 100755
> > --- a/tools/moveconfig.py
> > +++ b/tools/moveconfig.py
> > @@ -71,23 +71,23 @@ CONFIG_DATABASE = 'moveconfig.db'
> >   CONFIG_LEN = len('CONFIG_')
> >
> >   SIZES = {
> > -    "SZ_1":    0x00000001, "SZ_2":    0x00000002,
> > -    "SZ_4":    0x00000004, "SZ_8":    0x00000008,
> > -    "SZ_16":   0x00000010, "SZ_32":   0x00000020,
> > -    "SZ_64":   0x00000040, "SZ_128":  0x00000080,
> > -    "SZ_256":  0x00000100, "SZ_512":  0x00000200,
> > -    "SZ_1K":   0x00000400, "SZ_2K":   0x00000800,
> > -    "SZ_4K":   0x00001000, "SZ_8K":   0x00002000,
> > -    "SZ_16K":  0x00004000, "SZ_32K":  0x00008000,
> > -    "SZ_64K":  0x00010000, "SZ_128K": 0x00020000,
> > -    "SZ_256K": 0x00040000, "SZ_512K": 0x00080000,
> > -    "SZ_1M":   0x00100000, "SZ_2M":   0x00200000,
> > -    "SZ_4M":   0x00400000, "SZ_8M":   0x00800000,
> > -    "SZ_16M":  0x01000000, "SZ_32M":  0x02000000,
> > -    "SZ_64M":  0x04000000, "SZ_128M": 0x08000000,
> > -    "SZ_256M": 0x10000000, "SZ_512M": 0x20000000,
> > -    "SZ_1G":   0x40000000, "SZ_2G":   0x80000000,
> > -    "SZ_4G":  0x100000000
> > +    'SZ_1':    0x00000001, 'SZ_2':    0x00000002,
> > +    'SZ_4':    0x00000004, 'SZ_8':    0x00000008,
> > +    'SZ_16':   0x00000010, 'SZ_32':   0x00000020,
> > +    'SZ_64':   0x00000040, 'SZ_128':  0x00000080,
> > +    'SZ_256':  0x00000100, 'SZ_512':  0x00000200,
> > +    'SZ_1K':   0x00000400, 'SZ_2K':   0x00000800,
> > +    'SZ_4K':   0x00001000, 'SZ_8K':   0x00002000,
> > +    'SZ_16K':  0x00004000, 'SZ_32K':  0x00008000,
> > +    'SZ_64K':  0x00010000, 'SZ_128K': 0x00020000,
> > +    'SZ_256K': 0x00040000, 'SZ_512K': 0x00080000,
> > +    'SZ_1M':   0x00100000, 'SZ_2M':   0x00200000,
> > +    'SZ_4M':   0x00400000, 'SZ_8M':   0x00800000,
> > +    'SZ_16M':  0x01000000, 'SZ_32M':  0x02000000,
> > +    'SZ_64M':  0x04000000, 'SZ_128M': 0x08000000,
> > +    'SZ_256M': 0x10000000, 'SZ_512M': 0x20000000,
> > +    'SZ_1G':   0x40000000, 'SZ_2G':   0x80000000,
> > +    'SZ_4G':  0x100000000
> >   }
> >
> >   ### helper functions ###
> > @@ -536,12 +536,12 @@ def try_expand(line):
> >           aeval = asteval.Interpreter( usersyms=SIZES, minimal=True )
> >           cfg, val = re.split("=", line)
> >           val= val.strip('\"')
> > -        if re.search("[*+-/]|<<|SZ_+|\(([^\)]+)\)", val):
> > +        if re.search(r'[*+-/]|<<|SZ_+|\(([^\)]+)\)', val):
>
> In the commit message you could have mentioned that you mark a string as
> regular expression to fix a pylint warning.
>
> >               newval = hex(aeval(val))
> > -            print("\tExpanded expression %s to %s" % (val, newval))
> > +            print('\tExpanded expression %s to %s' % (val, newval))
> >               return cfg+'='+newval
> >       except:
> > -        print("\tFailed to expand expression in %s" % line)
> > +        print('\tFailed to expand expression in %s' % line)
> >
> >       return line
> >
> > @@ -735,10 +735,10 @@ class KconfigParser:
> >                   actlog = "Move '%s'" % value
> >                   log_color = COLOR_LIGHT_GREEN
> >               elif action == ACTION_NO_ENTRY:
> > -                actlog = "%s is not defined in Kconfig.  Do nothing." % value
> > +                actlog = '%s is not defined in Kconfig.  Do nothing.' % value
> >                   log_color = COLOR_LIGHT_BLUE
> >               elif action == ACTION_NO_ENTRY_WARN:
> > -                actlog = "%s is not defined in Kconfig (suspicious).  Do nothing." % value
> > +                actlog = '%s is not defined in Kconfig (suspicious).  Do nothing.' % value
> >                   log_color = COLOR_YELLOW
> >                   suspicious = True
> >               elif action == ACTION_NO_CHANGE:
> > @@ -746,10 +746,10 @@ class KconfigParser:
> >                            % value
> >                   log_color = COLOR_LIGHT_PURPLE
> >               elif action == ACTION_SPL_NOT_EXIST:
> > -                actlog = "SPL is not enabled for this defconfig.  Skip."
> > +                actlog = 'SPL is not enabled for this defconfig.  Skip.'
> >                   log_color = COLOR_PURPLE
> >               else:
> > -                sys.exit("Internal Error. This should not happen.")
> > +                sys.exit('Internal Error. This should not happen.')
> >
> >               log += color_text(self.options.color, log_color, actlog) + '\n'
> >
> > @@ -930,7 +930,7 @@ class Slot:
> >           elif self.state == STATE_SAVEDEFCONFIG:
> >               self.update_defconfig()
> >           else:
> > -            sys.exit("Internal Error. This should not happen.")
> > +            sys.exit('Internal Error. This should not happen.')
> >
> >           return True if self.state == STATE_IDLE else False
> >
> > @@ -938,7 +938,7 @@ class Slot:
> >           """Handle error cases."""
> >
> >           self.log += color_text(self.options.color, COLOR_LIGHT_RED,
> > -                               "Failed to process.\n")
> > +                               'Failed to process.\n')
> >           if self.options.verbose:
> >               self.log += color_text(self.options.color, COLOR_LIGHT_CYAN,
> >                                      self.ps.stderr.read().decode())
> > @@ -999,9 +999,9 @@ class Slot:
> >               return
> >           if updated:
> >               self.log += color_text(self.options.color, COLOR_LIGHT_GREEN,
> > -                                   "Syncing by savedefconfig...\n")
> > +                                   'Syncing by savedefconfig...\n')
> >           else:
> > -            self.log += "Syncing by savedefconfig (forced by option)...\n"
> > +            self.log += 'Syncing by savedefconfig (forced by option)...\n'
> >
> >           cmd = list(self.make_cmd)
> >           cmd.append('savedefconfig')
> > @@ -1022,7 +1022,7 @@ class Slot:
> >
> >           if updated:
> >               self.log += color_text(self.options.color, COLOR_LIGHT_BLUE,
> > -                                   "defconfig was updated.\n")
> > +                                   'defconfig was updated.\n')
> >
> >           if not self.options.dry_run and updated:
> >               shutil.move(new_defconfig, orig_defconfig)
> > @@ -1045,7 +1045,7 @@ class Slot:
> >
> >           if not success:
> >               if self.options.exit_on_error:
> > -                sys.exit("Exit on error.")
> > +                sys.exit('Exit on error.')
> >               # If --exit-on-error flag is not set, skip this board and continue.
> >               # Record the failed board.
> >               self.failed_boards.add(self.defconfig)
> > @@ -1137,9 +1137,9 @@ class Slots:
> >
> >           if boards:
> >               boards = '\n'.join(boards) + '\n'
> > -            msg = "The following boards were not processed due to error:\n"
> > +            msg = 'The following boards were not processed due to error:\n'
> >               msg += boards
> > -            msg += "(the list has been saved in %s)\n" % output_file
> > +            msg += '(the list has been saved in %s)\n' % output_file
> >               print(color_text(self.options.color, COLOR_LIGHT_RED,
> >                                               msg), file=sys.stderr)
> >
> > @@ -1156,10 +1156,10 @@ class Slots:
> >
> >           if boards:
> >               boards = '\n'.join(boards) + '\n'
> > -            msg = "The following boards might have been converted incorrectly.\n"
> > -            msg += "It is highly recommended to check them manually:\n"
> > +            msg = 'The following boards might have been converted incorrectly.\n'
> > +            msg += 'It is highly recommended to check them manually:\n'
> >               msg += boards
> > -            msg += "(the list has been saved in %s)\n" % output_file
> > +            msg += '(the list has been saved in %s)\n' % output_file
> >               print(color_text(self.options.color, COLOR_YELLOW,
> >                                               msg), file=sys.stderr)
> >
> > @@ -1177,7 +1177,7 @@ class ReferenceSource:
> >             commit: commit to git-clone
> >           """
> >           self.src_dir = tempfile.mkdtemp()
> > -        print("Cloning git repo to a separate work directory...")
> > +        print('Cloning git repo to a separate work directory...')
> >           subprocess.check_output(['git', 'clone', os.getcwd(), '.'],
> >                                   cwd=self.src_dir)
> >           print("Checkout '%s' to build the original autoconf.mk." % \
>
> After the patch a lot of string relates pylint warnings remain:
>
> tools/moveconfig.py:167:18: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:268:0: C0116: Missing function or method docstring
> (missing-function-docstring)
> tools/moveconfig.py:271:27: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:294:18: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:321:18: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:384:35: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:385:35: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:486:0: C0116: Missing function or method docstring
> (missing-function-docstring)
> tools/moveconfig.py:504:35: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:539:18: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:542:14: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:568:14: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:650:18: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:733:25: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:736:25: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:739:25: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:743:25: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:784:34: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:963:20: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1140:19: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1160:19: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1181:14: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1212:18: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1217:10: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1293:15: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1298:16: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1301:28: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1302:24: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1303:27: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1316:16: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1322:42: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1325:19: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1421:18: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1428:14: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1510:39: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1536:22: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1621:26: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1625:30: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1658:18: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1670:25: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
> tools/moveconfig.py:1672:29: C0209: Formatting a regular string which
> could be a f-string (consider-using-f-string)
>
> This could be fixed in a separate patch.
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Yes, quite a bit more work to do. This series is just a start.

Regards,
Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/6] moveconfig: Use single quotes
  2021-12-18 21:54 ` [PATCH 1/6] moveconfig: Use single quotes Simon Glass
  2021-12-18 22:16   ` Heinrich Schuchardt
@ 2022-01-25 15:57   ` Tom Rini
  1 sibling, 0 replies; 18+ messages in thread
From: Tom Rini @ 2022-01-25 15:57 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Masahiro Yamada,
	Trevor Woerner

[-- Attachment #1: Type: text/plain, Size: 327 bytes --]

On Sat, Dec 18, 2021 at 02:54:30PM -0700, Simon Glass wrote:

> Quite a few places use double quotes. Fix this to be consistent with
> other Python code in U-Boot.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] moveconfig: Convert to ArgumentParser
  2021-12-18 21:54 ` [PATCH 2/6] moveconfig: Convert to ArgumentParser Simon Glass
  2021-12-18 22:32   ` Heinrich Schuchardt
@ 2022-01-25 15:57   ` Tom Rini
  1 sibling, 0 replies; 18+ messages in thread
From: Tom Rini @ 2022-01-25 15:57 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Masahiro Yamada,
	Trevor Woerner

[-- Attachment #1: Type: text/plain, Size: 472 bytes --]

On Sat, Dec 18, 2021 at 02:54:31PM -0700, Simon Glass wrote:

> This is a newer library and is now preferred for Python scripts. Update
> the code to use it instead of optparse
> 
> Use 'args' instead of 'options' throughout, since this is the term used
> in that module. Also it helps to avoid confusion with CONFIG options, a
> term that is used in this file.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/6] moveconfig: Drop check for old Python
  2021-12-18 21:54 ` [PATCH 3/6] moveconfig: Drop check for old Python Simon Glass
  2021-12-18 22:27   ` Heinrich Schuchardt
@ 2022-01-25 15:58   ` Tom Rini
  1 sibling, 0 replies; 18+ messages in thread
From: Tom Rini @ 2022-01-25 15:58 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Masahiro Yamada,
	Trevor Woerner

[-- Attachment #1: Type: text/plain, Size: 377 bytes --]

On Sat, Dec 18, 2021 at 02:54:32PM -0700, Simon Glass wrote:

> Python 2 is not supported anymore and Python 3 has had subprocess.DEVNULL
> since version 3.3 which was released in 2012. Drop the unnecessary check.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/6] moveconfig: Use a function to write files
  2021-12-18 21:54 ` [PATCH 4/6] moveconfig: Use a function to write files Simon Glass
@ 2022-01-25 15:58   ` Tom Rini
  0 siblings, 0 replies; 18+ messages in thread
From: Tom Rini @ 2022-01-25 15:58 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Masahiro Yamada,
	Trevor Woerner

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

On Sat, Dec 18, 2021 at 02:54:33PM -0700, Simon Glass wrote:

> At present there is quite a bit of ad-hoc code writing to files. The
> treatment of newlines is different in some of them. Put it in a function
> and set the unicode encoding correctly.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/6] moveconfig: Use a function to read files
  2021-12-18 21:54 ` [PATCH 5/6] moveconfig: Use a function to read files Simon Glass
@ 2022-01-25 15:58   ` Tom Rini
  0 siblings, 0 replies; 18+ messages in thread
From: Tom Rini @ 2022-01-25 15:58 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Masahiro Yamada,
	Trevor Woerner

[-- Attachment #1: Type: text/plain, Size: 464 bytes --]

On Sat, Dec 18, 2021 at 02:54:34PM -0700, Simon Glass wrote:

> At present there is quite a bit of ad-hoc code reading from files. The
> most common case is to read the file as lines. Put it in a function and
> set the unicode encoding correctly.
> 
> Avoid writing back to a file when there are obviously no changes as this
> speeds things up slightly.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/6] moveconfig: Fix some pylint errors
  2021-12-18 21:54 ` [PATCH 6/6] moveconfig: Fix some pylint errors Simon Glass
@ 2022-01-25 15:58   ` Tom Rini
  0 siblings, 0 replies; 18+ messages in thread
From: Tom Rini @ 2022-01-25 15:58 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Masahiro Yamada,
	Trevor Woerner

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]

On Sat, Dec 18, 2021 at 02:54:35PM -0700, Simon Glass wrote:

> There are over 200 errors in this file. Fix some of them, starting at the
> beginning of the file. Future work can continue this effort.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-01-25 15:59 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-18 21:54 [PATCH 0/6] moveconfig: Improve the pylist score a little Simon Glass
2021-12-18 21:54 ` [PATCH 1/6] moveconfig: Use single quotes Simon Glass
2021-12-18 22:16   ` Heinrich Schuchardt
2021-12-28  8:33     ` Simon Glass
2022-01-25 15:57   ` Tom Rini
2021-12-18 21:54 ` [PATCH 2/6] moveconfig: Convert to ArgumentParser Simon Glass
2021-12-18 22:32   ` Heinrich Schuchardt
2021-12-18 23:33     ` Simon Glass
2022-01-25 15:57   ` Tom Rini
2021-12-18 21:54 ` [PATCH 3/6] moveconfig: Drop check for old Python Simon Glass
2021-12-18 22:27   ` Heinrich Schuchardt
2022-01-25 15:58   ` Tom Rini
2021-12-18 21:54 ` [PATCH 4/6] moveconfig: Use a function to write files Simon Glass
2022-01-25 15:58   ` Tom Rini
2021-12-18 21:54 ` [PATCH 5/6] moveconfig: Use a function to read files Simon Glass
2022-01-25 15:58   ` Tom Rini
2021-12-18 21:54 ` [PATCH 6/6] moveconfig: Fix some pylint errors Simon Glass
2022-01-25 15:58   ` Tom Rini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).