Patchwork [19,of,22] docker: simplify docker files

login
register
mail settings
Submitter Mads Kiilerich
Date May 20, 2014, 2:10 a.m.
Message ID <33b47ac7a7a12ae934a4.1400551814@mk-desktop>
Download mbox | patch
Permalink /patch/4841/
State Changes Requested
Headers show

Comments

Mads Kiilerich - May 20, 2014, 2:10 a.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1400551681 -7200
#      Tue May 20 04:08:01 2014 +0200
# Node ID 33b47ac7a7a12ae934a4c9037fd09aed83475f6b
# Parent  e314e1ed2c21a404ce5e51ee73f7d0fa924026ec
docker: simplify docker files

Less run commands executes faster and is simpler to maintain.
Matt Mackall - May 20, 2014, 5:04 p.m.
On Tue, 2014-05-20 at 04:10 +0200, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1400551681 -7200
> #      Tue May 20 04:08:01 2014 +0200
> # Node ID 33b47ac7a7a12ae934a4c9037fd09aed83475f6b
> # Parent  e314e1ed2c21a404ce5e51ee73f7d0fa924026ec
> docker: simplify docker files
> 
> Less run commands executes faster and is simpler to maintain.

You may have saved 10 seconds for people who are using our Docker script
for the first time, but you're wasting 20 minutes, half a gig of
bandwidth, and a bunch of disk space for everyone who's already built
the images? Churn here is expensive.

On the other hand, sticking new things at the end of existing
dockerfiles is cheap, so it is in fact quite natural to build things
with each step on a separate line.
Mads Kiilerich - May 20, 2014, 7:11 p.m.
On 05/20/2014 07:04 PM, Matt Mackall wrote:
> On Tue, 2014-05-20 at 04:10 +0200, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1400551681 -7200
>> #      Tue May 20 04:08:01 2014 +0200
>> # Node ID 33b47ac7a7a12ae934a4c9037fd09aed83475f6b
>> # Parent  e314e1ed2c21a404ce5e51ee73f7d0fa924026ec
>> docker: simplify docker files
>>
>> Less run commands executes faster and is simpler to maintain.
> You may have saved 10 seconds for people who are using our Docker script
> for the first time, but you're wasting 20 minutes, half a gig of
> bandwidth, and a bunch of disk space for everyone who's already built
> the images? Churn here is expensive.
>
> On the other hand, sticking new things at the end of existing
> dockerfiles is cheap, so it is in fact quite natural to build things
> with each step on a separate line.

I am not saying it is wrong to develop it by adding lines one by one 
(even though it would be more efficient to start out with the 
BuildRequires from the spec). But usually we don't care much how things 
were developed. We care about showing stuff in a logical, structured and 
maintainable way.

Do you object to adding comments explaining why the lines are needed? Is 
it ok to reorder the existing lines so lines not related to making the 
system Python available are grouped together?

If we want to support building packages with their own Python (or 
building other stuff that is closely related to Mercurial such as GUI 
tools or Python bindings), I guess we would like to have conditional 
sections in the "dockerfiles" instead of including everything every time 
... or just generate the commands in dockerbuild.

/Mads
Matt Mackall - May 20, 2014, 9:21 p.m.
On Tue, 2014-05-20 at 21:11 +0200, Mads Kiilerich wrote:
> On 05/20/2014 07:04 PM, Matt Mackall wrote:
> > On Tue, 2014-05-20 at 04:10 +0200, Mads Kiilerich wrote:
> >> # HG changeset patch
> >> # User Mads Kiilerich <madski@unity3d.com>
> >> # Date 1400551681 -7200
> >> #      Tue May 20 04:08:01 2014 +0200
> >> # Node ID 33b47ac7a7a12ae934a4c9037fd09aed83475f6b
> >> # Parent  e314e1ed2c21a404ce5e51ee73f7d0fa924026ec
> >> docker: simplify docker files
> >>
> >> Less run commands executes faster and is simpler to maintain.
> > You may have saved 10 seconds for people who are using our Docker script
> > for the first time, but you're wasting 20 minutes, half a gig of
> > bandwidth, and a bunch of disk space for everyone who's already built
> > the images? Churn here is expensive.
> >
> > On the other hand, sticking new things at the end of existing
> > dockerfiles is cheap, so it is in fact quite natural to build things
> > with each step on a separate line.
> 
> I am not saying it is wrong to develop it by adding lines one by one 
> (even though it would be more efficient to start out with the 
> BuildRequires from the spec). But usually we don't care much how things 
> were developed. We care about showing stuff in a logical, structured and 
> maintainable way.

Sure. But a Docker image is not a typical build target:

- it's (by definition) the size of an entire operating system
- changing the build rules is way more expensive
- it's thus NOT assumed that 'make clean; make' is common flow and
therefore "free"
- we don't autoclean old images between builds
- if we do add autoclean.. bisecting an RPM build issue is now hell
- ..as is building two versions of the packages on the same machine

In short, dockerfile churn is best avoided. I'd rather defer this until
we're forced to make a substantive change.

> Do you object to adding comments explaining why the lines are needed?

No (provided Docker is smart enough not to rebuild the image).

> Is it ok to reorder the existing lines so lines not related to making the 
> system Python available are grouped together?

Docker builds a nested image for each line in the dockerfile. Reordering
invalidates the nesting and is thus just as expensive as combining
lines.

> If we want to support building packages with their own Python (or 
> building other stuff that is closely related to Mercurial such as GUI 
> tools or Python bindings), I guess we would like to have conditional 
> sections in the "dockerfiles" instead of including everything every time 
> ... or just generate the commands in dockerbuild.

???

If we're going to go to the trouble of building a container image, we'd
better plan on reusing it as much as possible so rewriting the
dockerfile dynamically is going to be a problem. Building a second
container derived from the first but with Python available is probably
an option.

Patch

diff --git a/contrib/docker/centos5 b/contrib/docker/centos5
--- a/contrib/docker/centos5
+++ b/contrib/docker/centos5
@@ -1,3 +1,5 @@ 
 FROM saltstack/centos-5-minimal
+# For building Mercurial
 RUN yum install -y gcc make rpm-build gettext tar
+# For using the OS Python
 RUN yum install -y python-devel python-docutils
diff --git a/contrib/docker/centos6 b/contrib/docker/centos6
--- a/contrib/docker/centos6
+++ b/contrib/docker/centos6
@@ -1,7 +1,5 @@ 
 FROM centos
-RUN yum install -y gcc
+# For building Mercurial
+RUN yum install -y gcc make rpm-build gettext tar
+# For using the OS Python
 RUN yum install -y python-devel python-docutils
-RUN yum install -y make
-RUN yum install -y rpm-build
-RUN yum install -y gettext
-RUN yum install -y tar
diff --git a/contrib/docker/fedora b/contrib/docker/fedora
--- a/contrib/docker/fedora
+++ b/contrib/docker/fedora
@@ -1,6 +1,5 @@ 
 FROM fedora
-RUN yum install -y gcc
+# For building Mercurial
+RUN yum install -y gcc make rpm-build gettext tar
+# For using the OS Python
 RUN yum install -y python-devel python-docutils
-RUN yum install -y make
-RUN yum install -y rpm-build
-RUN yum install -y gettext