[PATCH v3 09/81] buildman: Add a partial test for ensure_board_list()

Simon Glass sjg at chromium.org
Sun Jul 16 02:35:37 CEST 2023


Create a new function which has the non-UI parts of ensure_board_list().
Add some tests for everything except the N: tag.

While we are here, fix the confusing usage of fname inside a loops that
also uses fname.

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

Changes in v3:
- Add new patch with a partial test for ensure_board_list()

 tools/buildman/board.py                       |  6 +-
 tools/buildman/boards.py                      | 58 ++++++++++++----
 tools/buildman/func_test.py                   | 66 +++++++++++++++++++
 tools/buildman/test/boards/board0/MAINTAINERS |  5 ++
 tools/buildman/test/boards/board2/MAINTAINERS |  5 ++
 5 files changed, 126 insertions(+), 14 deletions(-)
 create mode 100644 tools/buildman/test/boards/board0/MAINTAINERS
 create mode 100644 tools/buildman/test/boards/board2/MAINTAINERS

diff --git a/tools/buildman/board.py b/tools/buildman/board.py
index 8ef905b8ce1b..248d8bfff188 100644
--- a/tools/buildman/board.py
+++ b/tools/buildman/board.py
@@ -17,14 +17,14 @@ class Board:
             vendor: Name of vendor (e.g. armltd)
             board_name: Name of board (e.g. integrator)
             target: Target name (use make <target>_defconfig to configure)
-            cfg_name: Config name
+            cfg_name: Config-file name (in includes/configs/)
         """
         self.target = target
         self.arch = arch
         self.cpu = cpu
-        self.board_name = board_name
-        self.vendor = vendor
         self.soc = soc
+        self.vendor = vendor
+        self.board_name = board_name
         self.cfg_name = cfg_name
         self.props = [self.target, self.arch, self.cpu, self.board_name,
                       self.vendor, self.soc, self.cfg_name]
diff --git a/tools/buildman/boards.py b/tools/buildman/boards.py
index daf1f037b60a..541c82ff9960 100644
--- a/tools/buildman/boards.py
+++ b/tools/buildman/boards.py
@@ -328,13 +328,21 @@ class MaintainersDatabase:
 
         return ':'.join(self.database[target][1])
 
-    def parse_file(self, fname):
+    def parse_file(self, srcdir, fname):
         """Parse a MAINTAINERS file.
 
         Parse a MAINTAINERS file and accumulate board status and maintainers
         information in the self.database dict.
 
+        defconfig files are used to specify the target, e.g. xxx_defconfig is
+        used for target 'xxx'. If there is no defconfig file mentioned in the
+        MAINTAINERS file F: entries, then this function does nothing.
+
+        The N: name entries can be used to specify a defconfig file using
+        wildcards.
+
         Args:
+            srcdir (str): Directory containing source code (Kconfig files)
             fname (str): MAINTAINERS file to be parsed
         """
         targets = []
@@ -350,9 +358,12 @@ class MaintainersDatabase:
                     maintainers.append(rest)
                 elif tag == 'F:':
                     # expand wildcard and filter by 'configs/*_defconfig'
-                    for item in glob.glob(rest):
+                    glob_path = os.path.join(srcdir, rest)
+                    for item in glob.glob(glob_path):
                         front, match, rear = item.partition('configs/')
-                        if not front and match:
+                        if front.endswith('/'):
+                            front = front[:-1]
+                        if front == srcdir and match:
                             front, match, rear = rear.rpartition('_defconfig')
                             if match and not rear:
                                 targets.append(front)
@@ -361,9 +372,10 @@ class MaintainersDatabase:
                 elif tag == 'N:':
                     # Just scan the configs directory since that's all we care
                     # about
-                    for dirpath, _, fnames in os.walk('configs'):
-                        for fname in fnames:
-                            path = os.path.join(dirpath, fname)
+                    walk_path = os.walk(os.path.join(srcdir, 'configs'))
+                    for dirpath, _, fnames in walk_path:
+                        for cfg in fnames:
+                            path = os.path.join(dirpath, cfg)
                             front, match, rear = path.partition('configs/')
                             if not front and match:
                                 front, match, rear = rear.rpartition('_defconfig')
@@ -693,7 +705,7 @@ class Boards:
         return params_list
 
     @classmethod
-    def insert_maintainers_info(cls, params_list):
+    def insert_maintainers_info(cls, srcdir, params_list):
         """Add Status and Maintainers information to the board parameters list.
 
         Args:
@@ -703,9 +715,10 @@ class Boards:
             list of str: List of warnings collected due to missing status, etc.
         """
         database = MaintainersDatabase()
-        for (dirpath, _, filenames) in os.walk('.'):
+        for (dirpath, _, filenames) in os.walk(srcdir):
             if 'MAINTAINERS' in filenames:
-                database.parse_file(os.path.join(dirpath, 'MAINTAINERS'))
+                database.parse_file(srcdir,
+                                    os.path.join(dirpath, 'MAINTAINERS'))
 
         for i, params in enumerate(params_list):
             target = params['target']
@@ -748,6 +761,30 @@ class Boards:
         with open(output, 'w', encoding="utf-8") as outf:
             outf.write(COMMENT_BLOCK + '\n'.join(output_lines) + '\n')
 
+    def build_board_list(self, config_dir, srcdir, jobs=1):
+        """Generate a board-database file
+
+        This works by reading the Kconfig, then loading each board's defconfig
+        in to get the setting for each option. In particular, CONFIG_TARGET_xxx
+        is typically set by the defconfig, where xxx is the target to build.
+
+        Args:
+            config_dir (str): Directory containing the defconfig files
+            srcdir (str): Directory containing source code (Kconfig files)
+            jobs (int): The number of jobs to run simultaneously
+
+        Returns:
+            tuple:
+                list of dict: List of board parameters, each a dict:
+                    key: 'arch', 'cpu', 'soc', 'vendor', 'board', 'config',
+                         'target'
+                    value: string value of the key
+                list of str: Warnings that came up
+        """
+        params_list = self.scan_defconfigs(config_dir, srcdir, jobs)
+        warnings = self.insert_maintainers_info(srcdir, params_list)
+        return params_list, warnings
+
     def ensure_board_list(self, output, jobs=1, force=False, quiet=False):
         """Generate a board database file if needed.
 
@@ -767,8 +804,7 @@ class Boards:
             if not quiet:
                 print(f'{output} is up to date. Nothing to do.')
             return True
-        params_list = self.scan_defconfigs(CONFIG_DIR, os.getcwd(), jobs)
-        warnings = self.insert_maintainers_info(params_list)
+        params_list, warnings = self.build_board_list(CONFIG_DIR, '.', jobs)
         for warn in warnings:
             print(warn, file=sys.stderr)
         self.format_and_output(params_list, output)
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 70e2af19fd46..43087b4e08d2 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -877,3 +877,69 @@ Active  aarch64     armv8 - armltd total_compute board2
         Path(os.path.join(config_dir, 'board0_defconfig')).unlink()
         self.assertFalse(boards.output_is_new(boards_cfg, config_dir, src))
 
+    def test_maintainers(self):
+        """Test detecting boards without a MAINTAINERS entry"""
+        src = self._git_dir
+        main = os.path.join(src, 'boards', 'board0', 'MAINTAINERS')
+        other = os.path.join(src, 'boards', 'board2', 'MAINTAINERS')
+        config_dir = os.path.join(src, 'configs')
+        params_list, warnings = self._boards.build_board_list(config_dir, src)
+
+        # There should be two boards no warnings
+        self.assertEquals(2, len(params_list))
+        self.assertFalse(warnings)
+
+        # Set an invalid status line in the file
+        orig_data = tools.read_file(main, binary=False)
+        lines = ['S:      Other\n' if line.startswith('S:') else line
+                  for line in orig_data.splitlines(keepends=True)]
+        tools.write_file(main, ''.join(lines), binary=False)
+        params_list, warnings = self._boards.build_board_list(config_dir, src)
+        self.assertEquals(2, len(params_list))
+        params = params_list[0]
+        if params['target'] == 'board2':
+            params = params_list[1]
+        self.assertEquals('-', params['status'])
+        self.assertEquals(["WARNING: Other: unknown status for 'board0'"],
+                          warnings)
+
+        # Remove the status line (S:) from a file
+        lines = [line for line in orig_data.splitlines(keepends=True)
+                 if not line.startswith('S:')]
+        tools.write_file(main, ''.join(lines), binary=False)
+        params_list, warnings = self._boards.build_board_list(config_dir, src)
+        self.assertEquals(2, len(params_list))
+        self.assertEquals(["WARNING: -: unknown status for 'board0'"], warnings)
+
+        # Remove the configs/ line (F:) from a file - this is the last line
+        data = ''.join(orig_data.splitlines(keepends=True)[:-1])
+        tools.write_file(main, data, binary=False)
+        params_list, warnings = self._boards.build_board_list(config_dir, src)
+        self.assertEquals(2, len(params_list))
+        self.assertEquals(
+            ["WARNING: no status info for 'board0'",
+             "WARNING: no maintainers for 'board0'"], warnings)
+
+        # Remove the maintainer line (M:) from a file (this should be fine)
+        lines = [line for line in orig_data.splitlines(keepends=True)
+                 if not line.startswith('M:')]
+        tools.write_file(main, ''.join(lines), binary=False)
+        params_list, warnings = self._boards.build_board_list(config_dir, src)
+        self.assertEquals(2, len(params_list))
+        self.assertFalse(warnings)
+
+        # Move the contents of the second file into this one, removing the
+        # second file, to check multiple records in a single file.
+        data = orig_data + tools.read_file(other, binary=False)
+        tools.write_file(main, data, binary=False)
+        os.remove(other)
+        params_list, warnings = self._boards.build_board_list(config_dir, src)
+        self.assertEquals(2, len(params_list))
+        self.assertFalse(warnings)
+
+        # Add another record, this should be ignored
+        extra = '\n\nAnother\nM: Fred\nF: configs/board9_defconfig\nS: other\n'
+        tools.write_file(main, data + extra, binary=False)
+        params_list, warnings = self._boards.build_board_list(config_dir, src)
+        self.assertEquals(2, len(params_list))
+        self.assertFalse(warnings)
diff --git a/tools/buildman/test/boards/board0/MAINTAINERS b/tools/buildman/test/boards/board0/MAINTAINERS
new file mode 100644
index 000000000000..08207ff3f480
--- /dev/null
+++ b/tools/buildman/test/boards/board0/MAINTAINERS
@@ -0,0 +1,5 @@
+ARM Board 0
+M:      Mary Mary <quite at contrary.org>
+S:      Maintained
+F:      boards/board0
+F:      configs/board0_defconfig
diff --git a/tools/buildman/test/boards/board2/MAINTAINERS b/tools/buildman/test/boards/board2/MAINTAINERS
new file mode 100644
index 000000000000..c1547822026f
--- /dev/null
+++ b/tools/buildman/test/boards/board2/MAINTAINERS
@@ -0,0 +1,5 @@
+ARM Board 2
+M:      Old Mother <hubbard at cupboard.org>
+S:      Maintained
+F:      boards/board2
+F:      configs/board2_defconfig
-- 
2.41.0.455.g037347b96a-goog



More information about the U-Boot mailing list