* [PATCH 1/6] .gitignore: Ignore any html coverage directory
2021-11-22 3:48 [PATCH 0/6] Add a pylint check Simon Glass
@ 2021-11-22 3:48 ` Simon Glass
2022-01-25 15:56 ` Tom Rini
2021-11-22 3:48 ` [PATCH 2/6] dtoc: Fix up a code comment that confuses pylint Simon Glass
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-11-22 3:48 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Heinrich Schuchardt, Simon Glass
This is created when checking code coverage of Python tools. Ignore it.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
.gitignore | 3 +++
1 file changed, 3 insertions(+)
diff --git a/.gitignore b/.gitignore
index e66aa864da6..35034de6556 100644
--- a/.gitignore
+++ b/.gitignore
@@ -95,3 +95,6 @@ GTAGS
# Python cache
__pycache__
+
+# Python code coverage output (python3-coverage html)
+/htmlcov/
--
2.34.0.rc2.393.gf8c9666880-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 2/6] dtoc: Fix up a code comment that confuses pylint
2021-11-22 3:48 [PATCH 0/6] Add a pylint check Simon Glass
2021-11-22 3:48 ` [PATCH 1/6] .gitignore: Ignore any html coverage directory Simon Glass
@ 2021-11-22 3:48 ` Simon Glass
2021-11-22 6:45 ` Heinrich Schuchardt
2022-01-25 15:56 ` Tom Rini
2021-11-22 3:48 ` [PATCH 3/6] patman: Update the list of modules Simon Glass
` (3 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Simon Glass @ 2021-11-22 3:48 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Heinrich Schuchardt, Simon Glass
This produces a pylint error at present. Fix it.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/dtoc/dtb_platdata.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
index 3bb5c680f2e..a69a7889ce1 100644
--- a/tools/dtoc/dtb_platdata.py
+++ b/tools/dtoc/dtb_platdata.py
@@ -72,7 +72,7 @@ class Ftype(IntEnum):
# This holds information about each type of output file dtoc can create
-# type: Type of file (Ftype)
+# ftype: Type of file (Ftype)
# fname: Filename excluding directory, e.g. 'dt-plat.c'
# hdr_comment: Comment explaining the purpose of the file
OutputFile = collections.namedtuple('OutputFile',
--
2.34.0.rc2.393.gf8c9666880-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 2/6] dtoc: Fix up a code comment that confuses pylint
2021-11-22 3:48 ` [PATCH 2/6] dtoc: Fix up a code comment that confuses pylint Simon Glass
@ 2021-11-22 6:45 ` Heinrich Schuchardt
2022-01-25 15:56 ` Tom Rini
1 sibling, 0 replies; 18+ messages in thread
From: Heinrich Schuchardt @ 2021-11-22 6:45 UTC (permalink / raw)
To: Simon Glass; +Cc: Tom Rini, U-Boot Mailing List
On 11/22/21 04:48, Simon Glass wrote:
> This produces a pylint error at present. Fix it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> tools/dtoc/dtb_platdata.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
> index 3bb5c680f2e..a69a7889ce1 100644
> --- a/tools/dtoc/dtb_platdata.py
> +++ b/tools/dtoc/dtb_platdata.py
> @@ -72,7 +72,7 @@ class Ftype(IntEnum):
>
>
> # This holds information about each type of output file dtoc can create
> -# type: Type of file (Ftype)
> +# ftype: Type of file (Ftype)
Please, remove now superfluous ' (Ftype)'.
Best regards
Heinrich
> # fname: Filename excluding directory, e.g. 'dt-plat.c'
> # hdr_comment: Comment explaining the purpose of the file
> OutputFile = collections.namedtuple('OutputFile',
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 2/6] dtoc: Fix up a code comment that confuses pylint
2021-11-22 3:48 ` [PATCH 2/6] dtoc: Fix up a code comment that confuses pylint Simon Glass
2021-11-22 6:45 ` Heinrich Schuchardt
@ 2022-01-25 15:56 ` Tom Rini
1 sibling, 0 replies; 18+ messages in thread
From: Tom Rini @ 2022-01-25 15:56 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List, Heinrich Schuchardt
[-- Attachment #1: Type: text/plain, Size: 218 bytes --]
On Sun, Nov 21, 2021 at 08:48:37PM -0700, Simon Glass wrote:
> This produces a pylint error at present. Fix it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
Applied to u-boot/master, thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/6] patman: Update the list of modules
2021-11-22 3:48 [PATCH 0/6] Add a pylint check Simon Glass
2021-11-22 3:48 ` [PATCH 1/6] .gitignore: Ignore any html coverage directory Simon Glass
2021-11-22 3:48 ` [PATCH 2/6] dtoc: Fix up a code comment that confuses pylint Simon Glass
@ 2021-11-22 3:48 ` Simon Glass
2022-01-25 15:56 ` Tom Rini
2021-11-22 3:48 ` [PATCH 4/6] tools: Add init files for Python tools Simon Glass
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-11-22 3:48 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Heinrich Schuchardt, Simon Glass
Update the __init__ file to include recently added files.
Add a license header while we are here.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/__init__.py | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/patman/__init__.py b/tools/patman/__init__.py
index 7cbe5fa4b0c..c9d3e350529 100644
--- a/tools/patman/__init__.py
+++ b/tools/patman/__init__.py
@@ -1,3 +1,6 @@
-__all__ = ['checkpatch', 'command', 'commit', 'cros_subprocess',
- 'get_maintainer', 'gitutil', 'patchstream', 'project',
- 'series', 'settings', 'terminal', 'test']
+# SPDX-License-Identifier: GPL-2.0+
+
+__all__ = ['checkpatch', 'command', 'commit', 'control', 'cros_subprocess',
+ 'func_test', 'get_maintainer', 'gitutil', 'main', 'patchstream',
+ 'project', 'series', 'setup', 'settings', 'terminal',
+ 'test_checkpatch', 'test_util', 'tools', 'tout']
--
2.34.0.rc2.393.gf8c9666880-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 4/6] tools: Add init files for Python tools
2021-11-22 3:48 [PATCH 0/6] Add a pylint check Simon Glass
` (2 preceding siblings ...)
2021-11-22 3:48 ` [PATCH 3/6] patman: Update the list of modules Simon Glass
@ 2021-11-22 3:48 ` Simon Glass
2022-01-25 15:56 ` Tom Rini
2021-11-22 3:48 ` [PATCH 5/6] Makefile: Add a pylint checker to the build Simon Glass
2021-11-22 3:48 ` [PATCH 6/6] Azure/GitLab CI: Add the pylint checker Simon Glass
5 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-11-22 3:48 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Heinrich Schuchardt, Simon Glass
Add some empty __init__ files for binman, buildman and dtoc so that
pylint is able to recognise these as Python modules and produce more
useful pylint output.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/binman/__init__.py | 0
tools/buildman/__init__.py | 0
tools/dtoc/__init__.py | 0
3 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 tools/binman/__init__.py
create mode 100644 tools/buildman/__init__.py
create mode 100644 tools/dtoc/__init__.py
diff --git a/tools/binman/__init__.py b/tools/binman/__init__.py
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/tools/buildman/__init__.py b/tools/buildman/__init__.py
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/tools/dtoc/__init__.py b/tools/dtoc/__init__.py
new file mode 100644
index 00000000000..e69de29bb2d
--
2.34.0.rc2.393.gf8c9666880-goog
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 5/6] Makefile: Add a pylint checker to the build
2021-11-22 3:48 [PATCH 0/6] Add a pylint check Simon Glass
` (3 preceding siblings ...)
2021-11-22 3:48 ` [PATCH 4/6] tools: Add init files for Python tools Simon Glass
@ 2021-11-22 3:48 ` Simon Glass
2021-11-22 8:05 ` Heinrich Schuchardt
` (2 more replies)
2021-11-22 3:48 ` [PATCH 6/6] Azure/GitLab CI: Add the pylint checker Simon Glass
5 siblings, 3 replies; 18+ messages in thread
From: Simon Glass @ 2021-11-22 3:48 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Tom Rini, Heinrich Schuchardt, Simon Glass, Marek Behún,
Masahiro Yamada, Pali Rohár
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 <sjg@chromium.org>
---
.gitignore | 4 +
Makefile | 45 +++++++-
doc/develop/index.rst | 8 ++
doc/develop/python_cq.rst | 80 +++++++++++++++
scripts/pylint.base | 211 ++++++++++++++++++++++++++++++++++++++
5 files changed, 347 insertions(+), 1 deletion(-)
create mode 100644 doc/develop/python_cq.rst
create mode 100644 scripts/pylint.base
diff --git a/.gitignore b/.gitignore
index 35034de6556..28c439f09fd 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 338ae3341e6..ef2b0a853ea 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
@@ -2239,6 +2239,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
@@ -2257,6 +2299,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 9592d193fca..97a7f4ce14a 100644
--- a/doc/develop/index.rst
+++ b/doc/develop/index.rst
@@ -61,3 +61,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/
diff --git a/scripts/pylint.base b/scripts/pylint.base
new file mode 100644
index 00000000000..d848ebe9058
--- /dev/null
+++ b/scripts/pylint.base
@@ -0,0 +1,211 @@
+_testing 0.83
+atf_bl31 -6.00
+binman.cbfs_util 7.70
+binman.cbfs_util_test 9.19
+binman.cmdline 8.87
+binman.control 4.39
+binman.elf 6.42
+binman.elf_test 5.41
+binman.entry 2.39
+binman.entry_test 5.29
+binman.fdt_test 3.23
+binman.fmap_util 6.67
+binman.ftest 7.38
+binman.image 6.39
+binman.image_test 4.48
+binman.main 4.26
+binman.setup 5.00
+binman.state 3.30
+blob -1.94
+blob_dtb -10.00
+blob_ext -20.00
+blob_named_by_arg -7.78
+blob_phase -5.00
+buildman.board 7.11
+buildman.bsettings 0.98
+buildman.builder 6.55
+buildman.builderthread 7.35
+buildman.cmdline 8.85
+buildman.control 7.04
+buildman.func_test 6.38
+buildman.kconfiglib 7.48
+buildman.main 1.43
+buildman.test 6.10
+buildman.toolchain 5.81
+capsule_defs 5.00
+cbfs -1.52
+collection 2.33
+concurrencytest 6.77
+conftest -3.84
+conftest 1.25
+conftest 4.62
+conftest 6.43
+cros_ec_rw -6.00
+defs 6.67
+dtoc.dtb_platdata 7.82
+dtoc.fdt 3.47
+dtoc.fdt_util 4.53
+dtoc.main 7.33
+dtoc.setup 5.00
+dtoc.src_scan 8.75
+dtoc.test_dtoc 8.54
+dtoc.test_fdt 6.92
+dtoc.test_src_scan 9.43
+efivar 6.71
+endian-swap 8.93
+fdtmap -4.00
+files -7.43
+fill -6.43
+fit 5.32
+fmap -0.59
+fstest_defs 8.33
+fstest_helpers 4.29
+gbb -0.30
+genboardscfg 7.27
+image_header 5.58
+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.19
+mkimage 2.07
+moveconfig 7.31
+multiplexed_log 7.01
+opensbi -6.00
+patman 0.00
+patman.checkpatch 7.61
+patman.command 4.23
+patman.commit 2.75
+patman.control 8.14
+patman.cros_subprocess 7.41
+patman.func_test 7.87
+patman.get_maintainer 4.71
+patman.gitutil 4.55
+patman.main 8.23
+patman.patchstream 9.04
+patman.project 3.33
+patman.series 5.95
+patman.settings 5.63
+patman.setup 5.00
+patman.status 8.43
+patman.terminal 6.29
+patman.test_checkpatch 6.81
+patman.test_util 6.51
+patman.tools 3.47
+patman.tout 2.97
+powerpc_mpc85xx_bootpg_resetvec -10.00
+rkmux 6.76
+rmboard 7.76
+scp -6.00
+section 4.25
+sqfs_common 8.12
+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.40
+test_bind -2.99
+test_button 3.33
+test_capsule_firmware 3.89
+test_dfu 5.45
+test_dm 9.52
+test_efi_fit 7.59
+test_efi_loader 6.79
+test_efi_selftest 6.12
+test_env 6.68
+test_ext -0.25
+test_extension 2.14
+test_fit 6.20
+test_fit_ecdsa 7.50
+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.02
+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.12
+test_sqfs_ls 8.00
+test_stackprotector 5.71
+test_symlink 0.82
+test_tpm2 8.51
+test_ums 6.32
+test_unknown_cmd 5.00
+test_unlink 2.22
+test_unsigned 8.00
+test_ut 7.06
+test_vboot 6.00
+text -0.48
+u_boot -15.71
+u_boot_console_base 6.80
+u_boot_console_exec_attach 8.46
+u_boot_console_sandbox 6.94
+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 6.67
+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 4.69
+u_boot_with_ucode_ptr -0.71
+vblock -1.94
+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.34.0.rc2.393.gf8c9666880-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 5/6] Makefile: Add a pylint checker to the build
2021-11-22 3:48 ` [PATCH 5/6] Makefile: Add a pylint checker to the build Simon Glass
@ 2021-11-22 8:05 ` Heinrich Schuchardt
2021-11-25 0:13 ` Simon Glass
2022-01-25 15:56 ` Tom Rini
2022-01-25 15:57 ` Tom Rini
2 siblings, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2021-11-22 8:05 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Marek Behún, Masahiro Yamada, Pali Rohár,
U-Boot Mailing List
On 11/22/21 04:48, Simon Glass wrote:
> 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 <sjg@chromium.org>
> ---
>
> .gitignore | 4 +
> Makefile | 45 +++++++-
> doc/develop/index.rst | 8 ++
> doc/develop/python_cq.rst | 80 +++++++++++++++
> scripts/pylint.base | 211 ++++++++++++++++++++++++++++++++++++++
> 5 files changed, 347 insertions(+), 1 deletion(-)
> create mode 100644 doc/develop/python_cq.rst
> create mode 100644 scripts/pylint.base
>
> diff --git a/.gitignore b/.gitignore
> index 35034de6556..28c439f09fd 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 338ae3341e6..ef2b0a853ea 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
> @@ -2239,6 +2239,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
> +
This is over-engineered.
./scripts/pylint.base would have to be updated after every patch. Who
will do this?
Deleting superfluous but (according to pylint) correct code lines may
decrease the score. This is not because any new errors where introduced
but because the number of errors per line increases. But such a change
should pass CI.
This needs to be fixed before adding this series.
Your test does not detect new Python units with abysmal code.
We should simply set a minimum score (>= 8) for all units including old
ones.
Best regards
Heinrich
> backup:
> F=`basename $(srctree)` ; cd .. ; \
> gtar --force-local -zcvf `LC_ALL=C date "+$$F-%Y-%m-%d-%T.tar.gz"` $$F
> @@ -2257,6 +2299,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 9592d193fca..97a7f4ce14a 100644
> --- a/doc/develop/index.rst
> +++ b/doc/develop/index.rst
> @@ -61,3 +61,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/
> diff --git a/scripts/pylint.base b/scripts/pylint.base
> new file mode 100644
> index 00000000000..d848ebe9058
> --- /dev/null
> +++ b/scripts/pylint.base
> @@ -0,0 +1,211 @@
> +_testing 0.83
> +atf_bl31 -6.00
> +binman.cbfs_util 7.70
> +binman.cbfs_util_test 9.19
> +binman.cmdline 8.87
> +binman.control 4.39
> +binman.elf 6.42
> +binman.elf_test 5.41
> +binman.entry 2.39
> +binman.entry_test 5.29
> +binman.fdt_test 3.23
> +binman.fmap_util 6.67
> +binman.ftest 7.38
> +binman.image 6.39
> +binman.image_test 4.48
> +binman.main 4.26
> +binman.setup 5.00
> +binman.state 3.30
> +blob -1.94
> +blob_dtb -10.00
> +blob_ext -20.00
> +blob_named_by_arg -7.78
> +blob_phase -5.00
> +buildman.board 7.11
> +buildman.bsettings 0.98
> +buildman.builder 6.55
> +buildman.builderthread 7.35
> +buildman.cmdline 8.85
> +buildman.control 7.04
> +buildman.func_test 6.38
> +buildman.kconfiglib 7.48
> +buildman.main 1.43
> +buildman.test 6.10
> +buildman.toolchain 5.81
> +capsule_defs 5.00
> +cbfs -1.52
> +collection 2.33
> +concurrencytest 6.77
> +conftest -3.84
> +conftest 1.25
> +conftest 4.62
> +conftest 6.43
> +cros_ec_rw -6.00
> +defs 6.67
> +dtoc.dtb_platdata 7.82
> +dtoc.fdt 3.47
> +dtoc.fdt_util 4.53
> +dtoc.main 7.33
> +dtoc.setup 5.00
> +dtoc.src_scan 8.75
> +dtoc.test_dtoc 8.54
> +dtoc.test_fdt 6.92
> +dtoc.test_src_scan 9.43
> +efivar 6.71
> +endian-swap 8.93
> +fdtmap -4.00
> +files -7.43
> +fill -6.43
> +fit 5.32
> +fmap -0.59
> +fstest_defs 8.33
> +fstest_helpers 4.29
> +gbb -0.30
> +genboardscfg 7.27
> +image_header 5.58
> +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.19
> +mkimage 2.07
> +moveconfig 7.31
> +multiplexed_log 7.01
> +opensbi -6.00
> +patman 0.00
> +patman.checkpatch 7.61
> +patman.command 4.23
> +patman.commit 2.75
> +patman.control 8.14
> +patman.cros_subprocess 7.41
> +patman.func_test 7.87
> +patman.get_maintainer 4.71
> +patman.gitutil 4.55
> +patman.main 8.23
> +patman.patchstream 9.04
> +patman.project 3.33
> +patman.series 5.95
> +patman.settings 5.63
> +patman.setup 5.00
> +patman.status 8.43
> +patman.terminal 6.29
> +patman.test_checkpatch 6.81
> +patman.test_util 6.51
> +patman.tools 3.47
> +patman.tout 2.97
> +powerpc_mpc85xx_bootpg_resetvec -10.00
> +rkmux 6.76
> +rmboard 7.76
> +scp -6.00
> +section 4.25
> +sqfs_common 8.12
> +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.40
> +test_bind -2.99
> +test_button 3.33
> +test_capsule_firmware 3.89
> +test_dfu 5.45
> +test_dm 9.52
> +test_efi_fit 7.59
> +test_efi_loader 6.79
> +test_efi_selftest 6.12
> +test_env 6.68
> +test_ext -0.25
> +test_extension 2.14
> +test_fit 6.20
> +test_fit_ecdsa 7.50
> +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.02
> +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.12
> +test_sqfs_ls 8.00
> +test_stackprotector 5.71
> +test_symlink 0.82
> +test_tpm2 8.51
> +test_ums 6.32
> +test_unknown_cmd 5.00
> +test_unlink 2.22
> +test_unsigned 8.00
> +test_ut 7.06
> +test_vboot 6.00
> +text -0.48
> +u_boot -15.71
> +u_boot_console_base 6.80
> +u_boot_console_exec_attach 8.46
> +u_boot_console_sandbox 6.94
> +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 6.67
> +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 4.69
> +u_boot_with_ucode_ptr -0.71
> +vblock -1.94
> +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
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 5/6] Makefile: Add a pylint checker to the build
2021-11-22 8:05 ` Heinrich Schuchardt
@ 2021-11-25 0:13 ` Simon Glass
0 siblings, 0 replies; 18+ messages in thread
From: Simon Glass @ 2021-11-25 0:13 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Marek Behún, Masahiro Yamada, Pali Rohár,
U-Boot Mailing List
Hi Heinrich,
On Mon, 22 Nov 2021 at 01:05, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 11/22/21 04:48, Simon Glass wrote:
> > 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 <sjg@chromium.org>
> > ---
> >
> > .gitignore | 4 +
> > Makefile | 45 +++++++-
> > doc/develop/index.rst | 8 ++
> > doc/develop/python_cq.rst | 80 +++++++++++++++
> > scripts/pylint.base | 211 ++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 347 insertions(+), 1 deletion(-)
> > create mode 100644 doc/develop/python_cq.rst
> > create mode 100644 scripts/pylint.base
> >
> > diff --git a/.gitignore b/.gitignore
> > index 35034de6556..28c439f09fd 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 338ae3341e6..ef2b0a853ea 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
> > @@ -2239,6 +2239,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
> > +
>
> This is over-engineered.
>
> ./scripts/pylint.base would have to be updated after every patch. Who
> will do this?
It doesn't really need to be updated. Areyou suggesting I over-engeer
it further by making ti do that automatically?
>
> Deleting superfluous but (according to pylint) correct code lines may
> decrease the score. This is not because any new errors where introduced
> but because the number of errors per line increases. But such a change
> should pass CI.
>
> This needs to be fixed before adding this series.
Yes that's right, it is a pain. What do you suggest?
The nice thing is that you have to fix a few things when adding new code, right?
>
> Your test does not detect new Python units with abysmal code.
They need to have a line added, which will show the score.
>
> We should simply set a minimum score (>= 8) for all units including old
> ones.
Unfortunately I don't think that will have any effect. The point of
this is to encourage things to be improved. If it just stays at 2.4 no
one will care...?
Regards,
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] Makefile: Add a pylint checker to the build
2021-11-22 3:48 ` [PATCH 5/6] Makefile: Add a pylint checker to the build Simon Glass
2021-11-22 8:05 ` Heinrich Schuchardt
@ 2022-01-25 15:56 ` Tom Rini
2022-01-25 15:57 ` Tom Rini
2 siblings, 0 replies; 18+ messages in thread
From: Tom Rini @ 2022-01-25 15:56 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Heinrich Schuchardt, Marek Behún,
Masahiro Yamada, Pali Rohár
[-- Attachment #1: Type: text/plain, Size: 1702 bytes --]
On Sun, Nov 21, 2021 at 08:48:40PM -0700, Simon Glass wrote:
> 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 <sjg@chromium.org>
> ---
>
> .gitignore | 4 +
> Makefile | 45 +++++++-
> doc/develop/index.rst | 8 ++
> doc/develop/python_cq.rst | 80 +++++++++++++++
> scripts/pylint.base | 211 ++++++++++++++++++++++++++++++++++++++
> 5 files changed, 347 insertions(+), 1 deletion(-)
> create mode 100644 doc/develop/python_cq.rst
> create mode 100644 scripts/pylint.base
So, I'm applying this momentarily. But given the discussion at the
time, I wanted to follow up as well with some commentary. The scoring
mechanism is not perfect and I'm not applying the final part of this to
enable CI yet. In terms of minimum score? I don't know how that would
work, setting aside problems like removing code leads to a possibly
worse ratio. Maybe we need to get some people listed under MAINTAINERS
for python code? I'll be putting pylint in my local testing loop and
that'll at least make me question regressions but arbitrarily taking
"binman.fdt_test 3.23" I can't see what on earth is even related to that
score since there's nothing useful to me under "git grep fdt_test
tools/". But overall I think this at least starts us down the right
track of making it easier to make our python code more generally python
compliant.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] Makefile: Add a pylint checker to the build
2021-11-22 3:48 ` [PATCH 5/6] Makefile: Add a pylint checker to the build Simon Glass
2021-11-22 8:05 ` Heinrich Schuchardt
2022-01-25 15:56 ` Tom Rini
@ 2022-01-25 15:57 ` Tom Rini
2 siblings, 0 replies; 18+ messages in thread
From: Tom Rini @ 2022-01-25 15:57 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Heinrich Schuchardt, Marek Behún,
Masahiro Yamada, Pali Rohár
[-- Attachment #1: Type: text/plain, Size: 465 bytes --]
On Sun, Nov 21, 2021 at 08:48:40PM -0700, Simon Glass wrote:
> 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 <sjg@chromium.org>
Applied to u-boot/master, thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 6/6] Azure/GitLab CI: Add the pylint checker
2021-11-22 3:48 [PATCH 0/6] Add a pylint check Simon Glass
` (4 preceding siblings ...)
2021-11-22 3:48 ` [PATCH 5/6] Makefile: Add a pylint checker to the build Simon Glass
@ 2021-11-22 3:48 ` Simon Glass
2021-11-22 7:40 ` Heinrich Schuchardt
5 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-11-22 3:48 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Tom Rini, Heinrich Schuchardt, Simon Glass, Alper Nebi Yasak,
Bin Meng, Daniel Schwierzeck, Kristian Amlie
Add a check that new Python code does not regress the pylint score for
any module.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
.azure-pipelines.yml | 17 +++++++++++++++++
.gitlab-ci.yml | 12 ++++++++++++
2 files changed, 29 insertions(+)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 8801ff7d81b..6002477c905 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -172,6 +172,23 @@ jobs:
export PATH=/opt/gcc-11.1.0-nolibc/arm-linux-gnueabi/bin:$PATH
test/nokia_rx51_test.sh
+ - job: pylint
+ displayName: Check for any pylint regressions
+ pool:
+ vmImage: $(ubuntu_vm)
+ container:
+ image: $(ci_runner_image)
+ options: $(container_option)
+ steps:
+ - script: |
+ pip install -r test/py/requirements.txt
+ pip install asteval pylint pyopenssl
+ export PATH=${PATH}:~/.local/bin
+ echo "[MASTER]" >> .pylintrc
+ echo "load-plugins=pylint.extensions.docparams" >> .pylintrc
+ pylint --version
+ make pylint
+
- job: test_py
displayName: 'test.py'
pool:
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 4c89daeadcf..226595e04e8 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -180,6 +180,18 @@ Run tests for Nokia RX-51 (aka N900):
- export PATH=/opt/gcc-11.1.0-nolibc/arm-linux-gnueabi/bin:$PATH;
test/nokia_rx51_test.sh
+# Check for any pylint regressions
+Run pylint:
+ stage: testsuites
+ script:
+ - pip install -r test/py/requirements.txt
+ - pip install asteval pylint pyopenssl
+ - export PATH=${PATH}:~/.local/bin
+ - echo "[MASTER]" >> .pylintrc
+ - echo "load-plugins=pylint.extensions.docparams" >> .pylintrc
+ - pylint --version
+ - make pylint
+
# Test sandbox with test.py
sandbox test.py:
variables:
--
2.34.0.rc2.393.gf8c9666880-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 6/6] Azure/GitLab CI: Add the pylint checker
2021-11-22 3:48 ` [PATCH 6/6] Azure/GitLab CI: Add the pylint checker Simon Glass
@ 2021-11-22 7:40 ` Heinrich Schuchardt
2021-11-25 0:13 ` Simon Glass
0 siblings, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2021-11-22 7:40 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Alper Nebi Yasak, Bin Meng, Daniel Schwierzeck,
Kristian Amlie, U-Boot Mailing List
On 11/22/21 04:48, Simon Glass wrote:
> Add a check that new Python code does not regress the pylint score for
> any module.
How will ./scripts/pylint.base be updated?
Some or the worst scores are for binman. Any plans to address these?
Best regards
Heinrich
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] Azure/GitLab CI: Add the pylint checker
2021-11-22 7:40 ` Heinrich Schuchardt
@ 2021-11-25 0:13 ` Simon Glass
0 siblings, 0 replies; 18+ messages in thread
From: Simon Glass @ 2021-11-25 0:13 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Alper Nebi Yasak, Bin Meng, Daniel Schwierzeck,
Kristian Amlie, U-Boot Mailing List
Hi Heinrich,
On Mon, 22 Nov 2021 at 00:40, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 11/22/21 04:48, Simon Glass wrote:
> > Add a check that new Python code does not regress the pylint score for
> > any module.
>
> How will ./scripts/pylint.base be updated?
For the moment people will have to add it to their patches.
There is no automatic 'ratchet up' at present, though, if that is what
you are asking.
>
> Some or the worst scores are for binman. Any plans to address these?
Yes. A big one is changing the function naming to snake case. Another
is that classes override their parent methods and the comments are not
copied from the parent. I'm not sure whether we want to do that or
not.
Regards,
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread