support/docker: make the image more reproducible

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

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.

---
 support/docker/Dockerfile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index f01ac25f93..8c525f7cf1 100644
@@ -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

Hello,

Applied to master, thanks.

Thomas

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

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

---
Changes v1 -> v2:
- don't drop python-pip yet (Ricardo)

---
 support/docker/Dockerfile | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

index 8c525f7cf1..fe9e643a34 100644
@@ -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

Hello,

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

'git am' automatically removes this empty line.

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

Regards,
Ricardo

Ricardo, All,

On 2018-06-03 20:21 -0300, Ricardo Martincoski spake thusly:

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

[--SNIP--]

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

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.

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

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

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

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

index fe9e643a34..8d40aeecdf 100644
@@ -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

Hello,

Regards,
Ricardo

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.

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

index b03e891771..b0f28de450 100644
@@ -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
 

index f3f13ad97f..82756afefd 100644
@@ -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")

index 270e78cff7..76dd15e9f0 100755
@@ -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

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.

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]

                              ^

Hello,

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

Hello,

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

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! ;-)

---
 .gitlab-ci.yml            | 2 +-
 .gitlab-ci.yml.in         | 2 +-
 support/docker/Dockerfile | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

index e80491cdde..4e111b48bd 100644
@@ -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
 

index fb2650c5ce..bfe1eeccec 100644
@@ -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
 

index 8d40aeecdf..57c9ef78fa 100644
@@ -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