| Message ID | 20190519164417.20124-1-yann.morin.1998@free.fr |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | core: make it possible to check flake8 like we check package | expand |
Hello Yann, On Sun, 19 May 2019 18:44:17 +0200, "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > Move the code to run check-flake8 into the Makefile, like we > have for check-package, so that it is easy to run locally (and > not wait for someone to report a failure from their Gitlab > pipelines). > > Since there is no "post-rule" in Makefiles, we resort to using > a little trick to remove the temporary file. Also, there is a > slight change in behaviour: the list of files is not reported, > and the number of processed files is not reported either (there > is little value in that; all that is important are the errors, > if any). > > Regenerate .gitlab-ci.yml. > > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> > Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com> > Cc: Arnout Vandecappelle <arnout@mind.be> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > --- > .gitlab-ci.yml | 9 +-------- > .gitlab-ci.yml.in | 9 +-------- > Makefile | 9 ++++++++- > 3 files changed, 10 insertions(+), 17 deletions(-) > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > index 836944faf5..002ee07afb 100644 > --- a/.gitlab-ci.yml > +++ b/.gitlab-ci.yml > @@ -21,15 +21,8 @@ check-DEVELOPERS: > > check-flake8: > extends: .check_base > - before_script: > - # Help flake8 to find the Python files without .py extension. > - - find * -type f -name '*.py' > files.txt > - - find * -type f -print0 | xargs -0 file | grep 'Python script' | cut -d':' -f1 >> files.txt > - - sort -u files.txt | tee files.processed > script: > - - python -m flake8 --statistics --count --max-line-length=132 $(cat files.processed) > - after_script: > - - wc -l files.processed > + - make check-flake8 > > check-gitlab-ci.yml: > extends: .check_base > diff --git a/.gitlab-ci.yml.in b/.gitlab-ci.yml.in > index 33cb665d98..afa0d93500 100644 > --- a/.gitlab-ci.yml.in > +++ b/.gitlab-ci.yml.in > @@ -21,15 +21,8 @@ check-DEVELOPERS: > > check-flake8: > extends: .check_base > - before_script: > - # Help flake8 to find the Python files without .py extension. > - - find * -type f -name '*.py' > files.txt > - - find * -type f -print0 | xargs -0 file | grep 'Python script' | cut -d':' -f1 >> files.txt > - - sort -u files.txt | tee files.processed > script: > - - python -m flake8 --statistics --count --max-line-length=132 $(cat files.processed) > - after_script: > - - wc -l files.processed > + - make check-flake8 > > check-gitlab-ci.yml: > extends: .check_base > diff --git a/Makefile b/Makefile > index 7246376cf9..f78e5aad4d 100644 > --- a/Makefile > +++ b/Makefile > @@ -119,7 +119,7 @@ export BR2_VERSION_FULL := $(BR2_VERSION)$(shell $(TOPDIR)/support/scripts/setlo > noconfig_targets := menuconfig nconfig gconfig xconfig config oldconfig randconfig \ > defconfig %_defconfig allyesconfig allnoconfig alldefconfig syncconfig release \ > randpackageconfig allyespackageconfig allnopackageconfig \ > - print-version olddefconfig distclean manual manual-% check-package > + print-version olddefconfig distclean manual manual-% check-package check-flake8 > > # Some global targets do not trigger a build, but are used to collect > # metadata, or do various checks. When such targets are triggered, > @@ -1197,6 +1197,13 @@ release: > print-version: > @echo $(BR2_VERSION_FULL) > > +check-flake8: > + { find * -type f -name '*.py'; \ > + find * -type f -print0 | xargs -0 file | grep 'Python script' | cut -d':' -f1; \ > + } >files.txt > + $(Q)python -m flake8 --statistics --count --max-line-length=132 \ > + $$(sort -u files.txt; rm -f files.txt) > + > check-package: > find $(TOPDIR) -type f \( -name '*.mk' -o -name '*.hash' -o -name 'Config.*' \) \ > -exec ./utils/check-package {} + Did a local run with 'make check-flake8', produced 7636 warnings ;-) , you can add my Tested-by: Peter Seiderer <ps.report@gmx.net> Regards, Peter
Hello Yann, On Mon, 20 May 2019 19:34:16 +0200, Peter Seiderer <ps.report@gmx.net> wrote: > > Did a local run with 'make check-flake8', produced 7636 warnings ;-) , you can add my Most warnings where from the download/dl directory, without only 96 warnings in support and utils are left: support/testing/infra/__init__.py:91:29: W605 invalid escape sequence '\[' support/testing/infra/__init__.py:91:67: W605 invalid escape sequence '\]' support/testing/tests/core/test_timezone.py:7:9: E117 over-indented utils/check-package:48:40: W605 invalid escape sequence '\.' utils/check-package:48:42: W605 invalid escape sequence '\S' [...] utils/checkpackagelib/lib_config.py:65:48: W605 invalid escape sequence '\S' utils/checkpackagelib/lib_mk.py:22:28: W605 invalid escape sequence '\s' [...] utils/checkpackagelib/lib_mk.py:78:50: W605 invalid escape sequence '\+' utils/checkpackagelib/lib_mk.py:78:57: W605 invalid escape sequence '\s' utils/checkpackagelib/lib_mk.py:78:60: W605 invalid escape sequence '\$' utils/checkpackagelib/lib_mk.py:78:62: W605 invalid escape sequence '\(' utils/checkpackagelib/lib_mk.py:78:67: W605 invalid escape sequence '\)' [...] utils/checkpackagelib/lib_patch.py:13:32: W605 invalid escape sequence '\d' utils/checkpackagelib/lib_patch.py:23:42: W605 invalid escape sequence '\s' utils/checkpackagelib/lib_patch.py:23:45: W605 invalid escape sequence '\[' utils/checkpackagelib/lib_patch.py:23:52: W605 invalid escape sequence '\s' utils/checkpackagelib/lib_patch.py:23:55: W605 invalid escape sequence '\d' utils/checkpackagelib/lib_patch.py:23:59: W605 invalid escape sequence '\d' utils/checkpackagelib/lib_patch.py:23:62: W605 invalid escape sequence '\]' utils/checkpackagelib/lib.py:56:1: E302 expected 2 blank lines, found 1 utils/getdeveloperlib.py:12:36: W605 invalid escape sequence '\+' utils/getdeveloperlib.py:12:38: W605 invalid escape sequence '\$' utils/getdeveloperlib.py:12:40: W605 invalid escape sequence '\(' utils/getdeveloperlib.py:12:47: W605 invalid escape sequence '\$' utils/getdeveloperlib.py:12:49: W605 invalid escape sequence '\(' utils/getdeveloperlib.py:12:74: W605 invalid escape sequence '\)' utils/getdeveloperlib.py:12:76: W605 invalid escape sequence '\)' utils/getdeveloperlib.py:35:33: W605 invalid escape sequence '\$' utils/getdeveloperlib.py:35:35: W605 invalid escape sequence '\(' utils/getdeveloperlib.py:35:42: W605 invalid escape sequence '\$' utils/getdeveloperlib.py:35:44: W605 invalid escape sequence '\(' utils/getdeveloperlib.py:35:69: W605 invalid escape sequence '\)' utils/getdeveloperlib.py:35:71: W605 invalid escape sequence '\)' utils/getdeveloperlib.py:128:32: W605 invalid escape sequence '\s' utils/getdeveloperlib.py:142:41: W605 invalid escape sequence '\.' utils/getdeveloperlib.py:142:45: W605 invalid escape sequence '\.' utils/scanpypi:99:22: W605 invalid escape sequence '\w' utils/scanpypi:340:37: W605 invalid escape sequence '\w' utils/scanpypi:454:60: W605 invalid escape sequence '\(' utils/scanpypi:454:64: W605 invalid escape sequence '\)' Regards, Peter > > Tested-by: Peter Seiderer <ps.report@gmx.net> > > Regards, > Peter > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Peter, All, On 2019-05-20 20:03 +0200, Peter Seiderer spake thusly: > On Mon, 20 May 2019 19:34:16 +0200, Peter Seiderer <ps.report@gmx.net> wrote: > > Did a local run with 'make check-flake8', produced 7636 warnings ;-) , you can add my > Most warnings where from the download/dl directory, Aha! ;-) > without only 96 > warnings in support and utils are left: > > support/testing/infra/__init__.py:91:29: W605 invalid escape sequence '\[' Yes, like me you are running a pretty recent distribution, that has pycodestyle 2.4.0 (or more recent), which introduced a new warning, W605. I started fixing it locally, but I am not totally sure: 1- if we really need to fix it: the warning is about python3, while quite a few reported errors apply to python2 code. 2- how we should fix it. The obvious solution seems to prefix all such strings with a leading 'r', like so: r"the\.pattern\sstrings". However, doing so will change the behaviour, for example, of check-package, which starts reportign new issues with our mk files (more on that in a later mail...) However, this very patch is indeed about _calling_ flake8 from the Makefile instead of only from a gitlab pipeline. Thanks for the testing! :-) Regards, Yann E. MORIN. > support/testing/infra/__init__.py:91:67: W605 invalid escape sequence '\]' > support/testing/tests/core/test_timezone.py:7:9: E117 over-indented > utils/check-package:48:40: W605 invalid escape sequence '\.' > utils/check-package:48:42: W605 invalid escape sequence '\S' > > [...] > > utils/checkpackagelib/lib_config.py:65:48: W605 invalid escape sequence '\S' > utils/checkpackagelib/lib_mk.py:22:28: W605 invalid escape sequence '\s' > > [...] > > utils/checkpackagelib/lib_mk.py:78:50: W605 invalid escape sequence '\+' > utils/checkpackagelib/lib_mk.py:78:57: W605 invalid escape sequence '\s' > utils/checkpackagelib/lib_mk.py:78:60: W605 invalid escape sequence '\$' > utils/checkpackagelib/lib_mk.py:78:62: W605 invalid escape sequence '\(' > utils/checkpackagelib/lib_mk.py:78:67: W605 invalid escape sequence '\)' > > [...] > > utils/checkpackagelib/lib_patch.py:13:32: W605 invalid escape sequence '\d' > utils/checkpackagelib/lib_patch.py:23:42: W605 invalid escape sequence '\s' > utils/checkpackagelib/lib_patch.py:23:45: W605 invalid escape sequence '\[' > utils/checkpackagelib/lib_patch.py:23:52: W605 invalid escape sequence '\s' > utils/checkpackagelib/lib_patch.py:23:55: W605 invalid escape sequence '\d' > utils/checkpackagelib/lib_patch.py:23:59: W605 invalid escape sequence '\d' > utils/checkpackagelib/lib_patch.py:23:62: W605 invalid escape sequence '\]' > utils/checkpackagelib/lib.py:56:1: E302 expected 2 blank lines, found 1 > utils/getdeveloperlib.py:12:36: W605 invalid escape sequence '\+' > utils/getdeveloperlib.py:12:38: W605 invalid escape sequence '\$' > utils/getdeveloperlib.py:12:40: W605 invalid escape sequence '\(' > utils/getdeveloperlib.py:12:47: W605 invalid escape sequence '\$' > utils/getdeveloperlib.py:12:49: W605 invalid escape sequence '\(' > utils/getdeveloperlib.py:12:74: W605 invalid escape sequence '\)' > utils/getdeveloperlib.py:12:76: W605 invalid escape sequence '\)' > utils/getdeveloperlib.py:35:33: W605 invalid escape sequence '\$' > utils/getdeveloperlib.py:35:35: W605 invalid escape sequence '\(' > utils/getdeveloperlib.py:35:42: W605 invalid escape sequence '\$' > utils/getdeveloperlib.py:35:44: W605 invalid escape sequence '\(' > utils/getdeveloperlib.py:35:69: W605 invalid escape sequence '\)' > utils/getdeveloperlib.py:35:71: W605 invalid escape sequence '\)' > utils/getdeveloperlib.py:128:32: W605 invalid escape sequence '\s' > utils/getdeveloperlib.py:142:41: W605 invalid escape sequence '\.' > utils/getdeveloperlib.py:142:45: W605 invalid escape sequence '\.' > utils/scanpypi:99:22: W605 invalid escape sequence '\w' > utils/scanpypi:340:37: W605 invalid escape sequence '\w' > utils/scanpypi:454:60: W605 invalid escape sequence '\(' > utils/scanpypi:454:64: W605 invalid escape sequence '\)' > > Regards, > Peter > > > > > Tested-by: Peter Seiderer <ps.report@gmx.net> > > > > Regards, > > Peter > > _______________________________________________ > > buildroot mailing list > > buildroot@busybox.net > > http://lists.busybox.net/mailman/listinfo/buildroot > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: > Move the code to run check-flake8 into the Makefile, like we > have for check-package, so that it is easy to run locally (and > not wait for someone to report a failure from their Gitlab > pipelines). > Since there is no "post-rule" in Makefiles, we resort to using > a little trick to remove the temporary file. Also, there is a > slight change in behaviour: the list of files is not reported, > and the number of processed files is not reported either (there > is little value in that; all that is important are the errors, > if any). > Regenerate .gitlab-ci.yml. .. > +check-flake8: > + { find * -type f -name '*.py'; \ > + find * -type f -print0 | xargs -0 file | grep 'Python script' | cut -d':' -f1; \ > + } >files.txt > + $(Q)python -m flake8 --statistics --count --max-line-length=132 \ > + $$(sort -u files.txt; rm -f files.txt) > + I know we had this already like this, but it would be nice to make the following improvements: - Use git ls-tree -r --name-only HEAD instead of find so it doesn't check external files in output/build - No need for 2x find commands, file output for a .py file is presumably also 'Python script' (and if not, then it probably isn't a python file) - A temporary file in the toplevel directory isn't really nice, and it doesn't seem to be needed - Why not pipe the list of python scripts to sort -u | xargs python -m flake8 ..
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 836944faf5..002ee07afb 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -21,15 +21,8 @@ check-DEVELOPERS: check-flake8: extends: .check_base - before_script: - # Help flake8 to find the Python files without .py extension. - - find * -type f -name '*.py' > files.txt - - find * -type f -print0 | xargs -0 file | grep 'Python script' | cut -d':' -f1 >> files.txt - - sort -u files.txt | tee files.processed script: - - python -m flake8 --statistics --count --max-line-length=132 $(cat files.processed) - after_script: - - wc -l files.processed + - make check-flake8 check-gitlab-ci.yml: extends: .check_base diff --git a/.gitlab-ci.yml.in b/.gitlab-ci.yml.in index 33cb665d98..afa0d93500 100644 --- a/.gitlab-ci.yml.in +++ b/.gitlab-ci.yml.in @@ -21,15 +21,8 @@ check-DEVELOPERS: check-flake8: extends: .check_base - before_script: - # Help flake8 to find the Python files without .py extension. - - find * -type f -name '*.py' > files.txt - - find * -type f -print0 | xargs -0 file | grep 'Python script' | cut -d':' -f1 >> files.txt - - sort -u files.txt | tee files.processed script: - - python -m flake8 --statistics --count --max-line-length=132 $(cat files.processed) - after_script: - - wc -l files.processed + - make check-flake8 check-gitlab-ci.yml: extends: .check_base diff --git a/Makefile b/Makefile index 7246376cf9..f78e5aad4d 100644 --- a/Makefile +++ b/Makefile @@ -119,7 +119,7 @@ export BR2_VERSION_FULL := $(BR2_VERSION)$(shell $(TOPDIR)/support/scripts/setlo noconfig_targets := menuconfig nconfig gconfig xconfig config oldconfig randconfig \ defconfig %_defconfig allyesconfig allnoconfig alldefconfig syncconfig release \ randpackageconfig allyespackageconfig allnopackageconfig \ - print-version olddefconfig distclean manual manual-% check-package + print-version olddefconfig distclean manual manual-% check-package check-flake8 # Some global targets do not trigger a build, but are used to collect # metadata, or do various checks. When such targets are triggered, @@ -1197,6 +1197,13 @@ release: print-version: @echo $(BR2_VERSION_FULL) +check-flake8: + { find * -type f -name '*.py'; \ + find * -type f -print0 | xargs -0 file | grep 'Python script' | cut -d':' -f1; \ + } >files.txt + $(Q)python -m flake8 --statistics --count --max-line-length=132 \ + $$(sort -u files.txt; rm -f files.txt) + check-package: find $(TOPDIR) -type f \( -name '*.mk' -o -name '*.hash' -o -name 'Config.*' \) \ -exec ./utils/check-package {} +
Move the code to run check-flake8 into the Makefile, like we have for check-package, so that it is easy to run locally (and not wait for someone to report a failure from their Gitlab pipelines). Since there is no "post-rule" in Makefiles, we resort to using a little trick to remove the temporary file. Also, there is a slight change in behaviour: the list of files is not reported, and the number of processed files is not reported either (there is little value in that; all that is important are the errors, if any). Regenerate .gitlab-ci.yml. Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com> Cc: Arnout Vandecappelle <arnout@mind.be> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> --- .gitlab-ci.yml | 9 +-------- .gitlab-ci.yml.in | 9 +-------- Makefile | 9 ++++++++- 3 files changed, 10 insertions(+), 17 deletions(-)