From feafc61ea66c1f1f36aadda7d36a63814f086a4e Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 21 Nov 2021 20:48:40 -0700 Subject: Makefile: Add a pylint checker to the build At present the Python code in U-Boot is somewhat inconsistent, with some files passing pylint quite cleanly and others not. Add a way to track progress on this clean-up, by checking that no module has got any worse as a result of changes. This can be used with 'make pylint'. Signed-off-by: Simon Glass [trini: Re-generate pylint.base] --- doc/develop/index.rst | 8 +++++ doc/develop/python_cq.rst | 80 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 doc/develop/python_cq.rst (limited to 'doc/develop') diff --git a/doc/develop/index.rst b/doc/develop/index.rst index c84b10ea887..97148875ef4 100644 --- a/doc/develop/index.rst +++ b/doc/develop/index.rst @@ -62,3 +62,11 @@ Refactoring checkpatch coccinelle moveconfig + +Code quality +------------ + +.. toctree:: + :maxdepth: 1 + + python_cq diff --git a/doc/develop/python_cq.rst b/doc/develop/python_cq.rst new file mode 100644 index 00000000000..3f99f1d9c0b --- /dev/null +++ b/doc/develop/python_cq.rst @@ -0,0 +1,80 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Python code quality +=================== + +U-Boot has about 60k lines of Python code, mainly in the following areas: + +- tests +- pytest hooks +- patman patch submission tool +- buildman build / analysis tool +- dtoc devicetree-to-C tool +- binman firmware packaging tool + +`PEP 8`_ is used for the code style, with the single quote (') used by default for +strings and double quote for doc strings. All non-trivial functions should be +commented. + +Pylint is used to help check this code and keep a consistent code style. The +build system tracks the current 'score' of the source code and detects +regressions in any module. + +To run this locally you should use this version of pylint:: + + # pylint --version + pylint 2.11.1 + astroid 2.8.6 + Python 3.8.10 (default, Sep 28 2021, 16:10:42) + [GCC 9.3.0] + + +You should be able to select and this install other required tools with:: + + pip install pylint==2.11.1 + pip install -r test/py/requirements.txt + pip install asteval pyopenssl + +Note that if your distribution is a year or two old, you make need to use `pip3` +instead. + +To configure pylint, make sure it has docparams enabled, e.g. with:: + + echo "[MASTER]" >> .pylintrc + echo "load-plugins=pylint.extensions.docparams" >> .pylintrc + +Once everything is ready, use this to check the code:: + + make pylint + +This creates a directory called `pylint.out` which contains the pylint output +for each Python file in U-Boot. It also creates a summary file called +`pylint.cur` which shows the pylint score for each module:: + + _testing 0.83 + atf_bl31 -6.00 + atf_fip 0.49 + binman.cbfs_util 7.70 + binman.cbfs_util_test 9.19 + binman.cmdline 7.73 + binman.control 4.39 + binman.elf 6.42 + binman.elf_test 5.41 + ... + +This file is in alphabetical order. The build system compares the score of each +module to `scripts/pylint.base` (which must also be sorted and have exactly the +same modules in it) and reports any files where the score has dropped. Use +pylint to check what is wrong and fix up the code before you send out your +patches. + +New or removed files results in an error which can be resolved by updating the +`scripts/pylint.base` file to add/remove lines for those files, e.g.:: + + meld pylint.cur scripts/pylint.base + +If the pylint version is updated in CI, this may result in needing to regenerate +`scripts/pylint.base`. + + +.. _`PEP 8`: https://www.python.org/dev/peps/pep-0008/ -- cgit v1.3.1 From 65d7fcec5a05be15694613c6db709ede19bce85b Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 18 Dec 2021 08:09:46 -0700 Subject: moveconfig: Allow querying board configuration It is useful to be able to find out which boards define a particular option, or combination of options. This is not as easy as grepping the defconfig files since many options are implied by others. Add a -f option to the moveconfig tool to permit this. Update the documentation to cover this, including a better title for the doc page. Signed-off-by: Simon Glass --- doc/develop/moveconfig.rst | 25 ++++++++++++-- tools/moveconfig.py | 86 +++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 105 insertions(+), 6 deletions(-) (limited to 'doc/develop') diff --git a/doc/develop/moveconfig.rst b/doc/develop/moveconfig.rst index dcd4d927e40..2f53ea52b71 100644 --- a/doc/develop/moveconfig.rst +++ b/doc/develop/moveconfig.rst @@ -1,7 +1,7 @@ .. SPDX-License-Identifier: GPL-2.0+ -moveconfig -========== +moveconfig - Migrating and querying CONFIG options +================================================== Since Kconfig was introduced to U-Boot, we have worked on moving config options from headers to Kconfig (defconfig). @@ -129,6 +129,24 @@ To process CONFIG_CMD_FPGAD only for a subset of configs based on path match:: ./tools/moveconfig.py -Cy CONFIG_CMD_FPGAD -d - +Finding boards with particular CONFIG combinations +-------------------------------------------------- + +You can use `moveconfig.py` to figure out which boards have a CONFIG enabled, or +which do not. To use it, first build a database:: + + ./tools/moveconfig.py -b + +Then you can run queries using the `-f` flag followed by a list of CONFIG terms. +Each term is CONFIG name, with or without a tilde (~) prefix. The tool searches +for boards which match the CONFIG name, or do not match if tilde is used. For +example, to find boards which enabled CONFIG_SCSI but not CONFIG_BLK:: + + tools/moveconfig.py -f SCSI ~BLK + 3 matches + pg_wcom_seli8_defconfig highbank_defconfig pg_wcom_expu1_defconfig + + Finding implied CONFIGs ----------------------- @@ -235,6 +253,9 @@ Available options Specify a file containing a list of defconfigs to move. The defconfig files can be given with shell-style wildcards. Use '-' to read from stdin. + -f, --find + Find boards with a given config combination + -n, --dry-run Perform a trial run that does not make any changes. It is useful to see what is going to happen before one actually runs it. diff --git a/tools/moveconfig.py b/tools/moveconfig.py index 71a7736ca63..a86c07caa6e 100755 --- a/tools/moveconfig.py +++ b/tools/moveconfig.py @@ -1569,6 +1569,79 @@ def do_imply_config(config_list, add_imply, imply_flags, skip_added, add_imply_rule(config[CONFIG_LEN:], fname, linenum) +def do_find_config(config_list): + """Find boards with a given combination of CONFIGs + + Params: + config_list: List of CONFIG options to check (each a string consisting + of a config option, with or without a CONFIG_ prefix. If an option + is preceded by a tilde (~) then it must be false, otherwise it must + be true) + """ + all_configs, all_defconfigs, config_db, defconfig_db = read_database() + + # Get the whitelist + with open('scripts/config_whitelist.txt') as inf: + adhoc_configs = set(inf.read().splitlines()) + + # Start with all defconfigs + out = all_defconfigs + + # Work through each config in turn + adhoc = [] + for item in config_list: + # Get the real config name and whether we want this config or not + cfg = item + want = True + if cfg[0] == '~': + want = False + cfg = cfg[1:] + + if cfg in adhoc_configs: + adhoc.append(cfg) + continue + + # Search everything that is still in the running. If it has a config + # that we want, or doesn't have one that we don't, add it into the + # running for the next stage + in_list = out + out = set() + for defc in in_list: + has_cfg = cfg in config_db[defc] + if has_cfg == want: + out.add(defc) + if adhoc: + print(f"Error: Not in Kconfig: %s" % ' '.join(adhoc)) + else: + print(f'{len(out)} matches') + print(' '.join(out)) + + +def prefix_config(cfg): + """Prefix a config with CONFIG_ if needed + + This handles ~ operator, which indicates that the CONFIG should be disabled + + >>> prefix_config('FRED') + 'CONFIG_FRED' + >>> prefix_config('CONFIG_FRED') + 'CONFIG_FRED' + >>> prefix_config('~FRED') + '~CONFIG_FRED' + >>> prefix_config('~CONFIG_FRED') + '~CONFIG_FRED' + >>> prefix_config('A123') + 'CONFIG_A123' + """ + op = '' + if cfg[0] == '~': + op = cfg[0] + cfg = cfg[1:] + if not cfg.startswith('CONFIG_'): + cfg = 'CONFIG_' + cfg + return op + cfg + + def main(): try: cpu_count = multiprocessing.cpu_count() @@ -1596,6 +1669,8 @@ def main(): parser.add_option('-e', '--exit-on-error', action='store_true', default=False, help='exit immediately on any error') + parser.add_option('-f', '--find', action='store_true', default=False, + help='Find boards with a given config combination') parser.add_option('-H', '--headers-only', dest='cleanup_headers_only', action='store_true', default=False, help='only cleanup the headers') @@ -1631,13 +1706,12 @@ def main(): unittest.main() if len(configs) == 0 and not any((options.force_sync, options.build_db, - options.imply)): + options.imply, options.find)): parser.print_usage() sys.exit(1) # prefix the option name with CONFIG_ if missing - configs = [ config if config.startswith('CONFIG_') else 'CONFIG_' + config - for config in configs ] + configs = [prefix_config(cfg) for cfg in configs] check_top_directory() @@ -1663,6 +1737,10 @@ def main(): options.skip_added) return + if options.find: + do_find_config(configs) + return + config_db = {} db_queue = queue.Queue() t = DatabaseThread(config_db, db_queue) @@ -1705,4 +1783,4 @@ def main(): fd.write('\n') if __name__ == '__main__': - main() + sys.exit(main()) -- cgit v1.3.1