From feafc61ea66c1f1f36aadda7d36a63814f086a4e Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 21 Nov 2021 20:48:40 -0700 Subject: [PATCH] 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] --- .gitignore | 4 + Makefile | 45 +++++++- doc/develop/index.rst | 8 ++ doc/develop/python_cq.rst | 80 ++++++++++++++ scripts/pylint.base | 215 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 351 insertions(+), 1 deletion(-) create mode 100644 doc/develop/python_cq.rst create mode 100644 scripts/pylint.base diff --git a/.gitignore b/.gitignore index 35034de655..28c439f09f 100644 --- a/.gitignore +++ b/.gitignore @@ -98,3 +98,7 @@ __pycache__ # Python code coverage output (python3-coverage html) /htmlcov/ + +# pylint files +/pylint.cur +/pylint.out/ diff --git a/Makefile b/Makefile index b08bad4873..ebc99d24f2 100644 --- a/Makefile +++ b/Makefile @@ -521,7 +521,7 @@ env_h := include/generated/environment.h no-dot-config-targets := clean clobber mrproper distclean \ help %docs check% coccicheck \ - ubootversion backup tests check qcheck tcheck + ubootversion backup tests check qcheck tcheck pylint config-targets := 0 mixed-targets := 0 @@ -2257,6 +2257,48 @@ distclean: mrproper -type f -print | xargs rm -f @rm -f boards.cfg CHANGELOG +# See doc/develop/python_cq.rst +PHONY += pylint +PYLINT_BASE := scripts/pylint.base +PYLINT_CUR := pylint.cur +PYLINT_DIFF := pylint.diff +pylint: + $(Q)echo "Running pylint on all files (summary in $(PYLINT_CUR); output in pylint.out/)" + $(Q)mkdir -p pylint.out + $(Q)rm -f pylint.out/out* + $(Q)find tools test -name "*.py" \ + | xargs -n1 -P$(shell nproc 2>/dev/null || echo 1) \ + sh -c 'pylint --reports=y --exit-zero -f parseable --ignore-imports=yes $$@ > pylint.out/$$(echo $$@ | tr / _ | sed s/.py//)' _ + $(Q)sed -n 's/Your code has been rated at \([-0-9.]*\).*/\1/p; s/\*\** Module \(.*\)/\1/p' pylint.out/* \ + |sed '$!N;s/\n/ /' \ + |sort > $(PYLINT_CUR) + $(Q)base=$$(mktemp) cur=$$(mktemp); cut -d' ' -f1 $(PYLINT_BASE) >$$base; \ + cut -d' ' -f1 $(PYLINT_CUR) >$$cur; \ + comm -3 $$base $$cur > $(PYLINT_DIFF); \ + if [ -s $(PYLINT_DIFF) ]; then \ + echo "Files have been added/removed. Try:\n\tcp $(PYLINT_CUR) $(PYLINT_BASE)"; \ + echo; \ + echo "Added files:"; \ + comm -13 $$base $$cur; \ + echo; \ + echo "Removed files:"; \ + comm -23 $$base $$cur; \ + false; \ + else \ + rm $$base $$cur $(PYLINT_DIFF); \ + fi + $(Q)bad=false; while read base_file base_val <&3 && read cur_file cur_val <&4; do \ + if awk "BEGIN {exit !($$cur_val < $$base_val)}"; then \ + echo "$$base_file: Score was $$base_val, now $$cur_val"; \ + bad=true; fi; \ + done 3<$(PYLINT_BASE) 4<$(PYLINT_CUR); \ + if $$bad; then \ + echo "Some files have regressed, please fix"; \ + false; \ + else \ + echo "No pylint regressions"; \ + fi + backup: F=`basename $(srctree)` ; cd .. ; \ gtar --force-local -zcvf `LC_ALL=C date "+$$F-%Y-%m-%d-%T.tar.gz"` $$F @@ -2275,6 +2317,7 @@ help: @echo ' check - Run all automated tests that use sandbox' @echo ' qcheck - Run quick automated tests that use sandbox' @echo ' tcheck - Run quick automated tests on tools' + @echo ' pylint - Run pylint on all Python files' @echo '' @echo 'Other generic targets:' @echo ' all - Build all necessary images depending on configuration' diff --git a/doc/develop/index.rst b/doc/develop/index.rst index c84b10ea88..97148875ef 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 0000000000..3f99f1d9c0 --- /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/ diff --git a/scripts/pylint.base b/scripts/pylint.base new file mode 100644 index 0000000000..cefdc23555 --- /dev/null +++ b/scripts/pylint.base @@ -0,0 +1,215 @@ +_testing 0.83 +atf_bl31 -6.00 +atf_fip 0.29 +binman.cbfs_util 8.38 +binman.cbfs_util_test 9.30 +binman.cmdline 9.09 +binman.control 4.92 +binman.elf 6.73 +binman.elf_test 5.41 +binman.entry 3.38 +binman.entry_test 5.34 +binman.fdt_test 3.23 +binman.fip_util 9.86 +binman.fip_util_test 9.75 +binman.fmap_util 6.88 +binman.ftest 7.46 +binman.image 7.05 +binman.image_test 4.48 +binman.main 5.00 +binman.setup 5.00 +binman.state 4.15 +blob -1.58 +blob_dtb -10.00 +blob_ext -19.09 +blob_ext_list -0.32 +blob_named_by_arg -7.78 +blob_phase -5.00 +buildman.board 7.82 +buildman.bsettings 1.71 +buildman.builder 6.91 +buildman.builderthread 7.39 +buildman.cmdline 9.04 +buildman.control 8.10 +buildman.func_test 7.18 +buildman.kconfiglib 7.49 +buildman.main 1.43 +buildman.test 6.17 +buildman.toolchain 6.55 +capsule_defs 5.00 +cbfs -1.44 +collection 2.67 +concurrencytest 7.26 +conftest -3.29 +conftest 1.88 +conftest 5.13 +conftest 6.56 +cros_ec_rw -6.00 +defs 6.67 +dtoc.dtb_platdata 7.90 +dtoc.fdt 4.50 +dtoc.fdt_util 6.70 +dtoc.main 7.78 +dtoc.setup 5.00 +dtoc.src_scan 8.91 +dtoc.test_dtoc 8.56 +dtoc.test_fdt 6.96 +dtoc.test_src_scan 9.43 +efivar 6.71 +endian-swap 9.29 +fdtmap -3.28 +files -7.43 +fill -6.43 +fit 5.32 +fmap -0.29 +fstest_defs 8.33 +fstest_helpers 4.29 +gbb -0.30 +genboardscfg 7.95 +image_header 5.77 +intel_cmc -12.50 +intel_descriptor 4.62 +intel_fit 0.00 +intel_fit_ptr 2.35 +intel_fsp -12.50 +intel_fsp_m -12.50 +intel_fsp_s -12.50 +intel_fsp_t -12.50 +intel_ifwi 2.71 +intel_me -12.50 +intel_mrc -10.00 +intel_refcode -10.00 +intel_vbt -12.50 +intel_vga -12.50 +microcode-tool 7.25 +mkimage 2.57 +moveconfig 8.32 +multiplexed_log 7.49 +opensbi -6.00 +patman 0.00 +patman.checkpatch 8.04 +patman.command 4.74 +patman.commit 3.25 +patman.control 8.14 +patman.cros_subprocess 7.56 +patman.func_test 8.14 +patman.get_maintainer 6.47 +patman.gitutil 5.62 +patman.main 8.23 +patman.patchstream 9.11 +patman.project 6.67 +patman.series 6.16 +patman.settings 5.89 +patman.setup 5.00 +patman.status 8.62 +patman.terminal 7.05 +patman.test_checkpatch 6.81 +patman.test_util 6.89 +patman.tools 4.31 +patman.tout 3.12 +powerpc_mpc85xx_bootpg_resetvec -10.00 +rkmux 6.90 +rmboard 7.76 +scp -6.00 +section 4.68 +sqfs_common 8.41 +test 8.18 +test_000_version 7.50 +test_ab 6.50 +test_abootimg 6.09 +test_authvar 8.93 +test_avb 5.52 +test_basic 0.60 +test_bind -2.99 +test_button 3.33 +test_capsule_firmware 3.89 +test_dfu 5.45 +test_dm 9.52 +test_efi_fit 8.16 +test_efi_loader 7.38 +test_efi_selftest 6.36 +test_env 7.15 +test_ext 0.00 +test_extension 2.14 +test_fit 6.83 +test_fit_ecdsa 7.94 +test_fit_hashes 7.70 +test_fpga 1.81 +test_fs_cmd 8.00 +test_gpio 6.09 +test_gpt 7.67 +test_handoff 5.00 +test_help 5.00 +test_hush_if_test 9.27 +test_log 8.64 +test_lsblk 8.00 +test_md 3.64 +test_mkdir 1.96 +test_mmc_rd 6.05 +test_mmc_wr 3.33 +test_net 6.84 +test_ofplatdata 5.71 +test_part 8.00 +test_pinmux 3.27 +test_pstore 2.31 +test_qfw 8.75 +test_sandbox_exit 6.50 +test_scp03 3.33 +test_sf 7.13 +test_shell_basics 9.58 +test_signed 8.38 +test_signed_intca 8.10 +test_sleep 7.78 +test_spl 2.22 +test_sqfs_load 7.46 +test_sqfs_ls 8.00 +test_stackprotector 5.71 +test_symlink 1.22 +test_tpm2 8.51 +test_ums 6.32 +test_unknown_cmd 5.00 +test_unlink 2.78 +test_unsigned 8.00 +test_ut 7.06 +test_vboot 6.00 +text -0.48 +u_boot -15.71 +u_boot_console_base 7.08 +u_boot_console_exec_attach 9.23 +u_boot_console_sandbox 8.06 +u_boot_dtb -12.22 +u_boot_dtb_with_ucode 0.39 +u_boot_elf -8.42 +u_boot_env 0.74 +u_boot_expanded -10.00 +u_boot_img -15.71 +u_boot_nodtb -15.71 +u_boot_spawn 7.65 +u_boot_spl -10.91 +u_boot_spl_bss_pad -9.29 +u_boot_spl_dtb -12.22 +u_boot_spl_elf -15.71 +u_boot_spl_expanded -9.09 +u_boot_spl_nodtb -10.91 +u_boot_spl_with_ucode_ptr -5.00 +u_boot_tpl -10.91 +u_boot_tpl_bss_pad -9.29 +u_boot_tpl_dtb -12.22 +u_boot_tpl_dtb_with_ucode -7.50 +u_boot_tpl_elf -15.71 +u_boot_tpl_expanded -9.09 +u_boot_tpl_nodtb -10.91 +u_boot_tpl_with_ucode_ptr -20.83 +u_boot_ucode 1.52 +u_boot_utils 6.94 +u_boot_with_ucode_ptr -0.71 +vblock -1.61 +vboot_evil 8.95 +vboot_forge 9.22 +x86_reset16 -15.71 +x86_reset16_spl -15.71 +x86_reset16_tpl -15.71 +x86_start16 -15.71 +x86_start16_spl -15.71 +x86_start16_tpl -15.71 +zynqmp_pm_cfg_obj_convert 6.67 -- 2.39.5