* [Buildroot] [PATCH 1/5] support/docker: run apt-get update and apt-get install in two RUNs
2018-06-03 9:08 [Buildroot] [PATCH 0/5] support/docker: make the image more reproducible Yann E. MORIN
@ 2018-06-03 9:08 ` Yann E. MORIN
2018-06-03 18:31 ` Thomas Petazzoni
2018-06-03 9:08 ` [Buildroot] [PATCH 2/5] support/docker: sort the list of installed packages Yann E. MORIN
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Yann E. MORIN @ 2018-06-03 9:08 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Buildroot] [PATCH 1/5] support/docker: run apt-get update and apt-get install in two RUNs
2018-06-03 9:08 ` [Buildroot] [PATCH 1/5] support/docker: run apt-get update and apt-get install in two RUNs Yann E. MORIN
@ 2018-06-03 18:31 ` Thomas Petazzoni
0 siblings, 0 replies; 13+ messages in thread
From: Thomas Petazzoni @ 2018-06-03 18:31 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Buildroot] [PATCH 2/5] support/docker: sort the list of installed packages
2018-06-03 9:08 [Buildroot] [PATCH 0/5] support/docker: make the image more reproducible Yann E. MORIN
2018-06-03 9:08 ` [Buildroot] [PATCH 1/5] support/docker: run apt-get update and apt-get install in two RUNs Yann E. MORIN
@ 2018-06-03 9:08 ` Yann E. MORIN
2018-06-03 23:21 ` Ricardo Martincoski
2018-06-03 9:08 ` [Buildroot] [PATCH 3/5] support/docker: use the distro-provided flake8 Yann E. MORIN
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Yann E. MORIN @ 2018-06-03 9:08 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Buildroot] [PATCH 2/5] support/docker: sort the list of installed packages
2018-06-03 9:08 ` [Buildroot] [PATCH 2/5] support/docker: sort the list of installed packages Yann E. MORIN
@ 2018-06-03 23:21 ` Ricardo Martincoski
2018-06-04 16:11 ` Yann E. MORIN
0 siblings, 1 reply; 13+ messages in thread
From: Ricardo Martincoski @ 2018-06-03 23:21 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Buildroot] [PATCH 2/5] support/docker: sort the list of installed packages
2018-06-03 23:21 ` Ricardo Martincoski
@ 2018-06-04 16:11 ` Yann E. MORIN
0 siblings, 0 replies; 13+ messages in thread
From: Yann E. MORIN @ 2018-06-04 16:11 UTC (permalink / raw)
To: buildroot
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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Buildroot] [PATCH 3/5] support/docker: use the distro-provided flake8
2018-06-03 9:08 [Buildroot] [PATCH 0/5] support/docker: make the image more reproducible Yann E. MORIN
2018-06-03 9:08 ` [Buildroot] [PATCH 1/5] support/docker: run apt-get update and apt-get install in two RUNs Yann E. MORIN
2018-06-03 9:08 ` [Buildroot] [PATCH 2/5] support/docker: sort the list of installed packages Yann E. MORIN
@ 2018-06-03 9:08 ` Yann E. MORIN
2018-06-05 2:12 ` Ricardo Martincoski
2018-06-03 9:08 ` [Buildroot] [PATCH 4/5] support/testing: fix python syntax Yann E. MORIN
2018-06-03 9:08 ` [Buildroot] [PATCH 5/5] support/docker: update to python3-flavoured flake8 Yann E. MORIN
4 siblings, 1 reply; 13+ messages in thread
From: Yann E. MORIN @ 2018-06-03 9:08 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Buildroot] [PATCH 3/5] support/docker: use the distro-provided flake8
2018-06-03 9:08 ` [Buildroot] [PATCH 3/5] support/docker: use the distro-provided flake8 Yann E. MORIN
@ 2018-06-05 2:12 ` Ricardo Martincoski
0 siblings, 0 replies; 13+ messages in thread
From: Ricardo Martincoski @ 2018-06-05 2:12 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Buildroot] [PATCH 4/5] support/testing: fix python syntax
2018-06-03 9:08 [Buildroot] [PATCH 0/5] support/docker: make the image more reproducible Yann E. MORIN
` (2 preceding siblings ...)
2018-06-03 9:08 ` [Buildroot] [PATCH 3/5] support/docker: use the distro-provided flake8 Yann E. MORIN
@ 2018-06-03 9:08 ` Yann E. MORIN
2018-06-05 2:17 ` Ricardo Martincoski
2018-06-10 13:57 ` Thomas Petazzoni
2018-06-03 9:08 ` [Buildroot] [PATCH 5/5] support/docker: update to python3-flavoured flake8 Yann E. MORIN
4 siblings, 2 replies; 13+ messages in thread
From: Yann E. MORIN @ 2018-06-03 9:08 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Buildroot] [PATCH 4/5] support/testing: fix python syntax
2018-06-03 9:08 ` [Buildroot] [PATCH 4/5] support/testing: fix python syntax Yann E. MORIN
@ 2018-06-05 2:17 ` Ricardo Martincoski
2018-06-05 5:51 ` Thomas Petazzoni
2018-06-10 13:57 ` Thomas Petazzoni
1 sibling, 1 reply; 13+ messages in thread
From: Ricardo Martincoski @ 2018-06-05 2:17 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply [flat|nested] 13+ messages in thread* [Buildroot] [PATCH 4/5] support/testing: fix python syntax
2018-06-05 2:17 ` Ricardo Martincoski
@ 2018-06-05 5:51 ` Thomas Petazzoni
0 siblings, 0 replies; 13+ messages in thread
From: Thomas Petazzoni @ 2018-06-05 5:51 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Buildroot] [PATCH 4/5] support/testing: fix python syntax
2018-06-03 9:08 ` [Buildroot] [PATCH 4/5] support/testing: fix python syntax Yann E. MORIN
2018-06-05 2:17 ` Ricardo Martincoski
@ 2018-06-10 13:57 ` Thomas Petazzoni
1 sibling, 0 replies; 13+ messages in thread
From: Thomas Petazzoni @ 2018-06-10 13:57 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Buildroot] [PATCH 5/5] support/docker: update to python3-flavoured flake8
2018-06-03 9:08 [Buildroot] [PATCH 0/5] support/docker: make the image more reproducible Yann E. MORIN
` (3 preceding siblings ...)
2018-06-03 9:08 ` [Buildroot] [PATCH 4/5] support/testing: fix python syntax Yann E. MORIN
@ 2018-06-03 9:08 ` Yann E. MORIN
4 siblings, 0 replies; 13+ messages in thread
From: Yann E. MORIN @ 2018-06-03 9:08 UTC (permalink / raw)
To: buildroot
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
^ permalink raw reply related [flat|nested] 13+ messages in thread