Skip to content

Commit

Permalink
Makefile: Add a pylint checker to the build
Browse files Browse the repository at this point in the history
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 <[email protected]>
[trini: Re-generate pylint.base]
  • Loading branch information
sjg20 authored and trini committed Jan 24, 2022
1 parent c761cf7 commit feafc61
Show file tree
Hide file tree
Showing 5 changed files with 351 additions and 1 deletion.
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,7 @@ __pycache__

# Python code coverage output (python3-coverage html)
/htmlcov/

# pylint files
/pylint.cur
/pylint.out/
45 changes: 44 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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'
Expand Down
8 changes: 8 additions & 0 deletions doc/develop/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,11 @@ Refactoring
checkpatch
coccinelle
moveconfig

Code quality
------------

.. toctree::
:maxdepth: 1

python_cq
80 changes: 80 additions & 0 deletions doc/develop/python_cq.rst
Original file line number Diff line number Diff line change
@@ -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/
215 changes: 215 additions & 0 deletions scripts/pylint.base
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit feafc61

Please sign in to comment.