diff mbox series

[2/4] support/docker: sort the list of installed packages

Message ID ed5028f2849286aef3ec10937f4fe7d03a2137b8.1527977964.git.yann.morin.1998@free.fr
State Changes Requested
Headers show
Series [1/4] support/docker: run apt-get update and apt-get install in two RUNs | expand

Commit Message

Yann E. MORIN June 2, 2018, 10:19 p.m. UTC
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>
---
 support/docker/Dockerfile | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

Comments

Ricardo Martincoski June 3, 2018, 4:21 a.m. UTC | #1
Hello,

On Sat, Jun 02, 2018 at 07:19 PM, Yann E. MORIN wrote:

[snip]
>  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 && \

With only patches 1 and 2 applied the 'docker build' command will fail:
/usr/bin/python: No module named pip

This removal should be part of patch 4.

> +        locales \
> +        mercurial \
> +        python-nose2 \
> +        python-pexpect \
> +        qemu-system-arm \
> +        qemu-system-x86 \
> +        rsync \
> +        subversion \
> +        unzip \
> +        wget \
> +        && \

We usually don't put && in a separate line.
I am not really against it if no one else complains.


Regards,
Ricardo
Yann E. MORIN June 3, 2018, 7:24 a.m. UTC | #2
Ricardo, All,

On 2018-06-03 01:21 -0300, Ricardo Martincoski spake thusly:
> On Sat, Jun 02, 2018 at 07:19 PM, Yann E. MORIN wrote:
> >  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 && \
> 
> With only patches 1 and 2 applied the 'docker build' command will fail:
> /usr/bin/python: No module named pip

Oooh dang... I missed that one when doing the interactive git-add...

> This removal should be part of patch 4.

Obviously so, yes.

> > +        locales \
> > +        mercurial \
> > +        python-nose2 \
> > +        python-pexpect \
> > +        qemu-system-arm \
> > +        qemu-system-x86 \
> > +        rsync \
> > +        subversion \
> > +        unzip \
> > +        wget \
> > +        && \
> 
> We usually don't put && in a separate line.
> I am not really against it if no one else complains.

Well, I'm not too sure either. We have two coflicting rules here:

  - one package per-line, so that we can add/remove packages without
    touching the existing lines, so if we were top add a package after
    the last one, we'd have to touch two lines,

  - we almost exclusively put the && at the end of a line...

Note that the docker best practices put the && at the beginning of the
line when they need to split lines.

So, I don't care what rule we apply (as I prefer neither, as I favour
the one in the docker best practices).

Regards,
Yann E. MORIN.
diff mbox series

Patch

diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
index 8c525f7cf1..4f08a54f06 100644
--- a/support/docker/Dockerfile
+++ b/support/docker/Dockerfile
@@ -22,13 +22,28 @@  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 \
+        qemu-system-arm \
+        qemu-system-x86 \
+        rsync \
+        subversion \
+        unzip \
+        wget \
+        && \
     apt-get -y autoremove && \
     apt-get -y clean