Patchwork [1,of,3] makefile: mkdir is not needed on templatized docker builds

login
register
mail settings
Submitter via Mercurial-devel
Date April 14, 2018, 4:31 p.m.
Message ID <3c8463e542db60db1ad2.1523723484@cuben>
Download mbox | patch
Permalink /patch/31052/
State Accepted
Headers show

Comments

via Mercurial-devel - April 14, 2018, 4:31 p.m.
# HG changeset patch
# User Antonio Muci <a.mux@inwind.it>
# Date 1523714275 -7200
#      Sat Apr 14 15:57:55 2018 +0200
# Node ID 3c8463e542db60db1ad234ec80f39e290ed17658
# Parent  1541e1a8e87de9bd6b869ed498ad8e9b93c59d4d
makefile: mkdir is not needed on templatized docker builds

this imitates e63dfbbdbd07 and is a small addition to 231690dba9b4 and
5c1283713293
Anton Shestakov - April 15, 2018, 6:59 a.m.
On Sat, 14 Apr 2018 18:31:24 +0200
Antonio Muci via Mercurial-devel <mercurial-devel@mercurial-scm.org>
wrote:

> # HG changeset patch
> # User Antonio Muci <a.mux@inwind.it>
> # Date 1523714275 -7200
> #      Sat Apr 14 15:57:55 2018 +0200
> # Node ID 3c8463e542db60db1ad234ec80f39e290ed17658
> # Parent  1541e1a8e87de9bd6b869ed498ad8e9b93c59d4d
> makefile: mkdir is not needed on templatized docker builds
> 
> this imitates e63dfbbdbd07 and is a small addition to 231690dba9b4 and
> 5c1283713293

Nit: after the first line, regular sentences start (they need capital
letters and punctuation).

Otherwise, this series LGTM.
Yuya Nishihara - April 15, 2018, 12:59 p.m.
On Sun, 15 Apr 2018 14:59:45 +0800, Anton Shestakov wrote:
> On Sat, 14 Apr 2018 18:31:24 +0200
> Antonio Muci via Mercurial-devel <mercurial-devel@mercurial-scm.org>
> wrote:
> 
> > # HG changeset patch
> > # User Antonio Muci <a.mux@inwind.it>
> > # Date 1523714275 -7200
> > #      Sat Apr 14 15:57:55 2018 +0200
> > # Node ID 3c8463e542db60db1ad234ec80f39e290ed17658
> > # Parent  1541e1a8e87de9bd6b869ed498ad8e9b93c59d4d
> > makefile: mkdir is not needed on templatized docker builds
> > 
> > this imitates e63dfbbdbd07 and is a small addition to 231690dba9b4 and
> > 5c1283713293
> 
> Nit: after the first line, regular sentences start (they need capital
> letters and punctuation).
> 
> Otherwise, this series LGTM.

Queued, thanks.
via Mercurial-devel - April 15, 2018, 1:24 p.m.
Thanks Anton: that's true.

If you prefer so, I can send another series, or feel free to directly 
correct it if it's more practical for you.

On a side note: my real pain point yesterday was the way packages are 
built via Docker, because I feel it could be better. Unfortunately, I 
have not been able to find a simple solution yet, so I just sent this 
small series.

Basically, due to the way volumes are mounted by contrib/docker{deb,rpm}:

     $DOCKER run [...] -v $PWD/..:/mnt [...]

the container owns the working directory, and pollutes it while 
building. Probably, the only desired side effect of a build on the 
host's file system should be the generation of {deb,rpm}s in /packages, 
instead. Everything else should stay in the ephemeral container.

This causes troubles, for example, when tests fail while building. In 
that case the /debian directory does not get deleted, and subsequent 
builds fail with the error "debian control directory already exists".

On systems in which Docker must be run as root (for example, CentOS), 
this way of mounting volumes also results in files owned by root, which 
feels a bit weird.

Summarizing: I would try to find a more robust way of generating 
packages, but it will take some time.





On 15/04/2018 08:59, Anton Shestakov wrote:
> On Sat, 14 Apr 2018 18:31:24 +0200
> Antonio Muci via Mercurial-devel<mercurial-devel@mercurial-scm.org>
> wrote:
>
>> # HG changeset patch
>> # User Antonio Muci<a.mux@inwind.it>
>> # Date 1523714275 -7200
>> #      Sat Apr 14 15:57:55 2018 +0200
>> # Node ID 3c8463e542db60db1ad234ec80f39e290ed17658
>> # Parent  1541e1a8e87de9bd6b869ed498ad8e9b93c59d4d
>> makefile: mkdir is not needed on templatized docker builds
>>
>> this imitates e63dfbbdbd07 and is a small addition to 231690dba9b4 and
>> 5c1283713293
> Nit: after the first line, regular sentences start (they need capital
> letters and punctuation).
>
> Otherwise, this series LGTM.
>
Anton Shestakov - April 16, 2018, 6:15 a.m.
On Sun, 15 Apr 2018 15:24:28 +0200
Antonio Muci <a.mux@inwind.it> wrote:

> Summarizing: I would try to find a more robust way of generating 
> packages, but it will take some time.

Yep, people did discover that the way we do it now is not the most
"correct", but it's hard to formulate what is "correct" when it comes
to building packages in docker. Patches are welcome, and since code
freeze is scheduled for today, there's plenty of time for the search of
a better way: until May 1st or so only patches to stable will be
accepted.

Keep in mind that everything seems to be working not just on Linux
hosts, I think some people build packages on OS X too.

Patch

diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -213,11 +213,9 @@  contrib/docker/debian-%: contrib/docker/
 	sed "s/__CODENAME__/$*/" $< > $@
 
 docker-debian-jessie: contrib/docker/debian-jessie
-	mkdir -p packages/debian-jessie
 	contrib/dockerdeb debian jessie
 
 docker-debian-stretch: contrib/docker/debian-stretch
-	mkdir -p packages/debian-stretch
 	contrib/dockerdeb debian stretch
 
 contrib/docker/ubuntu-%: contrib/docker/ubuntu.template