[Qemu-devel] [PATCH for 2.10 v3 0/6] Various docker fixes

[Qemu-devel] [PATCH for 2.10 v3 0/6] Various docker fixes

[Alex Bennée]

Hi,

I didn't get around to posting the pull-req before I went on holiday
as there were some comments to address on "docker: reduce noise when
building travis.docker". If there are no other comments I shall roll
the pull request with this.

Alex Bennée (6):
  docker: ensure NOUSER for travis images
  docker: fix dirty/stash detection on some systems
  docker: ignore submodules when checking diff
  docker: docker.py make --no-cache skip checksum test
  docker: don't install device-tree-compiler build-deps in travis.docker
  docker: reduce noise when building travis.docker

 tests/docker/Makefile.include          | 3 ++-
 tests/docker/docker.py                 | 3 ++-
 tests/docker/dockerfiles/travis.docker | 4 ++--
 3 files changed, 6 insertions(+), 4 deletions(-)

-- 
2.13.0

[Qemu-devel] [PATCH for 2.10 v3 1/6] docker: ensure NOUSER for travis images

[Alex Bennée]

While adding the current user is a useful default behaviour for
creating new images it is not appropriate for Travis which already has
a default user.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/docker/Makefile.include | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index aaab1a4208..d7dafdbd27 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -71,6 +71,7 @@ docker-image-debian-ppc64el-cross: docker-image-debian9
 docker-image-debian-s390x-cross: docker-image-debian9
 docker-image-debian-win32-cross: docker-image-debian8-mxe
 docker-image-debian-win64-cross: docker-image-debian8-mxe
+docker-image-travis: NOUSER=1
 
 # Expand all the pre-requistes for each docker image and test combination
 $(foreach i,$(DOCKER_IMAGES), \
-- 
2.13.0

Re: [Qemu-devel] [PATCH for 2.10 v3 1/6] docker: ensure NOUSER for travis images

[Fam Zheng]

On Mon, 08/07 16:39, Alex Bennée wrote:
> While adding the current user is a useful default behaviour for
> creating new images it is not appropriate for Travis which already has
> a default user.

Target docker-travis@travis will not have NOUSER=1 automatically, right? Is that
a problem?

Fam

> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/docker/Makefile.include | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index aaab1a4208..d7dafdbd27 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -71,6 +71,7 @@ docker-image-debian-ppc64el-cross: docker-image-debian9
>  docker-image-debian-s390x-cross: docker-image-debian9
>  docker-image-debian-win32-cross: docker-image-debian8-mxe
>  docker-image-debian-win64-cross: docker-image-debian8-mxe
> +docker-image-travis: NOUSER=1
>  
>  # Expand all the pre-requistes for each docker image and test combination
>  $(foreach i,$(DOCKER_IMAGES), \
> -- 
> 2.13.0
> 
> 

Re: [Qemu-devel] [PATCH for 2.10 v3 1/6] docker: ensure NOUSER for travis images

[Alex Bennée]

Fam Zheng <famz@redhat.com> writes:

> On Mon, 08/07 16:39, Alex Bennée wrote:
>> While adding the current user is a useful default behaviour for
>> creating new images it is not appropriate for Travis which already has
>> a default user.
>
> Target docker-travis@travis will not have NOUSER=1 automatically, right? Is that
> a problem?

It doesn't until this patch. In fact it would be better if we ran travis
tests with the travis user rather than the mapped local user.

>
> Fam
>
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  tests/docker/Makefile.include | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>> index aaab1a4208..d7dafdbd27 100644
>> --- a/tests/docker/Makefile.include
>> +++ b/tests/docker/Makefile.include
>> @@ -71,6 +71,7 @@ docker-image-debian-ppc64el-cross: docker-image-debian9
>>  docker-image-debian-s390x-cross: docker-image-debian9
>>  docker-image-debian-win32-cross: docker-image-debian8-mxe
>>  docker-image-debian-win64-cross: docker-image-debian8-mxe
>> +docker-image-travis: NOUSER=1
>>
>>  # Expand all the pre-requistes for each docker image and test combination
>>  $(foreach i,$(DOCKER_IMAGES), \
>> --
>> 2.13.0
>>
>>


--
Alex Bennée

[Qemu-devel] [PATCH for 2.10 v3 2/6] docker: fix dirty/stash detection on some systems

[Alex Bennée]

I reported this a while ago but evidently forgot to push the patch
upstream. Without this I wasn't seeing the dirty tree state submitted
to the docker build which is very confusing.

Reference:
  Subject: [PATCH 1/6] tests/docker/Makefile.include: fix diff-index call
  Date: Fri, 28 Oct 2016 17:33:34 +0100
  Message-Id: <20161028163339.31096-2-alex.bennee@linaro.org>

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/docker/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index d7dafdbd27..ff580edfad 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -20,7 +20,7 @@ IMAGES ?= %
 # Make archive from git repo $1 to tar.gz $2
 make-archive-maybe = $(if $(wildcard $1/*), \
 	$(call quiet-command, \
-		(cd $1; if git diff-index --quiet HEAD -- &>/dev/null; then \
+		(cd $1; if git diff-index --quiet HEAD -- 2>/dev/null; then \
 			git archive -1 HEAD --format=tar.gz; \
 		else \
 			git archive -1 $$(git stash create) --format=tar.gz; \
-- 
2.13.0

[Qemu-devel] [PATCH for 2.10 v3 3/6] docker: ignore submodules when checking diff

[Alex Bennée]

The QEMU tree often has dirty submodules in it but this will confuse
things as git stash is not going to save their state directly. They
are likely just dirty from build files anyway. Ignore them in the
diff-index command.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/docker/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index ff580edfad..2a560a5327 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -20,7 +20,7 @@ IMAGES ?= %
 # Make archive from git repo $1 to tar.gz $2
 make-archive-maybe = $(if $(wildcard $1/*), \
 	$(call quiet-command, \
-		(cd $1; if git diff-index --quiet HEAD -- 2>/dev/null; then \
+		(cd $1; if git diff-index --quiet --ignore-submodules HEAD -- 2>/dev/null; then \
 			git archive -1 HEAD --format=tar.gz; \
 		else \
 			git archive -1 $$(git stash create) --format=tar.gz; \
-- 
2.13.0

[Qemu-devel] [PATCH for 2.10 v3 4/6] docker: docker.py make --no-cache skip checksum test

[Alex Bennée]

If you invoke with NOCACHE=1 we pass --no-cache in the argv to
docker.py but may still not force a rebuild if the dockerfile checksum
hasn't changed. By testing for its presence we can force builds
without having to manually remove the docker image.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/docker.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index ee40ca04d9..aab1648cc5 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -261,7 +261,8 @@ class BuildCommand(SubCommand):
         tag = args.tag
 
         dkr = Docker()
-        if dkr.image_matches_dockerfile(tag, dockerfile):
+        if "--no-cache" not in argv and \
+           dkr.image_matches_dockerfile(tag, dockerfile):
             if not args.quiet:
                 print "Image is up to date."
         else:
-- 
2.13.0

Re: [Qemu-devel] [PATCH for 2.10 v3 4/6] docker: docker.py make --no-cache skip checksum test

[Fam Zheng]

On Mon, 08/07 16:39, Alex Bennée wrote:
> If you invoke with NOCACHE=1 we pass --no-cache in the argv to
> docker.py but may still not force a rebuild if the dockerfile checksum
> hasn't changed. By testing for its presence we can force builds
> without having to manually remove the docker image.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/docker/docker.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index ee40ca04d9..aab1648cc5 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -261,7 +261,8 @@ class BuildCommand(SubCommand):
>          tag = args.tag
>  
>          dkr = Docker()
> -        if dkr.image_matches_dockerfile(tag, dockerfile):
> +        if "--no-cache" not in argv and \
> +           dkr.image_matches_dockerfile(tag, dockerfile):
>              if not args.quiet:
>                  print "Image is up to date."
>          else:
> -- 
> 2.13.0
> 
> 

NACK.

Please add "--no-cache" to BuildCommand.args, and use args.no_cache.

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index ee40ca04d9..530bc62d40 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -251,6 +251,8 @@ class BuildCommand(SubCommand):
         parser.add_argument("--add-current-user", "-u", dest="user",
                             action="store_true",
                             help="Add the current user to image's passwd")
+        parser.add_argument("--no-cache", action="store_true",
+                            help="Disable docker cache and checksum")
         parser.add_argument("tag",
                             help="Image Tag")
         parser.add_argument("dockerfile",

Re: [Qemu-devel] [PATCH for 2.10 v3 4/6] docker: docker.py make --no-cache skip checksum test

[Alex Bennée]

Fam Zheng <famz@redhat.com> writes:

> On Mon, 08/07 16:39, Alex Bennée wrote:
>> If you invoke with NOCACHE=1 we pass --no-cache in the argv to
>> docker.py but may still not force a rebuild if the dockerfile checksum
>> hasn't changed. By testing for its presence we can force builds
>> without having to manually remove the docker image.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  tests/docker/docker.py | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
>> index ee40ca04d9..aab1648cc5 100755
>> --- a/tests/docker/docker.py
>> +++ b/tests/docker/docker.py
>> @@ -261,7 +261,8 @@ class BuildCommand(SubCommand):
>>          tag = args.tag
>>
>>          dkr = Docker()
>> -        if dkr.image_matches_dockerfile(tag, dockerfile):
>> +        if "--no-cache" not in argv and \
>> +           dkr.image_matches_dockerfile(tag, dockerfile):
>>              if not args.quiet:
>>                  print "Image is up to date."
>>          else:
>> --
>> 2.13.0
>>
>>
>
> NACK.
>
> Please add "--no-cache" to BuildCommand.args, and use args.no_cache.
>
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index ee40ca04d9..530bc62d40 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -251,6 +251,8 @@ class BuildCommand(SubCommand):
>          parser.add_argument("--add-current-user", "-u", dest="user",
>                              action="store_true",
>                              help="Add the current user to image's passwd")
> +        parser.add_argument("--no-cache", action="store_true",
> +                            help="Disable docker cache and checksum")
>          parser.add_argument("tag",
>                              help="Image Tag")
>          parser.add_argument("dockerfile",

Fair enough, I just didn't want to clash with what usually gets passed
directly to the docker command.


--
Alex Bennée

Re: [Qemu-devel] [PATCH for 2.10 v3 4/6] docker: docker.py make --no-cache skip checksum test

[Fam Zheng]

On Tue, 08/08 09:51, Alex Bennée wrote:
> 
> Fam Zheng <famz@redhat.com> writes:
> 
> > On Mon, 08/07 16:39, Alex Bennée wrote:
> >> If you invoke with NOCACHE=1 we pass --no-cache in the argv to
> >> docker.py but may still not force a rebuild if the dockerfile checksum
> >> hasn't changed. By testing for its presence we can force builds
> >> without having to manually remove the docker image.
> >>
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> ---
> >>  tests/docker/docker.py | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> >> index ee40ca04d9..aab1648cc5 100755
> >> --- a/tests/docker/docker.py
> >> +++ b/tests/docker/docker.py
> >> @@ -261,7 +261,8 @@ class BuildCommand(SubCommand):
> >>          tag = args.tag
> >>
> >>          dkr = Docker()
> >> -        if dkr.image_matches_dockerfile(tag, dockerfile):
> >> +        if "--no-cache" not in argv and \
> >> +           dkr.image_matches_dockerfile(tag, dockerfile):
> >>              if not args.quiet:
> >>                  print "Image is up to date."
> >>          else:
> >> --
> >> 2.13.0
> >>
> >>
> >
> > NACK.
> >
> > Please add "--no-cache" to BuildCommand.args, and use args.no_cache.
> >
> > diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> > index ee40ca04d9..530bc62d40 100755
> > --- a/tests/docker/docker.py
> > +++ b/tests/docker/docker.py
> > @@ -251,6 +251,8 @@ class BuildCommand(SubCommand):
> >          parser.add_argument("--add-current-user", "-u", dest="user",
> >                              action="store_true",
> >                              help="Add the current user to image's passwd")
> > +        parser.add_argument("--no-cache", action="store_true",
> > +                            help="Disable docker cache and checksum")
> >          parser.add_argument("tag",
> >                              help="Image Tag")
> >          parser.add_argument("dockerfile",
> 
> Fair enough, I just didn't want to clash with what usually gets passed
> directly to the docker command.

Hmm, if we care about clashing maybe it's better to call it --no-checksum?

Fam

[Qemu-devel] [PATCH for 2.10 v3 5/6] docker: don't install device-tree-compiler build-deps in travis.docker

[Alex Bennée]

Installing the device-tree-compiler build-deps is a little extreme. We
only actually need the binary so include it with the other packages.

Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/dockerfiles/travis.docker | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/docker/dockerfiles/travis.docker b/tests/docker/dockerfiles/travis.docker
index 636fa590a5..57ac8e1419 100644
--- a/tests/docker/dockerfiles/travis.docker
+++ b/tests/docker/dockerfiles/travis.docker
@@ -1,6 +1,5 @@
 FROM quay.io/travisci/travis-ruby
 RUN apt-get update
 RUN apt-get -y build-dep qemu
-RUN apt-get -y build-dep device-tree-compiler
-RUN apt-get -y install python2.7 python-yaml dh-autoreconf gdb strace lsof net-tools
+RUN apt-get -y install device-tree-compiler python2.7 python-yaml dh-autoreconf gdb strace lsof net-tools
 ENV FEATURES pyyaml
-- 
2.13.0

Re: [Qemu-devel] [PATCH for 2.10 v3 5/6] docker: don't install device-tree-compiler build-deps in travis.docker

[Philippe Mathieu-Daudé]

On 08/07/2017 12:39 PM, Alex Bennée wrote:
> Installing the device-tree-compiler build-deps is a little extreme. We
> only actually need the binary so include it with the other packages.
> 
> Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

FWIW:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>   tests/docker/dockerfiles/travis.docker | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tests/docker/dockerfiles/travis.docker b/tests/docker/dockerfiles/travis.docker
> index 636fa590a5..57ac8e1419 100644
> --- a/tests/docker/dockerfiles/travis.docker
> +++ b/tests/docker/dockerfiles/travis.docker
> @@ -1,6 +1,5 @@
>   FROM quay.io/travisci/travis-ruby
>   RUN apt-get update
>   RUN apt-get -y build-dep qemu
> -RUN apt-get -y build-dep device-tree-compiler
> -RUN apt-get -y install python2.7 python-yaml dh-autoreconf gdb strace lsof net-tools
> +RUN apt-get -y install device-tree-compiler python2.7 python-yaml dh-autoreconf gdb strace lsof net-tools
>   ENV FEATURES pyyaml
> 

[Qemu-devel] [PATCH for 2.10 v3 6/6] docker: reduce noise when building travis.docker

[Alex Bennée]

Set the DEBIAN_FRONTEND and locale env vars to stop apt complaining so
much as we build the image. See the Travis build logs where it is done
by their scripts.

Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

---
v3
  - one line env, drop LANG
  - add r-b and t-b tags
---
 tests/docker/dockerfiles/travis.docker | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/docker/dockerfiles/travis.docker b/tests/docker/dockerfiles/travis.docker
index 57ac8e1419..03b7ef34de 100644
--- a/tests/docker/dockerfiles/travis.docker
+++ b/tests/docker/dockerfiles/travis.docker
@@ -1,4 +1,5 @@
 FROM quay.io/travisci/travis-ruby
+ENV DEBIAN_FRONTEND=noninteractive LC_ALL=en_US.UTF-8
 RUN apt-get update
 RUN apt-get -y build-dep qemu
 RUN apt-get -y install device-tree-compiler python2.7 python-yaml dh-autoreconf gdb strace lsof net-tools
-- 
2.13.0