Re: [Buildroot] [PATCH 14/16] Makefile: merge check-flake8 into check-package

[Arnout Vandecappelle <arnout@mind.be>]

On 24/07/2022 07:49, Ricardo Martincoski wrote:
> Teach check-package to detect python files by type and check them using
> flake8.
> Do not use subprocess to call 'python3 -m flake8' in order to avoid too
> many spawned shells, which in its turn would slow down the check for
> multiple files. (make check-package takes twice the time using a shell
> for each flake8 call, when compared of importing the main application)
> 
> Expand the runtime test and the unit tests for check-package.
> 
> Since 'make check-package' always run using the docker image, there is
> no dependency added to the host machine.
> 
> Remove check-flake8 from the makefile and also from the GitLab CI
> because the exact same checks become part of check-package.
> 
> Suggested-by:: Arnout Vandecappelle <arnout@mind.be>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>

  Applied to master, thanks.

  Regards,
  Arnout

> ---
>   Makefile                                      | 11 ++------
>   support/misc/gitlab-ci.yml.in                 |  4 ---
>   support/scripts/generate-gitlab-ci-yml        |  2 +-
>   .../tests/utils/br2-external/utils/x-python   |  2 ++
>   .../testing/tests/utils/test_check_package.py | 17 +++++++++++
>   utils/check-package                           |  3 ++
>   utils/checkpackagelib/lib_python.py           |  1 +
>   utils/checkpackagelib/test_tool.py            | 28 +++++++++++++++++++
>   utils/checkpackagelib/tool.py                 | 15 ++++++++++
>   9 files changed, 69 insertions(+), 14 deletions(-)
>   create mode 100644 support/testing/tests/utils/br2-external/utils/x-python
>   create mode 100644 utils/checkpackagelib/lib_python.py
> 
> diff --git a/Makefile b/Makefile
> index f42dc3151d..7c7b67a616 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -125,7 +125,7 @@ endif
>   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 check-flake8
> +	print-version olddefconfig distclean manual manual-% check-package
>   
>   # Some global targets do not trigger a build, but are used to collect
>   # metadata, or do various checks. When such targets are triggered,
> @@ -1240,19 +1240,12 @@ print-version:
>   check_inside_docker := $(shell if [ "`groups`" = 'br-user' ]; then echo y; else echo n; fi)
>   
>   # List of target that need to run inside docker image to ensure reproducible results
> -inside_docker_targets := check-package check-flake8
> +inside_docker_targets := check-package
>   
>   ifeq ($(check_inside_docker),n)
>   $(inside_docker_targets):
>   	$(Q)utils/docker-run $(MAKE) V=$(V) $@
>   else
> -check-flake8:
> -	$(Q)git ls-tree -r --name-only HEAD \
> -	| xargs file \
> -	| grep 'Python script' \
> -	| cut -d':' -f1 \
> -	| xargs -- python3 -m flake8 --statistics
> -
>   check-package:
>   	$(Q)./utils/check-package `git ls-tree -r --name-only HEAD`
>   endif
> diff --git a/support/misc/gitlab-ci.yml.in b/support/misc/gitlab-ci.yml.in
> index 3ac988a519..2fde904006 100644
> --- a/support/misc/gitlab-ci.yml.in
> +++ b/support/misc/gitlab-ci.yml.in
> @@ -6,10 +6,6 @@
>       script:
>           - utils/get-developers -v
>   
> -.check-flake8_base:
> -    script:
> -        - make check-flake8
> -
>   .check-package_base:
>       script:
>           - make check-package
> diff --git a/support/scripts/generate-gitlab-ci-yml b/support/scripts/generate-gitlab-ci-yml
> index aa43aac019..536ae0f042 100755
> --- a/support/scripts/generate-gitlab-ci-yml
> +++ b/support/scripts/generate-gitlab-ci-yml
> @@ -26,7 +26,7 @@ gen_tests() {
>       local do_basics do_defconfigs do_runtime do_testpkg
>       local defconfigs_ext cfg tst
>   
> -    basics=( check-package DEVELOPERS flake8 package )
> +    basics=( check-package DEVELOPERS package )
>   
>       defconfigs=( $(cd configs; LC_ALL=C ls -1 *_defconfig) )
>   
> diff --git a/support/testing/tests/utils/br2-external/utils/x-python b/support/testing/tests/utils/br2-external/utils/x-python
> new file mode 100644
> index 0000000000..63f77b6be1
> --- /dev/null
> +++ b/support/testing/tests/utils/br2-external/utils/x-python
> @@ -0,0 +1,2 @@
> +#!/usr/bin/env python3
> +
> diff --git a/support/testing/tests/utils/test_check_package.py b/support/testing/tests/utils/test_check_package.py
> index aeda0857e3..e655bff1ec 100644
> --- a/support/testing/tests/utils/test_check_package.py
> +++ b/support/testing/tests/utils/test_check_package.py
> @@ -249,3 +249,20 @@ class TestCheckPackage(unittest.TestCase):
>           self.assert_file_was_processed(m)
>           self.assert_warnings_generated_for_file(m)
>           self.assertIn("{}:0: run 'shellcheck' and fix the warnings".format(abs_file), w)
> +
> +        # python scripts are tested using flake8
> +        rel_file = "utils/x-python"
> +        abs_path = infra.filepath("tests/utils/br2-external")
> +        abs_file = os.path.join(abs_path, rel_file)
> +
> +        w, m = call_script(["check-package", "-vvv", "-b", rel_file],
> +                           self.WITH_UTILS_IN_PATH, abs_path)
> +        self.assert_file_was_processed(m)
> +        self.assert_warnings_generated_for_file(m)
> +        self.assertIn("{}:0: run 'flake8' and fix the warnings".format(rel_file), w)
> +
> +        w, m = call_script(["check-package", "-b", abs_file],
> +                           self.WITH_UTILS_IN_PATH, infra.basepath())
> +        self.assert_file_was_processed(m)
> +        self.assert_warnings_generated_for_file(m)
> +        self.assertIn("{}:0: run 'flake8' and fix the warnings".format(abs_file), w)
> diff --git a/utils/check-package b/utils/check-package
> index 601d899d3d..beebef0ddb 100755
> --- a/utils/check-package
> +++ b/utils/check-package
> @@ -14,6 +14,7 @@ import checkpackagelib.lib_config
>   import checkpackagelib.lib_hash
>   import checkpackagelib.lib_mk
>   import checkpackagelib.lib_patch
> +import checkpackagelib.lib_python
>   import checkpackagelib.lib_shellscript
>   import checkpackagelib.lib_sysv
>   
> @@ -94,6 +95,8 @@ def get_lib_from_filetype(fname):
>       filetype = magic.from_file(fname, mime=True)
>       if filetype == "text/x-shellscript":
>           return checkpackagelib.lib_shellscript
> +    if filetype in ["text/x-python", "text/x-script.python"]:
> +        return checkpackagelib.lib_python
>       return None
>   
>   
> diff --git a/utils/checkpackagelib/lib_python.py b/utils/checkpackagelib/lib_python.py
> new file mode 100644
> index 0000000000..f8c17ddc10
> --- /dev/null
> +++ b/utils/checkpackagelib/lib_python.py
> @@ -0,0 +1 @@
> +from checkpackagelib.tool import Flake8                # noqa: F401
> diff --git a/utils/checkpackagelib/test_tool.py b/utils/checkpackagelib/test_tool.py
> index a0bf88001d..cfa826f57c 100644
> --- a/utils/checkpackagelib/test_tool.py
> +++ b/utils/checkpackagelib/test_tool.py
> @@ -66,6 +66,34 @@ def test_NotExecutable_hint(testname, hint, filename, permissions, string, expec
>       assert warnings == expected
>   
>   
> +Flake8 = [
> +    ('empty',
> +     'empty.py',
> +     '',
> +     []),
> +    ('W391',
> +     'blank-line.py',
> +     '\n',
> +     ["dir/blank-line.py:0: run 'flake8' and fix the warnings",
> +      "dir/blank-line.py:1:1: W391 blank line at end of file"]),
> +    ('more than one warning',
> +     'file',
> +     'import os\n'
> +     'import re\n'
> +     '\n',
> +     ["dir/file:0: run 'flake8' and fix the warnings",
> +      "dir/file:1:1: F401 'os' imported but unused\n"
> +      "dir/file:2:1: F401 're' imported but unused\n"
> +      'dir/file:3:1: W391 blank line at end of file']),
> +    ]
> +
> +
> +@pytest.mark.parametrize('testname,filename,string,expected', Flake8)
> +def test_Flake8(testname, filename, string, expected):
> +    warnings = check_file(m.Flake8, filename, string)
> +    assert warnings == expected
> +
> +
>   Shellcheck = [
>       ('missing shebang',
>        'empty.sh',
> diff --git a/utils/checkpackagelib/tool.py b/utils/checkpackagelib/tool.py
> index 632aaa9f9e..907ada704f 100644
> --- a/utils/checkpackagelib/tool.py
> +++ b/utils/checkpackagelib/tool.py
> @@ -1,5 +1,7 @@
> +import flake8.main.application
>   import os
>   import subprocess
> +import tempfile
>   from checkpackagelib.base import _Tool
>   
>   
> @@ -14,6 +16,19 @@ class NotExecutable(_Tool):
>               return ["{}:0: This file does not need to be executable{}".format(self.filename, self.hint())]
>   
>   
> +class Flake8(_Tool):
> +    def run(self):
> +        with tempfile.NamedTemporaryFile() as output:
> +            app = flake8.main.application.Application()
> +            app.run(['--output-file={}'.format(output.name), self.filename])
> +            stdout = output.readlines()
> +            processed_output = [str(line.decode().rstrip()) for line in stdout if line]
> +            if len(stdout) == 0:
> +                return
> +            return ["{}:0: run 'flake8' and fix the warnings".format(self.filename),
> +                    '\n'.join(processed_output)]
> +
> +
>   class Shellcheck(_Tool):
>       def run(self):
>           cmd = ['shellcheck', self.filename]
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot