From 1b21842eab660ab1f80b89057abab99473b3bb5a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 19 Jul 2023 17:48:27 -0600 Subject: [PATCH] buildman: Add an option to check maintainers Rather than using the -R option to get this report as a side effect, add a dedicated option for it. Disable CI for now as there are some missing maintainers, unfortunately. Signed-off-by: Simon Glass --- .azure-pipelines.yml | 2 +- .gitlab-ci.yml | 2 +- tools/buildman/boards.py | 52 +++++++++++++++++++++++-------------- tools/buildman/buildman.rst | 13 ++++++++++ tools/buildman/cmdline.py | 2 ++ tools/buildman/control.py | 13 ++++++++-- tools/buildman/func_test.py | 6 +++-- 7 files changed, 64 insertions(+), 26 deletions(-) diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index ef7711d0f5..2678e5ae05 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -123,7 +123,7 @@ stages: options: $(container_option) steps: - script: | - ./tools/buildman/buildman -R + ./tools/buildman/buildman --maintainer-check || exit 0 - job: tools_only displayName: 'Ensure host tools build' diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 80dc587325..8010afae95 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -187,7 +187,7 @@ sloccount: Check for configs without MAINTAINERS entry: stage: testsuites script: - - ./tools/buildman/buildman -R + - ./tools/buildman/buildman --maintainer-check || exit 0 # Ensure host tools build Build tools-only: diff --git a/tools/buildman/boards.py b/tools/buildman/boards.py index e608f7990d..83adbf167c 100644 --- a/tools/buildman/boards.py +++ b/tools/buildman/boards.py @@ -213,11 +213,13 @@ class KconfigScanner: if self._tmpfile: try_remove(self._tmpfile) - def scan(self, defconfig): + def scan(self, defconfig, warn_targets): """Load a defconfig file to obtain board parameters. Args: defconfig (str): path to the defconfig file to be processed + warn_targets (bool): True to warn about missing or duplicate + CONFIG_TARGET options Returns: tuple: dictionary of board parameters. It has a form of: @@ -252,19 +254,20 @@ class KconfigScanner: params[key] = '-' # Check there is exactly one TARGET_xxx set - target = None - for name, sym in self._conf.syms.items(): - if name.startswith('TARGET_') and sym.str_value == 'y': - tname = name[7:].lower() - if target: - warnings.append( - f'WARNING: {leaf}: Duplicate TARGET_xxx: {target} and {tname}') - else: - target = tname - - if not target: - cfg_name = expect_target.replace('-', '_').upper() - warnings.append(f'WARNING: {leaf}: No TARGET_{cfg_name} enabled') + if warn_targets: + target = None + for name, sym in self._conf.syms.items(): + if name.startswith('TARGET_') and sym.str_value == 'y': + tname = name[7:].lower() + if target: + warnings.append( + f'WARNING: {leaf}: Duplicate TARGET_xxx: {target} and {tname}') + else: + target = tname + + if not target: + cfg_name = expect_target.replace('-', '_').upper() + warnings.append(f'WARNING: {leaf}: No TARGET_{cfg_name} enabled') params['target'] = expect_target @@ -666,7 +669,8 @@ class Boards: return result, warnings @classmethod - def scan_defconfigs_for_multiprocess(cls, srcdir, queue, defconfigs): + def scan_defconfigs_for_multiprocess(cls, srcdir, queue, defconfigs, + warn_targets): """Scan defconfig files and queue their board parameters This function is intended to be passed to multiprocessing.Process() @@ -678,10 +682,12 @@ class Boards: written into this. defconfigs (sequence of str): A sequence of defconfig files to be scanned. + warn_targets (bool): True to warn about missing or duplicate + CONFIG_TARGET options """ kconf_scanner = KconfigScanner(srcdir) for defconfig in defconfigs: - queue.put(kconf_scanner.scan(defconfig)) + queue.put(kconf_scanner.scan(defconfig, warn_targets)) @classmethod def read_queues(cls, queues, params_list, warnings): @@ -698,7 +704,7 @@ class Boards: params_list.append(params) warnings.update(warn) - def scan_defconfigs(self, config_dir, srcdir, jobs=1): + def scan_defconfigs(self, config_dir, srcdir, jobs=1, warn_targets=False): """Collect board parameters for all defconfig files. This function invokes multiple processes for faster processing. @@ -707,6 +713,8 @@ class Boards: 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 + warn_targets (bool): True to warn about missing or duplicate + CONFIG_TARGET options Returns: tuple: @@ -732,7 +740,7 @@ class Boards: que = multiprocessing.Queue(maxsize=-1) proc = multiprocessing.Process( target=self.scan_defconfigs_for_multiprocess, - args=(srcdir, que, defconfigs)) + args=(srcdir, que, defconfigs, warn_targets)) proc.start() processes.append(proc) queues.append(que) @@ -819,7 +827,8 @@ 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): + def build_board_list(self, config_dir=CONFIG_DIR, srcdir='.', jobs=1, + warn_targets=False): """Generate a board-database file This works by reading the Kconfig, then loading each board's defconfig @@ -830,6 +839,8 @@ class Boards: 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 + warn_targets (bool): True to warn about missing or duplicate + CONFIG_TARGET options Returns: tuple: @@ -839,7 +850,8 @@ class Boards: value: string value of the key list of str: Warnings that came up """ - params_list, warnings = self.scan_defconfigs(config_dir, srcdir, jobs) + params_list, warnings = self.scan_defconfigs(config_dir, srcdir, jobs, + warn_targets) m_warnings = self.insert_maintainers_info(srcdir, params_list) return params_list, warnings + m_warnings diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst index 6808727eb4..ebc552fae3 100644 --- a/tools/buildman/buildman.rst +++ b/tools/buildman/buildman.rst @@ -1311,6 +1311,19 @@ You should use 'buildman -nv ' instead of greoing the boards.cfg file, since it may be dropped altogether in future. +Checking maintainers +-------------------- + +Sometimes a board is added without a corresponding entry in a MAINTAINERS file. +Use the `--maintainer-check` option to check this:: + + $ buildman --maintainer-check + WARNING: board/mikrotik/crs3xx-98dx3236/MAINTAINERS: missing defconfig ending at line 7 + WARNING: no maintainers for 'clearfog_spi' + +Buildman returns with an exit code of 2 if there area any warnings. + + Checking the command -------------------- diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index a9cda24957..36acac5ee5 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -85,6 +85,8 @@ def ParseArgs(): parser.add_option( '-M', '--allow-missing', action='store_true', default=False, help='Tell binman to allow missing blobs and generate fake ones as needed'), + parser.add_option('--maintainer-check', action='store_true', + help='Check that maintainer entries exist for each board') parser.add_option( '--no-allow-missing', action='store_true', default=False, help='Disable telling binman to allow missing blobs'), diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 09a11f25b3..fc9e926d3a 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -202,6 +202,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None, brds=None, sys.exit(col.build(col.RED, '-w requires that you specify -o')) options.output_dir = '..' + nr_cups = options.threads or multiprocessing.cpu_count() + # Work out what subset of the boards we are building if not brds: if not os.path.exists(options.output_dir): @@ -209,8 +211,15 @@ def DoBuildman(options, args, toolchains=None, make_func=None, brds=None, board_file = os.path.join(options.output_dir, 'boards.cfg') brds = boards.Boards() - ok = brds.ensure_board_list(board_file, - options.threads or multiprocessing.cpu_count(), + if options.maintainer_check: + warnings = brds.build_board_list(jobs=nr_cups)[1] + if warnings: + for warn in warnings: + print(warn, file=sys.stderr) + return 2 + return 0 + + ok = brds.ensure_board_list(board_file, nr_cups, force=options.regen_board_list, quiet=not options.verbose) if options.regen_board_list: diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py index bb9eea335d..7f3d54cc76 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -976,7 +976,8 @@ config TARGET_OTHER endif ''') tools.write_file(kc_file, orig_kc_data + extra) - params_list, warnings = self._boards.build_board_list(config_dir, src) + params_list, warnings = self._boards.build_board_list(config_dir, src, + warn_targets=True) self.assertEquals(2, len(params_list)) self.assertEquals( ['WARNING: board2_defconfig: Duplicate TARGET_xxx: board2 and other'], @@ -986,7 +987,8 @@ endif lines = [b'' if line == b'config TARGET_BOARD2\n' else line for line in orig_kc_data.splitlines(keepends=True)] tools.write_file(kc_file, b''.join(lines)) - params_list, warnings = self._boards.build_board_list(config_dir, src) + params_list, warnings = self._boards.build_board_list(config_dir, src, + warn_targets=True) self.assertEquals(2, len(params_list)) self.assertEquals( ['WARNING: board2_defconfig: No TARGET_BOARD2 enabled'], -- 2.39.5