All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 00/14] fix Python code style
@ 2018-01-22  0:44 Ricardo Martincoski
  2018-01-22  0:44 ` [Buildroot] [PATCH 01/14] graph-depends: fix " Ricardo Martincoski
                   ` (15 more replies)
  0 siblings, 16 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

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(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 01/14] graph-depends: fix code style
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-01-22  0:44 ` [Buildroot] [PATCH 02/14] check-uniq-files: " Ricardo Martincoski
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

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'

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 support/scripts/brpkgutil.py  |  8 +++--
 support/scripts/graph-depends | 68 ++++++++++++++++++++++++++-----------------
 2 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/support/scripts/brpkgutil.py b/support/scripts/brpkgutil.py
index a0e2352..4c99ae9 100644
--- a/support/scripts/brpkgutil.py
+++ b/support/scripts/brpkgutil.py
@@ -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.
diff --git a/support/scripts/graph-depends b/support/scripts/graph-depends
index db3041b..85c9bf0 100755
--- a/support/scripts/graph-depends
+++ b/support/scripts/graph-depends
@@ -30,7 +30,7 @@ import brpkgutil
 
 # Modes of operation:
 MODE_FULL = 1   # draw full dependency graph for all selected packages
-MODE_PKG  = 2   # draw dependency graph for a given package
+MODE_PKG = 2    # draw dependency graph for a given package
 mode = 0
 
 # Limit drawing the dependency graph to this depth. 0 means 'no limit'.
@@ -49,18 +49,18 @@ parser.add_argument("--package", '-p', metavar="PACKAGE",
 parser.add_argument("--depth", '-d', metavar="DEPTH", dest="depth", type=int, default=0,
                     help="Limit the dependency graph to DEPTH levels; 0 means no limit.")
 parser.add_argument("--stop-on", "-s", metavar="PACKAGE", dest="stop_list", action="append",
-                    help="Do not graph past this package (can be given multiple times)." \
-                       + " Can be a package name or a glob, " \
-                       + " 'virtual' to stop on virtual packages, or " \
-                       + "'host' to stop on host packages.")
+                    help="Do not graph past this package (can be given multiple times)." +
+                         " Can be a package name or a glob, " +
+                         " 'virtual' to stop on virtual packages, or " +
+                         "'host' to stop on host packages.")
 parser.add_argument("--exclude", "-x", metavar="PACKAGE", dest="exclude_list", action="append",
                     help="Like --stop-on, but do not add PACKAGE to the graph.")
 parser.add_argument("--colours", "-c", metavar="COLOR_LIST", dest="colours",
                     default="lightblue,grey,gainsboro",
-                    help="Comma-separated list of the three colours to use" \
-                       + " to draw the top-level package, the target" \
-                       + " packages, and the host packages, in this order." \
-                       + " Defaults to: 'lightblue,grey,gainsboro'")
+                    help="Comma-separated list of the three colours to use" +
+                         " to draw the top-level package, the target" +
+                         " packages, and the host packages, in this order." +
+                         " Defaults to: 'lightblue,grey,gainsboro'")
 parser.add_argument("--transitive", dest="transitive", action='store_true',
                     default=False)
 parser.add_argument("--no-transitive", dest="transitive", action='store_false',
@@ -114,7 +114,7 @@ else:
 # Get the colours: we need exactly three colours,
 # so no need not split more than 4
 # We'll let 'dot' validate the colours...
-colours = args.colours.split(',',4)
+colours = args.colours.split(',', 4)
 if len(colours) != 3:
     sys.stderr.write("Error: incorrect colour list '%s'\n" % args.colours)
     sys.exit(1)
@@ -124,6 +124,7 @@ host_colour = colours[2]
 
 allpkgs = []
 
+
 # Execute the "make show-targets" command to get the list of the main
 # Buildroot PACKAGES and return it formatted as a Python list. This
 # list is used as the starting point for full dependency graphs
@@ -138,6 +139,7 @@ def get_targets():
         return []
     return output.split(' ')
 
+
 # Recursive function that builds the tree of dependencies for a given
 # list of packages. The dependencies are built in a list called
 # 'dependencies', which contains tuples of the form (pkg1 ->
@@ -179,10 +181,12 @@ def get_all_depends(pkgs):
 
     return dependencies
 
+
 # The Graphviz "dot" utility doesn't like dashes in node names. So for
 # node names, we strip all dashes.
 def pkg_node_name(pkg):
-    return pkg.replace("-","")
+    return pkg.replace("-", "")
+
 
 TARGET_EXCEPTIONS = [
     "target-finalize",
@@ -225,35 +229,39 @@ for dep in dependencies:
 # sub-dicts is "pkg2".
 is_dep_cache = {}
 
+
 def is_dep_cache_insert(pkg, pkg2, val):
     try:
         is_dep_cache[pkg].update({pkg2: val})
     except KeyError:
         is_dep_cache[pkg] = {pkg2: val}
 
+
 # Retrieves from the cache whether pkg2 is a transitive dependency
 # of pkg.
 # Note: raises a KeyError exception if the dependency is not known.
 def is_dep_cache_lookup(pkg, pkg2):
     return is_dep_cache[pkg][pkg2]
 
+
 # This function return True if pkg is a dependency (direct or
 # transitive) of pkg2, dependencies being listed in the deps
 # dictionary. Returns False otherwise.
 # This is the un-cached version.
-def is_dep_uncached(pkg,pkg2,deps):
+def is_dep_uncached(pkg, pkg2, deps):
     try:
         for p in deps[pkg2]:
             if pkg == p:
                 return True
-            if is_dep(pkg,p,deps):
+            if is_dep(pkg, p, deps):
                 return True
     except KeyError:
         pass
     return False
 
+
 # See is_dep_uncached() above; this is the cached version.
-def is_dep(pkg,pkg2,deps):
+def is_dep(pkg, pkg2, deps):
     try:
         return is_dep_cache_lookup(pkg, pkg2)
     except KeyError:
@@ -261,6 +269,7 @@ def is_dep(pkg,pkg2,deps):
         is_dep_cache_insert(pkg, pkg2, val)
         return val
 
+
 # This function eliminates transitive dependencies; for example, given
 # these dependency chain: A->{B,C} and B->{C}, the A->{C} dependency is
 # already covered by B->{C}, so C is a transitive dependency of A, via B.
@@ -269,30 +278,32 @@ def is_dep(pkg,pkg2,deps):
 #     - if d[i] is a dependency of any of the other dependencies d[j]
 #       - do not keep d[i]
 #     - otherwise keep d[i]
-def remove_transitive_deps(pkg,deps):
+def remove_transitive_deps(pkg, deps):
     d = deps[pkg]
     new_d = []
     for i in range(len(d)):
         keep_me = True
         for j in range(len(d)):
-            if j==i:
+            if j == i:
                 continue
-            if is_dep(d[i],d[j],deps):
+            if is_dep(d[i], d[j], deps):
                 keep_me = False
         if keep_me:
             new_d.append(d[i])
     return new_d
 
+
 # This function removes the dependency on some 'mandatory' package, like the
 # 'toolchain' package, or the 'skeleton' package
-def remove_mandatory_deps(pkg,deps):
+def remove_mandatory_deps(pkg, deps):
     return [p for p in deps[pkg] if p not in ['toolchain', 'skeleton']]
 
+
 # This function will check that there is no loop in the dependency chain
 # As a side effect, it builds up the dependency cache.
 def check_circular_deps(deps):
     def recurse(pkg):
-        if not pkg in list(deps.keys()):
+        if pkg not in list(deps.keys()):
             return
         if pkg in not_loop:
             return
@@ -314,24 +325,27 @@ def check_circular_deps(deps):
     for pkg in list(deps.keys()):
         recurse(pkg)
 
+
 # This functions trims down the dependency list of all packages.
 # It applies in sequence all the dependency-elimination methods.
 def remove_extra_deps(deps):
     for pkg in list(deps.keys()):
         if not pkg == 'all':
-            deps[pkg] = remove_mandatory_deps(pkg,deps)
+            deps[pkg] = remove_mandatory_deps(pkg, deps)
     for pkg in list(deps.keys()):
         if not transitive or pkg == 'all':
-            deps[pkg] = remove_transitive_deps(pkg,deps)
+            deps[pkg] = remove_transitive_deps(pkg, deps)
     return deps
 
+
 check_circular_deps(dict_deps)
 if check_only:
     sys.exit(0)
 
 dict_deps = remove_extra_deps(dict_deps)
 dict_version = brpkgutil.get_version([pkg for pkg in allpkgs
-                                if pkg != "all" and not pkg.startswith("root")])
+                                      if pkg != "all" and not pkg.startswith("root")])
+
 
 # Print the attributes of a node: label and fill-color
 def print_attrs(pkg):
@@ -344,8 +358,8 @@ def print_attrs(pkg):
         color = root_colour
     else:
         if pkg.startswith('host') \
-        or pkg.startswith('toolchain') \
-        or pkg.startswith('rootfs'):
+                or pkg.startswith('toolchain') \
+                or pkg.startswith('rootfs'):
             color = host_colour
         else:
             color = target_colour
@@ -356,6 +370,7 @@ def print_attrs(pkg):
         outfile.write("%s [label = \"%s\"]\n" % (name, label))
     outfile.write("%s [color=%s,style=filled]\n" % (name, color))
 
+
 # Print the dependency graph of a package
 def print_pkg_deps(depth, pkg):
     if pkg in done_deps:
@@ -381,12 +396,13 @@ def print_pkg_deps(depth, pkg):
                 continue
             add = True
             for p in exclude_list:
-                if fnmatch(d,p):
+                if fnmatch(d, p):
                     add = False
                     break
             if add:
                 outfile.write("%s -> %s [dir=%s]\n" % (pkg_node_name(pkg), pkg_node_name(d), arrow_dir))
-                print_pkg_deps(depth+1, d)
+                print_pkg_deps(depth + 1, d)
+
 
 # Start printing the graph data
 outfile.write("digraph G {\n")
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 02/14] check-uniq-files: fix code style
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
  2018-01-22  0:44 ` [Buildroot] [PATCH 01/14] graph-depends: fix " Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-01-22  0:44 ` [Buildroot] [PATCH 03/14] graph-build-time: " Ricardo Martincoski
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

Fix 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

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
---
 support/scripts/check-uniq-files | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/support/scripts/check-uniq-files b/support/scripts/check-uniq-files
index ea5afdb..a3d1767 100755
--- a/support/scripts/check-uniq-files
+++ b/support/scripts/check-uniq-files
@@ -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())
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 03/14] graph-build-time: fix code style
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
  2018-01-22  0:44 ` [Buildroot] [PATCH 01/14] graph-depends: fix " Ricardo Martincoski
  2018-01-22  0:44 ` [Buildroot] [PATCH 02/14] check-uniq-files: " Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-01-22  0:44 ` [Buildroot] [PATCH 04/14] pycompile: " Ricardo Martincoski
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

Fix 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

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
---
 support/scripts/graph-build-time | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/support/scripts/graph-build-time b/support/scripts/graph-build-time
index 9d05f22..415d431 100755
--- a/support/scripts/graph-build-time
+++ b/support/scripts/graph-build-time
@@ -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)")
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 04/14] pycompile: fix code style
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
                   ` (2 preceding siblings ...)
  2018-01-22  0:44 ` [Buildroot] [PATCH 03/14] graph-build-time: " Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-01-22  8:50   ` Yegor Yefremov
  2018-01-22  0:44 ` [Buildroot] [PATCH 05/14] size-stats: " Ricardo Martincoski
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

Fix these warnings:
E302 expected 2 blank lines, found 1
E305 expected 2 blank lines after class or function definition, found 1

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
---
 support/scripts/pycompile.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
index 8e164ab..9192a70 100644
--- a/support/scripts/pycompile.py
+++ b/support/scripts/pycompile.py
@@ -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

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 05/14] size-stats: fix code style
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
                   ` (3 preceding siblings ...)
  2018-01-22  0:44 ` [Buildroot] [PATCH 04/14] pycompile: " Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-01-22  0:44 ` [Buildroot] [PATCH 06/14] testing/tests/boot/test_atf: " Ricardo Martincoski
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

Fix 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'

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 support/scripts/size-stats | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/support/scripts/size-stats b/support/scripts/size-stats
index 85e7c15..deea92e 100755
--- a/support/scripts/size-stats
+++ b/support/scripts/size-stats
@@ -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,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 06/14] testing/tests/boot/test_atf: fix code style
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
                   ` (4 preceding siblings ...)
  2018-01-22  0:44 ` [Buildroot] [PATCH 05/14] size-stats: " Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-01-22  0:44 ` [Buildroot] [PATCH 07/14] check-package: " Ricardo Martincoski
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

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

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 support/testing/tests/boot/test_atf.py | 124 +++++++++++++++++----------------
 1 file changed, 63 insertions(+), 61 deletions(-)

diff --git a/support/testing/tests/boot/test_atf.py b/support/testing/tests/boot/test_atf.py
index f1e58af..8288116 100644
--- a/support/testing/tests/boot/test_atf.py
+++ b/support/testing/tests/boot/test_atf.py
@@ -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
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 07/14] check-package: fix code style
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
                   ` (5 preceding siblings ...)
  2018-01-22  0:44 ` [Buildroot] [PATCH 06/14] testing/tests/boot/test_atf: " Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-01-22  0:44 ` [Buildroot] [PATCH 08/14] diffconfig: " Ricardo Martincoski
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

Ignore these warnings:
F401 'lib.ConsecutiveEmptyLines' imported but unused

And remove comments that are not needed anymore.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 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(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index 26ebb39..11d885f 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -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):
diff --git a/utils/checkpackagelib/lib_hash.py b/utils/checkpackagelib/lib_hash.py
index c4d055e..6d4cc9f 100644
--- a/utils/checkpackagelib/lib_hash.py
+++ b/utils/checkpackagelib/lib_hash.py
@@ -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):
diff --git a/utils/checkpackagelib/lib_mk.py b/utils/checkpackagelib/lib_mk.py
index a6cfeb6..2df2e1a 100644
--- a/utils/checkpackagelib/lib_mk.py
+++ b/utils/checkpackagelib/lib_mk.py
@@ -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):
diff --git a/utils/checkpackagelib/lib_patch.py b/utils/checkpackagelib/lib_patch.py
index 591f13c..555621a 100644
--- a/utils/checkpackagelib/lib_patch.py
+++ b/utils/checkpackagelib/lib_patch.py
@@ -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):
diff --git a/utils/checkpackagelib/readme.txt b/utils/checkpackagelib/readme.txt
index 85e10fe..3bfe289 100644
--- a/utils/checkpackagelib/readme.txt
+++ b/utils/checkpackagelib/readme.txt
@@ -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
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 08/14] diffconfig: fix code style
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
                   ` (6 preceding siblings ...)
  2018-01-22  0:44 ` [Buildroot] [PATCH 07/14] check-package: " Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-01-29 22:11   ` Thomas Petazzoni
  2018-01-22  0:44 ` [Buildroot] [PATCH 09/14] genrandconfig: " Ricardo Martincoski
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

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 line

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 utils/diffconfig | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/utils/diffconfig b/utils/diffconfig
index 5862a62..c8cd04c 100755
--- a/utils/diffconfig
+++ b/utils/diffconfig
@@ -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()
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 09/14] genrandconfig: fix code style
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
                   ` (7 preceding siblings ...)
  2018-01-22  0:44 ` [Buildroot] [PATCH 08/14] diffconfig: " Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-01-29 22:12   ` Thomas Petazzoni
  2018-01-22  0:44 ` [Buildroot] [PATCH 10/14] get-developers: " Ricardo Martincoski
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

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):
   ...="' + 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.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 utils/genrandconfig | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/utils/genrandconfig b/utils/genrandconfig
index 8833225..021d0f9 100755
--- a/utils/genrandconfig
+++ b/utils/genrandconfig
@@ -127,7 +127,7 @@ def get_toolchain_configs(toolchains_csv, buildrootdir):
 
     with open(toolchains_csv) as r:
         # filter empty lines and comments
-        lines = [ t for t in r.readlines() if len(t.strip()) > 0 and t[0] != '#' ]
+        lines = [t for t in r.readlines() if len(t.strip()) > 0 and t[0] != '#']
         toolchains = decode_byte_list(lines)
     configs = []
 
@@ -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 \
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 10/14] get-developers: fix code style
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
                   ` (8 preceding siblings ...)
  2018-01-22  0:44 ` [Buildroot] [PATCH 09/14] genrandconfig: " Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-01-22  0:44 ` [Buildroot] [PATCH 11/14] scanpypi: " Ricardo Martincoski
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

Fix 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

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 utils/get-developers     |  4 +++-
 utils/getdeveloperlib.py | 52 +++++++++++++++++++++++++++++-------------------
 2 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/utils/get-developers b/utils/get-developers
index 6a028b4..f525ff2 100755
--- a/utils/get-developers
+++ b/utils/get-developers
@@ -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__()
diff --git a/utils/getdeveloperlib.py b/utils/getdeveloperlib.py
index 6519107..2c8d477 100644
--- a/utils/getdeveloperlib.py
+++ b/utils/getdeveloperlib.py
@@ -1,7 +1,5 @@
-import sys
 import os
 import re
-import argparse
 import glob
 import subprocess
 
@@ -11,6 +9,7 @@ import subprocess
 
 FIND_INFRA_IN_PATCH = re.compile("^\+\$\(eval \$\((host-)?([^-]*)-package\)\)$")
 
+
 def analyze_patch(patch):
     """Parse one patch and return the list of files modified, added or
     removed by the patch."""
@@ -24,14 +23,16 @@ def analyze_patch(patch):
         if not line.startswith("+++ "):
             continue
         line.strip()
-        fname = line[line.find("/") + 1 : ].strip()
+        fname = line[line.find("/") + 1:].strip()
         if fname == "dev/null":
             continue
         files.add(fname)
     return (files, infras)
 
+
 FIND_INFRA_IN_MK = re.compile("^\$\(eval \$\((host-)?([^-]*)-package\)\)$")
 
+
 def fname_get_package_infra(fname):
     """Checks whether the file name passed as argument is a Buildroot .mk
     file describing a package, and find the infrastructure it's using."""
@@ -42,13 +43,14 @@ def fname_get_package_infra(fname):
         return None
 
     with open(fname, "r") as f:
-        for l in f:
-            l = l.strip()
-            m = FIND_INFRA_IN_MK.match(l)
+        for line in f:
+            line = line.strip()
+            m = FIND_INFRA_IN_MK.match(line)
             if m:
                 return m.group(2)
     return None
 
+
 def get_infras(files):
     """Search in the list of files for .mk files, and collect the package
     infrastructures used by those .mk files."""
@@ -59,6 +61,7 @@ def get_infras(files):
             infras.add(infra)
     return infras
 
+
 def analyze_patches(patches):
     """Parse a list of patches and returns the list of files modified,
     added or removed by the patches, as well as the list of package
@@ -72,6 +75,7 @@ def analyze_patches(patches):
     allinfras = allinfras | get_infras(allfiles)
     return (allfiles, allinfras)
 
+
 #
 # DEVELOPERS file parsing functions
 #
@@ -91,6 +95,7 @@ class Developer:
                 return True
         return False
 
+
 def parse_developer_packages(fnames):
     """Given a list of file patterns, travel through the Buildroot source
     tree to find which packages are implemented by those file
@@ -105,25 +110,27 @@ def parse_developer_packages(fnames):
                     packages.add(pkg)
     return packages
 
+
 def parse_arches_from_config_in(fname):
     """Given a path to an arch/Config.in.* file, parse it to get the list
     of BR2_ARCH values for this architecture."""
     arches = set()
     with open(fname, "r") as f:
         parsing_arches = False
-        for l in f:
-            l = l.strip()
-            if l == "config BR2_ARCH":
+        for line in f:
+            line = line.strip()
+            if line == "config BR2_ARCH":
                 parsing_arches = True
                 continue
             if parsing_arches:
-                m = re.match("^\s*default \"([^\"]*)\".*", l)
+                m = re.match("^\s*default \"([^\"]*)\".*", line)
                 if m:
                     arches.add(m.group(1))
                 else:
                     parsing_arches = False
     return arches
 
+
 def parse_developer_architectures(fnames):
     """Given a list of file names, find the ones starting by
     'arch/Config.in.', and use that to determine the architecture a
@@ -135,6 +142,7 @@ def parse_developer_architectures(fnames):
         arches = arches | parse_arches_from_config_in(fname)
     return arches
 
+
 def parse_developer_infras(fnames):
     infras = set()
     for fname in fnames:
@@ -143,37 +151,38 @@ def parse_developer_infras(fnames):
             infras.add(m.group(1))
     return infras
 
+
 def parse_developers(basepath=None):
     """Parse the DEVELOPERS file and return a list of Developer objects."""
     developers = []
     linen = 0
-    if basepath == None:
+    if basepath is None:
         basepath = os.getcwd()
     with open(os.path.join(basepath, "DEVELOPERS"), "r") as f:
         files = []
         name = None
-        for l in f:
-            l = l.strip()
-            if l.startswith("#"):
+        for line in f:
+            line = line.strip()
+            if line.startswith("#"):
                 continue
-            elif l.startswith("N:"):
+            elif line.startswith("N:"):
                 if name is not None or len(files) != 0:
                     print("Syntax error in DEVELOPERS file, line %d" % linen)
-                name = l[2:].strip()
-            elif l.startswith("F:"):
-                fname = l[2:].strip()
+                name = line[2:].strip()
+            elif line.startswith("F:"):
+                fname = line[2:].strip()
                 dev_files = glob.glob(os.path.join(basepath, fname))
                 if len(dev_files) == 0:
                     print("WARNING: '%s' doesn't match any file" % fname)
                 files += dev_files
-            elif l == "":
+            elif line == "":
                 if not name:
                     continue
                 developers.append(Developer(name, files))
                 files = []
                 name = None
             else:
-                print("Syntax error in DEVELOPERS file, line %d: '%s'" % (linen, l))
+                print("Syntax error in DEVELOPERS file, line %d: '%s'" % (linen, line))
                 return None
             linen += 1
     # handle last developer
@@ -181,10 +190,11 @@ def parse_developers(basepath=None):
         developers.append(Developer(name, files))
     return developers
 
+
 def check_developers(developers, basepath=None):
     """Look at the list of files versioned in Buildroot, and returns the
     list of files that are not handled by any developer"""
-    if basepath == None:
+    if basepath is None:
         basepath = os.getcwd()
     cmd = ["git", "--git-dir", os.path.join(basepath, ".git"), "ls-files"]
     files = subprocess.check_output(cmd).strip().split("\n")
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 11/14] scanpypi: fix code style
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
                   ` (9 preceding siblings ...)
  2018-01-22  0:44 ` [Buildroot] [PATCH 10/14] get-developers: " Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-01-22  8:49   ` Yegor Yefremov
  2018-01-22  0:44 ` [Buildroot] [PATCH 12/14] size-stats-compare: " Ricardo Martincoski
                   ` (4 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

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

Ignore these warnings:
E402 module level import not at top of file

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
---
 utils/scanpypi | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/utils/scanpypi b/utils/scanpypi
index 88fcb4a..d043135 100755
--- a/utils/scanpypi
+++ b/utils/scanpypi
@@ -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

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 12/14] size-stats-compare: fix code style
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
                   ` (10 preceding siblings ...)
  2018-01-22  0:44 ` [Buildroot] [PATCH 11/14] scanpypi: " Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-01-29 22:13   ` Thomas Petazzoni
  2018-01-22  0:44 ` [Buildroot] [PATCH 13/14] support/dockerfile: allow to install packages Ricardo Martincoski
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

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

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 utils/size-stats-compare | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/utils/size-stats-compare b/utils/size-stats-compare
index e5a1ec3..f69c3ed 100755
--- a/utils/size-stats-compare
+++ b/utils/size-stats-compare
@@ -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"""
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 13/14] support/dockerfile: allow to install packages
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
                   ` (11 preceding siblings ...)
  2018-01-22  0:44 ` [Buildroot] [PATCH 12/14] size-stats-compare: " Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-02-03 15:26   ` Yann E. MORIN
  2018-01-22  0:44 ` [Buildroot] [PATCH 14/14] .gitlab-ci.yml: check flake8 Ricardo Martincoski
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

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.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Peter Korsgaard <peter@korsgaard.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
---
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(-)

diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
index ebb471f..9d115d8 100644
--- a/support/docker/Dockerfile
+++ b/support/docker/Dockerfile
@@ -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
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 14/14] .gitlab-ci.yml: check flake8
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
                   ` (12 preceding siblings ...)
  2018-01-22  0:44 ` [Buildroot] [PATCH 13/14] support/dockerfile: allow to install packages Ricardo Martincoski
@ 2018-01-22  0:44 ` Ricardo Martincoski
  2018-01-29 22:14 ` [Buildroot] [PATCH 00/14] fix Python code style Thomas Petazzoni
  2018-03-11  5:15 ` [Buildroot] [PATCH v2 0/7] fix Python code style v2 Ricardo Martincoski
  15 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-01-22  0:44 UTC (permalink / raw)
  To: buildroot

Add 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.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
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(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index bf975fc..b665410 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -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 {} +
diff --git a/.gitlab-ci.yml.in b/.gitlab-ci.yml.in
index f763fdd..798404e 100644
--- a/.gitlab-ci.yml.in
+++ b/.gitlab-ci.yml.in
@@ -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 {} +
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 11/14] scanpypi: fix code style
  2018-01-22  0:44 ` [Buildroot] [PATCH 11/14] scanpypi: " Ricardo Martincoski
@ 2018-01-22  8:49   ` Yegor Yefremov
  0 siblings, 0 replies; 45+ messages in thread
From: Yegor Yefremov @ 2018-01-22  8:49 UTC (permalink / raw)
  To: buildroot

On Mon, Jan 22, 2018 at 1:44 AM, Ricardo Martincoski
<ricardo.martincoski@gmail.com> wrote:
> 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
>
> Ignore these warnings:
> E402 module level import not at top of file
>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Yegor Yefremov <yegorslists@googlemail.com>
> ---

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

>  utils/scanpypi | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/utils/scanpypi b/utils/scanpypi
> index 88fcb4a..d043135 100755
> --- a/utils/scanpypi
> +++ b/utils/scanpypi
> @@ -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
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 04/14] pycompile: fix code style
  2018-01-22  0:44 ` [Buildroot] [PATCH 04/14] pycompile: " Ricardo Martincoski
@ 2018-01-22  8:50   ` Yegor Yefremov
  0 siblings, 0 replies; 45+ messages in thread
From: Yegor Yefremov @ 2018-01-22  8:50 UTC (permalink / raw)
  To: buildroot

On Mon, Jan 22, 2018 at 1:44 AM, Ricardo Martincoski
<ricardo.martincoski@gmail.com> wrote:
> Fix these warnings:
> E302 expected 2 blank lines, found 1
> E305 expected 2 blank lines after class or function definition, found 1
>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Yegor Yefremov <yegorslists@googlemail.com>

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

> ---
>  support/scripts/pycompile.py | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/support/scripts/pycompile.py b/support/scripts/pycompile.py
> index 8e164ab..9192a70 100644
> --- a/support/scripts/pycompile.py
> +++ b/support/scripts/pycompile.py
> @@ -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
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 08/14] diffconfig: fix code style
  2018-01-22  0:44 ` [Buildroot] [PATCH 08/14] diffconfig: " Ricardo Martincoski
@ 2018-01-29 22:11   ` Thomas Petazzoni
  2018-01-30 20:25     ` Marcus Folkesson
  0 siblings, 1 reply; 45+ messages in thread
From: Thomas Petazzoni @ 2018-01-29 22:11 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 21 Jan 2018 22:44:36 -0200, Ricardo Martincoski wrote:
> 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 line
> 
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Marcus Folkesson <marcus.folkesson@gmail.com>

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 09/14] genrandconfig: fix code style
  2018-01-22  0:44 ` [Buildroot] [PATCH 09/14] genrandconfig: " Ricardo Martincoski
@ 2018-01-29 22:12   ` Thomas Petazzoni
  2018-02-13  3:12     ` Ricardo Martincoski
  0 siblings, 1 reply; 45+ messages in thread
From: Thomas Petazzoni @ 2018-01-29 22:12 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 21 Jan 2018 22:44:37 -0200, Ricardo Martincoski wrote:
> 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):
>    ...="' + 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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 12/14] size-stats-compare: fix code style
  2018-01-22  0:44 ` [Buildroot] [PATCH 12/14] size-stats-compare: " Ricardo Martincoski
@ 2018-01-29 22:13   ` Thomas Petazzoni
  2018-02-03 15:24     ` Yann E. MORIN
  0 siblings, 1 reply; 45+ messages in thread
From: Thomas Petazzoni @ 2018-01-29 22:13 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 21 Jan 2018 22:44:40 -0200, Ricardo Martincoski wrote:
> Fix these warnings:
> E129 visually indented line with same indent as next logical line

Really?

>      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'):

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 00/14] fix Python code style
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
                   ` (13 preceding siblings ...)
  2018-01-22  0:44 ` [Buildroot] [PATCH 14/14] .gitlab-ci.yml: check flake8 Ricardo Martincoski
@ 2018-01-29 22:14 ` Thomas Petazzoni
  2018-03-11  5:15 ` [Buildroot] [PATCH v2 0/7] fix Python code style v2 Ricardo Martincoski
  15 siblings, 0 replies; 45+ messages in thread
From: Thomas Petazzoni @ 2018-01-29 22:14 UTC (permalink / raw)
  To: buildroot

Hello,

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.

On Sun, 21 Jan 2018 22:44:28 -0200, Ricardo Martincoski wrote:

> 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

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.

> 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.

>   diffconfig: fix code style

Not applied.

>   genrandconfig: fix code style

Not applied.

>   get-developers: fix code style
>   scanpypi: fix code style

Both of these applied.

>   size-stats-compare: fix code style

Not applied.

>   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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 08/14] diffconfig: fix code style
  2018-01-29 22:11   ` Thomas Petazzoni
@ 2018-01-30 20:25     ` Marcus Folkesson
  2018-02-13  3:14       ` Ricardo Martincoski
  0 siblings, 1 reply; 45+ messages in thread
From: Marcus Folkesson @ 2018-01-30 20:25 UTC (permalink / raw)
  To: buildroot

Hi,

On Mon, Jan 29, 2018 at 11:11:52PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Sun, 21 Jan 2018 22:44:36 -0200, Ricardo Martincoski wrote:
> > 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 line
> > 
> > Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> > Cc: Marcus Folkesson <marcus.folkesson@gmail.com>
> 
> 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.

> 
> Best regards,
> 
> Thomas
> -- 
> 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>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 12/14] size-stats-compare: fix code style
  2018-01-29 22:13   ` Thomas Petazzoni
@ 2018-02-03 15:24     ` Yann E. MORIN
  2018-02-13  3:28       ` Ricardo Martincoski
  0 siblings, 1 reply; 45+ messages in thread
From: Yann E. MORIN @ 2018-02-03 15:24 UTC (permalink / raw)
  To: buildroot

Thomas, Ricardo, All,

On 2018-01-29 23:13 +0100, Thomas Petazzoni spake thusly:
> On Sun, 21 Jan 2018 22:44:40 -0200, Ricardo Martincoski wrote:
> > Fix these warnings:
> > E129 visually indented line with same indent as next logical line
> 
> Really?
> 
> >      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'):
> 
> 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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 13/14] support/dockerfile: allow to install packages
  2018-01-22  0:44 ` [Buildroot] [PATCH 13/14] support/dockerfile: allow to install packages Ricardo Martincoski
@ 2018-02-03 15:26   ` Yann E. MORIN
  2018-02-13  3:42     ` Ricardo Martincoski
  0 siblings, 1 reply; 45+ messages in thread
From: Yann E. MORIN @ 2018-02-03 15:26 UTC (permalink / raw)
  To: buildroot

Ricardo, All,

On 2018-01-21 22:44 -0200, Ricardo Martincoski spake thusly:
> 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.

> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
> 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(-)
> 
> diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
> index ebb471f..9d115d8 100644
> --- a/support/docker/Dockerfile
> +++ b/support/docker/Dockerfile
> @@ -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
> -- 
> 2.7.4
> 

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

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 09/14] genrandconfig: fix code style
  2018-01-29 22:12   ` Thomas Petazzoni
@ 2018-02-13  3:12     ` Ricardo Martincoski
  0 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-02-13  3:12 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon, Jan 29, 2018 at 08:12 PM, Thomas Petazzoni wrote:

> On Sun, 21 Jan 2018 22:44:37 -0200, Ricardo Martincoski wrote:
[snip]
>>  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.

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

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 08/14] diffconfig: fix code style
  2018-01-30 20:25     ` Marcus Folkesson
@ 2018-02-13  3:14       ` Ricardo Martincoski
  2018-02-13  7:49         ` Thomas Petazzoni
  0 siblings, 1 reply; 45+ messages in thread
From: Ricardo Martincoski @ 2018-02-13  3:14 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue, Jan 30, 2018 at 06:25 PM, Marcus Folkesson wrote:

> On Mon, Jan 29, 2018 at 11:11:52PM +0100, Thomas Petazzoni wrote:
[snip]
>> 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.

> 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

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 12/14] size-stats-compare: fix code style
  2018-02-03 15:24     ` Yann E. MORIN
@ 2018-02-13  3:28       ` Ricardo Martincoski
  2018-02-13  7:51         ` Thomas Petazzoni
  0 siblings, 1 reply; 45+ messages in thread
From: Ricardo Martincoski @ 2018-02-13  3:28 UTC (permalink / raw)
  To: buildroot

Hello,

On Sat, Feb 03, 2018 at 01:24 PM, Yann E. MORIN wrote:

> On 2018-01-29 23:13 +0100, Thomas Petazzoni spake thusly:
>> On Sun, 21 Jan 2018 22:44:40 -0200, Ricardo Martincoski wrote:
>> > Fix these warnings:
>> > E129 visually indented line with same indent as next logical line
>> 
>> Really?
>> 
>> >      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'):
>> 
>> 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...

> 
> 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.

> 
> 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 "

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

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 13/14] support/dockerfile: allow to install packages
  2018-02-03 15:26   ` Yann E. MORIN
@ 2018-02-13  3:42     ` Ricardo Martincoski
  0 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-02-13  3:42 UTC (permalink / raw)
  To: buildroot

Hello,

On Sat, Feb 03, 2018 at 01:26 PM, Yann E. MORIN wrote:

> On 2018-01-21 22:44 -0200, Ricardo Martincoski spake thusly:
>> 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

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 08/14] diffconfig: fix code style
  2018-02-13  3:14       ` Ricardo Martincoski
@ 2018-02-13  7:49         ` Thomas Petazzoni
  2018-02-13  8:44           ` Marcus Folkesson
  0 siblings, 1 reply; 45+ messages in thread
From: Thomas Petazzoni @ 2018-02-13  7:49 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue, 13 Feb 2018 01:14:40 -0200, Ricardo Martincoski wrote:

> 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.

Agreed, the latter seems preferable to me.

Thanks!

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

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 12/14] size-stats-compare: fix code style
  2018-02-13  3:28       ` Ricardo Martincoski
@ 2018-02-13  7:51         ` Thomas Petazzoni
  2018-02-13 20:53           ` Thomas De Schampheleire
  0 siblings, 1 reply; 45+ messages in thread
From: Thomas Petazzoni @ 2018-02-13  7:51 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue, 13 Feb 2018 01:28:31 -0200, Ricardo Martincoski wrote:

> ... 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
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 08/14] diffconfig: fix code style
  2018-02-13  7:49         ` Thomas Petazzoni
@ 2018-02-13  8:44           ` Marcus Folkesson
  2018-02-14  0:32             ` Ricardo Martincoski
  0 siblings, 1 reply; 45+ messages in thread
From: Marcus Folkesson @ 2018-02-13  8:44 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue, Feb 13, 2018 at 08:49:30AM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Tue, 13 Feb 2018 01:14:40 -0200, Ricardo Martincoski wrote:
> 
> > 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.
> 
> Agreed, the latter seems preferable to me.

Yes, please do that.

Could you please add utils/config as well?

> 
> Thanks!
> 
> Thomas
> -- 
> 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>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 12/14] size-stats-compare: fix code style
  2018-02-13  7:51         ` Thomas Petazzoni
@ 2018-02-13 20:53           ` Thomas De Schampheleire
  0 siblings, 0 replies; 45+ messages in thread
From: Thomas De Schampheleire @ 2018-02-13 20:53 UTC (permalink / raw)
  To: buildroot


On Tue, Feb 13, 2018 at 08:51:34AM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Tue, 13 Feb 2018 01:28:31 -0200, Ricardo Martincoski wrote:
> 
> > ... 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.

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

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH 08/14] diffconfig: fix code style
  2018-02-13  8:44           ` Marcus Folkesson
@ 2018-02-14  0:32             ` Ricardo Martincoski
  0 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-02-14  0:32 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue, Feb 13, 2018 at 06:44 AM, Marcus Folkesson wrote:
[snip]
> 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

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH v2 0/7] fix Python code style v2
  2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
                   ` (14 preceding siblings ...)
  2018-01-29 22:14 ` [Buildroot] [PATCH 00/14] fix Python code style Thomas Petazzoni
@ 2018-03-11  5:15 ` Ricardo Martincoski
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 1/7] genrandconfig: fix code style Ricardo Martincoski
                     ` (6 more replies)
  15 siblings, 7 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-03-11  5:15 UTC (permalink / raw)
  To: buildroot

Hello,

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

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

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

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

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)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH v2 1/7] genrandconfig: fix code style
  2018-03-11  5:15 ` [Buildroot] [PATCH v2 0/7] fix Python code style v2 Ricardo Martincoski
@ 2018-03-11  5:15   ` Ricardo Martincoski
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 2/7] size-stats-compare: " Ricardo Martincoski
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-03-11  5:15 UTC (permalink / raw)
  To: buildroot

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

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Changes v1 -> v2:  (suggested by Thomas Petazzoni)
  - fix the long lines instead of ignoring them

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

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

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH v2 2/7] size-stats-compare: fix code style
  2018-03-11  5:15 ` [Buildroot] [PATCH v2 0/7] fix Python code style v2 Ricardo Martincoski
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 1/7] genrandconfig: fix code style Ricardo Martincoski
@ 2018-03-11  5:15   ` Ricardo Martincoski
  2018-03-11  8:57     ` Yann E. MORIN
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 3/7] test_python_cryptography: " Ricardo Martincoski
                     ` (4 subsequent siblings)
  6 siblings, 1 reply; 45+ messages in thread
From: Ricardo Martincoski @ 2018-03-11  5:15 UTC (permalink / raw)
  To: buildroot

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

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
---
Changes v1 -> v2:  (suggested by Thomas Petazzoni)
  - the fix for E129 made the code less readable, use another fix.
---
 utils/size-stats-compare | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

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

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH v2 3/7] test_python_cryptography: fix code style
  2018-03-11  5:15 ` [Buildroot] [PATCH v2 0/7] fix Python code style v2 Ricardo Martincoski
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 1/7] genrandconfig: fix code style Ricardo Martincoski
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 2/7] size-stats-compare: " Ricardo Martincoski
@ 2018-03-11  5:15   ` Ricardo Martincoski
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 4/7] scanpypi: " Ricardo Martincoski
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-03-11  5:15 UTC (permalink / raw)
  To: buildroot

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

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
---
Changes 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(-)

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

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH v2 4/7] scanpypi: fix code style
  2018-03-11  5:15 ` [Buildroot] [PATCH v2 0/7] fix Python code style v2 Ricardo Martincoski
                     ` (2 preceding siblings ...)
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 3/7] test_python_cryptography: " Ricardo Martincoski
@ 2018-03-11  5:15   ` Ricardo Martincoski
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 5/7] support/dockerfile: install flake8 Ricardo Martincoski
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-03-11  5:15 UTC (permalink / raw)
  To: buildroot

Fix these warnings:
E401 multiple imports on one line

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
---
Changes v1 -> v2:
  - new patch.
---
 utils/scanpypi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

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

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH v2 5/7] support/dockerfile: install flake8
  2018-03-11  5:15 ` [Buildroot] [PATCH v2 0/7] fix Python code style v2 Ricardo Martincoski
                     ` (3 preceding siblings ...)
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 4/7] scanpypi: " Ricardo Martincoski
@ 2018-03-11  5:15   ` Ricardo Martincoski
  2018-03-11  8:51     ` Yann E. MORIN
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 6/7] .gitlab-ci.yml: check flake8 Ricardo Martincoski
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 7/7] .flake8: ignore utils/diffconfig Ricardo Martincoski
  6 siblings, 1 reply; 45+ messages in thread
From: Ricardo Martincoski @ 2018-03-11  5:15 UTC (permalink / raw)
  To: buildroot

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 <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Peter Korsgaard <peter@korsgaard.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
---
Changes 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(-)

diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
index ce3fdd9..f2ba064 100644
--- a/support/docker/Dockerfile
+++ b/support/docker/Dockerfile
@@ -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
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH v2 6/7] .gitlab-ci.yml: check flake8
  2018-03-11  5:15 ` [Buildroot] [PATCH v2 0/7] fix Python code style v2 Ricardo Martincoski
                     ` (4 preceding siblings ...)
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 5/7] support/dockerfile: install flake8 Ricardo Martincoski
@ 2018-03-11  5:15   ` Ricardo Martincoski
  2018-03-11  8:54     ` Yann E. MORIN
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 7/7] .flake8: ignore utils/diffconfig Ricardo Martincoski
  6 siblings, 1 reply; 45+ messages in thread
From: Ricardo Martincoski @ 2018-03-11  5:15 UTC (permalink / raw)
  To: buildroot

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

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

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

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
---
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(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 4cdaa09..efd2587 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -30,6 +30,17 @@ check-DEVELOPERS:
     script:
         - "! utils/get-developers | grep -v 'No action specified'"
 
+check-flake8:
+    before_script:
+        # Help flake8 to find the Python files without .py extension.
+        - find * -type f -name '*.py' > files.txt
+        - find * -type f -print0 | xargs -0 file | grep 'Python script' | cut -d':' -f1 >> files.txt
+        - sort -u files.txt | tee files.processed
+    script:
+        - python -m flake8 --statistics --count $(cat files.processed)
+    after_script:
+        - wc -l files.processed
+
 check-package:
     script:
         - find . -type f \( -name '*.mk' -o -name '*.hash' \) -exec ./utils/check-package {} +
diff --git a/.gitlab-ci.yml.in b/.gitlab-ci.yml.in
index cb3eb71..95fc025 100644
--- a/.gitlab-ci.yml.in
+++ b/.gitlab-ci.yml.in
@@ -30,6 +30,17 @@ check-DEVELOPERS:
     script:
         - "! utils/get-developers | grep -v 'No action specified'"
 
+check-flake8:
+    before_script:
+        # Help flake8 to find the Python files without .py extension.
+        - find * -type f -name '*.py' > files.txt
+        - find * -type f -print0 | xargs -0 file | grep 'Python script' | cut -d':' -f1 >> files.txt
+        - sort -u files.txt | tee files.processed
+    script:
+        - python -m flake8 --statistics --count $(cat files.processed)
+    after_script:
+        - wc -l files.processed
+
 check-package:
     script:
         - find . -type f \( -name '*.mk' -o -name '*.hash' \) -exec ./utils/check-package {} +
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH v2 7/7] .flake8: ignore utils/diffconfig
  2018-03-11  5:15 ` [Buildroot] [PATCH v2 0/7] fix Python code style v2 Ricardo Martincoski
                     ` (5 preceding siblings ...)
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 6/7] .gitlab-ci.yml: check flake8 Ricardo Martincoski
@ 2018-03-11  5:15   ` Ricardo Martincoski
  6 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-03-11  5:15 UTC (permalink / raw)
  To: buildroot

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

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

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Marcus Folkesson <marcus.folkesson@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Changes v1 -> v2:
  - instead of tweaking even more the script from the kernel source,
    ignore any warnings for it;  (suggested by Thomas Petazzoni)
  - move the patch after the one adding the GitLab job so I can
    reference it in the commit message.
---
 .flake8 | 3 +++
 1 file changed, 3 insertions(+)

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

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH v2 5/7] support/dockerfile: install flake8
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 5/7] support/dockerfile: install flake8 Ricardo Martincoski
@ 2018-03-11  8:51     ` Yann E. MORIN
  2018-03-12  1:19       ` Ricardo Martincoski
  0 siblings, 1 reply; 45+ messages in thread
From: Yann E. MORIN @ 2018-03-11  8:51 UTC (permalink / raw)
  To: buildroot

Ricardo, All,

On 2018-03-11 02:15 -0300, Ricardo Martincoski spake thusly:
> Use the latest version of the tool because it is actively maintained.
> But use a fixed version of the tool and its dependencies to get stable
> results. It can be manually bumped from time to time.
> 
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
> Changes 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(-)
> 
> diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
> index ce3fdd9..f2ba064 100644
> --- a/support/docker/Dockerfile
> +++ b/support/docker/Dockerfile
> @@ -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

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.0

Note: 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.

> +
>  # To be able to generate a toolchain with locales, enable one UTF-8 locale
>  RUN sed -i 's/# \(en_US.UTF-8\)/\1/' /etc/locale.gen && \
>      /usr/sbin/locale-gen
> -- 
> 2.7.4
> 

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

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH v2 6/7] .gitlab-ci.yml: check flake8
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 6/7] .gitlab-ci.yml: check flake8 Ricardo Martincoski
@ 2018-03-11  8:54     ` Yann E. MORIN
  0 siblings, 0 replies; 45+ messages in thread
From: Yann E. MORIN @ 2018-03-11  8:54 UTC (permalink / raw)
  To: buildroot

Ricardo, All,

On 2018-03-11 02:15 -0300, Ricardo Martincoski spake thusly:
> 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.

> Search files by type in order to help flake8 to find the Python scripts
> without .py extension. But don't rely only in the output of 'file' as it
> uses heuristics and sometimes it is wrong (specially identifying Python
> files as C++ source for the 'file' version currently in the Docker
> image).
> 
> Include in the output:
>  - the list of Python files processed;
>  - statistics for each kind of warning;
>  - the total number of warnings;
>  - the number of Python files processed.
> 
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>

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

Regards,
Yann E. MORIN.

> ---
> 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(+)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 4cdaa09..efd2587 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -30,6 +30,17 @@ check-DEVELOPERS:
>      script:
>          - "! utils/get-developers | grep -v 'No action specified'"
>  
> +check-flake8:
> +    before_script:
> +        # Help flake8 to find the Python files without .py extension.
> +        - find * -type f -name '*.py' > files.txt
> +        - find * -type f -print0 | xargs -0 file | grep 'Python script' | cut -d':' -f1 >> files.txt
> +        - sort -u files.txt | tee files.processed
> +    script:
> +        - python -m flake8 --statistics --count $(cat files.processed)
> +    after_script:
> +        - wc -l files.processed
> +
>  check-package:
>      script:
>          - find . -type f \( -name '*.mk' -o -name '*.hash' \) -exec ./utils/check-package {} +
> diff --git a/.gitlab-ci.yml.in b/.gitlab-ci.yml.in
> index cb3eb71..95fc025 100644
> --- a/.gitlab-ci.yml.in
> +++ b/.gitlab-ci.yml.in
> @@ -30,6 +30,17 @@ check-DEVELOPERS:
>      script:
>          - "! utils/get-developers | grep -v 'No action specified'"
>  
> +check-flake8:
> +    before_script:
> +        # Help flake8 to find the Python files without .py extension.
> +        - find * -type f -name '*.py' > files.txt
> +        - find * -type f -print0 | xargs -0 file | grep 'Python script' | cut -d':' -f1 >> files.txt
> +        - sort -u files.txt | tee files.processed
> +    script:
> +        - python -m flake8 --statistics --count $(cat files.processed)
> +    after_script:
> +        - wc -l files.processed
> +
>  check-package:
>      script:
>          - find . -type f \( -name '*.mk' -o -name '*.hash' \) -exec ./utils/check-package {} +
> -- 
> 2.7.4
> 

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

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH v2 2/7] size-stats-compare: fix code style
  2018-03-11  5:15   ` [Buildroot] [PATCH v2 2/7] size-stats-compare: " Ricardo Martincoski
@ 2018-03-11  8:57     ` Yann E. MORIN
  0 siblings, 0 replies; 45+ messages in thread
From: Yann E. MORIN @ 2018-03-11  8:57 UTC (permalink / raw)
  To: buildroot

Ricardo, All,

On 2018-03-11 02:15 -0300, Ricardo Martincoski spake thusly:
> Fix these warnings:
> E129 visually indented line with same indent as next logical line
> E302 expected 2 blank lines, found 1
> 
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>

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

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

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [Buildroot] [PATCH v2 5/7] support/dockerfile: install flake8
  2018-03-11  8:51     ` Yann E. MORIN
@ 2018-03-12  1:19       ` Ricardo Martincoski
  0 siblings, 0 replies; 45+ messages in thread
From: Ricardo Martincoski @ 2018-03-12  1:19 UTC (permalink / raw)
  To: buildroot

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.

On Sun, Mar 11, 2018 at 05:51 AM, Yann E. MORIN wrote:

> On 2018-03-11 02:15 -0300, Ricardo Martincoski spake thusly:
[snip]
>> +# 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.

OK.

> 
> 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.

> 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.

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

^ permalink raw reply	[flat|nested] 45+ messages in thread

end of thread, other threads:[~2018-03-12  1:19 UTC | newest]

Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-22  0:44 [Buildroot] [PATCH 00/14] fix Python code style Ricardo Martincoski
2018-01-22  0:44 ` [Buildroot] [PATCH 01/14] graph-depends: fix " Ricardo Martincoski
2018-01-22  0:44 ` [Buildroot] [PATCH 02/14] check-uniq-files: " Ricardo Martincoski
2018-01-22  0:44 ` [Buildroot] [PATCH 03/14] graph-build-time: " Ricardo Martincoski
2018-01-22  0:44 ` [Buildroot] [PATCH 04/14] pycompile: " Ricardo Martincoski
2018-01-22  8:50   ` Yegor Yefremov
2018-01-22  0:44 ` [Buildroot] [PATCH 05/14] size-stats: " Ricardo Martincoski
2018-01-22  0:44 ` [Buildroot] [PATCH 06/14] testing/tests/boot/test_atf: " Ricardo Martincoski
2018-01-22  0:44 ` [Buildroot] [PATCH 07/14] check-package: " Ricardo Martincoski
2018-01-22  0:44 ` [Buildroot] [PATCH 08/14] diffconfig: " Ricardo Martincoski
2018-01-29 22:11   ` Thomas Petazzoni
2018-01-30 20:25     ` Marcus Folkesson
2018-02-13  3:14       ` Ricardo Martincoski
2018-02-13  7:49         ` Thomas Petazzoni
2018-02-13  8:44           ` Marcus Folkesson
2018-02-14  0:32             ` Ricardo Martincoski
2018-01-22  0:44 ` [Buildroot] [PATCH 09/14] genrandconfig: " Ricardo Martincoski
2018-01-29 22:12   ` Thomas Petazzoni
2018-02-13  3:12     ` Ricardo Martincoski
2018-01-22  0:44 ` [Buildroot] [PATCH 10/14] get-developers: " Ricardo Martincoski
2018-01-22  0:44 ` [Buildroot] [PATCH 11/14] scanpypi: " Ricardo Martincoski
2018-01-22  8:49   ` Yegor Yefremov
2018-01-22  0:44 ` [Buildroot] [PATCH 12/14] size-stats-compare: " Ricardo Martincoski
2018-01-29 22:13   ` Thomas Petazzoni
2018-02-03 15:24     ` Yann E. MORIN
2018-02-13  3:28       ` Ricardo Martincoski
2018-02-13  7:51         ` Thomas Petazzoni
2018-02-13 20:53           ` Thomas De Schampheleire
2018-01-22  0:44 ` [Buildroot] [PATCH 13/14] support/dockerfile: allow to install packages Ricardo Martincoski
2018-02-03 15:26   ` Yann E. MORIN
2018-02-13  3:42     ` Ricardo Martincoski
2018-01-22  0:44 ` [Buildroot] [PATCH 14/14] .gitlab-ci.yml: check flake8 Ricardo Martincoski
2018-01-29 22:14 ` [Buildroot] [PATCH 00/14] fix Python code style Thomas Petazzoni
2018-03-11  5:15 ` [Buildroot] [PATCH v2 0/7] fix Python code style v2 Ricardo Martincoski
2018-03-11  5:15   ` [Buildroot] [PATCH v2 1/7] genrandconfig: fix code style Ricardo Martincoski
2018-03-11  5:15   ` [Buildroot] [PATCH v2 2/7] size-stats-compare: " Ricardo Martincoski
2018-03-11  8:57     ` Yann E. MORIN
2018-03-11  5:15   ` [Buildroot] [PATCH v2 3/7] test_python_cryptography: " Ricardo Martincoski
2018-03-11  5:15   ` [Buildroot] [PATCH v2 4/7] scanpypi: " Ricardo Martincoski
2018-03-11  5:15   ` [Buildroot] [PATCH v2 5/7] support/dockerfile: install flake8 Ricardo Martincoski
2018-03-11  8:51     ` Yann E. MORIN
2018-03-12  1:19       ` Ricardo Martincoski
2018-03-11  5:15   ` [Buildroot] [PATCH v2 6/7] .gitlab-ci.yml: check flake8 Ricardo Martincoski
2018-03-11  8:54     ` Yann E. MORIN
2018-03-11  5:15   ` [Buildroot] [PATCH v2 7/7] .flake8: ignore utils/diffconfig Ricardo Martincoski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.