[Buildroot] [PATCH 1/5] support/docker: run apt-get update and apt-get install in two RUNs

[Buildroot] [PATCH 1/5] support/docker: run apt-get update and apt-get install in two RUNs

[Yann E. MORIN]

In commit 7517aef4d (support/docker: limit the number of layers),
we reduced the number of layers by coalescing multiple RUN commands
into less commands.

In doing so, we especially coalesced "apt-get update" with "apt-get
install".

However, the distribution we used is a pinned version of stretch, so
we know that running apt-get update will always yield the same apt
database.

If we split the two apt-get commands, then we can re-use any local
intermediate image when we need to update the list of packages to
install; this helps quite a bit when testing the docker files over
and over again, with just slight variants in the packages list.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Peter Korsgaard <peter@korsgaard.com>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 support/docker/Dockerfile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
index f01ac25f93..8c525f7cf1 100644
--- a/support/docker/Dockerfile
+++ b/support/docker/Dockerfile
@@ -20,8 +20,8 @@ COPY apt-sources.list /etc/apt/sources.list
 
 # The container has no package lists, so need to update first
 RUN dpkg --add-architecture i386 && \
-    apt-get update -y && \
-    apt-get install -y --no-install-recommends \
+    apt-get update -y
+RUN apt-get install -y --no-install-recommends \
         build-essential cmake libc6:i386 g++-multilib \
         bc ca-certificates file locales rsync \
         cvs bzr git mercurial subversion wget \
-- 
2.14.1

[Buildroot] [PATCH 0/5] support/docker: make the image more reproducible

[Yann E. MORIN]

Hello All!

Currently, the base test image is not entirely reproducible, because we
install packages from PyPI without specifying all the versions of all
the packages we install from there.

Instead of relying on a third-party to provide packages, just use those
from the distribution, because we are using a pinned version and thus
we will always get the same packages from the distro.

Eventually, we also update to a python3-flavoured flake8, as that
catches even more issues. Those issues are also fixed. Note that, even
if we decide against switching to a python3 flake8, we can still apply
patch 4, to fix the issues that it reported.

The result of using this series can be seen at:
    https://gitlab.com/ymorin/buildroot-ci/pipelines/23095050

If/when this series is applied, I'll regenerate and updalod the new
buildroot/base image, and send a furhter patch to use it from our
gitlab-ci.yml.


Regards,
Yann E. MORIN.


The following changes since commit 6a29c8ae3ee2f2ef1a7bdac48fa96d29d9e9b62f

  librsync: bump to version 2.0.2 (2018-06-02 16:06:02 +0200)


are available in the git repository at:

  git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to ed69b61801983e7f424b062eb586669b6d59a969

  support/docker: update to python3-flavoured flake8 (2018-06-03 10:55:23 +0200)


----------------------------------------------------------------
Yann E. MORIN (5):
      support/docker: run apt-get update and apt-get install in two RUNs
      support/docker: sort the list of installed packages
      support/docker: use the distro-provided flake8
      support/testing: fix python syntax
      support/docker: update to python3-flavoured flake8

 .gitlab-ci.yml                    |  2 +-
 .gitlab-ci.yml.in                 |  2 +-
 support/docker/Dockerfile         | 40 +++++++++++++++++++++++----------------
 support/testing/infra/__init__.py |  6 +++---
 support/testing/infra/basetest.py |  4 ++--
 support/testing/run-tests         | 26 ++++++++++++-------------
 6 files changed, 44 insertions(+), 36 deletions(-)

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

[Buildroot] [PATCH 2/5] support/docker: sort the list of installed packages

[Yann E. MORIN]

As suggested in the docker best practices [0], order the package list
alphabetically, and list only one package per line.

This will be much usefull later, we need to update the list of installed
packages, like adding new ones for example.

[0] https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#sort-multi-line-arguments

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>

---
Changes v1 -> v2:
  - don't drop python-pip yet  (Ricardo)
---
 support/docker/Dockerfile | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
index 8c525f7cf1..fe9e643a34 100644
--- a/support/docker/Dockerfile
+++ b/support/docker/Dockerfile
@@ -22,13 +22,29 @@ COPY apt-sources.list /etc/apt/sources.list
 RUN dpkg --add-architecture i386 && \
     apt-get update -y
 RUN apt-get install -y --no-install-recommends \
-        build-essential cmake libc6:i386 g++-multilib \
-        bc ca-certificates file locales rsync \
-        cvs bzr git mercurial subversion wget \
-        cpio unzip \
+        bc \
+        build-essential \
+        bzr \
+        ca-certificates \
+        cpio \
+        cvs \
+        file \
+        g++-multilib \
+        git \
+        libc6:i386 \
         libncurses5-dev \
-        python-nose2 python-pexpect qemu-system-arm qemu-system-x86 \
-        python-pip && \
+        locales \
+        mercurial \
+        python-nose2 \
+        python-pexpect \
+        python-pip \
+        qemu-system-arm \
+        qemu-system-x86 \
+        rsync \
+        subversion \
+        unzip \
+        wget \
+        && \
     apt-get -y autoremove && \
     apt-get -y clean
 
-- 
2.14.1

[Buildroot] [PATCH 3/5] support/docker: use the distro-provided flake8

[Yann E. MORIN]

Currently, we install flake8 and its dependencies via pip. We
tried to be reproducible by pinning the version of those python
packages, but we did forget quite a few of them, and thus some
dependencies for flake8 are installed as uncontrolled versions.

Furthermore, before we install flake8 and its dependencies, we
forcibly update pip, setuptools, and wheels packages to their
latest versions. This explicitly breaks reproducibility.

While we could enforce a specific version of all those packages
and still grab them from PyPI, we can simply grab them from the
distribution-provided packages instead.

Since we're using a pinned version of stretch, this already
guarantees we'll reproducibly get the same versions over and
over again. Besides, we just need to list flake8 as a package to
install to automatically get all its dependencies (again, in a
reproducible way).

This has the slight unfortunate drawback of downgrading flake8
to version 3.2.1, from version 3.5.0, as well as downgrading a
few of flake8's dependencies, as noticed by Ricardo:
    http://lists.busybox.net/pipermail/buildroot/2018-May/222376.html

However, as Ricardo said, there isn't "any serious limitation of
this old version, the release notes for a version in the between
mentions 'Dramatically improve the performance' but we have a
limited number of scripts and running on Gitlab for all of them
still takes less than 5 minutes".

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>

---
Changes v1 -> v2:
  - improve commit log to mention downgrading of flake8  (Ricardo)
  - correctly use python2's version of flake8, not python3's  (Ricardo)
  - fix typoes  (Ricardo)
---
 support/docker/Dockerfile | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
index fe9e643a34..8d40aeecdf 100644
--- a/support/docker/Dockerfile
+++ b/support/docker/Dockerfile
@@ -35,9 +35,9 @@ RUN apt-get install -y --no-install-recommends \
         libncurses5-dev \
         locales \
         mercurial \
+        python-flake8 \
         python-nose2 \
         python-pexpect \
-        python-pip \
         qemu-system-arm \
         qemu-system-x86 \
         rsync \
@@ -48,14 +48,6 @@ RUN apt-get install -y --no-install-recommends \
     apt-get -y autoremove && \
     apt-get -y clean
 
-# For check-flake8
-RUN python -m pip install --upgrade pip setuptools wheel && \
-    pip install -q \
-        flake8==3.5.0 \
-        mccabe==0.6.1 \
-        pycodestyle==2.3.1 \
-        pyflakes==1.6.0
-
 # To be able to generate a toolchain with locales, enable one UTF-8 locale
 RUN sed -i 's/# \(en_US.UTF-8\)/\1/' /etc/locale.gen && \
     /usr/sbin/locale-gen
-- 
2.14.1

[Buildroot] [PATCH 4/5] support/testing: fix python syntax

[Yann E. MORIN]

Fix three issues with code style in our test infra:
  - 'print' is now a function,
  - exceptions need to be caught-assigned with the 'as' keyword,
  - old-style "%s"%() formatting is deprecated.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>

---
Changes v1 -> v2:
  - this was previously caught because of a spurious switch to
    python3-flake8, as noticed by Ricardo. We're now back to using
    python-flake8, so the rationale has changed. Still, we do want
    to fix those because it makes sense anyway.
---
 support/testing/infra/__init__.py |  6 +++---
 support/testing/infra/basetest.py |  4 ++--
 support/testing/run-tests         | 26 +++++++++++++-------------
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/support/testing/infra/__init__.py b/support/testing/infra/__init__.py
index b03e891771..b0f28de450 100644
--- a/support/testing/infra/__init__.py
+++ b/support/testing/infra/__init__.py
@@ -34,17 +34,17 @@ def download(dldir, filename):
         os.makedirs(dldir)
 
     tmpfile = tempfile.mktemp(dir=dldir)
-    print "Downloading to {}".format(tmpfile)
+    print("Downloading to {0}".format(tmpfile))
 
     try:
         url_fh = urlopen(os.path.join(ARTIFACTS_URL, filename))
         with open(tmpfile, "w+") as tmpfile_fh:
             tmpfile_fh.write(url_fh.read())
-    except (HTTPError, URLError), err:
+    except (HTTPError, URLError) as err:
         os.unlink(tmpfile)
         raise err
 
-    print "Renaming from %s to %s" % (tmpfile, finalpath)
+    print("Renaming from {0} to {1}".format(tmpfile, finalpath))
     os.rename(tmpfile, finalpath)
     return finalpath
 
diff --git a/support/testing/infra/basetest.py b/support/testing/infra/basetest.py
index f3f13ad97f..82756afefd 100644
--- a/support/testing/infra/basetest.py
+++ b/support/testing/infra/basetest.py
@@ -46,8 +46,8 @@ class BRTest(unittest.TestCase):
         self.config += "\nBR2_JLEVEL={}\n".format(self.jlevel)
 
     def show_msg(self, msg):
-        print "{} {:40s} {}".format(datetime.datetime.now().strftime("%H:%M:%S"),
-                                    self.testname, msg)
+        print("{0} {1:40s} {2}".format(datetime.datetime.now().strftime("%H:%M:%S"),
+                                       self.testname, msg))
 
     def setUp(self):
         self.show_msg("Starting")
diff --git a/support/testing/run-tests b/support/testing/run-tests
index 270e78cff7..76dd15e9f0 100755
--- a/support/testing/run-tests
+++ b/support/testing/run-tests
@@ -41,7 +41,7 @@ def main():
         BRTest.logtofile = False
 
     if args.list:
-        print "List of tests"
+        print("List of tests")
         nose2.discover(argv=[script_path,
                              "-s", test_dir,
                              "-v",
@@ -52,16 +52,16 @@ def main():
     if args.download is None:
         args.download = os.getenv("BR2_DL_DIR")
         if args.download is None:
-            print "Missing download directory, please use -d/--download"
-            print ""
+            print("Missing download directory, please use -d/--download")
+            print("")
             parser.print_help()
             return 1
 
     BRTest.downloaddir = os.path.abspath(args.download)
 
     if args.output is None:
-        print "Missing output directory, please use -o/--output"
-        print ""
+        print("Missing output directory, please use -o/--output")
+        print("")
         parser.print_help()
         return 1
 
@@ -71,8 +71,8 @@ def main():
     BRTest.outputdir = os.path.abspath(args.output)
 
     if args.all is False and len(args.testname) == 0:
-        print "No test selected"
-        print ""
+        print("No test selected")
+        print("")
         parser.print_help()
         return 1
 
@@ -80,8 +80,8 @@ def main():
 
     if args.testcases != 1:
         if args.testcases < 1:
-            print "Invalid number of testcases to run simultaneously"
-            print ""
+            print("Invalid number of testcases to run simultaneously")
+            print("")
             parser.print_help()
             return 1
         # same default BR2_JLEVEL as package/Makefile.in
@@ -93,16 +93,16 @@ def main():
 
     if args.jlevel:
         if args.jlevel < 0:
-            print "Invalid BR2_JLEVEL to use for each testcase"
-            print ""
+            print("Invalid BR2_JLEVEL to use for each testcase")
+            print("")
             parser.print_help()
             return 1
         # the user can override the auto calculated value
         BRTest.jlevel = args.jlevel
 
     if args.timeout_multiplier < 1:
-        print "Invalid multiplier for timeout values"
-        print ""
+        print("Invalid multiplier for timeout values")
+        print("")
         parser.print_help()
         return 1
     BRTest.timeout_multiplier = args.timeout_multiplier
-- 
2.14.1

[Buildroot] [PATCH 5/5] support/docker: update to python3-flavoured flake8

[Yann E. MORIN]

We are currently using python2-flavoured flake8, but the way to the
future is with python3. Besides, the python3-flavoured flake8 catches
more problems.

So, we leap into the future and switch to using python3's flake8.

Future, here we come! ;-)

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 .gitlab-ci.yml            | 2 +-
 .gitlab-ci.yml.in         | 2 +-
 support/docker/Dockerfile | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index e80491cdde..4e111b48bd 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -37,7 +37,7 @@ check-flake8:
         - 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)
+        - python3 -m flake8 --statistics --count $(cat files.processed)
     after_script:
         - wc -l files.processed
 
diff --git a/.gitlab-ci.yml.in b/.gitlab-ci.yml.in
index fb2650c5ce..bfe1eeccec 100644
--- a/.gitlab-ci.yml.in
+++ b/.gitlab-ci.yml.in
@@ -37,7 +37,7 @@ check-flake8:
         - 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)
+        - python3 -m flake8 --statistics --count $(cat files.processed)
     after_script:
         - wc -l files.processed
 
diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
index 8d40aeecdf..57c9ef78fa 100644
--- a/support/docker/Dockerfile
+++ b/support/docker/Dockerfile
@@ -29,13 +29,13 @@ RUN apt-get install -y --no-install-recommends \
         cpio \
         cvs \
         file \
+        flake8 \
         g++-multilib \
         git \
         libc6:i386 \
         libncurses5-dev \
         locales \
         mercurial \
-        python-flake8 \
         python-nose2 \
         python-pexpect \
         qemu-system-arm \
-- 
2.14.1

[Buildroot] [PATCH 1/5] support/docker: run apt-get update and apt-get install in two RUNs

[Thomas Petazzoni]

Hello,

On Sun,  3 Jun 2018 11:08:18 +0200, Yann E. MORIN wrote:
> In commit 7517aef4d (support/docker: limit the number of layers),
> we reduced the number of layers by coalescing multiple RUN commands
> into less commands.
> 
> In doing so, we especially coalesced "apt-get update" with "apt-get
> install".
> 
> However, the distribution we used is a pinned version of stretch, so
> we know that running apt-get update will always yield the same apt
> database.
> 
> If we split the two apt-get commands, then we can re-use any local
> intermediate image when we need to update the list of packages to
> install; this helps quite a bit when testing the docker files over
> and over again, with just slight variants in the packages list.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> ---
>  support/docker/Dockerfile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied to master, thanks.

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

[Buildroot] [PATCH 2/5] support/docker: sort the list of installed packages

[Ricardo Martincoski]

Hello,

Nit: you forgot to use -v2 with format-patch.

On Sun, Jun 03, 2018 at 06:08 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:

> As suggested in the docker best practices [0], order the package list
> alphabetically, and list only one package per line.
> 
> This will be much usefull later, we need to update the list of installed
> packages, like adding new ones for example.
> 
> [0] https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#sort-multi-line-arguments
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> 

'git am' automatically removes this empty line.

> ---
> Changes v1 -> v2:
>   - don't drop python-pip yet  (Ricardo)
> ---
>  support/docker/Dockerfile | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
> index 8c525f7cf1..fe9e643a34 100644
> --- a/support/docker/Dockerfile
> +++ b/support/docker/Dockerfile
> @@ -22,13 +22,29 @@ COPY apt-sources.list /etc/apt/sources.list
>  RUN dpkg --add-architecture i386 && \
>      apt-get update -y
>  RUN apt-get install -y --no-install-recommends \
> -        build-essential cmake libc6:i386 g++-multilib \

I missed that when reviewing the first iteration...

> -        bc ca-certificates file locales rsync \
> -        cvs bzr git mercurial subversion wget \
> -        cpio unzip \
> +        bc \
> +        build-essential \
> +        bzr \
> +        ca-certificates \

'cmake' was removed by mistake.

Obviously, since host-cmake will be built when needed, it should not make any
current or upcoming test case to fail. I actually tested with the current test
cases (still running while I write this):
https://gitlab.com/RicardoMartincoski/buildroot/pipelines/23112714
I didn't checked if any current test case is already requiring cmake.

But removing cmake will make any test case that needs it to take longer to run,
as the version seems suitable to be used:

br-user at b976f947b114:~$ cmake --version
cmake version 3.7.2

The build of host-cmake is probably already covered by the autobuilders.
And if we want to have a test case in the test infra for this the best way IMO
is to write a specific test for it.

> +        cpio \
> +        cvs \
> +        file \
> +        g++-multilib \
> +        git \
> +        libc6:i386 \
>          libncurses5-dev \
> -        python-nose2 python-pexpect qemu-system-arm qemu-system-x86 \
> -        python-pip && \
> +        locales \
> +        mercurial \
> +        python-nose2 \
> +        python-pexpect \
> +        python-pip \
> +        qemu-system-arm \
> +        qemu-system-x86 \
> +        rsync \
> +        subversion \
> +        unzip \
> +        wget \
> +        && \
>      apt-get -y autoremove && \
>      apt-get -y clean
>  
> -- 
> 2.14.1

[I am not against using && in a separate line as we discussed in
 http://lists.busybox.net/pipermail/buildroot/2018-June/222901.html .
 With cmake added back to the list please add]
 Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>

Regards,
Ricardo

[Buildroot] [PATCH 2/5] support/docker: sort the list of installed packages

[Yann E. MORIN]

Ricardo, All,

On 2018-06-03 20:21 -0300, Ricardo Martincoski spake thusly:
> Nit: you forgot to use -v2 with format-patch.

Yeah. I renamed the branch locally before re-pushing, and in doing so I
lost the metadata associated with the old one, so the iteration version
got lost...

> On Sun, Jun 03, 2018 at 06:08 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
[--SNIP--]
> > diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
> > index 8c525f7cf1..fe9e643a34 100644
> > --- a/support/docker/Dockerfile
> > +++ b/support/docker/Dockerfile
> > @@ -22,13 +22,29 @@ COPY apt-sources.list /etc/apt/sources.list
> >  RUN dpkg --add-architecture i386 && \
> >      apt-get update -y
> >  RUN apt-get install -y --no-install-recommends \
> > -        build-essential cmake libc6:i386 g++-multilib \
> 
> I missed that when reviewing the first iteration...
> 
> > -        bc ca-certificates file locales rsync \
> > -        cvs bzr git mercurial subversion wget \
> > -        cpio unzip \
> > +        bc \
> > +        build-essential \
> > +        bzr \
> > +        ca-certificates \
> 
> 'cmake' was removed by mistake.

Sag mir wo, das cmake ist; wo ist es geblieben?
Sag mir wo, das cmake ist; was ist geschehen?

> Obviously, since host-cmake will be built when needed, it should not make any
> current or upcoming test case to fail. I actually tested with the current test
> cases (still running while I write this):
> https://gitlab.com/RicardoMartincoski/buildroot/pipelines/23112714
> I didn't checked if any current test case is already requiring cmake.
> 
> But removing cmake will make any test case that needs it to take longer to run,
> as the version seems suitable to be used:
> 
> br-user at b976f947b114:~$ cmake --version
> cmake version 3.7.2
> 
> The build of host-cmake is probably already covered by the autobuilders.
> And if we want to have a test case in the test infra for this the best way IMO
> is to write a specific test for it.

Nah, this is definitely an oversight on my side; we want to keep cmake
installed in the distro.

And even if did not (but we do!), this should be a separate patch anyway.

> > +        cpio \
> > +        cvs \
> > +        file \
> > +        g++-multilib \
> > +        git \
> > +        libc6:i386 \
> >          libncurses5-dev \
> > -        python-nose2 python-pexpect qemu-system-arm qemu-system-x86 \
> > -        python-pip && \
> > +        locales \
> > +        mercurial \
> > +        python-nose2 \
> > +        python-pexpect \
> > +        python-pip \
> > +        qemu-system-arm \
> > +        qemu-system-x86 \
> > +        rsync \
> > +        subversion \
> > +        unzip \
> > +        wget \
> > +        && \
> >      apt-get -y autoremove && \
> >      apt-get -y clean
> >  
> > -- 
> > 2.14.1
> 
> [I am not against using && in a separate line as we discussed in
>  http://lists.busybox.net/pipermail/buildroot/2018-June/222901.html .

Yeah... I haven't found a better solution, except moving the && at the
begining of the next line, in which case we'd have to do it everywhere
else...

>  With cmake added back to the list please add]
>  Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>

Thanks! :-)

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH 3/5] support/docker: use the distro-provided flake8

[Ricardo Martincoski]

Hello,

On Sun, Jun 03, 2018 at 06:08 AM, Yann E. MORIN wrote:

> Currently, we install flake8 and its dependencies via pip. We
> tried to be reproducible by pinning the version of those python
> packages, but we did forget quite a few of them, and thus some
> dependencies for flake8 are installed as uncontrolled versions.
> 
> Furthermore, before we install flake8 and its dependencies, we
> forcibly update pip, setuptools, and wheels packages to their
> latest versions. This explicitly breaks reproducibility.
> 
> While we could enforce a specific version of all those packages
> and still grab them from PyPI, we can simply grab them from the
> distribution-provided packages instead.
> 
> Since we're using a pinned version of stretch, this already
> guarantees we'll reproducibly get the same versions over and
> over again. Besides, we just need to list flake8 as a package to
> install to automatically get all its dependencies (again, in a
> reproducible way).
> 
> This has the slight unfortunate drawback of downgrading flake8
> to version 3.2.1, from version 3.5.0, as well as downgrading a
> few of flake8's dependencies, as noticed by Ricardo:
>     http://lists.busybox.net/pipermail/buildroot/2018-May/222376.html
> 
> However, as Ricardo said, there isn't "any serious limitation of
> this old version, the release notes for a version in the between
> mentions 'Dramatically improve the performance' but we have a
> limited number of scripts and running on Gitlab for all of them
> still takes less than 5 minutes".
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Acked-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>


Regards,
Ricardo

[Buildroot] [PATCH 4/5] support/testing: fix python syntax

[Ricardo Martincoski]

Hello,

The endless patch series... each iteration I suggest new patches for it :-)

+ Thomas P

There is nothing really wrong with the changes this patch do to the code.
But see below 2 nits that could lead us (or not) to choose one way or another
and standardize all Python scripts in the tree.

On Sun, Jun 03, 2018 at 06:08 AM, Yann E. MORIN wrote:

> Fix three issues with code style in our test infra:
>   - 'print' is now a function,
>   - exceptions need to be caught-assigned with the 'as' keyword,
>   - old-style "%s"%() formatting is deprecated.

This patch also do:
  - add indices in format string

We currently do that only for check-uniq-files.
Everywhere else we use '{}' instead of '{0}' when possible, as this is not
really needed for Python 3 when we use each arguments once in the same order
they are passed to format().

BUT... looking at "62fa5e17cb support/scripts/check-uniq-files: add indices in
format string" maybe it would be good to adopt a simplistic policy of always
using indices in format string, as it seems to be compatible to Python >= 2.6
and then we don't need to think about whether the script will be used during
the build (and therefore need to support old distros) or not when reviewing.

Thoughts?

[snip]
> -    print "Downloading to {}".format(tmpfile)
> +    print("Downloading to {0}".format(tmpfile))
                              ^
Example of index mentioned above.
2 more occurrences in the patch.

[snip]
> -            print "Missing download directory, please use -d/--download"
> -            print ""
> +            print("Missing download directory, please use -d/--download")
> +            print("")

I was about to suggest to change to 'print()' as it is already used in scanpypi.
But I didn't tested yet any form on python2.6.
5 more occurrences in the patch.

Thoughts?


Regards,
Ricardo

[Buildroot] [PATCH 4/5] support/testing: fix python syntax

[Thomas Petazzoni]

Hello,

On Mon, 04 Jun 2018 23:17:12 -0300, Ricardo Martincoski wrote:

> This patch also do:
>   - add indices in format string
> 
> We currently do that only for check-uniq-files.
> Everywhere else we use '{}' instead of '{0}' when possible, as this is not
> really needed for Python 3 when we use each arguments once in the same order
> they are passed to format().
> 
> BUT... looking at "62fa5e17cb support/scripts/check-uniq-files: add indices in
> format string" maybe it would be good to adopt a simplistic policy of always
> using indices in format string, as it seems to be compatible to Python >= 2.6
> and then we don't need to think about whether the script will be used during
> the build (and therefore need to support old distros) or not when reviewing.
> 
> Thoughts?

I am not sure it is useful (and realistic) to have all our Python
scripts compatible with Python 2.6. This puts a pretty hard constraint
which might be annoying down the road. So Python scripts involved in
the build itself should definitely be compatible with Python 2.6, but
all the utility Python scripts around that are not essential for the
build, I don't think we should enforce this constraint.

Best regards,

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

[Buildroot] [PATCH 4/5] support/testing: fix python syntax

[Thomas Petazzoni]

Hello,

On Sun,  3 Jun 2018 11:08:21 +0200, Yann E. MORIN wrote:
> Fix three issues with code style in our test infra:
>   - 'print' is now a function,
>   - exceptions need to be caught-assigned with the 'as' keyword,
>   - old-style "%s"%() formatting is deprecated.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> 
> ---
> Changes v1 -> v2:
>   - this was previously caught because of a spurious switch to
>     python3-flake8, as noticed by Ricardo. We're now back to using
>     python-flake8, so the rationale has changed. Still, we do want
>     to fix those because it makes sense anyway.
> ---
>  support/testing/infra/__init__.py |  6 +++---
>  support/testing/infra/basetest.py |  4 ++--
>  support/testing/run-tests         | 26 +++++++++++++-------------
>  3 files changed, 18 insertions(+), 18 deletions(-)

I've dropped the changes that added indices to format strings (since
they would only be needed for Python 2.6 compatibility, and I'm not
sure that it's a worthwhile goal to be able to run the test suite on
Python 2.6 machines). And then I've applied to master. Thanks!

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