[Buildroot] [PATCH v3 0/7] fix Python code style v3

[Buildroot] [PATCH v3 0/7] fix Python code style v3

[Ricardo Martincoski]

Hello,

This series fixes code style warnings reported by flake8 in the Python scripts
in the tree. Many patches from v1 were already applied.

I am ignoring this script for now:
support/scripts/xorg-release
I opened a thread about it:
http://lists.busybox.net/pipermail/buildroot/2018-January/211557.html

The first 4 patches fix each script.
I tried to do the minimum to accomplish the task avoiding any unrelated
refactoring, what IMO brings these advantages:
 - small chance of introducing regressions;
 - easy code review;
 - less testing effort.
The commit messages are terse as the changes I introduced are minimal.

Patch 5 adds flake8 to the Docker image, needed by the next patch.

After this patch, the docker image must be built, published, and have its tag
added to .gitlab.yml{,.in}

Patch 6 creates a job in GitLab to run flake8:
https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/57046920

Patch 7 ignores a file from the check.

And here a complete run on GitLab using a preview of the new Docker image:
https://gitlab.com/RicardoMartincoski/buildroot/pipelines/18789118

Regards,
Ricardo
--
Changes v2 -> v3:
  - manually accepted review tags.
  - dockerfile: improve commit log and use best practices.  (suggested by
    Yann E. MORIN)

Changes v1 -> v2:
  - genrandconfig: fix the long lines instead of ignoring them.  (suggested by
    Thomas Petazzoni)
  - size-stats-compare: the fix for E129 made the code less readable, use
    another fix.  (suggested by Thomas Petazzoni)
  - test_python_cryptography: new patch.
  - scanpypi: new patch.
  - dockerfile: install flake8 to the base docker image instead of adding sudo
    to install tools on the fly.  (suggested by Yann E. MORIN)
  - .gitlab-ci.yml: use flake8 pre-installed.  (suggested by Yann E. MORIN)
  - utils/diffconfig: ignore warnings for this file that comes from kernel
    source.  (suggested by Thomas Petazzoni)
--
Ricardo Martincoski (7):
  genrandconfig: fix code style
  size-stats-compare: fix code style
  test_python_cryptography: fix code style
  scanpypi: fix code style
  support/dockerfile: install flake8
  .gitlab-ci.yml: check flake8
  .flake8: ignore utils/diffconfig

 .flake8                                            |  3 ++
 .gitlab-ci.yml                                     | 11 +++++++
 .gitlab-ci.yml.in                                  | 11 +++++++
 support/docker/Dockerfile                          | 11 ++++++-
 .../tests/package/test_python_cryptography.py      | 23 ++++++++------
 utils/genrandconfig                                | 36 ++++++++++++----------
 utils/scanpypi                                     |  4 ++-
 utils/size-stats-compare                           |  7 +++--
 8 files changed, 75 insertions(+), 31 deletions(-)

-- 
2.7.4

[Buildroot] [PATCH v3 1/7] genrandconfig: fix code style

[Ricardo Martincoski]

Fix these warnings:
E201 whitespace after '['
E202 whitespace before ']'
E501 line too long (138 > 132 characters)
 -> isolate the common part of the external toolchain url in a variable
 to make the long lines shorter and more readable.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Changes v2 -> v3:
  - no changes

Changes v1 -> v2:  (suggested by Thomas Petazzoni)
  - fix the long lines instead of ignoring them

Tested using this script:
https://gitlab.com/RicardoMartincoski/buildroot/commit/19edfa388981e656216af7b8b566142161fa6142
---
 utils/genrandconfig | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/utils/genrandconfig b/utils/genrandconfig
index 0d08570..d958664 100755
--- a/utils/genrandconfig
+++ b/utils/genrandconfig
@@ -127,7 +127,7 @@ def get_toolchain_configs(toolchains_csv, buildrootdir):
 
     with open(toolchains_csv) as r:
         # filter empty lines and comments
-        lines = [ t for t in r.readlines() if len(t.strip()) > 0 and t[0] != '#' ]
+        lines = [t for t in r.readlines() if len(t.strip()) > 0 and t[0] != '#']
         toolchains = decode_byte_list(lines)
     configs = []
 
@@ -210,6 +210,8 @@ def fixup_config(configfile):
     with open(configfile) as configf:
         configlines = configf.readlines()
 
+    BR2_TOOLCHAIN_EXTERNAL_URL = 'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/'
+
     if "BR2_NEEDS_HOST_JAVA=y\n" in configlines and not sysinfo.has("java"):
         return False
     if "BR2_NEEDS_HOST_JAVAC=y\n" in configlines and not sysinfo.has("javac"):
@@ -221,36 +223,36 @@ def fixup_config(configfile):
         return False
     # The ctng toolchain is affected by PR58854
     if 'BR2_PACKAGE_LTTNG_TOOLS=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/armv5-ctng-linux-gnueabi.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'armv5-ctng-linux-gnueabi.tar.xz"\n' in configlines:
         return False
     # The ctng toolchain tigger an assembler error with guile package when compiled with -Os (same issue as for CS ARM 2014.05-29)
     if 'BR2_PACKAGE_GUILE=y\n' in configlines and \
        'BR2_OPTIMIZE_S=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/armv5-ctng-linux-gnueabi.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'armv5-ctng-linux-gnueabi.tar.xz"\n' in configlines:
         return False
     # The ctng toolchain is affected by PR58854
     if 'BR2_PACKAGE_LTTNG_TOOLS=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/armv6-ctng-linux-uclibcgnueabi.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'armv6-ctng-linux-uclibcgnueabi.tar.xz"\n' in configlines:
         return False
     # The ctng toolchain is affected by PR58854
     if 'BR2_PACKAGE_LTTNG_TOOLS=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/armv7-ctng-linux-gnueabihf.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'armv7-ctng-linux-gnueabihf.tar.xz"\n' in configlines:
         return False
     # The ctng toolchain is affected by PR60155
     if 'BR2_PACKAGE_SDL=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/powerpc-ctng-linux-uclibc.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'powerpc-ctng-linux-uclibc.tar.xz"\n' in configlines:
         return False
     # The ctng toolchain is affected by PR60155
     if 'BR2_PACKAGE_LIBMPEG2=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/powerpc-ctng-linux-uclibc.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'powerpc-ctng-linux-uclibc.tar.xz"\n' in configlines:
         return False
     # This MIPS toolchain uses eglibc-2.18 which lacks SYS_getdents64
     if 'BR2_PACKAGE_STRONGSWAN=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/mips64el-ctng_n64-linux-gnu.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'mips64el-ctng_n64-linux-gnu.tar.xz"\n' in configlines:
         return False
     # This MIPS toolchain uses eglibc-2.18 which lacks SYS_getdents64
     if 'BR2_PACKAGE_PYTHON3=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/mips64el-ctng_n64-linux-gnu.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'mips64el-ctng_n64-linux-gnu.tar.xz"\n' in configlines:
         return False
     # libffi not available on sh2a and ARMv7-M, but propagating libffi
     # arch dependencies in Buildroot is really too much work, so we
@@ -266,37 +268,37 @@ def fixup_config(configfile):
         configlines.append('BR2_PACKAGE_SUNXI_BOARDS_FEX_FILE="a10/hackberry.fex"\n')
     # This MIPS uClibc toolchain fails to build the gdb package
     if 'BR2_PACKAGE_GDB=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines:
         return False
     # This MIPS uClibc toolchain fails to build the rt-tests package
     if 'BR2_PACKAGE_RT_TESTS=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines:
         return False
     # This MIPS uClibc toolchain fails to build the civetweb package
     if 'BR2_PACKAGE_CIVETWEB=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines:
         return False
     # This MIPS ctng toolchain fails to build the python3 package
     if 'BR2_PACKAGE_PYTHON3=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/mips64el-ctng_n64-linux-gnu.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'mips64el-ctng_n64-linux-gnu.tar.xz"\n' in configlines:
         return False
     # This MIPS uClibc toolchain fails to build the strace package
     if 'BR2_PACKAGE_STRACE=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines:
         return False
     # This MIPS uClibc toolchain fails to build the cdrkit package
     if 'BR2_PACKAGE_CDRKIT=y\n' in configlines and \
        'BR2_STATIC_LIBS=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines:
         return False
     # uClibc vfork static linking issue
     if 'BR2_PACKAGE_ALSA_LIB=y\n' in configlines and \
        'BR2_STATIC_LIBS=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/i486-ctng-linux-uclibc.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'i486-ctng-linux-uclibc.tar.xz"\n' in configlines:
         return False
     # This MIPS uClibc toolchain fails to build the weston package
     if 'BR2_PACKAGE_WESTON=y\n' in configlines and \
-       'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines:
+       BR2_TOOLCHAIN_EXTERNAL_URL + 'mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines:
         return False
     # The cs nios2 2017.02 toolchain is affected by binutils PR19405
     if 'BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII=y\n' in configlines and \
-- 
2.7.4

[Buildroot] [PATCH v3 2/7] size-stats-compare: fix code style

[Ricardo Martincoski]

Fix these warnings:
E129 visually indented line with same indent as next logical line
E302 expected 2 blank lines, found 1

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
Changes v2 -> v3:
  - manually accepted review tag

Changes v1 -> v2:  (suggested by Thomas Petazzoni)
  - the fix for E129 made the code less readable, use another fix.
---
 utils/size-stats-compare | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/utils/size-stats-compare b/utils/size-stats-compare
index e5a1ec3..a3d7f25 100755
--- a/utils/size-stats-compare
+++ b/utils/size-stats-compare
@@ -24,14 +24,15 @@ import csv
 import argparse
 import sys
 
+
 def read_file_size_csv(inputf, detail=None):
     """Extract package or file sizes from CSV file into size dictionary"""
     sizes = {}
     reader = csv.reader(inputf)
 
     header = next(reader)
-    if (header[0] != 'File name' or header[1] != 'Package name' or
-        header[2] != 'File size' or header[3] != 'Package size'):
+    if header[0] != 'File name' or header[1] != 'Package name' or \
+       header[2] != 'File size' or header[3] != 'Package size':
         print(("Input file %s does not contain the expected header. Are you "
                "sure this file corresponds to the file-size-stats.csv "
                "file created by 'make graph-size'?") % inputf.name)
@@ -45,6 +46,7 @@ def read_file_size_csv(inputf, detail=None):
 
     return sizes
 
+
 def compare_sizes(old, new):
     """Return delta/added/removed dictionaries based on two input size
     dictionaries"""
@@ -64,6 +66,7 @@ def compare_sizes(old, new):
 
     return delta
 
+
 def print_results(result, threshold):
     """Print the given result dictionary sorted by size, ignoring any entries
     below or equal to threshold"""
-- 
2.7.4

[Buildroot] [PATCH v3 3/7] test_python_cryptography: fix code style

[Ricardo Martincoski]

Fix these warnings:
E122 continuation line missing indentation or outdented
E301 expected 1 blank line, found 0
E302 expected 2 blank lines, found 1
F401 'os' imported but unused

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
---
Changes v2 -> v3:
  - no changes

Changes v1 -> v2:
  - new patch (v1 of this series was not sent yet during the review of
    http://patchwork.ozlabs.org/patch/811011/ so I asked to fix all in
    the same series.

https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/57046960
https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/57046961
---
 .../tests/package/test_python_cryptography.py      | 23 ++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/support/testing/tests/package/test_python_cryptography.py b/support/testing/tests/package/test_python_cryptography.py
index b60152d..78c3ef5 100644
--- a/support/testing/tests/package/test_python_cryptography.py
+++ b/support/testing/tests/package/test_python_cryptography.py
@@ -1,7 +1,6 @@
-import os
-
 from tests.package.test_python import TestPythonBase
 
+
 class TestPythonCryptography(TestPythonBase):
     def fernet_test(self, timeout=-1):
         cmd = self.interpreter + " -c 'from cryptography.fernet import Fernet;"
@@ -10,22 +9,26 @@ class TestPythonCryptography(TestPythonBase):
         _, exit_code = self.emulator.run(cmd, timeout)
         self.assertEqual(exit_code, 0)
 
+
 class TestPythonPy2Cryptography(TestPythonCryptography):
     config = TestPythonBase.config + \
-"""
-BR2_PACKAGE_PYTHON=y
-BR2_PACKAGE_PYTHON_CRYPTOGRAPHY=y
-"""
+        """
+        BR2_PACKAGE_PYTHON=y
+        BR2_PACKAGE_PYTHON_CRYPTOGRAPHY=y
+        """
+
     def test_run(self):
         self.login()
         self.fernet_test(40)
 
+
 class TestPythonPy3Cryptography(TestPythonCryptography):
     config = TestPythonBase.config + \
-"""
-BR2_PACKAGE_PYTHON3=y
-BR2_PACKAGE_PYTHON_CRYPTOGRAPHY=y
-"""
+        """
+        BR2_PACKAGE_PYTHON3=y
+        BR2_PACKAGE_PYTHON_CRYPTOGRAPHY=y
+        """
+
     def test_run(self):
         self.login()
         self.fernet_test(40)
-- 
2.7.4

[Buildroot] [PATCH v3 4/7] scanpypi: fix code style

[Ricardo Martincoski]

Fix these warnings:
E401 multiple imports on one line

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
---
Changes v2 -> v3:
  - no changes

Changes v1 -> v2:
  - new patch.
---
 utils/scanpypi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/utils/scanpypi b/utils/scanpypi
index b9006ee..505ba0a 100755
--- a/utils/scanpypi
+++ b/utils/scanpypi
@@ -10,7 +10,9 @@ from __future__ import print_function
 from __future__ import absolute_import
 import argparse
 import json
-import six.moves.urllib.request, six.moves.urllib.error, six.moves.urllib.parse
+import six.moves.urllib.request
+import six.moves.urllib.error
+import six.moves.urllib.parse
 import sys
 import os
 import shutil
-- 
2.7.4

[Buildroot] [PATCH v3 5/7] support/dockerfile: install flake8

[Ricardo Martincoski]

Use the latest version of the tool because it is actively maintained.
But use a fixed version of the tool and its dependencies to get stable
results. It can be manually bumped from time to time.

Before installing any Python packages, ensure pip, setuptools, and wheel
are up to date as recommended in the docs [1].

[1] https://packaging.python.org/tutorials/installing-packages/

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Peter Korsgaard <peter@korsgaard.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
---
Changes v2 -> v3:  (suggested by Yann E. MORIN)
  - minimise the number of intermediate layers;
  - explain why install setuptools separately using the latest version
    (I actually just used the exact command line from the docs and
    referenced it in the commit log);
  - use a single package on each line, sorted.

Changes v1 -> v2:  (suggested by Yann E. MORIN)
  - install flake8 to the base docker image instead of adding sudo to
    install tools on the fly.
---
 support/docker/Dockerfile | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
index ce3fdd9..f01ac25 100644
--- a/support/docker/Dockerfile
+++ b/support/docker/Dockerfile
@@ -27,10 +27,19 @@ RUN dpkg --add-architecture i386 && \
         cvs bzr git mercurial subversion wget \
         cpio unzip \
         libncurses5-dev \
-        python-nose2 python-pexpect qemu-system-arm qemu-system-x86 && \
+        python-nose2 python-pexpect qemu-system-arm qemu-system-x86 \
+        python-pip && \
     apt-get -y autoremove && \
     apt-get -y clean
 
+# For check-flake8
+RUN python -m pip install --upgrade pip setuptools wheel && \
+    pip install -q \
+        flake8==3.5.0 \
+        mccabe==0.6.1 \
+        pycodestyle==2.3.1 \
+        pyflakes==1.6.0
+
 # To be able to generate a toolchain with locales, enable one UTF-8 locale
 RUN sed -i 's/# \(en_US.UTF-8\)/\1/' /etc/locale.gen && \
     /usr/sbin/locale-gen
-- 
2.7.4

[Buildroot] [PATCH v3 6/7] .gitlab-ci.yml: check flake8

[Ricardo Martincoski]

Add a test to check Python code style in the whole buildroot tree.

Search files by type in order to help flake8 to find the Python scripts
without .py extension. But don't rely only in the output of 'file' as it
uses heuristics and sometimes it is wrong (specially identifying Python
files as C++ source for the 'file' version currently in the Docker
image).

Include in the output:
 - the list of Python files processed;
 - statistics for each kind of warning;
 - the total number of warnings;
 - the number of Python files processed.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
BEFORE APPLYING, see this note from Yann:
Note: this can only be applied _after_ we regenerate the docker image.
Then we'll have to update the version of the image at the top of the
gitlab-ci.yml file.

Changes v2 -> v3:
  - manually accepted review tag

Changes v1 -> v2:  (suggested by Yann E. MORIN)
  - install flake8 to the base docker image (in the previous patch)

https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/57046920
---
 .gitlab-ci.yml    | 11 +++++++++++
 .gitlab-ci.yml.in | 11 +++++++++++
 2 files changed, 22 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index f44c3a9..9ad4c5f 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -30,6 +30,17 @@ check-DEVELOPERS:
     script:
         - "! utils/get-developers | grep -v 'No action specified'"
 
+check-flake8:
+    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 $(cat files.processed)
+    after_script:
+        - wc -l files.processed
+
 check-package:
     script:
         - find . -type f \( -name '*.mk' -o -name '*.hash' \) -exec ./utils/check-package {} +
diff --git a/.gitlab-ci.yml.in b/.gitlab-ci.yml.in
index cb3eb71..95fc025 100644
--- a/.gitlab-ci.yml.in
+++ b/.gitlab-ci.yml.in
@@ -30,6 +30,17 @@ check-DEVELOPERS:
     script:
         - "! utils/get-developers | grep -v 'No action specified'"
 
+check-flake8:
+    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 $(cat files.processed)
+    after_script:
+        - wc -l files.processed
+
 check-package:
     script:
         - find . -type f \( -name '*.mk' -o -name '*.hash' \) -exec ./utils/check-package {} +
-- 
2.7.4

[Buildroot] [PATCH v3 7/7] .flake8: ignore utils/diffconfig

[Ricardo Martincoski]

This script comes from the kernel source, so ignore any code style
warnings for it in order to keep it as close as possible to the original
one, making synchronization between repos easier.

The option --exclude for flake8/pycodestyle is an absolute list and has
a default, so ideally the default values should be added too.
But the use cases for flake8 in the tree are:
 - when developing a new script or changing an existing one, the
   developer calls flake8 only on that script;
 - in the GitLab job, a list of all Python files to be tested is created
   and then passed to flake8.
None of these involve calling 'flake8' without parameters, so don't care
about adding the default value.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Marcus Folkesson <marcus.folkesson@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Changes v2 -> v3:
  - no changes

Changes v1 -> v2:
  - instead of tweaking even more the script from the kernel source,
    ignore any warnings for it;  (suggested by Thomas Petazzoni)
  - move the patch after the one adding the GitLab job so I can
    reference it in the commit message.
---
 .flake8 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.flake8 b/.flake8
index afdb967..7dd7b54 100644
--- a/.flake8
+++ b/.flake8
@@ -1,2 +1,5 @@
 [flake8]
+exclude=
+    # copied from the kernel sources
+    utils/diffconfig
 max-line-length=132
-- 
2.7.4

[Buildroot] [PATCH v3 4/7] scanpypi: fix code style

[Yegor Yefremov]

On Tue, Mar 13, 2018 at 4:09 AM, Ricardo Martincoski
<ricardo.martincoski@gmail.com> wrote:
> Fix these warnings:
> E401 multiple imports on one line
>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Yegor Yefremov <yegorslists@googlemail.com>

Reviewed-by: Yegor Yefremov <yegorslists@googlemail.com>

> ---
> Changes v2 -> v3:
>   - no changes
>
> Changes v1 -> v2:
>   - new patch.
> ---
>  utils/scanpypi | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/utils/scanpypi b/utils/scanpypi
> index b9006ee..505ba0a 100755
> --- a/utils/scanpypi
> +++ b/utils/scanpypi
> @@ -10,7 +10,9 @@ from __future__ import print_function
>  from __future__ import absolute_import
>  import argparse
>  import json
> -import six.moves.urllib.request, six.moves.urllib.error, six.moves.urllib.parse
> +import six.moves.urllib.request
> +import six.moves.urllib.error
> +import six.moves.urllib.parse
>  import sys
>  import os
>  import shutil
> --
> 2.7.4
>

[Buildroot] [PATCH v3 3/7] test_python_cryptography: fix code style

[Yegor Yefremov]

On Tue, Mar 13, 2018 at 4:09 AM, Ricardo Martincoski
<ricardo.martincoski@gmail.com> wrote:
> Fix these warnings:
> E122 continuation line missing indentation or outdented
> E301 expected 1 blank line, found 0
> E302 expected 2 blank lines, found 1
> F401 'os' imported but unused
>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Yegor Yefremov <yegorslists@googlemail.com>

Reviewed-by: Yegor Yefremov <yegorslists@googlemail.com>

> ---
> Changes v2 -> v3:
>   - no changes
>
> Changes v1 -> v2:
>   - new patch (v1 of this series was not sent yet during the review of
>     http://patchwork.ozlabs.org/patch/811011/ so I asked to fix all in
>     the same series.
>
> https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/57046960
> https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/57046961
> ---
>  .../tests/package/test_python_cryptography.py      | 23 ++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/support/testing/tests/package/test_python_cryptography.py b/support/testing/tests/package/test_python_cryptography.py
> index b60152d..78c3ef5 100644
> --- a/support/testing/tests/package/test_python_cryptography.py
> +++ b/support/testing/tests/package/test_python_cryptography.py
> @@ -1,7 +1,6 @@
> -import os
> -
>  from tests.package.test_python import TestPythonBase
>
> +
>  class TestPythonCryptography(TestPythonBase):
>      def fernet_test(self, timeout=-1):
>          cmd = self.interpreter + " -c 'from cryptography.fernet import Fernet;"
> @@ -10,22 +9,26 @@ class TestPythonCryptography(TestPythonBase):
>          _, exit_code = self.emulator.run(cmd, timeout)
>          self.assertEqual(exit_code, 0)
>
> +
>  class TestPythonPy2Cryptography(TestPythonCryptography):
>      config = TestPythonBase.config + \
> -"""
> -BR2_PACKAGE_PYTHON=y
> -BR2_PACKAGE_PYTHON_CRYPTOGRAPHY=y
> -"""
> +        """
> +        BR2_PACKAGE_PYTHON=y
> +        BR2_PACKAGE_PYTHON_CRYPTOGRAPHY=y
> +        """
> +
>      def test_run(self):
>          self.login()
>          self.fernet_test(40)
>
> +
>  class TestPythonPy3Cryptography(TestPythonCryptography):
>      config = TestPythonBase.config + \
> -"""
> -BR2_PACKAGE_PYTHON3=y
> -BR2_PACKAGE_PYTHON_CRYPTOGRAPHY=y
> -"""
> +        """
> +        BR2_PACKAGE_PYTHON3=y
> +        BR2_PACKAGE_PYTHON_CRYPTOGRAPHY=y
> +        """
> +
>      def test_run(self):
>          self.login()
>          self.fernet_test(40)
> --
> 2.7.4
>

[Buildroot] [PATCH v3 5/7] support/dockerfile: install flake8

[Yann E. MORIN]

Ricardo, All,

On 2018-03-13 00:09 -0300, Ricardo Martincoski spake thusly:
> Use the latest version of the tool because it is actively maintained.
> But use a fixed version of the tool and its dependencies to get stable
> results. It can be manually bumped from time to time.
> 
> Before installing any Python packages, ensure pip, setuptools, and wheel
> are up to date as recommended in the docs [1].
> 
> [1] https://packaging.python.org/tutorials/installing-packages/
> 
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
> Changes v2 -> v3:  (suggested by Yann E. MORIN)
>   - minimise the number of intermediate layers;
>   - explain why install setuptools separately using the latest version
>     (I actually just used the exact command line from the docs and
>     referenced it in the commit log);
>   - use a single package on each line, sorted.
> 
> Changes v1 -> v2:  (suggested by Yann E. MORIN)
>   - install flake8 to the base docker image instead of adding sudo to
>     install tools on the fly.
> ---
>  support/docker/Dockerfile | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
> index ce3fdd9..f01ac25 100644
> --- a/support/docker/Dockerfile
> +++ b/support/docker/Dockerfile
> @@ -27,10 +27,19 @@ RUN dpkg --add-architecture i386 && \
>          cvs bzr git mercurial subversion wget \
>          cpio unzip \
>          libncurses5-dev \
> -        python-nose2 python-pexpect qemu-system-arm qemu-system-x86 && \
> +        python-nose2 python-pexpect qemu-system-arm qemu-system-x86 \
> +        python-pip && \
>      apt-get -y autoremove && \
>      apt-get -y clean
>  
> +# For check-flake8
> +RUN python -m pip install --upgrade pip setuptools wheel && \
> +    pip install -q \
> +        flake8==3.5.0 \
> +        mccabe==0.6.1 \
> +        pycodestyle==2.3.1 \
> +        pyflakes==1.6.0
> +
>  # To be able to generate a toolchain with locales, enable one UTF-8 locale
>  RUN sed -i 's/# \(en_US.UTF-8\)/\1/' /etc/locale.gen && \
>      /usr/sbin/locale-gen
> -- 
> 2.7.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v3 6/7] .gitlab-ci.yml: check flake8

[Yann E. MORIN]

Ricardo, All,

On 2018-03-13 00:09 -0300, Ricardo Martincoski spake thusly:
> Add a test to check Python code style in the whole buildroot tree.
> 
> Search files by type in order to help flake8 to find the Python scripts
> without .py extension. But don't rely only in the output of 'file' as it
> uses heuristics and sometimes it is wrong (specially identifying Python
> files as C++ source for the 'file' version currently in the Docker
> image).
> 
> Include in the output:
>  - the list of Python files processed;
>  - statistics for each kind of warning;
>  - the total number of warnings;
>  - the number of Python files processed.
> 
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> BEFORE APPLYING, see this note from Yann:
> Note: this can only be applied _after_ we regenerate the docker image.
> Then we'll have to update the version of the image at the top of the
> gitlab-ci.yml file.

Indeed, so I've marked this patch as "deferred" in patchwork, so it does
not get accidentally applied.

Also, please consider inverting the order of patch 6 and 7: we need
flake8 to succeeds before enabling the automatic check, so we need to
exclude diffconfig first.

Regards,
Yann E. MORIN.

> Changes v2 -> v3:
>   - manually accepted review tag
> 
> Changes v1 -> v2:  (suggested by Yann E. MORIN)
>   - install flake8 to the base docker image (in the previous patch)
> 
> https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/57046920
> ---
>  .gitlab-ci.yml    | 11 +++++++++++
>  .gitlab-ci.yml.in | 11 +++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index f44c3a9..9ad4c5f 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -30,6 +30,17 @@ check-DEVELOPERS:
>      script:
>          - "! utils/get-developers | grep -v 'No action specified'"
>  
> +check-flake8:
> +    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 $(cat files.processed)
> +    after_script:
> +        - wc -l files.processed
> +
>  check-package:
>      script:
>          - find . -type f \( -name '*.mk' -o -name '*.hash' \) -exec ./utils/check-package {} +
> diff --git a/.gitlab-ci.yml.in b/.gitlab-ci.yml.in
> index cb3eb71..95fc025 100644
> --- a/.gitlab-ci.yml.in
> +++ b/.gitlab-ci.yml.in
> @@ -30,6 +30,17 @@ check-DEVELOPERS:
>      script:
>          - "! utils/get-developers | grep -v 'No action specified'"
>  
> +check-flake8:
> +    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 $(cat files.processed)
> +    after_script:
> +        - wc -l files.processed
> +
>  check-package:
>      script:
>          - find . -type f \( -name '*.mk' -o -name '*.hash' \) -exec ./utils/check-package {} +
> -- 
> 2.7.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v3 1/7] genrandconfig: fix code style

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:

 > Fix these warnings:
 > E201 whitespace after '['
 > E202 whitespace before ']'
 > E501 line too long (138 > 132 characters)
 -> isolate the common part of the external toolchain url in a variable
 >  to make the long lines shorter and more readable.

 > Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
 > Cc: Arnout Vandecappelle <arnout@mind.be>
 > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > ---
 > Changes v2 -> v3:
 >   - no changes

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v3 2/7] size-stats-compare: fix code style

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:

 > Fix these warnings:
 > E129 visually indented line with same indent as next logical line
 > E302 expected 2 blank lines, found 1

 > Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
 > Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
 > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Cc: Yann E. MORIN <yann.morin.1998@free.fr>
 > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
 > ---
 > Changes v2 -> v3:
 >   - manually accepted review tag

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v3 3/7] test_python_cryptography: fix code style

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:

 > Fix these warnings:
 > E122 continuation line missing indentation or outdented
 > E301 expected 1 blank line, found 0
 > E302 expected 2 blank lines, found 1
 > F401 'os' imported but unused

 > Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
 > Cc: Yegor Yefremov <yegorslists@googlemail.com>
 > ---
 > Changes v2 -> v3:
 >   - no changes

 > Changes v1 -> v2:
 >   - new patch (v1 of this series was not sent yet during the review of
 >     http://patchwork.ozlabs.org/patch/811011/ so I asked to fix all in
 >     the same series.

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v3 4/7] scanpypi: fix code style

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:

 > Fix these warnings:
 > E401 multiple imports on one line

 > Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
 > Cc: Yegor Yefremov <yegorslists@googlemail.com>
 > ---
 > Changes v2 -> v3:
 >   - no changes

 > Changes v1 -> v2:
 >   - new patch.

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v3 5/7] support/dockerfile: install flake8

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:

 > Use the latest version of the tool because it is actively maintained.
 > But use a fixed version of the tool and its dependencies to get stable
 > results. It can be manually bumped from time to time.

 > Before installing any Python packages, ensure pip, setuptools, and wheel
 > are up to date as recommended in the docs [1].

 > [1] https://packaging.python.org/tutorials/installing-packages/

 > Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
 > Cc: Arnout Vandecappelle <arnout@mind.be>
 > Cc: Peter Korsgaard <peter@korsgaard.com>
 > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Cc: Yann E. MORIN <yann.morin.1998@free.fr>
 > ---
 > Changes v2 -> v3:  (suggested by Yann E. MORIN)
 >   - minimise the number of intermediate layers;
 >   - explain why install setuptools separately using the latest version
 >     (I actually just used the exact command line from the docs and
 >     referenced it in the commit log);
 >   - use a single package on each line, sorted.

 > Changes v1 -> v2:  (suggested by Yann E. MORIN)
 >   - install flake8 to the base docker image instead of adding sudo to
 >     install tools on the fly.

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v3 7/7] .flake8: ignore utils/diffconfig

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes:

 > This script comes from the kernel source, so ignore any code style
 > warnings for it in order to keep it as close as possible to the original
 > one, making synchronization between repos easier.

 > The option --exclude for flake8/pycodestyle is an absolute list and has
 > a default, so ideally the default values should be added too.
 > But the use cases for flake8 in the tree are:
 >  - when developing a new script or changing an existing one, the
 >    developer calls flake8 only on that script;
 >  - in the GitLab job, a list of all Python files to be tested is created
 >    and then passed to flake8.
 > None of these involve calling 'flake8' without parameters, so don't care
 > about adding the default value.

 > Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
 > Cc: Marcus Folkesson <marcus.folkesson@gmail.com>
 > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > ---
 > Changes v2 -> v3:
 >   - no changes

 > Changes v1 -> v2:
 >   - instead of tweaking even more the script from the kernel source,
 >     ignore any warnings for it;  (suggested by Thomas Petazzoni)
 >   - move the patch after the one adding the GitLab job so I can
 >     reference it in the commit message.

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v3 6/7] .gitlab-ci.yml: check flake8

[Ricardo Martincoski]

Hello,

On Tue, Mar 13, 2018 at 03:15 PM, Yann E. MORIN wrote:

> On 2018-03-13 00:09 -0300, Ricardo Martincoski spake thusly:
>> ---
>> BEFORE APPLYING, see this note from Yann:
>> Note: this can only be applied _after_ we regenerate the docker image.
>> Then we'll have to update the version of the image at the top of the
>> gitlab-ci.yml file.
> 
> Indeed, so I've marked this patch as "deferred" in patchwork, so it does
> not get accidentally applied.

Could you regenerate the image, please?
I don't have the docker push permission.

Regards,
Ricardo

[Buildroot] [PATCH v3 6/7] .gitlab-ci.yml: check flake8

[Yann E. MORIN]

Ricardo, All,

On 2018-03-14 20:16 -0300, Ricardo Martincoski spake thusly:
> On Tue, Mar 13, 2018 at 03:15 PM, Yann E. MORIN wrote:
> > On 2018-03-13 00:09 -0300, Ricardo Martincoski spake thusly:
> >> ---
> >> BEFORE APPLYING, see this note from Yann:
> >> Note: this can only be applied _after_ we regenerate the docker image.
> >> Then we'll have to update the version of the image at the top of the
> >> gitlab-ci.yml file.
> > 
> > Indeed, so I've marked this patch as "deferred" in patchwork, so it does
> > not get accidentally applied.
> 
> Could you regenerate the image, please?
> I don't have the docker push permission.

Ok, I'm doing it now. Thanks!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v3 6/7] .gitlab-ci.yml: check flake8

[Yann E. MORIN]

Ricardo, All,

On 2018-03-14 20:16 -0300, Ricardo Martincoski spake thusly:
> On Tue, Mar 13, 2018 at 03:15 PM, Yann E. MORIN wrote:
> > On 2018-03-13 00:09 -0300, Ricardo Martincoski spake thusly:
> >> ---
> >> BEFORE APPLYING, see this note from Yann:
> >> Note: this can only be applied _after_ we regenerate the docker image.
> >> Then we'll have to update the version of the image at the top of the
> >> gitlab-ci.yml file.
> > Indeed, so I've marked this patch as "deferred" in patchwork, so it does
> > not get accidentally applied.
> Could you regenerate the image, please?
> I don't have the docker push permission.

Not sure if you were aware of it, but this has been done a little while
ago now. Tag 20180318.1724 is available:
    https://hub.docker.com/r/buildroot/base/tags/

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v3 6/7] .gitlab-ci.yml: check flake8

[Ricardo Martincoski]

Hello,

On Fri, Mar 30, 2018 at 06:12 PM, Yann E. MORIN wrote:

> Not sure if you were aware of it, but this has been done a little while
> ago now. Tag 20180318.1724 is available:
>     https://hub.docker.com/r/buildroot/base/tags/

Thank you. I was not aware of it.
I am testing it now before I send a patch to bump the image tag.

Regards,
Ricardo

[Buildroot] [PATCH v3 6/7] .gitlab-ci.yml: check flake8

[Thomas Petazzoni]

Hello,

On Tue, 13 Mar 2018 00:09:44 -0300, Ricardo Martincoski wrote:
> Add a test to check Python code style in the whole buildroot tree.
> 
> Search files by type in order to help flake8 to find the Python scripts
> without .py extension. But don't rely only in the output of 'file' as it
> uses heuristics and sometimes it is wrong (specially identifying Python
> files as C++ source for the 'file' version currently in the Docker
> image).
> 
> Include in the output:
>  - the list of Python files processed;
>  - statistics for each kind of warning;
>  - the total number of warnings;
>  - the number of Python files processed.
> 
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v3 5/7] support/dockerfile: install flake8

[Yann E. MORIN]

Ricardo, All,

On 2018-03-13 00:09 -0300, Ricardo Martincoski spake thusly:
> Use the latest version of the tool because it is actively maintained.
> But use a fixed version of the tool and its dependencies to get stable
> results. It can be manually bumped from time to time.
[--SNIP--]
> +# For check-flake8
> +RUN python -m pip install --upgrade pip setuptools wheel && \

So, back when you submnitted this patch, I ACKed it. But I missed that
this command really means we can't reproduce the docker file, because it
will use versions of pip, setuptools, and wheel that are current at the
time the command is run.

I.e. today I could very well get something that is different from the
version we got back at 20180205.0730 when we last built the docker
image.

This is not really nice... :-(

Do you think we could get away without running this command at all, and
just run the pip-install one, below, since we force the version of the
modules we isntall?

Regards,
Yann E. MORIN.

> +    pip install -q \
> +        flake8==3.5.0 \
> +        mccabe==0.6.1 \
> +        pycodestyle==2.3.1 \
> +        pyflakes==1.6.0
> +
>  # To be able to generate a toolchain with locales, enable one UTF-8 locale
>  RUN sed -i 's/# \(en_US.UTF-8\)/\1/' /etc/locale.gen && \
>      /usr/sbin/locale-gen
> -- 
> 2.7.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v3 5/7] support/dockerfile: install flake8

[Ricardo Martincoski]

Hello,

First of all: I don't think we need to hurry to fix it in the master branch.
The release is so close. Unless of course we need to generate a new docker
image right now for some reason.
Do you disagree?

On Sun, May 27, 2018 at 01:42 PM, Yann E. MORIN wrote:

> On 2018-03-13 00:09 -0300, Ricardo Martincoski spake thusly:
>> Use the latest version of the tool because it is actively maintained.
>> But use a fixed version of the tool and its dependencies to get stable
>> results. It can be manually bumped from time to time.
> [--SNIP--]
>> +# For check-flake8
>> +RUN python -m pip install --upgrade pip setuptools wheel && \
> 
> So, back when you submnitted this patch, I ACKed it. But I missed that
> this command really means we can't reproduce the docker file, because it
> will use versions of pip, setuptools, and wheel that are current at the
> time the command is run.
> 
> I.e. today I could very well get something that is different from the
> version we got back at 20180205.0730 when we last built the docker
> image.
> 
> This is not really nice... :-(

You are correct. Sorry I missed it too.

And this is not the whole problem...
I fixed the version of the direct dependencies of flake8.
But a better approach for the general case would be to fix all versions, as
dependencies between Python packages can include ranges:
package foo 1.0 can depend on bar >= 1.1

> Do you think we could get away without running this command at all, and
> just run the pip-install one, below, since we force the version of the
> modules we isntall?

No. But certainly we can come up with some command that generates reproducible
images.

By trying to install packages without that line we would get errors [1] and [2].
The only version that we don't *need* to upgrade is pip. apt-get installs 9.0.1,
and the pip --upgrade installed version 9.0.2 at the time the current image was
generated.
An image generated today would have pip 10.0.1, demonstrating the problem you
found in the current Dockerfile.

Using 'pip freeze' and 'pip list' inside the current image we can dump all the
current package versions [3].
Since the list of packages to install will become longer we could even move it
to a separate file, as suggested in [4] and also in the example (but not the
text) from the 'Best practices' guide which url you shared a while ago [5].

So a way to generate an image equivalent to the current one would be:

@ support/docker/Dockerfile:
# For check-flake8
COPY requirements.txt /tmp/
RUN pip install -q \
        pip==9.0.2 \
        setuptools==39.0.1 \
        wheel==0.30.0 && \
    pip install -q -r /tmp/requirements.txt

@ support/docker/requirements.txt
bzr==2.8.0.dev1
configobj==5.0.6
configparser==3.5.0
enum34==1.1.6
flake8==3.5.0
mccabe==0.6.1
mercurial==4.0
nose2==0.6.5
pexpect==4.2.1
ptyprocess==0.5.1
pycodestyle==2.3.1
pyflakes==1.6.0
six==1.10.0

And maybe a comment at the first line of the new file would be nice:
# output from 'pip freeze'

What do you think about this approach?
Are you preparing a patch?


[1] error message without setuptools and wheel:
Collecting configparser; python_version < "3.2" (from flake8==3.5.0)
  Using cached https://files.pythonhosted.org/packages/7c/69/c2ce7e91c89dc073eb1aa74c0621c3eefbffe8216b3f9af9d3885265c01c/configparser-3.5.0.t
ar.gz
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
    ImportError: No module named setuptools

[2] error message with setuptools but without wheel:
Building wheels for collected packages: configparser
  Running setup.py bdist_wheel for configparser ... error
  Complete output from command /usr/bin/python -u -c "import setuptools, tokenize;__file__='/tmp/pip-build-HGBa5l/configparser/setup.py';f=get
attr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" bdist_wheel -d /tm
p/tmpAST7T9pip-wheel- --python-tag cp27:
  usage: -c [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
     or: -c --help [cmd1 cmd2 ...]
     or: -c --help-commands
     or: -c cmd --help

  error: invalid command 'bdist_wheel'

[3] versions for python packages used in buildroot/base:20180318.1724 :
br-user at fa92a79b0862:~$ pip freeze
bzr==2.8.0.dev1
configobj==5.0.6
configparser==3.5.0
enum34==1.1.6
flake8==3.5.0
mccabe==0.6.1
mercurial==4.0
nose2==0.6.5
pexpect==4.2.1
ptyprocess==0.5.1
pycodestyle==2.3.1
pyflakes==1.6.0
six==1.10.0
You are using pip version 9.0.2, however version 10.0.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
br-user at fa92a79b0862:~$ pip list --format=columns | grep 'pip\|setuptools\|wheel'
pip          9.0.2
setuptools   39.0.1
wheel        0.30.0

[4] https://unix.stackexchange.com/questions/349227/dockerfile-docker-image-and-reproducible-environment

[5] https://docs.docker.com/v17.09/engine/userguide/eng-image/dockerfile_best-practices/#add-or-copy


Regards,
Ricardo

[Buildroot] [PATCH v3 5/7] support/dockerfile: install flake8

[Yann E. MORIN]

Ricardo, All,

On 2018-05-28 00:00 -0300, Ricardo Martincoski spake thusly:
> First of all: I don't think we need to hurry to fix it in the master branch.
> The release is so close. Unless of course we need to generate a new docker
> image right now for some reason.
> Do you disagree?

Absolutely no urgency at all. ;-)

[--SNIP--]
> > Do you think we could get away without running this command at all, and
> > just run the pip-install one, below, since we force the version of the
> > modules we isntall?
> 
> No. But certainly we can come up with some command that generates reproducible
> images.
> 
> By trying to install packages without that line we would get errors [1] and [2].
> The only version that we don't *need* to upgrade is pip. apt-get installs 9.0.1,
> and the pip --upgrade installed version 9.0.2 at the time the current image was
> generated.
> An image generated today would have pip 10.0.1, demonstrating the problem you
> found in the current Dockerfile.
> 
> Using 'pip freeze' and 'pip list' inside the current image we can dump all the
> current package versions [3].
> Since the list of packages to install will become longer we could even move it
> to a separate file, as suggested in [4] and also in the example (but not the
> text) from the 'Best practices' guide which url you shared a while ago [5].
> 
> So a way to generate an image equivalent to the current one would be:
> 
> @ support/docker/Dockerfile:
> # For check-flake8
> COPY requirements.txt /tmp/
> RUN pip install -q \
>         pip==9.0.2 \
>         setuptools==39.0.1 \
>         wheel==0.30.0 && \
>     pip install -q -r /tmp/requirements.txt
> 
> @ support/docker/requirements.txt
> bzr==2.8.0.dev1
> configobj==5.0.6
> configparser==3.5.0
> enum34==1.1.6
> flake8==3.5.0
> mccabe==0.6.1
> mercurial==4.0
> nose2==0.6.5
> pexpect==4.2.1
> ptyprocess==0.5.1
> pycodestyle==2.3.1
> pyflakes==1.6.0
> six==1.10.0
> 
> And maybe a comment at the first line of the new file would be nice:
> # output from 'pip freeze'
> 
> What do you think about this approach?

Why do we even use pip to install those? Can't we just rely on the
verions actually packaged in the distro instead? I.e.

    apt-get install python-configobj python-configparser etc...

Of course, provided that the module is indeed packaged in the distro...
As far as I can see, there should be everything we need, no?

The only two for which I have a doubt are bzr (python-bzrlib) and
mercurial (python-hglib); all the others are in stretch-20171210.

But if we apt-get install python-flake8, then it should bring all its
dependencies I guess:

    root at 39b33276de75:/# apt-get install python-flake8
    Reading package lists... Done
    Building dependency tree
    Reading state information... Done
    The following additional packages will be installed:
      dh-python javascript-common libjs-jquery libjs-sphinxdoc libjs-underscore libmpdec2 libpython3-stdlib
      libpython3.5-minimal libpython3.5-stdlib pyflakes pyflakes3 python-configparser python-enum34
      python-mccabe python-pycodestyle python-pyflakes python-setuptools python3 python3-minimal
      python3-pkg-resources python3-pyflakes python3.5 python3.5-minimal
    Suggested packages:
      apache2 | lighttpd | httpd python-enum34-doc python-mock python-setuptools-doc python3-doc python3-tk
      python3-venv python3-setuptools python3.5-venv python3.5-doc binfmt-support
    The following NEW packages will be installed:
      dh-python javascript-common libjs-jquery libjs-sphinxdoc libjs-underscore libmpdec2 libpython3-stdlib
      libpython3.5-minimal libpython3.5-stdlib pyflakes pyflakes3 python-configparser python-enum34
      python-flake8 python-mccabe python-pycodestyle python-pyflakes python-setuptools python3
      python3-minimal python3-pkg-resources python3-pyflakes python3.5 python3.5-minimal

Note that I don't care if stretch packages a different version than what
we curently have; I'd prefer that we use the distro-packaged versions,
since we know that apt-get *is* reproducible as we use a fixed version
of the distro.

> Are you preparing a patch?

When we agree on the directions, yes I can.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v3 5/7] support/dockerfile: install flake8

[Ricardo Martincoski]

Hello,

On Mon, May 28, 2018 at 04:44 PM, Yann E. MORIN wrote:

[snip]
> Why do we even use pip to install those? Can't we just rely on the
> verions actually packaged in the distro instead? I.e.
> 
>     apt-get install python-configobj python-configparser etc...
> 
> Of course, provided that the module is indeed packaged in the distro...
> As far as I can see, there should be everything we need, no?

Indeed. Yes.

> The only two for which I have a doubt are bzr (python-bzrlib) and
> mercurial (python-hglib); all the others are in stretch-20171210.

Don't worry about these. They were probably installed by the apt-get command,
pip only listed them.

> But if we apt-get install python-flake8, then it should bring all its
> dependencies I guess:

Yes. We don't even need to install python-pip.

[snip]
> Note that I don't care if stretch packages a different version than what
> we curently have; I'd prefer that we use the distro-packaged versions,
> since we know that apt-get *is* reproducible as we use a fixed version
> of the distro.

OK. Simple and effective solution.

Notice we will be downgrading flake8 from
3.5.0 (mccabe: 0.6.1, pycodestyle: 2.3.1, pyflakes: 1.6.0) from 2017-10-23
to
3.2.1 (pyflakes: 1.3.0, pycodestyle: 2.2.0, mccabe: 0.5.3) from 2016-11-21

Anyone curious can see the changelog here:
http://flake8.pycqa.org/en/latest/release-notes/index.html#x-release-series

>> Are you preparing a patch?
> 
> When we agree on the directions, yes I can.

Reproducible images with less code and less maintenance effort in exchange of
using a not-so-bleeding-edge version. Makes sense to me.

I am only in doubt how future-proof is this solution, if some day we will
*need* a new version, i.e. to something related to Python3 in 2020 (it's only a
wild guess!, I don't know any current limitation, I don't know the internals of
flake8).
But then probably debian will have a newer version of flake8, so we can update
the stretch version. And we can always go back to using pip if really needed.


Regards,
Ricardo

[Buildroot] [PATCH v3 5/7] support/dockerfile: install flake8

[Yann E. MORIN]

Ricardo, All,

On 2018-05-29 00:43 -0300, Ricardo Martincoski spake thusly:
> On Mon, May 28, 2018 at 04:44 PM, Yann E. MORIN wrote:
> > Why do we even use pip to install those? Can't we just rely on the
> > verions actually packaged in the distro instead? I.e.
[--SNIP--]
> >> Are you preparing a patch?
> > When we agree on the directions, yes I can.
> 
> Reproducible images with less code and less maintenance effort in exchange of
> using a not-so-bleeding-edge version. Makes sense to me.

OK, so I'll send a patch to implement this soilution.

Thanks! :-)

> I am only in doubt how future-proof is this solution, if some day we will
> *need* a new version, i.e. to something related to Python3 in 2020 (it's only a
> wild guess!, I don't know any current limitation, I don't know the internals of
> flake8).
> But then probably debian will have a newer version of flake8, so we can update
> the stretch version. And we can always go back to using pip if really needed.

Yes, then we can update to a newer version of the distro.

There was also a wild suggestion that we provide more Docker images,
for each of the mainstream distros.

Because sometimes, build issues only occur on some distros and not
others, e.g. arch-linux, which uses musl, sometimes break the build of
some host packages, and it is difficult to diagnose and fix...

I was also looking into providing additional Dockerfiles for additional
distros. I'll get this in the same series. Soon. Sooooon.... ;-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'