From 47a97d2b52b169b882bd501df91bb6be20ed7cea Mon Sep 17 00:00:00 2001 From: Fu Hanxi Date: Fri, 30 Oct 2020 17:28:24 +0800 Subject: [PATCH] check_kconfigs and check_deprecated_kconfigs now use files arguments --- .pre-commit-config.yaml | 10 +- tools/ci/check_deprecated_kconfigs.py | 81 +++++++------ tools/ci/check_kconfigs.py | 163 +++++++++++++++----------- tools/ci/idf_ci_utils.py | 9 +- 4 files changed, 151 insertions(+), 112 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 41423c3ad0..c9ce52d2d6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -34,16 +34,14 @@ repos: always_run: true - id: check-kconfigs name: Validate Kconfig files - entry: tools/ci/check_kconfigs.py --exclude-submodules + entry: tools/ci/check_kconfigs.py language: python - pass_filenames: false - always_run: true + files: '^Kconfig$|Kconfig.*$' - id: check-deprecated-kconfigs-options name: Check if any Kconfig Options Deprecated - entry: tools/ci/check_deprecated_kconfigs.py --exclude-submodules + entry: tools/ci/check_deprecated_kconfigs.py language: python - pass_filenames: false - always_run: true + files: 'sdkconfig\.ci$|sdkconfig\.rename$|sdkconfig.*$' - id: cmake-lint name: Check CMake Files Format entry: cmakelint --linelength=120 --spaces=4 diff --git a/tools/ci/check_deprecated_kconfigs.py b/tools/ci/check_deprecated_kconfigs.py index b8e55a6e88..3182f3933a 100755 --- a/tools/ci/check_deprecated_kconfigs.py +++ b/tools/ci/check_deprecated_kconfigs.py @@ -16,10 +16,13 @@ from __future__ import print_function from __future__ import unicode_literals + import argparse import os import sys from io import open + +from check_kconfigs import valid_directory from idf_ci_utils import get_submodule_dirs # FILES_TO_CHECK used as "startswith" pattern to match sdkconfig.defaults variants @@ -49,54 +52,66 @@ def _valid_directory(path): def main(): - default_path = os.getenv('IDF_PATH', None) - parser = argparse.ArgumentParser(description='Kconfig options checker') - parser.add_argument('--directory', '-d', help='Path to directory to check recursively ' - '(for example $IDF_PATH)', - type=_valid_directory, - required=default_path is None, - default=default_path) - parser.add_argument('--exclude-submodules', action='store_true', help='Exclude submodules') + parser.add_argument('files', nargs='*', + help='Kconfig files') + parser.add_argument('--includes', '-d', nargs='*', + help='Extra paths for recursively searching Kconfig files. (for example $IDF_PATH)', + type=valid_directory) + parser.add_argument('--exclude-submodules', action='store_true', + help='Exclude submodules') args = parser.parse_args() - # IGNORE_DIRS makes sense when the required directory is IDF_PATH - check_ignore_dirs = default_path is not None and os.path.abspath(args.directory) == os.path.abspath(default_path) + success_counter = 0 + failure_counter = 0 + ignore_counter = 0 - ignores = 0 - files_to_check = [] deprecated_options = set() - ret = 0 ignore_dirs = IGNORE_DIRS if args.exclude_submodules: - for submodule in get_submodule_dirs(): + for submodule in get_submodule_dirs(full_path=True): ignore_dirs = ignore_dirs + tuple(submodule) - for root, dirnames, filenames in os.walk(args.directory): - for filename in filenames: - full_path = os.path.join(root, filename) - path_in_idf = os.path.relpath(full_path, args.directory) + files = [os.path.abspath(file_path) for file_path in args.files] - if filename.startswith(FILES_TO_CHECK): - if check_ignore_dirs and path_in_idf.startswith(ignore_dirs): - print('{}: Ignored'.format(path_in_idf)) - ignores += 1 - continue - files_to_check.append(full_path) - elif filename == 'sdkconfig.rename': - deprecated_options |= _parse_path(full_path) + if args.includes: + for directory in args.includes: + for root, dirnames, filenames in os.walk(directory): + for filename in filenames: + full_path = os.path.join(root, filename) + if filename.startswith(FILES_TO_CHECK): + files.append(full_path) + elif filename == 'sdkconfig.rename': + deprecated_options |= _parse_path(full_path) - for path in files_to_check: - used_options = _parse_path(path, '=') + for full_path in files: + if full_path.startswith(ignore_dirs): + print('{}: Ignored'.format(full_path)) + ignore_counter += 1 + continue + used_options = _parse_path(full_path, '=') used_deprecated_options = deprecated_options & used_options if len(used_deprecated_options) > 0: - print('{}: The following options are deprecated: {}'.format(path, ', '.join(used_deprecated_options))) - ret = 1 + print('{}: The following options are deprecated: {}' + .format(full_path, ', '.join(used_deprecated_options))) + failure_counter += 1 + else: + print('{}: OK'.format(full_path)) + success_counter += 1 - if ignores > 0: - print('{} files have been ignored.'.format(ignores)) - return ret + if ignore_counter > 0: + print('{} files have been ignored.'.format(ignore_counter)) + if success_counter > 0: + print('{} files have been successfully checked.'.format(success_counter)) + if failure_counter > 0: + print('{} files have errors. Please take a look at the log.'.format(failure_counter)) + return 1 + + if not files: + print('WARNING: no files specified. Please specify files or use ' + '"--includes" to search Kconfig files recursively') + return 0 if __name__ == "__main__": diff --git a/tools/ci/check_kconfigs.py b/tools/ci/check_kconfigs.py index 0f2b504ed9..a12a11f186 100755 --- a/tools/ci/check_kconfigs.py +++ b/tools/ci/check_kconfigs.py @@ -16,12 +16,14 @@ from __future__ import print_function from __future__ import unicode_literals -import os -import sys -import re + import argparse +import os +import re +import sys from io import open -from idf_ci_utils import get_submodule_dirs + +from idf_ci_utils import get_submodule_dirs, IDF_PATH # regular expression for matching Kconfig files RE_KCONFIG = r'^Kconfig(\.projbuild)?(\.in)?$' @@ -34,10 +36,10 @@ OUTPUT_SUFFIX = '.new' # accepts tuples but no lists. IGNORE_DIRS = ( # Kconfigs from submodules need to be ignored: - os.path.join('components', 'mqtt', 'esp-mqtt'), + os.path.join(IDF_PATH, 'components', 'mqtt', 'esp-mqtt'), # Test Kconfigs are also ignored - os.path.join('tools', 'ldgen', 'test', 'data'), - os.path.join('tools', 'kconfig_new', 'test'), + os.path.join(IDF_PATH, 'tools', 'ldgen', 'test', 'data'), + os.path.join(IDF_PATH, 'tools', 'kconfig_new', 'test'), ) SPACES_PER_INDENT = 4 @@ -283,8 +285,8 @@ class IndentAndNameChecker(BaseChecker): common_prefix_len = len(common_prefix) if common_prefix_len < self.min_prefix_length: raise InputError(self.path_in_idf, line_number, - 'The common prefix for the config names of the menu ending at this line is "{}". ' - 'All config names in this menu should start with the same prefix of {} characters ' + 'The common prefix for the config names of the menu ending at this line is "{}".\n' + '\tAll config names in this menu should start with the same prefix of {} characters ' 'or more.'.format(common_prefix, self.min_prefix_length), line) # no suggested correction for this if len(self.prefix_stack) > 0: @@ -371,84 +373,105 @@ def valid_directory(path): return path -def main(): - default_path = os.getenv('IDF_PATH', None) +def validate_kconfig_file(kconfig_full_path, verbose=False): # type: (str, bool) -> bool + suggestions_full_path = kconfig_full_path + OUTPUT_SUFFIX + fail = False + with open(kconfig_full_path, 'r', encoding='utf-8') as f, \ + open(suggestions_full_path, 'w', encoding='utf-8', newline='\n') as f_o, \ + LineRuleChecker(kconfig_full_path) as line_checker, \ + SourceChecker(kconfig_full_path) as source_checker, \ + IndentAndNameChecker(kconfig_full_path, debug=verbose) as indent_and_name_checker: + try: + for line_number, line in enumerate(f, start=1): + try: + for checker in [line_checker, indent_and_name_checker, source_checker]: + checker.process_line(line, line_number) + # The line is correct therefore we echo it to the output file + f_o.write(line) + except InputError as e: + print(e) + fail = True + f_o.write(e.suggested_line) + except UnicodeDecodeError: + raise ValueError("The encoding of {} is not Unicode.".format(kconfig_full_path)) + + if fail: + print('\t{} has been saved with suggestions for resolving the issues.\n' + '\tPlease note that the suggestions can be wrong and ' + 'you might need to re-run the checker several times ' + 'for solving all issues'.format(suggestions_full_path)) + print('\tPlease fix the errors and run {} for checking the correctness of ' + 'Kconfig files.'.format(os.path.abspath(__file__))) + return False + else: + print('{}: OK'.format(kconfig_full_path)) + try: + os.remove(suggestions_full_path) + except Exception: + # not a serious error is when the file cannot be deleted + print('{} cannot be deleted!'.format(suggestions_full_path)) + finally: + return True + + +def main(): parser = argparse.ArgumentParser(description='Kconfig style checker') - parser.add_argument('--verbose', '-v', help='Print more information (useful for debugging)', - action='store_true', default=False) - parser.add_argument('--directory', '-d', help='Path to directory where Kconfigs should be recursively checked ' - '(for example $IDF_PATH)', - type=valid_directory, - required=default_path is None, - default=default_path) - parser.add_argument('--exclude-submodules', action='store_true', help='Exclude submodules') + parser.add_argument('files', nargs='*', + help='Kconfig files') + parser.add_argument('--verbose', '-v', action='store_true', + help='Print more information (useful for debugging)') + parser.add_argument('--includes', '-d', nargs='*', + help='Extra paths for recursively searching Kconfig files. (for example $IDF_PATH)', + type=valid_directory) + parser.add_argument('--exclude-submodules', action='store_true', + help='Exclude submodules') args = parser.parse_args() success_counter = 0 + failure_counter = 0 ignore_counter = 0 - failure = False ignore_dirs = IGNORE_DIRS if args.exclude_submodules: - for submodule in get_submodule_dirs(): - ignore_dirs = ignore_dirs + tuple(submodule) + ignore_dirs = ignore_dirs + tuple(get_submodule_dirs(full_path=True)) - # IGNORE_DIRS makes sense when the required directory is IDF_PATH - check_ignore_dirs = default_path is not None and os.path.abspath(args.directory) == os.path.abspath(default_path) + files = [os.path.abspath(file_path) for file_path in args.files] - for root, dirnames, filenames in os.walk(args.directory): - for filename in filenames: - full_path = os.path.join(root, filename) - path_in_idf = os.path.relpath(full_path, args.directory) - if re.search(RE_KCONFIG, filename): - if check_ignore_dirs and path_in_idf.startswith(ignore_dirs): - print('{}: Ignored'.format(path_in_idf)) - ignore_counter += 1 - continue - suggestions_full_path = full_path + OUTPUT_SUFFIX - with open(full_path, 'r', encoding='utf-8') as f, \ - open(suggestions_full_path, 'w', encoding='utf-8', newline='\n') as f_o, \ - LineRuleChecker(path_in_idf) as line_checker, \ - SourceChecker(path_in_idf) as source_checker, \ - IndentAndNameChecker(path_in_idf, debug=args.verbose) as indent_and_name_checker: - try: - for line_number, line in enumerate(f, start=1): - try: - for checker in [line_checker, indent_and_name_checker, source_checker]: - checker.process_line(line, line_number) - # The line is correct therefore we echo it to the output file - f_o.write(line) - except InputError as e: - print(e) - failure = True - f_o.write(e.suggested_line) - except UnicodeDecodeError: - raise ValueError("The encoding of {} is not Unicode.".format(path_in_idf)) + if args.includes: + for directory in args.includes: + for root, dirnames, filenames in os.walk(directory): + for filename in filenames: + full_path = os.path.join(root, filename) + if re.search(RE_KCONFIG, filename): + files.append(full_path) + elif re.search(RE_KCONFIG, filename, re.IGNORECASE): + # On Windows Kconfig files are working with different cases! + print('{}: Incorrect filename. The case should be "Kconfig"!'.format(full_path)) + failure_counter += 1 - if failure: - print('{} has been saved with suggestions for resolving the issues. Please note that the ' - 'suggestions can be wrong and you might need to re-run the checker several times ' - 'for solving all issues'.format(path_in_idf + OUTPUT_SUFFIX)) - print('Please fix the errors and run {} for checking the correctness of ' - 'Kconfigs.'.format(os.path.relpath(os.path.abspath(__file__), args.directory))) - return 1 - else: - success_counter += 1 - print('{}: OK'.format(path_in_idf)) - try: - os.remove(suggestions_full_path) - except Exception: - # not a serious error is when the file cannot be deleted - print('{} cannot be deleted!'.format(suggestions_full_path)) - elif re.search(RE_KCONFIG, filename, re.IGNORECASE): - # On Windows Kconfig files are working with different cases! - raise ValueError('Incorrect filename of {}. The case should be "Kconfig"!'.format(path_in_idf)) + for full_path in files: + if full_path.startswith(ignore_dirs): + print('{}: Ignored'.format(full_path)) + ignore_counter += 1 + continue + is_valid = validate_kconfig_file(full_path, args.verbose) + if is_valid: + success_counter += 1 + else: + failure_counter += 1 if ignore_counter > 0: print('{} files have been ignored.'.format(ignore_counter)) if success_counter > 0: print('{} files have been successfully checked.'.format(success_counter)) + if failure_counter > 0: + print('{} files have errors. Please take a look at the log.'.format(failure_counter)) + return 1 + + if not files: + print('WARNING: no files specified. Please specify files or use ' + '"--includes" to search Kconfig files recursively') return 0 diff --git a/tools/ci/idf_ci_utils.py b/tools/ci/idf_ci_utils.py index e76a9ce23a..4522454b1b 100644 --- a/tools/ci/idf_ci_utils.py +++ b/tools/ci/idf_ci_utils.py @@ -20,10 +20,10 @@ import logging import os import subprocess -IDF_PATH = os.getenv('IDF_PATH', os.getcwd()) +IDF_PATH = os.getenv('IDF_PATH', os.path.join(os.path.dirname(__file__), '..', '..')) -def get_submodule_dirs(): # type: () -> list +def get_submodule_dirs(full_path=False): # type: (bool) -> list """ To avoid issue could be introduced by multi-os or additional dependency, we use python and git to get this output @@ -36,7 +36,10 @@ def get_submodule_dirs(): # type: () -> list '--get-regexp', 'path']).decode('utf8').strip().split('\n') for line in lines: _, path = line.split(' ') - dirs.append(path) + if full_path: + dirs.append(os.path.join(IDF_PATH, path)) + else: + dirs.append(path) except Exception as e: logging.warning(str(e))