fix Python code style
cover fix Python code style
Hello,
This series fixes code style warnings reported by flake8 in the Python scripts in the tree.
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 12 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.
Most of the commit messages are terse as the changes I introduced are minimal. If I missed to explain any change that is not obvious, please ask me on the review.
Patch 13 adds support to install flake8 on the fly to the Docker image, needed by the next patch.
Patch 14 creates a job in gitlab to run flake8:
https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/48876752
And here a complete run on gitlab using current Docker image:
https://gitlab.com/RicardoMartincoski/buildroot/pipelines/16432662
Regards,
Ricardo
Ricardo Martincoski (14):
graph-depends: fix code style
check-uniq-files: fix code style
graph-build-time: fix code style
pycompile: fix code style
size-stats: fix code style
testing/tests/boot/test_atf: fix code style
check-package: fix code style
diffconfig: fix code style
genrandconfig: fix code style
get-developers: fix code style
scanpypi: fix code style
size-stats-compare: fix code style
support/dockerfile: allow to install packages
.gitlab-ci.yml: check flake8
.gitlab-ci.yml | 16 +++++ .gitlab-ci.yml.in | 16 +++++ support/docker/Dockerfile | 8 ++- support/scripts/brpkgutil.py | 8 ++- support/scripts/check-uniq-files | 4 +- support/scripts/graph-build-time | 24 ++++--- support/scripts/graph-depends | 68 +++++++++++------- support/scripts/pycompile.py | 3 + support/scripts/size-stats | 8 ++- support/testing/tests/boot/test_atf.py | 124 +++++++++++++++++---------------- utils/checkpackagelib/lib_config.py | 9 ++- utils/checkpackagelib/lib_hash.py | 9 ++- utils/checkpackagelib/lib_mk.py | 9 ++- utils/checkpackagelib/lib_patch.py | 3 +- utils/checkpackagelib/readme.txt | 2 - utils/diffconfig | 22 +++--- utils/genrandconfig | 34 ++++----- utils/get-developers | 4 +- utils/getdeveloperlib.py | 52 ++++++++------ utils/scanpypi | 13 ++-- utils/size-stats-compare | 5 +- 21 files changed, 269 insertions(+), 172 deletions(-)
Signature · 1 line
2.7.4
PATCH 1/14 graph-depends: fix code style
Fix these warnings:
E122 continuation line missing indentation or outdented
E127 continuation line over-indented for visual indent
E128 continuation line under-indented for visual indent
E202 whitespace before ']'
E221 multiple spaces before operator
E225 missing whitespace around operator
E231 missing whitespace after ','
E302 expected 2 blank lines, found 1
E305 expected 2 blank lines after class or function definition, found 1 E502 the backslash is redundant between brackets
E713 test for membership should be 'not in'
--- support/scripts/brpkgutil.py | 8 +++-- support/scripts/graph-depends | 68 ++++++++++++++++++++++++++----------------- 2 files changed, 48 insertions(+), 28 deletions(-)
support/scripts/brpkgutil.py +6 -2
@@ -3,11 +3,12 @@ import sys import subprocess + # Execute the "make <pkg>-show-version" command to get the version of a given # list of packages, and return the version formatted as a Python dictionary. def get_version(pkgs): sys.stderr.write("Getting version for %s\n" % pkgs) - cmd = ["make", "-s", "--no-print-directory" ] + cmd = ["make", "-s", "--no-print-directory"] for pkg in pkgs: cmd.append("%s-show-version" % pkg) p = subprocess.Popen(cmd, stdout=subprocess.PIPE, universal_newlines=True)
@@ -25,9 +26,10 @@ def get_version(pkgs): version[pkg] = output[i] return version + def _get_depends(pkgs, rule): sys.stderr.write("Getting dependencies for %s\n" % pkgs) - cmd = ["make", "-s", "--no-print-directory" ] + cmd = ["make", "-s", "--no-print-directory"] for pkg in pkgs: cmd.append("%s-%s" % (pkg, rule)) p = subprocess.Popen(cmd, stdout=subprocess.PIPE, universal_newlines=True)
@@ -49,12 +51,14 @@ def _get_depends(pkgs, rule): deps[pkg] = pkg_deps return deps + # Execute the "make <pkg>-show-depends" command to get the list of # dependencies of a given list of packages, and return the list of # dependencies formatted as a Python dictionary. def get_depends(pkgs): return _get_depends(pkgs, 'show-depends') + # Execute the "make <pkg>-show-rdepends" command to get the list of # reverse dependencies of a given list of packages, and return the # list of dependencies formatted as a Python dictionary.
support/scripts/graph-depends +42 -26
217 lines not on this page · raw message
Signature · 1 line
2.7.4
PATCH 2/14 check-uniq-files: fix code style
In reply to Ricardo MartincoskiFix these warnings:
E128 continuation line under-indented for visual indent
E302 expected 2 blank lines, found 1
E305 expected 2 blank lines after class or function definition, found 1
--- support/scripts/check-uniq-files | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
support/scripts/check-uniq-files +3 -1
@@ -7,10 +7,11 @@ from collections import defaultdict warn = 'Warning: {} file "{}" is touched by more than one package: {}\n' + def main(): parser = argparse.ArgumentParser() parser.add_argument('packages_file_list', nargs='*', - help='The packages-file-list to check from') + help='The packages-file-list to check from') parser.add_argument('-t', '--type', metavar="TYPE", help='Report as a TYPE file (TYPE is either target, staging, or host)')
@@ -36,5 +37,6 @@ def main(): if len(file_to_pkg[file]) > 1: sys.stderr.write(warn.format(args.type, file, file_to_pkg[file])) + if __name__ == "__main__": sys.exit(main())
Signature · 1 line
2.7.4
PATCH 3/14 graph-build-time: fix code style
In reply to Ricardo MartincoskiFix these warnings:
E201 whitespace after '['
E202 whitespace before ']'
E302 expected 2 blank lines, found 1
E305 expected 2 blank lines after class or function definition, found 1
Ignore these warnings:
E402 module level import not at top of file
--- support/scripts/graph-build-time | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)
support/scripts/graph-build-time +15 -9
@@ -64,14 +64,14 @@ except ImportError: # Note: matplotlib.use() must be called *before* matplotlib.pyplot. mpl.use('Agg') -import matplotlib.pyplot as plt -import matplotlib.font_manager as fm -import csv -import argparse +import matplotlib.pyplot as plt # noqa: E402 +import matplotlib.font_manager as fm # noqa: E402 +import csv # noqa: E402 +import argparse # noqa: E402 -steps = [ 'extract', 'patch', 'configure', 'build', - 'install-target', 'install-staging', 'install-images', - 'install-host'] +steps = ['extract', 'patch', 'configure', 'build', + 'install-target', 'install-staging', 'install-images', + 'install-host'] default_colors = ['#e60004', '#009836', '#2e1d86', '#ffed00', '#0068b5', '#f28e00', '#940084', '#97c000']
@@ -79,6 +79,7 @@ default_colors = ['#e60004', '#009836', '#2e1d86', '#ffed00', alternate_colors = ['#00e0e0', '#3f7f7f', '#ff0000', '#00c000', '#0080ff', '#c000ff', '#00eeee', '#e0e000'] + class Package: def __init__(self, name): self.name = name
@@ -104,6 +105,7 @@ class Package: return self.steps_duration[step] return 0 + # Generate an histogram of the time spent in each step of each # package. def pkg_histogram(data, output, order="build"):
@@ -132,10 +134,10 @@ def pkg_histogram(data, output, order="build"): for i in range(0, len(vals)): b = plt.bar(ind+0.1, vals[i], width=0.8, color=colors[i], bottom=bottom, linewidth=0.25) legenditems.append(b[0]) - bottom = [ bottom[j] + vals[i][j] for j in range(0, len(vals[i])) ] + bottom = [bottom[j] + vals[i][j] for j in range(0, len(vals[i]))] # Draw the package names - plt.xticks(ind + .6, [ p.name for p in data ], rotation=-60, rotation_mode="anchor", fontsize=8, ha='left') + plt.xticks(ind + .6, [p.name for p in data], rotation=-60, rotation_mode="anchor", fontsize=8, ha='left') # Adjust size of graph depending on the number of packages # Ensure a minimal size twice as the default
@@ -172,6 +174,7 @@ def pkg_histogram(data, output, order="build"): # Save graph plt.savefig(output) + # Generate a pie chart with the time spent building each package. def pkg_pie_time_per_package(data, output): # Compute total build duration
@@ -210,6 +213,7 @@ def pkg_pie_time_per_package(data, output): plt.title('Build time per package') plt.savefig(output) + # Generate a pie chart with a portion for the overall time spent in # each step for all packages. def pkg_pie_time_per_step(data, output):
@@ -236,6 +240,7 @@ def pkg_pie_time_per_step(data, output): plt.title('Build time per step') plt.savefig(output) + # Parses the csv file passed on standard input and returns a list of # Package objects, filed with the duration of each step and the total # duration of the package.
@@ -269,6 +274,7 @@ def read_data(input_file): return pkgs + parser = argparse.ArgumentParser(description='Draw build time graphs') parser.add_argument("--type", '-t', metavar="GRAPH_TYPE", help="Type of graph (histogram, pie-packages, pie-steps)")
Signature · 1 line
2.7.4
PATCH 4/14 pycompile: fix code style
In reply to Ricardo MartincoskiFix these warnings:
E302 expected 2 blank lines, found 1
E305 expected 2 blank lines after class or function definition, found 1
--- support/scripts/pycompile.py | 3 +++ 1 file changed, 3 insertions(+)
support/scripts/pycompile.py +3 -0
@@ -12,6 +12,7 @@ import py_compile import compileall import argparse + def check_for_errors(comparison): '''Wrap comparison operator with code checking for PyCompileError. If PyCompileError was raised, re-raise it again to abort execution,
@@ -28,6 +29,7 @@ def check_for_errors(comparison): return operator + class ReportProblem(int): '''Class that pretends to be an int() object but implements all of its comparison operators such that it'd detect being called in
@@ -55,6 +57,7 @@ class ReportProblem(int): def __ne__(self, other): return not self == other + parser = argparse.ArgumentParser(description='Compile Python source files in a directory tree.') parser.add_argument("target", metavar='DIRECTORY', help='Directory to scan')
Signature · 1 line
2.7.4
PATCH 4/14 pycompile: fix code style
Quoted 7 lines · Ricardo Martincoski
Fix these warnings:
E302 expected 2 blank lines, found 1
E305 expected 2 blank lines after class or function definition, found 1Signed-off-by Ricardo Martincoski <[email protected]>Cc Thomas Petazzoni <[email protected]>Cc Yegor Yefremov <[email protected]>
Quoted 35 lines
--- support/scripts/pycompile.py | 3 +++ 1 file changed, 3 insertions(+)support/scripts/pycompile.py +3
-1@@ -12,6 +12,7 @@ import py_compile import compileall import argparse + def check_for_errors(comparison): '''Wrap comparison operator with code checking for PyCompileError. If PyCompileError was raised, re-raise it again to abort execution,@@ -28,6 +29,7 @@ def check_for_errors(comparison): return operator + class ReportProblem(int): '''Class that pretends to be an int() object but implements all of its comparison operators such that it'd detect being called in@@ -55,6 +57,7 @@ class ReportProblem(int): def __ne__(self, other): return not self == other + parser = argparse.ArgumentParser(description='Compile Python source files in a directory tree.') parser.add_argument("target", metavar='DIRECTORY', help='Directory to scan') --2.7.4
PATCH 5/14 size-stats: fix code style
In reply to Ricardo MartincoskiFix these warnings:
E302 expected 2 blank lines, found 1
E305 expected 2 blank lines after class or function definition, found 1 E713 test for membership should be 'not in'
--- support/scripts/size-stats | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
support/scripts/size-stats +7 -1
@@ -35,6 +35,7 @@ except ImportError: colors = ['#e60004', '#009836', '#2e1d86', '#ffed00', '#0068b5', '#f28e00', '#940084', '#97c000'] + # # This function adds a new file to 'filesdict', after checking its # size. The 'filesdict' contain the relative path of the file as the
@@ -54,6 +55,7 @@ def add_file(filesdict, relpath, abspath, pkg): sz = os.stat(abspath).st_size filesdict[relpath] = (pkg, sz) + # # This function returns a dict where each key is the path of a file in # the root filesystem, and the value is a tuple containing two
@@ -73,6 +75,7 @@ def build_package_dict(builddir): add_file(filesdict, fpath, fullpath, pkg) return filesdict + # # This function builds a dictionary that contains the name of a # package as key, and the size of the files installed by this package
@@ -103,7 +106,7 @@ def build_package_size(filesdict, builddir): seeninodes.add(st.st_ino) frelpath = os.path.relpath(fpath, os.path.join(builddir, "target")) - if not frelpath in filesdict: + if frelpath not in filesdict: print("WARNING: %s is not part of any package" % frelpath) pkg = "unknown" else:
@@ -113,6 +116,7 @@ def build_package_size(filesdict, builddir): return pkgsize + # # Given a dict returned by build_package_size(), this function # generates a pie chart of the size installed by each package.
@@ -150,6 +154,7 @@ def draw_graph(pkgsize, outputf): plt.title("Total filesystem size: %d kB" % (total / 1000.), fontsize=10, y=.96) plt.savefig(outputf) + # # Generate a CSV file with statistics about the size of each file, its # size contribution to the package and to the overall system.
@@ -208,6 +213,7 @@ def gen_packages_csv(pkgsizes, outputf): for (pkg, size) in pkgsizes.items(): wr.writerow([pkg, size, "%.1f" % (float(size) / total * 100)]) + parser = argparse.ArgumentParser(description='Draw size statistics graphs') parser.add_argument("--builddir", '-i', metavar="BUILDDIR", required=True,
Signature · 1 line
2.7.4
PATCH 6/14 testing/tests/boot/test_atf: fix code style
In reply to Ricardo MartincoskiFix these warnings:
E122 continuation line missing indentation or outdented E302 expected 2 blank lines, found 1
F401 'os' imported but unused
--- support/testing/tests/boot/test_atf.py | 124 +++++++++++++++++---------------- 1 file changed, 63 insertions(+), 61 deletions(-)
support/testing/tests/boot/test_atf.py +63 -61
@@ -1,80 +1,82 @@ -import os import infra.basetest + class TestATFVexpress(infra.basetest.BRTest): config = \ - """ - BR2_aarch64=y - BR2_TOOLCHAIN_EXTERNAL=y - BR2_TOOLCHAIN_EXTERNAL_LINARO_AARCH64=y - BR2_TARGET_ARM_TRUSTED_FIRMWARE=y - BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_GIT=y - BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_URL="https://github.com/ARM-software/arm-trusted-firmware.git" - BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_VERSION="v1.2" - BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM="juno" - BR2_TARGET_ARM_TRUSTED_FIRMWARE_FIP=y - BR2_TARGET_ARM_TRUSTED_FIRMWARE_UBOOT_AS_BL33=y - BR2_TARGET_UBOOT=y - BR2_TARGET_UBOOT_BOARDNAME="vexpress_aemv8a_juno" - BR2_TARGET_UBOOT_CUSTOM_VERSION=y - BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2016.03" - BR2_TARGET_VEXPRESS_FIRMWARE=y - """ + """ + BR2_aarch64=y + BR2_TOOLCHAIN_EXTERNAL=y + BR2_TOOLCHAIN_EXTERNAL_LINARO_AARCH64=y + BR2_TARGET_ARM_TRUSTED_FIRMWARE=y + BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_GIT=y + BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_URL="https://github.com/ARM-software/arm-trusted-firmware.git" + BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_VERSION="v1.2" + BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM="juno" + BR2_TARGET_ARM_TRUSTED_FIRMWARE_FIP=y + BR2_TARGET_ARM_TRUSTED_FIRMWARE_UBOOT_AS_BL33=y + BR2_TARGET_UBOOT=y + BR2_TARGET_UBOOT_BOARDNAME="vexpress_aemv8a_juno" + BR2_TARGET_UBOOT_CUSTOM_VERSION=y + BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2016.03" + BR2_TARGET_VEXPRESS_FIRMWARE=y + """ def test_run(self): pass + class TestATFAllwinner(infra.basetest.BRTest): config = \ - """ - BR2_aarch64=y - BR2_TOOLCHAIN_EXTERNAL=y - BR2_TOOLCHAIN_EXTERNAL_LINARO_AARCH64=y - BR2_TARGET_ARM_TRUSTED_FIRMWARE=y - BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_GIT=y - BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_URL="https://github.com/apritzel/arm-trusted-firmware.git" - BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM="sun50iw1p1" - BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_VERSION="aa75c8da415158a94b82a430b2b40000778e851f" - BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31=y - BR2_TARGET_UBOOT=y - BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y - BR2_TARGET_UBOOT_CUSTOM_VERSION=y - BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2017.09" - BR2_TARGET_UBOOT_BOARD_DEFCONFIG="bananapi_m64" - BR2_TARGET_UBOOT_NEEDS_DTC=y - BR2_TARGET_UBOOT_NEEDS_ATF_BL31=y - BR2_TARGET_UBOOT_FORMAT_CUSTOM=y - BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME="u-boot.itb" - BR2_TARGET_UBOOT_SPL=y - BR2_TARGET_UBOOT_SPL_NAME="spl/sunxi-spl.bin" - """ + """ + BR2_aarch64=y + BR2_TOOLCHAIN_EXTERNAL=y + BR2_TOOLCHAIN_EXTERNAL_LINARO_AARCH64=y + BR2_TARGET_ARM_TRUSTED_FIRMWARE=y + BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_GIT=y + BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_URL="https://github.com/apritzel/arm-trusted-firmware.git" + BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM="sun50iw1p1" + BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_VERSION="aa75c8da415158a94b82a430b2b40000778e851f" + BR2_TARGET_ARM_TRUSTED_FIRMWARE_BL31=y + BR2_TARGET_UBOOT=y + BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y + BR2_TARGET_UBOOT_CUSTOM_VERSION=y + BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2017.09" + BR2_TARGET_UBOOT_BOARD_DEFCONFIG="bananapi_m64" + BR2_TARGET_UBOOT_NEEDS_DTC=y + BR2_TARGET_UBOOT_NEEDS_ATF_BL31=y + BR2_TARGET_UBOOT_FORMAT_CUSTOM=y + BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME="u-boot.itb" + BR2_TARGET_UBOOT_SPL=y + BR2_TARGET_UBOOT_SPL_NAME="spl/sunxi-spl.bin" + """ def test_run(self): pass + class TestATFMarvell(infra.basetest.BRTest): config = \ - """ - BR2_aarch64=y - BR2_TOOLCHAIN_EXTERNAL=y - BR2_TOOLCHAIN_EXTERNAL_LINARO_AARCH64=y - BR2_TARGET_ARM_TRUSTED_FIRMWARE=y - BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_GIT=y - BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_URL="https://github.com/MarvellEmbeddedProcessors/atf-marvell.git" - BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_VERSION="atf-v1.3-armada-17.10" - BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM="a80x0_mcbin" - BR2_TARGET_ARM_TRUSTED_FIRMWARE_FIP=y - BR2_TARGET_ARM_TRUSTED_FIRMWARE_UBOOT_AS_BL33=y - BR2_TARGET_ARM_TRUSTED_FIRMWARE_ADDITIONAL_VARIABLES="USE_COHERENT_MEM=0" - BR2_TARGET_BINARIES_MARVELL=y - BR2_TARGET_BINARIES_MARVELL_8040=y - BR2_TARGET_MV_DDR_MARVELL=y - BR2_TARGET_UBOOT=y - BR2_TARGET_UBOOT_BOARDNAME="mvebu_mcbin-88f8040" - BR2_TARGET_UBOOT_CUSTOM_VERSION=y - BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2017.09" - BR2_TARGET_UBOOT_NEEDS_DTC=y - """ + """ + BR2_aarch64=y + BR2_TOOLCHAIN_EXTERNAL=y + BR2_TOOLCHAIN_EXTERNAL_LINARO_AARCH64=y + BR2_TARGET_ARM_TRUSTED_FIRMWARE=y + BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_GIT=y + BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_URL="https://github.com/MarvellEmbeddedProcessors/atf-marvell.git" + BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_REPO_VERSION="atf-v1.3-armada-17.10" + BR2_TARGET_ARM_TRUSTED_FIRMWARE_PLATFORM="a80x0_mcbin" + BR2_TARGET_ARM_TRUSTED_FIRMWARE_FIP=y + BR2_TARGET_ARM_TRUSTED_FIRMWARE_UBOOT_AS_BL33=y + BR2_TARGET_ARM_TRUSTED_FIRMWARE_ADDITIONAL_VARIABLES="USE_COHERENT_MEM=0" + BR2_TARGET_BINARIES_MARVELL=y + BR2_TARGET_BINARIES_MARVELL_8040=y + BR2_TARGET_MV_DDR_MARVELL=y + BR2_TARGET_UBOOT=y + BR2_TARGET_UBOOT_BOARDNAME="mvebu_mcbin-88f8040" + BR2_TARGET_UBOOT_CUSTOM_VERSION=y + BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2017.09" + BR2_TARGET_UBOOT_NEEDS_DTC=y + """ def test_run(self): pass
Signature · 1 line
2.7.4
PATCH 7/14 check-package: fix code style
In reply to Ricardo MartincoskiIgnore these warnings:
F401 'lib.ConsecutiveEmptyLines' imported but unused
And remove comments that are not needed anymore.
--- utils/checkpackagelib/lib_config.py | 9 ++++----- utils/checkpackagelib/lib_hash.py | 9 ++++----- utils/checkpackagelib/lib_mk.py | 9 ++++----- utils/checkpackagelib/lib_patch.py | 3 +-- utils/checkpackagelib/readme.txt | 2 -- 5 files changed, 13 insertions(+), 19 deletions(-)
utils/checkpackagelib/lib_config.py +4 -5
@@ -6,11 +6,10 @@ import re from base import _CheckFunction -# Notice: ignore 'imported but unused' from pyflakes for check functions. -from lib import ConsecutiveEmptyLines -from lib import EmptyLastLine -from lib import NewlineAtEof -from lib import TrailingSpace +from lib import ConsecutiveEmptyLines # noqa: F401 +from lib import EmptyLastLine # noqa: F401 +from lib import NewlineAtEof # noqa: F401 +from lib import TrailingSpace # noqa: F401 def _empty_or_comment(text):
utils/checkpackagelib/lib_hash.py +4 -5
@@ -6,11 +6,10 @@ import re from base import _CheckFunction -# Notice: ignore 'imported but unused' from pyflakes for check functions. -from lib import ConsecutiveEmptyLines -from lib import EmptyLastLine -from lib import NewlineAtEof -from lib import TrailingSpace +from lib import ConsecutiveEmptyLines # noqa: F401 +from lib import EmptyLastLine # noqa: F401 +from lib import NewlineAtEof # noqa: F401 +from lib import TrailingSpace # noqa: F401 def _empty_line_or_comment(text):
utils/checkpackagelib/lib_mk.py +4 -5
@@ -7,11 +7,10 @@ import re from base import _CheckFunction -# Notice: ignore 'imported but unused' from pyflakes for check functions. -from lib import ConsecutiveEmptyLines -from lib import EmptyLastLine -from lib import NewlineAtEof -from lib import TrailingSpace +from lib import ConsecutiveEmptyLines # noqa: F401 +from lib import EmptyLastLine # noqa: F401 +from lib import NewlineAtEof # noqa: F401 +from lib import TrailingSpace # noqa: F401 class Indent(_CheckFunction):
utils/checkpackagelib/lib_patch.py +1 -2
@@ -6,8 +6,7 @@ import re from base import _CheckFunction -# Notice: ignore 'imported but unused' from pyflakes for check functions. -from lib import NewlineAtEof +from lib import NewlineAtEof # noqa: F401 class ApplyOrder(_CheckFunction):
utils/checkpackagelib/readme.txt +0 -2
@@ -28,8 +28,6 @@ Some hints when changing this code: - when there is no other reason for ordering, use alphabetical order (e.g. keep the check functions in alphabetical order, keep the imports in alphabetical order, and so on). -- use pyflakes to detect and fix potential problems. -- use pep8 formatting. - keep in mind that for every class the method before() will be called before any line is served to be checked by the method check_line(). A class that checks the filename should only implement the method before(). A function that
Signature · 1 line
2.7.4
PATCH 8/14 diffconfig: fix code style
In reply to Ricardo MartincoskiFix these warnings:
E225 missing whitespace around operator
E231 missing whitespace after ','
E302 expected 2 blank lines, found 1
E305 expected 2 blank lines after class or function definition, found 1 E401 multiple imports on one line
--- utils/diffconfig | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)
utils/diffconfig +14 -8
@@ -8,7 +8,9 @@ # Adapted to Buildroot 2017 by Marcus Folkesson # -import sys, os +import sys +import os + def usage(): print("""Usage: diffconfig [-h] [-m] [<config1> <config2>]
@@ -40,6 +42,7 @@ Example usage: """) sys.exit(0) + # returns a dictionary of name/value pairs for config items in the file def readconfig(config_file): d = {}
@@ -52,23 +55,25 @@ def readconfig(config_file): d[line[6:-11]] = "n" return d + def print_config(op, config, value, new_value): global merge_style if merge_style: if new_value: - if new_value=="n": + if new_value == "n": print("# BR2_%s is not set" % config) else: print("BR2_%s=%s" % (config, new_value)) else: - if op=="-": + if op == "-": print("-%s %s" % (config, value)) - elif op=="+": + elif op == "+": print("+%s %s" % (config, new_value)) else: print(" %s %s -> %s" % (config, value, new_value)) + def main(): global merge_style
@@ -82,15 +87,15 @@ def main(): sys.argv.remove("-m") argc = len(sys.argv) - if not (argc==1 or argc == 3): + if not (argc == 1 or argc == 3): print("Error: incorrect number of arguments or unrecognized option") usage() if argc == 1: # if no filenames given, assume .config and .config.old - build_dir="" + build_dir = "" if "KBUILD_OUTPUT" in os.environ: - build_dir = os.environ["KBUILD_OUTPUT"]+"/" + build_dir = os.environ["KBUILD_OUTPUT"] + "/" configa_filename = build_dir + ".config.old" configb_filename = build_dir + ".config" else:
@@ -102,7 +107,7 @@ def main(): b = readconfig(open(configb_filename)) except (IOError): e = sys.exc_info()[1] - print("I/O error[%s]: %s\n" % (e.args[0],e.args[1])) + print("I/O error[%s]: %s\n" % (e.args[0], e.args[1])) usage() # print items in a but not b (accumulate, sort and print)
@@ -133,4 +138,5 @@ def main(): for config in new: print_config("+", config, None, b[config]) + main()
Signature · 1 line
2.7.4
PATCH 8/14 diffconfig: fix code style
Hello,
Quoted 9 lines · Ricardo Martincoski
Fix these warnings:
E225 missing whitespace around operator
E231 missing whitespace after ','
E302 expected 2 blank lines, found 1
E305 expected 2 blank lines after class or function definition, found 1 E401 multiple imports on one lineSigned-off-by Ricardo Martincoski <[email protected]>Cc Marcus Folkesson <[email protected]>
I haven't applied this one, because I don't really like the fact that we're further tweaking a script that comes from the kernel sources. On the other hand, we have already tweaked it for our purposes.
What do others think? Should we stay as close to the original upstream version as possible? Or because we have anyway already tweaked it, we don't really care and can now do whatever changes we believe are useful?
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 8/14 diffconfig: fix code style
Hi,
Quoted 13 lines · Thomas Petazzoni
Hello,
Quoted 9 lines · Ricardo Martincoski
Fix these warnings:
E225 missing whitespace around operator
E231 missing whitespace after ','
E302 expected 2 blank lines, found 1
E305 expected 2 blank lines after class or function definition, found 1 E401 multiple imports on one lineSigned-off-by Ricardo Martincoski <[email protected]>Cc Marcus Folkesson <[email protected]>
Quoted 7 lines
I haven't applied this one, because I don't really like the fact that we're further tweaking a script that comes from the kernel sources. On the other hand, we have already tweaked it for our purposes.
What do others think? Should we stay as close to the original upstream version as possible? Or because we have anyway already tweaked it, we don't really care and can now do whatever changes we believe are useful?
Well, I will try to keep the script reasonably synchronized between the buildroot and kernel repo, and these type of patches does not really help with that.
However, these scripts does not change that much, so I guess it does not really matters.
Quoted 8 lines
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Best regards
Marcus Folkesson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20180130/b6d5af48/attachment.asc>
PATCH 8/14 diffconfig: fix code style
Hello,
Quoted 1 line · Marcus Folkesson
On Mon, Jan 29, 2018 at 11:11:52PM +0100, Thomas Petazzoni wrote:
[snip]
Quoted 7 lines
Quoted 7 lines
I haven't applied this one, because I don't really like the fact that we're further tweaking a script that comes from the kernel sources. On the other hand, we have already tweaked it for our purposes.
What do others think? Should we stay as close to the original upstream version as possible? Or because we have anyway already tweaked it, we don't really care and can now do whatever changes we believe are useful?
I am happy either way.
If no one argues against, I will make flake8 ignore the file.
Quoted 3 lines
Well, I will try to keep the script reasonably synchronized between the buildroot and kernel repo, and these type of patches does not really help with that.
Ignoring the file can be accomplished by 2 ways:
Adding this line to utils/diffconfig
# flake8: noqa
Adding this line to .flake8
exclude=utils/diffconfig
It looks to me the later is preferable. This way a script that don't need any tweak can be a raw copy from the original.
Regards,
Ricardo
PATCH 8/14 diffconfig: fix code style
Hello,
Quoted 5 lines · Ricardo Martincoski
Adding this line to .flake8
exclude=utils/diffconfigIt looks to me the later is preferable. This way a script that don't need any tweak can be a raw copy from the original.
Agreed, the latter seems preferable to me.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com
PATCH 8/14 diffconfig: fix code style
Hello,
Quoted 11 lines · Thomas Petazzoni
Hello,
Quoted 5 lines · Ricardo Martincoski
Adding this line to .flake8
exclude=utils/diffconfigIt looks to me the later is preferable. This way a script that don't need any tweak can be a raw copy from the original.
Agreed, the latter seems preferable to me.
Yes, please do that.
Could you please add utils/config as well?
Quoted 8 lines
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com
Thank you,
Marcus Folkesson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20180213/fd3d44c0/attachment.asc>
PATCH 8/14 diffconfig: fix code style
Hello,
On Tue, Feb 13, 2018 at 06:44 AM, Marcus Folkesson wrote:
[snip]
Quoted 1 line
Could you please add utils/config as well?
It's not needed right now because only Python scripts will be checked in the GitLab job with this series applied.
Regards,
Ricardo
PATCH 9/14 genrandconfig: fix code style
In reply to Ricardo MartincoskiFix these warnings:
E201 whitespace after '['
E202 whitespace before ']'
Ignore these warnings:
E501 line too long (138 > 132 characters)
-> flake8 has options to ignore all warnings in a single file, to ignore a given warning for all files, and to ignore a given warning for a line. Unfortunately it does not have an option to ignore a given warning for a single file, so add the magic comments to all lines with the warning. It makes the lines even longer, but keeps the check for the rest of the file and stops generating warning for them. Alternative solutions would be to isolate the common part of the strings as a variable and concatenate on the fly the strings (using a bit more processing):
...="' + COMMON_URL + 'armv5-ctng-linux-gnueabi.tar.xz"\n' in ...
, or to break the strings in two (the interpreter would join them):
...="http://autobuild.buildroot.org/toolchains/tarballs/' \
'armv5-ctng-linux-gnueabi.tar.xz"\n' in ...
Also, don't try to align all comments because a new toolchain exception added to the script in the future can easily break this alignment.
--- utils/genrandconfig | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)
utils/genrandconfig +17 -17
@@ -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 = []
@@ -221,36 +221,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="http://autobuild.buildroot.org/toolchains/tarballs/armv5-ctng-linux-gnueabi.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/armv5-ctng-linux-gnueabi.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/armv6-ctng-linux-uclibcgnueabi.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/armv7-ctng-linux-gnueabihf.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/powerpc-ctng-linux-uclibc.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/powerpc-ctng-linux-uclibc.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/mips64el-ctng_n64-linux-gnu.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/mips64el-ctng_n64-linux-gnu.tar.xz"\n' in configlines: # noqa E501 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 +266,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="http://autobuild.buildroot.org/toolchains/tarballs/mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/mips64el-ctng_n64-linux-gnu.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/i486-ctng-linux-uclibc.tar.xz"\n' in configlines: # noqa E501 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="http://autobuild.buildroot.org/toolchains/tarballs/mipsel-ctng-linux-uclibc.tar.xz"\n' in configlines: # noqa E501 return False # The cs nios2 2017.02 toolchain is affected by binutils PR19405 if 'BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII=y\n' in configlines and \
Signature · 1 line
2.7.4
PATCH 9/14 genrandconfig: fix code style
Hello,
Quoted 15 lines · Ricardo Martincoski
Fix these warnings:
E201 whitespace after '['
E202 whitespace before ']'Ignore these warnings:
E501 line too long (138 > 132 characters)
-> flake8 has options to ignore all warnings in a single file, to ignore a given warning for all files, and to ignore a given warning for a line. Unfortunately it does not have an option to ignore a given warning for a single file, so add the magic comments to all lines with the warning. It makes the lines even longer, but keeps the check for the rest of the file and stops generating warning for them. Alternative solutions would be to isolate the common part of the strings as a variable and concatenate on the fly the strings (using a bit more processing):
Quoted 1 line
...="' + COMMON_URL + 'armv5-ctng-linux-gnueabi.tar.xz"\n' in ...
Yes, please do this. Or perhaps we could use some regexp, because we really don't care about the full URL, but just about the tarball name, no?
But I believe all those warning exceptions are really annoying to have. I'd really prefer a smarter solution here.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 9/14 genrandconfig: fix code style
Hello,
Quoted 1 line · Thomas Petazzoni
On Sun, 21 Jan 2018 22:44:37 -0200, Ricardo Martincoski wrote:
[snip]
Quoted 8 lines
Quoted 4 lines
Alternative solutions would be to isolate the common part of the strings as a variable and concatenate on the fly the strings (using a bit more processing):
...="' + COMMON_URL + 'armv5-ctng-linux-gnueabi.tar.xz"\n' in ...Yes, please do this. Or perhaps we could use some regexp, because we really don't care about the full URL, but just about the tarball name, no?
Regexp would work, but to avoid joining configlines into a string to search in, I think we should change how the config file is opened in this method:
with open(configfile) as configf:
config = configf.read()
...
if re.search(r'^BR2_PACKAGE_PYTHON_NFC=y$', config, re.M) and not sysinfo.has("bzr"):
return False
# The ctng toolchain is affected by PR58854
if re.search(r'^BR2_PACKAGE_LTTNG_TOOLS=y$', config, re.M) and \
re.search(r'^BR2_TOOLCHAIN_EXTERNAL_URL=".*armv5-ctng-linux-gnueabi.tar.xz"$', config, re.M):
return False
Notice all 'if' lines will be changed.
The nice thing about keeping using strings is that configlines is already the result of readlines(), so a list of strings, therefore each 'if' searches a string in a list.
BR2_TOOLCHAIN_EXTERNAL_URL = 'BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/'
...
if 'BR2_PACKAGE_PYTHON_NFC=y\n' in configlines and not sysinfo.has("bzr"):
return False
# The ctng toolchain is affected by PR58854
if 'BR2_PACKAGE_LTTNG_TOOLS=y\n' in configlines and \
BR2_TOOLCHAIN_EXTERNAL_URL + 'armv5-ctng-linux-gnueabi.tar.xz"\n' in configlines:
return False
We also have this code that takes advantage of configlines being a list:
configlines.remove('BR2_PACKAGE_SUNXI_BOARDS_FEX_FILE=""\n')
And only the long lines need to be changed.
Quoted 3 lines
But I believe all those warning exceptions are really annoying to have. I'd really prefer a smarter solution here.
From the two snippets above I prefer the later. What do you think? Any of the two, or another solution?
Regards,
Ricardo
PATCH 10/14 get-developers: fix code style
In reply to Ricardo MartincoskiFix these warnings:
E202 whitespace before ']'
E203 whitespace before ':'
E302 expected 2 blank lines, found 1
E305 expected 2 blank lines after class or function definition, found 1 E711 comparison to None should be 'if cond is None:'
E741 ambiguous variable name 'l'
F401 'sys' imported but unused
W391 blank line at end of file
--- utils/get-developers | 4 +++- utils/getdeveloperlib.py | 52 +++++++++++++++++++++++++++++------------------- 2 files changed, 34 insertions(+), 22 deletions(-)
utils/get-developers +3 -1
@@ -5,6 +5,7 @@ import getdeveloperlib import sys import os + def parse_args(): parser = argparse.ArgumentParser() parser.add_argument('patches', metavar='P', type=argparse.FileType('r'), nargs='*',
@@ -19,6 +20,7 @@ def parse_args(): const=True, help='list files not handled by any developer') return parser.parse_args() + def __main__(): devs = getdeveloperlib.parse_developers() if devs is None:
@@ -95,5 +97,5 @@ def __main__(): if result != "": print("git send-email %s" % result) -__main__() +__main__()
utils/getdeveloperlib.py +31 -21
167 lines not on this page · raw message
Signature · 1 line
2.7.4
PATCH 11/14 scanpypi: fix code style
In reply to Ricardo MartincoskiFix these warnings:
E101 indentation contains mixed spaces and tabs
E128 continuation line under-indented for visual indent
E231 missing whitespace after ','
E261 at least two spaces before inline comment
E302 expected 2 blank lines, found 1
E305 expected 2 blank lines after class or function definition, found 1 W191 indentation contains tabs
Ignore these warnings:
E402 module level import not at top of file
--- utils/scanpypi | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
utils/scanpypi +9 -4
@@ -35,6 +35,7 @@ except ImportError: 'pip install spdx_lookup') liclookup = None + def setup_decorator(func, method): """ Decorator for distutils.core.setup and setuptools.setup.
@@ -55,11 +56,12 @@ def setup_decorator(func, method): return closure # monkey patch -import setuptools +import setuptools # noqa E402 setuptools.setup = setup_decorator(setuptools.setup, 'setuptools') -import distutils +import distutils # noqa E402 distutils.core.setup = setup_decorator(setuptools.setup, 'distutils') + def find_file_upper_case(filenames, path='./'): """ List generator:
@@ -91,9 +93,11 @@ def pkg_buildroot_name(pkg_name): name = pattern.sub(r'python-\1', name) return name + class DownloadFailed(Exception): pass + class BuildrootPackage(): """This class's methods are not meant to be used individually please use them in the correct order:
@@ -267,14 +271,14 @@ class BuildrootPackage(): # called through the if __name__ == '__main__' directive. # In this case, we can only pray that it is called through a # function called main() in setup.py. - setup.main() # Will raise AttributeError if not found + setup.main() # Will raise AttributeError if not found self.setup_metadata = self.setup_args[self.metadata_name] # Here we must remove the module the hard way. # We must do this because of a very specific case: if a package calls # setup from the __main__ but does not come with a 'main()' function, # for some reason setup.main() will successfully call the main # function of a previous package... - sys.modules.pop('setup',None) + sys.modules.pop('setup', None) del setup os.chdir(current_dir) sys.path.remove(self.tmp_extract)
@@ -694,5 +698,6 @@ def main(): finally: shutil.rmtree(tmp_path) + if __name__ == "__main__": main()
Signature · 1 line
2.7.4
PATCH 11/14 scanpypi: fix code style
Quoted 9 lines · Ricardo Martincoski
Fix these warnings:
E101 indentation contains mixed spaces and tabs
E128 continuation line under-indented for visual indent
E231 missing whitespace after ','
E261 at least two spaces before inline comment
E302 expected 2 blank lines, found 1
E305 expected 2 blank lines after class or function definition, found 1 W191 indentation contains tabs
Quoted 7 lines
Ignore these warnings:
E402 module level import not at top of fileSigned-off-by Ricardo Martincoski <[email protected]>Cc Thomas Petazzoni <[email protected]>Cc Yegor Yefremov <[email protected]>---
Quoted 69 lines
utils/scanpypi | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)utils/scanpypi +9
-5@@ -35,6 +35,7 @@ except ImportError: 'pip install spdx_lookup') liclookup = None + def setup_decorator(func, method): """ Decorator for distutils.core.setup and setuptools.setup.@@ -55,11 +56,12 @@ def setup_decorator(func, method): return closure # monkey patch -import setuptools +import setuptools # noqa E402 setuptools.setup = setup_decorator(setuptools.setup, 'setuptools') -import distutils +import distutils # noqa E402 distutils.core.setup = setup_decorator(setuptools.setup, 'distutils') + def find_file_upper_case(filenames, path='./'): """ List generator:@@ -91,9 +93,11 @@ def pkg_buildroot_name(pkg_name): name = pattern.sub(r'python-\1', name) return name + class DownloadFailed(Exception): pass + class BuildrootPackage(): """This class's methods are not meant to be used individually please use them in the correct order:@@ -267,14 +271,14 @@ class BuildrootPackage(): # called through the if __name__ == '__main__' directive. # In this case, we can only pray that it is called through a # function called main() in setup.py. - setup.main() # Will raise AttributeError if not found + setup.main() # Will raise AttributeError if not found self.setup_metadata = self.setup_args[self.metadata_name] # Here we must remove the module the hard way. # We must do this because of a very specific case: if a package calls # setup from the __main__ but does not come with a 'main()' function, # for some reason setup.main() will successfully call the main # function of a previous package... - sys.modules.pop('setup',None) + sys.modules.pop('setup', None) del setup os.chdir(current_dir) sys.path.remove(self.tmp_extract)@@ -694,5 +698,6 @@ def main(): finally: shutil.rmtree(tmp_path) + if __name__ == "__main__": main() --2.7.4
PATCH 12/14 size-stats-compare: fix code style
In reply to Ricardo MartincoskiFix these warnings:
E129 visually indented line with same indent as next logical line E302 expected 2 blank lines, found 1
--- utils/size-stats-compare | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
utils/size-stats-compare +4 -1
@@ -24,6 +24,7 @@ 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 = {}
@@ -31,7 +32,7 @@ def read_file_size_csv(inputf, detail=None): header = next(reader) if (header[0] != 'File name' or header[1] != 'Package name' or - header[2] != 'File size' or header[3] != 'Package size'): + 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"""
Signature · 1 line
2.7.4
PATCH 12/14 size-stats-compare: fix code style
Hello,
Quoted 2 lines · Ricardo Martincoski
Fix these warnings:
E129 visually indented line with same indent as next logical line
Really?
utils/size-stats-compare def read_file_size_csv(inputf, detail=None): · line 35
Quoted 4 lines
header = next(reader)
if (header[0] != 'File name' or header[1] != 'Package name' orutils/size-stats-compare +1
-1- header[2] != 'File size' or header[3] != 'Package size'): + header[2] != 'File size' or header[3] != 'Package size'):
This looks totally bogus to me. The code was properly and nicely indented before the change, and now it looks badly indented. Is this really what flake8 wants? If so, flake8 is very strange.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 12/14 size-stats-compare: fix code style
Thomas, Ricardo, All,
On 2018-01-29 23:13 +0100, Thomas Petazzoni spake thusly:
Quoted 6 lines · Ricardo Martincoski
Quoted 2 lines
Fix these warnings:
E129 visually indented line with same indent as next logical lineReally?
Quoted 8 lines
utils/size-stats-compare def read_file_size_csv(inputf, detail=None): · line 35
Quoted 4 lines
header = next(reader)
if (header[0] != 'File name' or header[1] != 'Package name' orutils/size-stats-compare +1
-1- header[2] != 'File size' or header[3] != 'Package size'): + header[2] != 'File size' or header[3] != 'Package size'):This looks totally bogus to me. The code was properly and nicely indented before the change, and now it looks badly indented. Is this really what flake8 wants? If so, flake8 is very strange.
I guess it 's probably more about the following line than about hte previous one:
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 "... where there could be confusion with the 'printf' line.
Maybe the printf line could be indented one-level mnore, instead? Nope, that does not solve the issue. I guess flake8 does not do look-ahead...
In this case, I would be happy with an exception... # noqa E129
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | 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. | '------------------------------^-------^------------------^--------------------'
PATCH 12/14 size-stats-compare: fix code style
Hello,
Quoted 15 lines · Yann E. MORIN
On 2018-01-29 23:13 +0100, Thomas Petazzoni spake thusly:
Quoted 14 lines · Ricardo Martincoski
Quoted 2 lines
Fix these warnings:
E129 visually indented line with same indent as next logical lineReally?
utils/size-stats-compare def read_file_size_csv(inputf, detail=None): · line 35
Quoted 4 lines
header = next(reader)
if (header[0] != 'File name' or header[1] != 'Package name' orutils/size-stats-compare +1
-1- header[2] != 'File size' or header[3] != 'Package size'): + header[2] != 'File size' or header[3] != 'Package size'):This looks totally bogus to me. The code was properly and nicely indented before the change, and now it looks badly indented. Is this really what flake8 wants? If so, flake8 is very strange.
Sorry. I failed to explore all possible acceptable formats, see below...
Quoted 9 lines
I guess it 's probably more about the following line than about hte previous one:
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 "... where there could be confusion with the 'printf' line.
Yes. I think that's it.
Quoted 3 lines
Maybe the printf line could be indented one-level mnore, instead? Nope, that does not solve the issue. I guess flake8 does not do look-ahead...
Maybe one of these?
This I think it is uglier:
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 "... This is present in other scripts and I missed that in v1. I plan to use it if you don't oppose to.
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 "Quoted 2 lines
In this case, I would be happy with an exception... # noqa E129
If we don't agree with this rule, perhaps is better to ignore it for any new script, by adding this line to .flake8:
ignore=E129
Regards,
Ricardo
PATCH 12/14 size-stats-compare: fix code style
Hello,
Quoted 6 lines · Ricardo Martincoski
... This is present in other scripts and I missed that in v1. I plan to use it if you don't oppose to.
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 "
This version looks good to me. The parenthesis around the if condition are useless in Python.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com
PATCH 12/14 size-stats-compare: fix code style
Quoted 7 lines · Thomas Petazzoni
Hello,
Quoted 3 lines · Ricardo Martincoski
... This is present in other scripts and I missed that in v1. I plan to use it if you don't oppose to.
Quoted 6 lines
Quoted 3 lines
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 "This version looks good to me. The parenthesis around the if condition are useless in Python.
That is not completely accurate: the parentheses are not needed for the if, they are an alternative to providing a line-continuation character. I.e.
if (expression1 or
expression2):
is equivalent to:
if expression1 or \
expression2:Some people/groups prefer to avoid line continuation characters by using the parentheses style.
/Thomas
PATCH 13/14 support/dockerfile: allow to install packages
In reply to Ricardo MartincoskiSome tools in the Docker image can be useful to a single test only. Allow tests to install packages on the fly instead of adding everything to the base Docker image.
---
For a demo, see the beginning of the log at this job:
https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/48876752
--- support/docker/Dockerfile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
support/docker/Dockerfile +7 -1
@@ -20,7 +20,8 @@ RUN apt-get install -y -qq --no-install-recommends \ 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 \ + sudo RUN apt-get -q -y autoremove RUN apt-get -q -y clean
@@ -31,6 +32,11 @@ RUN /usr/sbin/locale-gen RUN useradd -ms /bin/bash br-user RUN chown -R br-user:br-user /home/br-user +# Allow user to install packages +RUN adduser br-user sudo +RUN echo "br-user ALL = NOPASSWD : /usr/bin/apt-get" > /etc/sudoers.d/apt-get +RUN chmod 440 /etc/sudoers.d/apt-get + USER br-user WORKDIR /home/br-user ENV HOME /home/br-user
Signature · 1 line
2.7.4
PATCH 13/14 support/dockerfile: allow to install packages
Ricardo, All,
On 2018-01-21 22:44 -0200, Ricardo Martincoski spake thusly:
Quoted 3 lines
Some tools in the Docker image can be useful to a single test only. Allow tests to install packages on the fly instead of adding everything to the base Docker image.
I think I would prefer that our base image has all the tools we need.
Regards,
Yann E. MORIN.
Quoted 41 lines
Signed-off-by Ricardo Martincoski <[email protected]>Cc Arnout Vandecappelle <[email protected]>Cc Peter Korsgaard <[email protected]>Cc Thomas Petazzoni <[email protected]>Cc Yann E. MORIN <[email protected]>---
For a demo, see the beginning of the log at this job:
https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/48876752--- support/docker/Dockerfile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)support/docker/Dockerfile +7
-1@@ -20,7 +20,8 @@ RUN apt-get install -y -qq --no-install-recommends \ 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 \ + sudo RUN apt-get -q -y autoremove RUN apt-get -q -y clean@@ -31,6 +32,11 @@ RUN /usr/sbin/locale-gen RUN useradd -ms /bin/bash br-user RUN chown -R br-user:br-user /home/br-user +# Allow user to install packages +RUN adduser br-user sudo +RUN echo "br-user ALL = NOPASSWD : /usr/bin/apt-get" > /etc/sudoers.d/apt-get +RUN chmod 440 /etc/sudoers.d/apt-get + USER br-user WORKDIR /home/br-user ENV HOME /home/br-userSignature · 1 line
2.7.4
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | 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. | '------------------------------^-------^------------------^--------------------'
PATCH 13/14 support/dockerfile: allow to install packages
Hello,
Quoted 6 lines · Yann E. MORIN
On 2018-01-21 22:44 -0200, Ricardo Martincoski spake thusly:
Quoted 3 lines
Some tools in the Docker image can be useful to a single test only. Allow tests to install packages on the fly instead of adding everything to the base Docker image.
I think I would prefer that our base image has all the tools we need.
I will do, if no one argues against.
To me, this patch is only a means to an end.
I am happy with any solution.
In the past Arnout suggested the opposite way
http://lists.busybox.net/pipermail/buildroot/2017-October/205253.html
but that was before we started to use the image as br-user.
Regards,
Ricardo
PATCH 14/14 .gitlab-ci.yml: check flake8
In reply to Ricardo MartincoskiAdd a test to check Python code style in the whole buildroot tree.
Install the needed tool on the fly instead of adding it to the Docker image since it's only used for this one test.
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.
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.
---
For a demo, see the end of the log at this job:
https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/48876752
--- .gitlab-ci.yml | 16 ++++++++++++++++ .gitlab-ci.yml.in | 16 ++++++++++++++++ 2 files changed, 32 insertions(+)
.gitlab-ci.yml +16 -0
@@ -30,6 +30,22 @@ check-DEVELOPERS: script: - "! utils/get-developers | grep -v 'No action specified'" +check-flake8: + before_script: + # Use a fixed version of the tool in order to get stable results. + - sudo apt-get update -qq + - sudo apt-get install -y -qq --no-install-recommends python-pip + - pip install -q setuptools + - pip install -q flake8==3.5.0 mccabe==0.6.1 pycodestyle==2.3.1 pyflakes==1.6.0 + # 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 {} +
.gitlab-ci.yml.in +16 -0
@@ -30,6 +30,22 @@ check-DEVELOPERS: script: - "! utils/get-developers | grep -v 'No action specified'" +check-flake8: + before_script: + # Use a fixed version of the tool in order to get stable results. + - sudo apt-get update -qq + - sudo apt-get install -y -qq --no-install-recommends python-pip + - pip install -q setuptools + - pip install -q flake8==3.5.0 mccabe==0.6.1 pycodestyle==2.3.1 pyflakes==1.6.0 + # 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 {} +
Signature · 1 line
2.7.4
cover fix Python code style
In reply to Ricardo MartincoskiHello,
Thanks for working on this. I've applied a number of patches, but not all of them. I'll say below which ones have been applied, and I will comment on the patches themselves for those that have not been applied.
Quoted 15 lines · Ricardo Martincoski
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.htmlThe first 12 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.
Most of the commit messages are terse as the changes I introduced are minimal. If I missed to explain any change that is not obvious, please ask me on the review.
Quoted 8 lines
Patch 13 adds support to install flake8 on the fly to the Docker image, needed by the next patch.
Patch 14 creates a job in gitlab to run flake8:
https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/48876752And here a complete run on gitlab using current Docker image:
https://gitlab.com/RicardoMartincoski/buildroot/pipelines/16432662
One rule that I find a bit silly in flake8 checks is "every function must be separated from the previous function with two blank lines". But OK, if flake8 wants that, and Python developers like it, why not.
Quoted 8 lines
Ricardo Martincoski (14):
graph-depends: fix code style
check-uniq-files: fix code style
graph-build-time: fix code style
pycompile: fix code style
size-stats: fix code style
testing/tests/boot/test_atf: fix code style
check-package: fix code style
All those applied.
Quoted 1 line
diffconfig: fix code style
Not applied.
Quoted 1 line
genrandconfig: fix code style
Not applied.
Quoted 2 lines
get-developers: fix code style
scanpypi: fix code style
Both of these applied.
Quoted 1 line
size-stats-compare: fix code style
Not applied.
Quoted 2 lines
support/dockerfile: allow to install packages
.gitlab-ci.yml: check flake8
And of course, I haven't applied those, because not all problems have been fixed yet, due to me not applying all patches.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
v2 cover fix Python code style v2
In reply to Ricardo MartincoskiHello,
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.
Patch 6 creates a job in GitLab to run flake8:
https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/56769344
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/18703075
Regards,
Ricardo
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 | 7 ++++- .../tests/package/test_python_cryptography.py | 23 ++++++++------ utils/genrandconfig | 36 ++++++++++++---------- utils/scanpypi | 4 ++- utils/size-stats-compare | 7 +++-- 8 files changed, 71 insertions(+), 31 deletions(-)
--
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)
Signature · 1 line
2.7.4
PATCH v2 1/7 genrandconfig: fix code style
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.
---
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(-)
utils/genrandconfig +19 -17
@@ -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 \
Signature · 1 line
2.7.4
PATCH v2 2/7 size-stats-compare: fix code style
In reply to Ricardo MartincoskiFix these warnings:
E129 visually indented line with same indent as next logical line E302 expected 2 blank lines, found 1
---
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(-)
utils/size-stats-compare +5 -2
@@ -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"""
Signature · 1 line
2.7.4
PATCH v2 2/7 size-stats-compare: fix code style
Ricardo, All,
On 2018-03-11 02:15 -0300, Ricardo Martincoski spake thusly:
Quoted 8 lines
Fix these warnings:
E129 visually indented line with same indent as next logical line E302 expected 2 blank lines, found 1Signed-off-by Ricardo Martincoski <[email protected]>Cc Thomas De Schampheleire <[email protected]>Cc Thomas Petazzoni <[email protected]>Cc Yann E. MORIN <[email protected]>
Quoted 48 lines
---
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(-)utils/size-stats-compare +5
-2@@ -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"""Signature · 1 line
2.7.4
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | 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. | '------------------------------^-------^------------------^--------------------'
PATCH v2 3/7 test_python_cryptography: fix code style
In reply to Ricardo MartincoskiFix 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
---
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/56769384
https://gitlab.com/RicardoMartincoski/buildroot/-/jobs/56769385
--- .../tests/package/test_python_cryptography.py | 23 ++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)
support/testing/tests/package/test_python_cryptography.py +13 -10
@@ -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)
Signature · 1 line
2.7.4
PATCH v2 4/7 scanpypi: fix code style
In reply to Ricardo MartincoskiFix these warnings:
E401 multiple imports on one line
---
Changes v1 -> v2:
- new patch.
--- utils/scanpypi | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
utils/scanpypi +3 -1
@@ -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
Signature · 1 line
2.7.4
PATCH v2 5/7 support/dockerfile: install flake8
In reply to Ricardo MartincoskiUse 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.
---
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 | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
support/docker/Dockerfile +6 -1
@@ -27,10 +27,15 @@ 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 pip install -q setuptools +RUN 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
Signature · 1 line
2.7.4
PATCH v2 5/7 support/dockerfile: install flake8
Ricardo, All,
On 2018-03-11 02:15 -0300, Ricardo Martincoski spake thusly:
Quoted 24 lines
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.
Signed-off-by Ricardo Martincoski <[email protected]>Cc Arnout Vandecappelle <[email protected]>Cc Peter Korsgaard <[email protected]>Cc Thomas Petazzoni <[email protected]>Cc Yann E. MORIN <[email protected]>---
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 | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)support/docker/Dockerfile +0
-0@@ -27,10 +27,15 @@ RUN dpkg --add-architecture i386 && \ cvs bzr git mercurial subversion wget \ cpio unzip \
support/docker/Dockerfile RUN dpkg --add-architecture i386 && \ · line 37
Quoted 10 lines
support/docker/Dockerfile +5
-1libncurses5-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 pip install -q setuptools +RUN pip install -q flake8==3.5.0 mccabe==0.6.1 pycodestyle==2.3.1 pyflakes==1.6.0
Please, run this with a single RUN command, to minimise the inter;ediate layers.
Also, you need to explain why you install setuptools separately, and why you don't force the version for it.
Finally, , have a single pacakge on each line, so that it is easier to maintain.
RUN pip install -q setuptools && \
pip install -q \
flake8==3.5.0 \
mccabe==0.6.1 \
pycodestyle==2.3.1 \
pyflakes==1.6.0Note: I know the list of deb packages is neither sorted nor one-package per line, but that's historical; best practices suggest a single package per line:
https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#minimize-the-number-of-layers
https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#sort-multi-line-arguments
Regards,
Yann E. MORIN.
Quoted 7 lines
support/docker/Dockerfile +1
-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-genSignature · 1 line
2.7.4
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | 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. | '------------------------------^-------^------------------^--------------------'
PATCH v2 5/7 support/dockerfile: install flake8
Hello,
Thank you for your review.
Now I see the entire series is missing from patchwork. Well... I need to send v3 anyway.
I will manually accept your review tags in the other patches.
Quoted 1 line · Yann E. MORIN
On 2018-03-11 02:15 -0300, Ricardo Martincoski spake thusly:
[snip]
Quoted 6 lines
support/docker/Dockerfile RUN dpkg --add-architecture i386 && \ · line 37
Quoted 3 lines
support/docker/Dockerfile +3
-0+# For check-flake8 +RUN pip install -q setuptools +RUN pip install -q flake8==3.5.0 mccabe==0.6.1 pycodestyle==2.3.1 pyflakes==1.6.0Please, run this with a single RUN command, to minimise the inter;ediate layers.
OK.
Quoted 2 lines
Also, you need to explain why you install setuptools separately, and why
I don't know the root cause for that. I tried (few months ago in the docker image) and installing setuptools and packages together didn't worked. Even in the docs, setuptools is assumed to be installed before installing packages.
https://packaging.python.org/tutorials/installing-packages/#requirements-for-installing-packages
But I will add this info to the commit log.
Quoted 1 line
you don't force the version for it.
Same here. I don't know the internals to say why we should always use the latest version. But maybe an acceptable approach is to use the exact command line the docs recommend (I didn't tried yet, but it should work):
python -m pip install --upgrade pip setuptools wheel
This way the docker image will be ready to install any package we eventually need.
Quoted 10 lines
Finally, , have a single pacakge on each line, so that it is easier to maintain.
RUN pip install -q setuptools && \ pip install -q \ flake8==3.5.0 \ mccabe==0.6.1 \ pycodestyle==2.3.1 \ pyflakes==1.6.0
Indeed, much easier to maintain. I will do.
Regards,
Ricardo
PATCH v2 6/7 .gitlab-ci.yml: check flake8
In reply to Ricardo MartincoskiAdd 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.
---
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/56769344
--- .gitlab-ci.yml | 11 +++++++++++ .gitlab-ci.yml.in | 11 +++++++++++ 2 files changed, 22 insertions(+)
.gitlab-ci.yml +11 -0
@@ -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 {} +
.gitlab-ci.yml.in +11 -0
@@ -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 {} +
Signature · 1 line
2.7.4
PATCH v2 6/7 .gitlab-ci.yml: check flake8
Ricardo, All,
On 2018-03-11 02:15 -0300, Ricardo Martincoski spake thusly:
Quoted 1 line
Add a test to check Python code style in the whole buildroot tree.
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.
Quoted 12 lines
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.
Quoted 3 lines
Signed-off-by Ricardo Martincoski <[email protected]>Cc Arnout Vandecappelle <[email protected]>Cc Yann E. MORIN <[email protected]>
Regards,
Yann E. MORIN.
Quoted 57 lines
---
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/56769344
--- .gitlab-ci.yml | 11 +++++++++++ .gitlab-ci.yml.in | 11 +++++++++++ 2 files changed, 22 insertions(+).gitlab-ci.yml +11
-0@@ -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 {} +.gitlab-ci.yml.in +11
-0@@ -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 {} +Signature · 1 line
2.7.4
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | 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. | '------------------------------^-------^------------------^--------------------'
PATCH v2 7/7 .flake8: ignore utils/diffconfig
In reply to Ricardo MartincoskiThis 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.
---
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(+)
.flake8 +3 -0
@@ -1,2 +1,5 @@ [flake8] +exclude= + # copied from the kernel sources + utils/diffconfig max-line-length=132
Signature · 1 line
2.7.4