[PATCH 11/19] buildman: Tidy up pylint problems in boards module

Simon Glass sjg at chromium.org
Tue Jul 12 03:04:05 CEST 2022


Fix all the pylint warnings. Also tidy up the comments so that they show
type information, as required.

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

 tools/buildman/boards.py | 352 +++++++++++++++++++++++----------------
 1 file changed, 204 insertions(+), 148 deletions(-)

diff --git a/tools/buildman/boards.py b/tools/buildman/boards.py
index c18914253e4..e16f3268ab1 100644
--- a/tools/buildman/boards.py
+++ b/tools/buildman/boards.py
@@ -24,19 +24,26 @@ from buildman import kconfiglib
 OUTPUT_FILE = 'boards.cfg'
 CONFIG_DIR = 'configs'
 SLEEP_TIME = 0.03
-COMMENT_BLOCK = '''#
+COMMENT_BLOCK = f'''#
 # List of boards
-#   Automatically generated by %s: don't edit
+#   Automatically generated by {__file__}: don't edit
 #
 # Status, Arch, CPU, SoC, Vendor, Board, Target, Options, Maintainers
 
-''' % __file__
+'''
 
 
-def try_remove(f):
-    """Remove a file ignoring 'No such file or directory' error."""
+def try_remove(fname):
+    """Remove a file ignoring 'No such file or directory' error.
+
+    Args:
+        fname (str): Filename to remove
+
+    Raises:
+        OSError: output file exists but could not be removed
+    """
     try:
-        os.remove(f)
+        os.remove(fname)
     except OSError as exception:
         # Ignore 'No such file or directory' error
         if exception.errno != errno.ENOENT:
@@ -46,20 +53,30 @@ def try_remove(f):
 def output_is_new(output):
     """Check if the output file is up to date.
 
+    Looks at defconfig and Kconfig files to make sure none is newer than the
+    output file. Also ensures that the boards.cfg does not mention any removed
+    boards.
+
+    Args:
+        output (str): Filename to check
+
     Returns:
-      True if the given output file exists and is newer than any of
-      *_defconfig, MAINTAINERS and Kconfig*.  False otherwise.
+        True if the given output file exists and is newer than any of
+        *_defconfig, MAINTAINERS and Kconfig*.  False otherwise.
+
+    Raises:
+        OSError: output file exists but could not be opened
     """
+    # pylint: disable=too-many-branches
     try:
         ctime = os.path.getctime(output)
     except OSError as exception:
         if exception.errno == errno.ENOENT:
             # return False on 'No such file or directory' error
             return False
-        else:
-            raise
+        raise
 
-    for (dirpath, dirnames, filenames) in os.walk(CONFIG_DIR):
+    for (dirpath, _, filenames) in os.walk(CONFIG_DIR):
         for filename in fnmatch.filter(filenames, '*_defconfig'):
             if fnmatch.fnmatch(filename, '.*'):
                 continue
@@ -67,7 +84,7 @@ def output_is_new(output):
             if ctime < os.path.getctime(filepath):
                 return False
 
-    for (dirpath, dirnames, filenames) in os.walk('.'):
+    for (dirpath, _, filenames) in os.walk('.'):
         for filename in filenames:
             if (fnmatch.fnmatch(filename, '*~') or
                 not fnmatch.fnmatch(filename, 'Kconfig*') and
@@ -79,8 +96,8 @@ def output_is_new(output):
 
     # Detect a board that has been removed since the current board database
     # was generated
-    with open(output, encoding="utf-8") as f:
-        for line in f:
+    with open(output, encoding="utf-8") as inf:
+        for line in inf:
             if line[0] == '#' or line == '\n':
                 continue
             defconfig = line.split()[6] + '_defconfig'
@@ -97,7 +114,7 @@ class Expr:
         """Set up a new Expr object.
 
         Args:
-            expr: String cotaining regular expression to store
+            expr (str): String cotaining regular expression to store
         """
         self._expr = expr
         self._re = re.compile(expr)
@@ -106,7 +123,7 @@ class Expr:
         """Check if any of the properties match the regular expression.
 
         Args:
-           props: List of properties to check
+           props (list of str): List of properties to check
         Returns:
            True if any of the properties match the regular expression
         """
@@ -132,7 +149,7 @@ class Term:
         """Add an Expr object to the list to check.
 
         Args:
-            expr: New Expr object to add to the list of those that must
+            expr (Expr): New Expr object to add to the list of those that must
                   match for a board to be built.
         """
         self._expr_list.append(Expr(expr))
@@ -147,7 +164,7 @@ class Term:
         Each of the expressions in the term is checked. All must match.
 
         Args:
-           props: List of properties to check
+           props (list of str): List of properties to check
         Returns:
            True if all of the expressions in the Term match, else False
         """
@@ -178,6 +195,7 @@ class KconfigScanner:
         os.environ['srctree'] = os.getcwd()
         os.environ['UBOOTVERSION'] = 'dummy'
         os.environ['KCONFIG_OBJDIR'] = ''
+        self._tmpfile = None
         self._conf = kconfiglib.Kconfig(warn=False)
 
     def __del__(self):
@@ -188,37 +206,38 @@ class KconfigScanner:
         the temporary file might be left over.  In that case, it should be
         deleted in this destructor.
         """
-        if hasattr(self, '_tmpfile') and self._tmpfile:
+        if self._tmpfile:
             try_remove(self._tmpfile)
 
     def scan(self, defconfig):
         """Load a defconfig file to obtain board parameters.
 
-        Arguments:
-          defconfig: path to the defconfig file to be processed
+        Args:
+            defconfig (str): path to the defconfig file to be processed
 
         Returns:
-          A dictionary of board parameters.  It has a form of:
-          {
-              'arch': <arch_name>,
-              'cpu': <cpu_name>,
-              'soc': <soc_name>,
-              'vendor': <vendor_name>,
-              'board': <board_name>,
-              'target': <target_name>,
-              'config': <config_header_name>,
-              'options': <extra_options>
-          }
+            Dictionary of board parameters.  It has a form:
+            {
+                'arch': <arch_name>,
+                'cpu': <cpu_name>,
+                'soc': <soc_name>,
+                'vendor': <vendor_name>,
+                'board': <board_name>,
+                'target': <target_name>,
+                'config': <config_header_name>,
+                'options': <extra_options>
+            }
         """
         # strip special prefixes and save it in a temporary file
-        fd, self._tmpfile = tempfile.mkstemp()
-        with os.fdopen(fd, 'w') as f:
-            for line in open(defconfig):
-                colon = line.find(':CONFIG_')
-                if colon == -1:
-                    f.write(line)
-                else:
-                    f.write(line[colon + 1:])
+        outfd, self._tmpfile = tempfile.mkstemp()
+        with os.fdopen(outfd, 'w') as outf:
+            with open(defconfig, encoding='utf-8') as inf:
+                for line in inf:
+                    colon = line.find(':CONFIG_')
+                    if colon == -1:
+                        outf.write(line)
+                    else:
+                        outf.write(line[colon + 1:])
 
         self._conf.load_config(self._tmpfile)
         try_remove(self._tmpfile)
@@ -237,7 +256,7 @@ class KconfigScanner:
 
         defconfig = os.path.basename(defconfig)
         params['target'], match, rear = defconfig.partition('_defconfig')
-        assert match and not rear, '%s : invalid defconfig' % defconfig
+        assert match and not rear, f'{defconfig} : invalid defconfig'
 
         # fix-up for aarch64
         if params['arch'] == 'arm' and params['cpu'] == 'armv8':
@@ -256,7 +275,15 @@ class KconfigScanner:
 
 class MaintainersDatabase:
 
-    """The database of board status and maintainers."""
+    """The database of board status and maintainers.
+
+    Properties:
+        database: dict:
+            key: Board-target name (e.g. 'snow')
+            value: tuple:
+                str: Board status (e.g. 'Active')
+                str: List of maintainers, separated by :
+    """
 
     def __init__(self):
         """Create an empty database."""
@@ -269,73 +296,78 @@ class MaintainersDatabase:
         Display a warning message and return '-' if status information
         is not found.
 
+        Args:
+            target (str): Build-target name
+
         Returns:
-          'Active', 'Orphan' or '-'.
+            str: 'Active', 'Orphan' or '-'.
         """
         if not target in self.database:
-            print("WARNING: no status info for '%s'" % target, file=sys.stderr)
+            print(f"WARNING: no status info for '{target}'", file=sys.stderr)
             return '-'
 
         tmp = self.database[target][0]
         if tmp.startswith('Maintained'):
             return 'Active'
-        elif tmp.startswith('Supported'):
+        if tmp.startswith('Supported'):
             return 'Active'
-        elif tmp.startswith('Orphan'):
+        if tmp.startswith('Orphan'):
             return 'Orphan'
-        else:
-            print(("WARNING: %s: unknown status for '%s'" %
-                                  (tmp, target)), file=sys.stderr)
-            return '-'
+        print(f"WARNING: {tmp}: unknown status for '{target}'", file=sys.stderr)
+        return '-'
 
     def get_maintainers(self, target):
         """Return the maintainers of the given board.
 
+        Args:
+            target (str): Build-target name
+
         Returns:
-          Maintainers of the board.  If the board has two or more maintainers,
-          they are separated with colons.
+            str: Maintainers of the board.  If the board has two or more
+            maintainers, they are separated with colons.
         """
         if not target in self.database:
-            print("WARNING: no maintainers for '%s'" % target, file=sys.stderr)
+            print(f"WARNING: no maintainers for '{target}'", file=sys.stderr)
             return ''
 
         return ':'.join(self.database[target][1])
 
-    def parse_file(self, file):
+    def parse_file(self, fname):
         """Parse a MAINTAINERS file.
 
-        Parse a MAINTAINERS file and accumulates board status and
-        maintainers information.
+        Parse a MAINTAINERS file and accumulate board status and maintainers
+        information in the self.database dict.
 
-        Arguments:
-          file: MAINTAINERS file to be parsed
+        Args:
+            fname (str): MAINTAINERS file to be parsed
         """
         targets = []
         maintainers = []
         status = '-'
-        for line in open(file, encoding="utf-8"):
-            # Check also commented maintainers
-            if line[:3] == '#M:':
-                line = line[1:]
-            tag, rest = line[:2], line[2:].strip()
-            if tag == 'M:':
-                maintainers.append(rest)
-            elif tag == 'F:':
-                # expand wildcard and filter by 'configs/*_defconfig'
-                for f in glob.glob(rest):
-                    front, match, rear = f.partition('configs/')
-                    if not front and match:
-                        front, match, rear = rear.rpartition('_defconfig')
-                        if match and not rear:
-                            targets.append(front)
-            elif tag == 'S:':
-                status = rest
-            elif line == '\n':
-                for target in targets:
-                    self.database[target] = (status, maintainers)
-                targets = []
-                maintainers = []
-                status = '-'
+        with open(fname, encoding="utf-8") as inf:
+            for line in inf:
+                # Check also commented maintainers
+                if line[:3] == '#M:':
+                    line = line[1:]
+                tag, rest = line[:2], line[2:].strip()
+                if tag == 'M:':
+                    maintainers.append(rest)
+                elif tag == 'F:':
+                    # expand wildcard and filter by 'configs/*_defconfig'
+                    for item in glob.glob(rest):
+                        front, match, rear = item.partition('configs/')
+                        if not front and match:
+                            front, match, rear = rear.rpartition('_defconfig')
+                            if match and not rear:
+                                targets.append(front)
+                elif tag == 'S:':
+                    status = rest
+                elif line == '\n':
+                    for target in targets:
+                        self.database[target] = (status, maintainers)
+                    targets = []
+                    maintainers = []
+                    status = '-'
         if targets:
             for target in targets:
                 self.database[target] = (status, maintainers)
@@ -353,7 +385,7 @@ class Boards:
         The board's target member must not already exist in the board list.
 
         Args:
-            brd: board to add
+            brd (Board): board to add
         """
         self._boards.append(brd)
 
@@ -363,7 +395,7 @@ class Boards:
         Create a Board object for each and add it to our _boards list.
 
         Args:
-            fname: Filename of boards.cfg file
+            fname (str): Filename of boards.cfg file
         """
         with open(fname, 'r', encoding='utf-8') as inf:
             for line in inf:
@@ -452,9 +484,10 @@ class Boards:
         a board to be selected.
 
         Args:
-            args: List of command line arguments
+            args (list of str): List of command line arguments
+
         Returns:
-            A list of Term objects
+            list of Term: A list of Term objects
         """
         syms = []
         for arg in args:
@@ -493,11 +526,12 @@ class Boards:
         If brds and args are both empty, all boards are selected.
 
         Args:
-            args: List of strings specifying boards to include, either named,
-                  or by their target, architecture, cpu, vendor or soc. If
-                  empty, all boards are selected.
-            exclude: List of boards to exclude, regardless of 'args'
-            brds: List of boards to build
+            args (list of str): List of strings specifying boards to include,
+                either named, or by their target, architecture, cpu, vendor or
+                soc. If empty, all boards are selected.
+            exclude (list of str): List of boards to exclude, regardless of
+                'args', or None for none
+            brds (list of Board): List of boards to build, or None/[] for all
 
         Returns:
             Tuple
@@ -505,21 +539,21 @@ class Boards:
                     due to each argument, arranged by argument.
                 List of errors found
         """
-        result = OrderedDict()
-        warnings = []
-        terms = self._build_terms(args)
+        def _check_board(brd):
+            """Check whether to include or exclude a board
 
-        result['all'] = []
-        for term in terms:
-            result[str(term)] = []
+            Checks the various terms and decide whether to build it or not (the
+            'build_it' variable).
 
-        exclude_list = []
-        if exclude:
-            for expr in exclude:
-                exclude_list.append(Expr(expr))
+            If it is built, add the board to the result[term] list so we know
+            which term caused it to be built. Add it to result['all'] also.
 
-        found = []
-        for brd in self._boards:
+            Keep a list of boards we found in 'found', so we can report boards
+            which appear in self._boards but not in brds.
+
+            Args:
+                brd (Board): Board to check
+            """
             matching_term = None
             build_it = False
             if terms:
@@ -547,6 +581,23 @@ class Boards:
                     result[matching_term].append(brd.target)
                 result['all'].append(brd.target)
 
+        result = OrderedDict()
+        warnings = []
+        terms = self._build_terms(args)
+
+        result['all'] = []
+        for term in terms:
+            result[str(term)] = []
+
+        exclude_list = []
+        if exclude:
+            for expr in exclude:
+                exclude_list.append(Expr(expr))
+
+        found = []
+        for brd in self._boards:
+            _check_board(brd)
+
         if brds:
             remaining = set(brds) - set(found)
             if remaining:
@@ -554,37 +605,40 @@ class Boards:
 
         return result, warnings
 
-    def scan_defconfigs_for_multiprocess(self, queue, defconfigs):
+    @classmethod
+    def scan_defconfigs_for_multiprocess(cls, queue, defconfigs):
         """Scan defconfig files and queue their board parameters
 
-        This function is intended to be passed to
-        multiprocessing.Process() constructor.
+        This function is intended to be passed to multiprocessing.Process()
+        constructor.
 
-        Arguments:
-        queue: An instance of multiprocessing.Queue().
-                The resulting board parameters are written into it.
-        defconfigs: A sequence of defconfig files to be scanned.
+        Args:
+            queue (multiprocessing.Queue): The resulting board parameters are
+                written into this.
+            defconfigs (sequence of str): A sequence of defconfig files to be
+                scanned.
         """
         kconf_scanner = KconfigScanner()
         for defconfig in defconfigs:
             queue.put(kconf_scanner.scan(defconfig))
 
-    def read_queues(self, queues, params_list):
+    @classmethod
+    def read_queues(cls, queues, params_list):
         """Read the queues and append the data to the paramers list"""
-        for q in queues:
-            while not q.empty():
-                params_list.append(q.get())
+        for que in queues:
+            while not que.empty():
+                params_list.append(que.get())
 
     def scan_defconfigs(self, jobs=1):
         """Collect board parameters for all defconfig files.
 
         This function invokes multiple processes for faster processing.
 
-        Arguments:
-        jobs: The number of jobs to run simultaneously
+        Args:
+            jobs (int): The number of jobs to run simultaneously
         """
         all_defconfigs = []
-        for (dirpath, dirnames, filenames) in os.walk(CONFIG_DIR):
+        for (dirpath, _, filenames) in os.walk(CONFIG_DIR):
             for filename in fnmatch.filter(filenames, '*_defconfig'):
                 if fnmatch.fnmatch(filename, '.*'):
                     continue
@@ -596,42 +650,43 @@ class Boards:
         for i in range(jobs):
             defconfigs = all_defconfigs[total_boards * i // jobs :
                                         total_boards * (i + 1) // jobs]
-            q = multiprocessing.Queue(maxsize=-1)
-            p = multiprocessing.Process(
+            que = multiprocessing.Queue(maxsize=-1)
+            proc = multiprocessing.Process(
                 target=self.scan_defconfigs_for_multiprocess,
-                args=(q, defconfigs))
-            p.start()
-            processes.append(p)
-            queues.append(q)
+                args=(que, defconfigs))
+            proc.start()
+            processes.append(proc)
+            queues.append(que)
 
         # The resulting data should be accumulated to this list
         params_list = []
 
         # Data in the queues should be retrieved preriodically.
         # Otherwise, the queues would become full and subprocesses would get stuck.
-        while any([p.is_alive() for p in processes]):
+        while any(p.is_alive() for p in processes):
             self.read_queues(queues, params_list)
             # sleep for a while until the queues are filled
             time.sleep(SLEEP_TIME)
 
         # Joining subprocesses just in case
         # (All subprocesses should already have been finished)
-        for p in processes:
-            p.join()
+        for proc in processes:
+            proc.join()
 
         # retrieve leftover data
         self.read_queues(queues, params_list)
 
         return params_list
 
-    def insert_maintainers_info(self, params_list):
+    @classmethod
+    def insert_maintainers_info(cls, params_list):
         """Add Status and Maintainers information to the board parameters list.
 
-        Arguments:
-        params_list: A list of the board parameters
+        Args:
+            params_list (list of dict): A list of the board parameters
         """
         database = MaintainersDatabase()
-        for (dirpath, dirnames, filenames) in os.walk('.'):
+        for (dirpath, _, filenames) in os.walk('.'):
             if 'MAINTAINERS' in filenames:
                 database.parse_file(os.path.join(dirpath, 'MAINTAINERS'))
 
@@ -641,51 +696,52 @@ class Boards:
             params['maintainers'] = database.get_maintainers(target)
             params_list[i] = params
 
-    def format_and_output(self, params_list, output):
+    @classmethod
+    def format_and_output(cls, params_list, output):
         """Write board parameters into a file.
 
         Columnate the board parameters, sort lines alphabetically,
         and then write them to a file.
 
-        Arguments:
-        params_list: The list of board parameters
-        output: The path to the output file
+        Args:
+            params_list (list of dict): The list of board parameters
+            output (str): The path to the output file
         """
-        FIELDS = ('status', 'arch', 'cpu', 'soc', 'vendor', 'board', 'target',
-                'options', 'maintainers')
+        fields = ('status', 'arch', 'cpu', 'soc', 'vendor', 'board', 'target',
+                  'options', 'maintainers')
 
         # First, decide the width of each column
-        max_length = dict([ (f, 0) for f in FIELDS])
+        max_length = {f: 0 for f in fields}
         for params in params_list:
-            for f in FIELDS:
-                max_length[f] = max(max_length[f], len(params[f]))
+            for field in fields:
+                max_length[field] = max(max_length[field], len(params[field]))
 
         output_lines = []
         for params in params_list:
             line = ''
-            for f in FIELDS:
+            for field in fields:
                 # insert two spaces between fields like column -t would
-                line += '  ' + params[f].ljust(max_length[f])
+                line += '  ' + params[field].ljust(max_length[field])
             output_lines.append(line.strip())
 
         # ignore case when sorting
         output_lines.sort(key=str.lower)
 
-        with open(output, 'w', encoding="utf-8") as f:
-            f.write(COMMENT_BLOCK + '\n'.join(output_lines) + '\n')
+        with open(output, 'w', encoding="utf-8") as outf:
+            outf.write(COMMENT_BLOCK + '\n'.join(output_lines) + '\n')
 
     def ensure_board_list(self, output, jobs=1, force=False, quiet=False):
         """Generate a board database file if needed.
 
-        Arguments:
-        output: The name of the output file
-        jobs: The number of jobs to run simultaneously
-        force: Force to generate the output even if it is new
-        quiet: True to avoid printing a message if nothing needs doing
+        Args:
+            output (str): The name of the output file
+            jobs (int): The number of jobs to run simultaneously
+            force (bool): Force to generate the output even if it is new
+            quiet (bool): True to avoid printing a message if nothing needs doing
         """
         if not force and output_is_new(output):
             if not quiet:
-                print("%s is up to date. Nothing to do." % output)
+                print(f'{output} is up to date. Nothing to do.')
             return
         params_list = self.scan_defconfigs(jobs)
         self.insert_maintainers_info(params_list)
-- 
2.37.0.144.g8ac04bfd2-goog



More information about the U-Boot mailing list