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

[Ricardo Martincoski <ricardo.martincoski@gmail.com>]

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>
---
 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]
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot