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 <s...@chromium.org> --- Changes in v3: - Add new patch with an option to check maintainers - Use the new --maintainer-check option in CI .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 06c46b681c3f..8499372d6763 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 cfd58513c377..fc25e900ab5b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -184,7 +184,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 25fa167b7bed..b9e21f9b2857 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 @@ -662,7 +665,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() @@ -674,10 +678,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): @@ -694,7 +700,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. @@ -703,6 +709,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: @@ -728,7 +736,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) @@ -815,7 +823,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 @@ -826,6 +835,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: @@ -835,7 +846,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 c8b0db3d8b9c..ec24744f7469 100644 --- a/tools/buildman/buildman.rst +++ b/tools/buildman/buildman.rst @@ -1315,6 +1315,19 @@ You should use 'buildman -nv <criteria>' 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 a9cda2495727..36acac5ee5b0 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 09a11f25b3f3..fc9e926d3ad1 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 7f21746792b6..506d6db0107a 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -959,7 +959,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'], @@ -969,7 +970,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.41.0.455.g037347b96a-goog