All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/7] docker tests fixes
@ 2016-08-24 18:30 Sascha Silbe
  2016-08-24 18:30 ` [Qemu-devel] [PATCH v2 1/7] docker.py: don't hang on large docker output Sascha Silbe
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Sascha Silbe @ 2016-08-24 18:30 UTC (permalink / raw)
  To: qemu-devel, Fam Zheng; +Cc: Alex Bennée

A couple of fixes for issues encountered while trying out the new
docker test support. As of v2 building the debian-debootstrap image
now works on my laptop, too.

Thanks for the docker test support, BTW. The centos6 test came in
rather handy today for testing the glib < 2.30 compatibility code.

v1→v2:
  - found a good place to stick the warning about EXECUTABLE
  - additional fixes for the debian-debootstrap image

Sascha Silbe (7):
  docker.py: don't hang on large docker output
  docker: avoid dependency on 'realpath' package
  docker: debian-bootstrap.pre: print helpful message if
    DEB_ARCH/DEB_TYPE unset
  docker: print warning if EXECUTABLE is not set when building
    debootstrap image
  docker: make sure debootstrap is at least 1.0.67
  docker: build debootstrap after cloning
  docker: silence debootstrap when --quiet is given

 tests/docker/Makefile.include                 |  5 ++++-
 tests/docker/docker.py                        | 12 ++++++++----
 tests/docker/dockerfiles/debian-bootstrap.pre | 23 +++++++++++++++++++++++
 3 files changed, 35 insertions(+), 5 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 1/7] docker.py: don't hang on large docker output
  2016-08-24 18:30 [Qemu-devel] [PATCH v2 0/7] docker tests fixes Sascha Silbe
@ 2016-08-24 18:30 ` Sascha Silbe
  2016-09-06  1:30   ` Fam Zheng
  2016-08-24 18:30 ` [Qemu-devel] [PATCH v2 2/7] docker: avoid dependency on 'realpath' package Sascha Silbe
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Sascha Silbe @ 2016-08-24 18:30 UTC (permalink / raw)
  To: qemu-devel, Fam Zheng; +Cc: Alex Bennée

Unlike Popen.communicate(), subprocess.call() doesn't read from the
stdout file descriptor. If the child process produces more output than
fits into the pipe buffer, it will block indefinitely.

If we don't intend to consume the output, just send it straight to
/dev/null to avoid this issue.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
This fixes a hang for me when building the Ubuntu docker image (empty
docker image cache).

 tests/docker/docker.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 222a105..efb2bf4 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -25,6 +25,10 @@ from tarfile import TarFile, TarInfo
 from StringIO import StringIO
 from shutil import copy, rmtree
 
+
+DEVNULL = open(os.devnull, 'wb')
+
+
 def _text_checksum(text):
     """Calculate a digest string unique to the text content"""
     return hashlib.sha1(text).hexdigest()
@@ -34,8 +38,7 @@ def _guess_docker_command():
     commands = [["docker"], ["sudo", "-n", "docker"]]
     for cmd in commands:
         if subprocess.call(cmd + ["images"],
-                           stdout=subprocess.PIPE,
-                           stderr=subprocess.PIPE) == 0:
+                           stdout=DEVNULL, stderr=DEVNULL) == 0:
             return cmd
     commands_txt = "\n".join(["  " + " ".join(x) for x in commands])
     raise Exception("Cannot find working docker command. Tried:\n%s" % \
@@ -98,7 +101,7 @@ class Docker(object):
 
     def _do(self, cmd, quiet=True, infile=None, **kwargs):
         if quiet:
-            kwargs["stdout"] = subprocess.PIPE
+            kwargs["stdout"] = DEVNULL
         if infile:
             kwargs["stdin"] = infile
         return subprocess.call(self._command + cmd, **kwargs)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 2/7] docker: avoid dependency on 'realpath' package
  2016-08-24 18:30 [Qemu-devel] [PATCH v2 0/7] docker tests fixes Sascha Silbe
  2016-08-24 18:30 ` [Qemu-devel] [PATCH v2 1/7] docker.py: don't hang on large docker output Sascha Silbe
@ 2016-08-24 18:30 ` Sascha Silbe
  2016-08-24 18:30 ` [Qemu-devel] [PATCH v2 3/7] docker: debian-bootstrap.pre: print helpful message if DEB_ARCH/DEB_TYPE unset Sascha Silbe
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Sascha Silbe @ 2016-08-24 18:30 UTC (permalink / raw)
  To: qemu-devel, Fam Zheng; +Cc: Alex Bennée

The 'realpath' executable is shipped in a separate package that isn't
installed by default on some distros.

We already use 'readlink -e' (provided by GNU coreutils) in some other
part of the code, so let's settle for that instead.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
Too bad there isn't a POSIX equivalent of this.

 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 4f4707d..1b20db0 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -116,7 +116,7 @@ docker-run-%: docker-qemu-src
 				-e EXTRA_CONFIGURE_OPTS=$(EXTRA_CONFIGURE_OPTS) \
 				-e V=$V -e J=$J -e DEBUG=$(DEBUG)\
 				-e CCACHE_DIR=/var/tmp/ccache \
-				-v $$(realpath $(DOCKER_SRC_COPY)):/var/tmp/qemu:z$(COMMA)ro \
+				-v $$(readlink -e $(DOCKER_SRC_COPY)):/var/tmp/qemu:z$(COMMA)ro \
 				-v $(DOCKER_CCACHE_DIR):/var/tmp/ccache:z \
 				qemu:$(IMAGE) \
 				/var/tmp/qemu/run \
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 3/7] docker: debian-bootstrap.pre: print helpful message if DEB_ARCH/DEB_TYPE unset
  2016-08-24 18:30 [Qemu-devel] [PATCH v2 0/7] docker tests fixes Sascha Silbe
  2016-08-24 18:30 ` [Qemu-devel] [PATCH v2 1/7] docker.py: don't hang on large docker output Sascha Silbe
  2016-08-24 18:30 ` [Qemu-devel] [PATCH v2 2/7] docker: avoid dependency on 'realpath' package Sascha Silbe
@ 2016-08-24 18:30 ` Sascha Silbe
  2016-09-06  1:47   ` Fam Zheng
  2016-08-24 18:31 ` [Qemu-devel] [PATCH v2 4/7] docker: print warning if EXECUTABLE is not set when building debootstrap image Sascha Silbe
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Sascha Silbe @ 2016-08-24 18:30 UTC (permalink / raw)
  To: qemu-devel, Fam Zheng; +Cc: Alex Bennée

The debian-bootstrap image doesn't choose a default architecture and
distribution version, instead the user has to set both DEB_ARCH and
DEB_TYPE in the environment. Print a reasonably helpful message if
either of them isn't set instead of complaining about "qemu-" being
missing or erroring out because we cannot cd to the mirror URL.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
 tests/docker/dockerfiles/debian-bootstrap.pre | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/docker/dockerfiles/debian-bootstrap.pre b/tests/docker/dockerfiles/debian-bootstrap.pre
index 5d9c8d5..2ae363f 100755
--- a/tests/docker/dockerfiles/debian-bootstrap.pre
+++ b/tests/docker/dockerfiles/debian-bootstrap.pre
@@ -15,6 +15,19 @@ exit_and_skip()
 if [ -z $FAKEROOT ]; then
     echo "Please install fakeroot to enable bootstraping"
     exit_and_skip
+
+fi
+
+if [ -z "${DEB_ARCH}" ]; then
+    echo "Please set DEB_ARCH to choose an architecture (e.g. armhf)"
+    exit_and_skip
+
+fi
+
+if [ -z "${DEB_TYPE}" ]; then
+    echo "Please set DEB_TYPE to a Debian archive name (e.g. testing)"
+    exit_and_skip
+
 fi
 
 # We check in order for
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 4/7] docker: print warning if EXECUTABLE is not set when building debootstrap image
  2016-08-24 18:30 [Qemu-devel] [PATCH v2 0/7] docker tests fixes Sascha Silbe
                   ` (2 preceding siblings ...)
  2016-08-24 18:30 ` [Qemu-devel] [PATCH v2 3/7] docker: debian-bootstrap.pre: print helpful message if DEB_ARCH/DEB_TYPE unset Sascha Silbe
@ 2016-08-24 18:31 ` Sascha Silbe
  2016-08-24 18:31 ` [Qemu-devel] [PATCH v2 5/7] docker: make sure debootstrap is at least 1.0.67 Sascha Silbe
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Sascha Silbe @ 2016-08-24 18:31 UTC (permalink / raw)
  To: qemu-devel, Fam Zheng; +Cc: Alex Bennée

Building the debian-debootstrap image will usually fail if EXECUTABLE
isn't set (when using the Makefile). Warn the user in this case so
they know why it's failing.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
 tests/docker/Makefile.include | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 1b20db0..19d4cc7 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -44,6 +44,9 @@ docker-image: ${DOCKER_TARGETS}
 
 # General rule for building docker images
 docker-image-%: $(DOCKER_FILES_DIR)/%.docker
+	@if test "$@" = docker-image-debian-bootstrap -a -z "$(EXECUTABLE)"; then \
+		echo WARNING: EXECUTABLE is not set, debootstrap may fail. 2>&1 ; \
+	fi
 	$(call quiet-command,\
 		$(SRC_PATH)/tests/docker/docker.py build qemu:$* $< \
 		$(if $V,,--quiet) $(if $(NOCACHE),--no-cache) \
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 5/7] docker: make sure debootstrap is at least 1.0.67
  2016-08-24 18:30 [Qemu-devel] [PATCH v2 0/7] docker tests fixes Sascha Silbe
                   ` (3 preceding siblings ...)
  2016-08-24 18:31 ` [Qemu-devel] [PATCH v2 4/7] docker: print warning if EXECUTABLE is not set when building debootstrap image Sascha Silbe
@ 2016-08-24 18:31 ` Sascha Silbe
  2016-09-06  1:43   ` Fam Zheng
  2016-08-24 18:31 ` [Qemu-devel] [PATCH v2 6/7] docker: build debootstrap after cloning Sascha Silbe
  2016-08-24 18:31 ` [Qemu-devel] [PATCH v2 7/7] docker: silence debootstrap when --quiet is given Sascha Silbe
  6 siblings, 1 reply; 16+ messages in thread
From: Sascha Silbe @ 2016-08-24 18:31 UTC (permalink / raw)
  To: qemu-devel, Fam Zheng; +Cc: Alex Bennée

debootstrap prior to 1.0.67 generated an empty sources.list during
foreign bootstraps (Debian#732255 [1]). Fall back to the git checkout
if the installed debootstrap version is too old.

[1] https://bugs.debian.org/732255

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
Not sure if this used to work in even older debootstrap versions; I
don't remember running into this before. But then I needed to set up
several config files manually after a debootstrap anyway (both foreign
and native), so sources.list might just have been another one to fix
up manually.

 tests/docker/dockerfiles/debian-bootstrap.pre | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tests/docker/dockerfiles/debian-bootstrap.pre b/tests/docker/dockerfiles/debian-bootstrap.pre
index 2ae363f..3c3e781 100755
--- a/tests/docker/dockerfiles/debian-bootstrap.pre
+++ b/tests/docker/dockerfiles/debian-bootstrap.pre
@@ -3,6 +3,8 @@
 # Simple wrapper for debootstrap, run in the docker build context
 #
 FAKEROOT=`which fakeroot 2> /dev/null`
+# debootstrap < 1.0.67 generates empty sources.list, see Debian#732255
+MIN_DEBOOTSTRAP_VERSION=1.0.67
 
 exit_and_skip()
 {
@@ -40,9 +42,16 @@ fi
 #
 
 if [ -z $DEBOOTSTRAP_DIR ]; then
+    NEED_DEBOOTSTRAP=false
     DEBOOTSTRAP=`which debootstrap 2> /dev/null`
     if [ -z $DEBOOTSTRAP ]; then
         echo "No debootstrap installed, attempting to install from SCM"
+        NEED_DEBOOTSTRAP=true
+    elif ! (echo "${MIN_DEBOOTSTRAP_VERSION}" ; "${DEBOOTSTRAP}" --version |cut -d ' ' -f 2) |sort -VC; then
+        echo "debootstrap too old, attempting to install from SCM"
+        NEED_DEBOOTSTRAP=true
+    fi
+    if $NEED_DEBOOTSTRAP; then
         DEBOOTSTRAP_SOURCE=https://anonscm.debian.org/git/d-i/debootstrap.git
         git clone ${DEBOOTSTRAP_SOURCE} ./debootstrap.git
         export DEBOOTSTRAP_DIR=./debootstrap.git
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 6/7] docker: build debootstrap after cloning
  2016-08-24 18:30 [Qemu-devel] [PATCH v2 0/7] docker tests fixes Sascha Silbe
                   ` (4 preceding siblings ...)
  2016-08-24 18:31 ` [Qemu-devel] [PATCH v2 5/7] docker: make sure debootstrap is at least 1.0.67 Sascha Silbe
@ 2016-08-24 18:31 ` Sascha Silbe
  2016-08-24 18:31 ` [Qemu-devel] [PATCH v2 7/7] docker: silence debootstrap when --quiet is given Sascha Silbe
  6 siblings, 0 replies; 16+ messages in thread
From: Sascha Silbe @ 2016-08-24 18:31 UTC (permalink / raw)
  To: qemu-devel, Fam Zheng; +Cc: Alex Bennée

When using the git version of debootstrap (because no usable version
of debootstrap was installed on the host), we need to run 'make' so
that devices.tar.gz gets built. Otherwise the first debootstrap stage
will fail without printing any error message.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
 tests/docker/dockerfiles/debian-bootstrap.pre | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/docker/dockerfiles/debian-bootstrap.pre b/tests/docker/dockerfiles/debian-bootstrap.pre
index 3c3e781..a633224 100755
--- a/tests/docker/dockerfiles/debian-bootstrap.pre
+++ b/tests/docker/dockerfiles/debian-bootstrap.pre
@@ -56,6 +56,7 @@ if [ -z $DEBOOTSTRAP_DIR ]; then
         git clone ${DEBOOTSTRAP_SOURCE} ./debootstrap.git
         export DEBOOTSTRAP_DIR=./debootstrap.git
         DEBOOTSTRAP=./debootstrap.git/debootstrap
+        (cd "${DEBOOTSTRAP_DIR}" && "${FAKEROOT}" make )
     fi
 else
     DEBOOTSTRAP=${DEBOOTSTRAP_DIR}/debootstrap
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v2 7/7] docker: silence debootstrap when --quiet is given
  2016-08-24 18:30 [Qemu-devel] [PATCH v2 0/7] docker tests fixes Sascha Silbe
                   ` (5 preceding siblings ...)
  2016-08-24 18:31 ` [Qemu-devel] [PATCH v2 6/7] docker: build debootstrap after cloning Sascha Silbe
@ 2016-08-24 18:31 ` Sascha Silbe
  6 siblings, 0 replies; 16+ messages in thread
From: Sascha Silbe @ 2016-08-24 18:31 UTC (permalink / raw)
  To: qemu-devel, Fam Zheng; +Cc: Alex Bennée

If we silence docker when --quiet is given, we should also silence the
.pre script (i.e. debootstrap).

Only discards stdout, so some diagnostics (e.g. from git clone) are
still printed. Most of the verbose output is gone however and this way
we still have a chance to see error messages.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
 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 efb2bf4..b85c165 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -239,8 +239,9 @@ class BuildCommand(SubCommand):
             # Is there a .pre file to run in the build context?
             docker_pre = os.path.splitext(args.dockerfile)[0]+".pre"
             if os.path.exists(docker_pre):
+                stdout = DEVNULL if args.quiet else None
                 rc = subprocess.call(os.path.realpath(docker_pre),
-                                     cwd=docker_dir)
+                                     cwd=docker_dir, stdout=stdout)
                 if rc == 3:
                     print "Skip"
                     return 0
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/7] docker.py: don't hang on large docker output
  2016-08-24 18:30 ` [Qemu-devel] [PATCH v2 1/7] docker.py: don't hang on large docker output Sascha Silbe
@ 2016-09-06  1:30   ` Fam Zheng
  2016-09-06 18:31     ` Sascha Silbe
  0 siblings, 1 reply; 16+ messages in thread
From: Fam Zheng @ 2016-09-06  1:30 UTC (permalink / raw)
  To: Sascha Silbe; +Cc: qemu-devel, Alex Bennée

On Wed, 08/24 20:30, Sascha Silbe wrote:
> Unlike Popen.communicate(), subprocess.call() doesn't read from the
> stdout file descriptor. If the child process produces more output than
> fits into the pipe buffer, it will block indefinitely.
> 
> If we don't intend to consume the output, just send it straight to
> /dev/null to avoid this issue.
> 
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> ---
> This fixes a hang for me when building the Ubuntu docker image (empty
> docker image cache).
> 
>  tests/docker/docker.py | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index 222a105..efb2bf4 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -25,6 +25,10 @@ from tarfile import TarFile, TarInfo
>  from StringIO import StringIO
>  from shutil import copy, rmtree
>  
> +
> +DEVNULL = open(os.devnull, 'wb')
> +
> +

Too many blank lines? Otherwise looks good.

>  def _text_checksum(text):
>      """Calculate a digest string unique to the text content"""
>      return hashlib.sha1(text).hexdigest()
> @@ -34,8 +38,7 @@ def _guess_docker_command():
>      commands = [["docker"], ["sudo", "-n", "docker"]]
>      for cmd in commands:
>          if subprocess.call(cmd + ["images"],
> -                           stdout=subprocess.PIPE,
> -                           stderr=subprocess.PIPE) == 0:
> +                           stdout=DEVNULL, stderr=DEVNULL) == 0:
>              return cmd
>      commands_txt = "\n".join(["  " + " ".join(x) for x in commands])
>      raise Exception("Cannot find working docker command. Tried:\n%s" % \
> @@ -98,7 +101,7 @@ class Docker(object):
>  
>      def _do(self, cmd, quiet=True, infile=None, **kwargs):
>          if quiet:
> -            kwargs["stdout"] = subprocess.PIPE
> +            kwargs["stdout"] = DEVNULL
>          if infile:
>              kwargs["stdin"] = infile
>          return subprocess.call(self._command + cmd, **kwargs)
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 5/7] docker: make sure debootstrap is at least 1.0.67
  2016-08-24 18:31 ` [Qemu-devel] [PATCH v2 5/7] docker: make sure debootstrap is at least 1.0.67 Sascha Silbe
@ 2016-09-06  1:43   ` Fam Zheng
  2016-09-06 18:54     ` Sascha Silbe
  0 siblings, 1 reply; 16+ messages in thread
From: Fam Zheng @ 2016-09-06  1:43 UTC (permalink / raw)
  To: Sascha Silbe; +Cc: qemu-devel, Alex Bennée

On Wed, 08/24 20:31, Sascha Silbe wrote:
> debootstrap prior to 1.0.67 generated an empty sources.list during
> foreign bootstraps (Debian#732255 [1]). Fall back to the git checkout
> if the installed debootstrap version is too old.
> 
> [1] https://bugs.debian.org/732255
> 
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> ---
> Not sure if this used to work in even older debootstrap versions; I
> don't remember running into this before. But then I needed to set up
> several config files manually after a debootstrap anyway (both foreign
> and native), so sources.list might just have been another one to fix
> up manually.
> 
>  tests/docker/dockerfiles/debian-bootstrap.pre | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tests/docker/dockerfiles/debian-bootstrap.pre b/tests/docker/dockerfiles/debian-bootstrap.pre
> index 2ae363f..3c3e781 100755
> --- a/tests/docker/dockerfiles/debian-bootstrap.pre
> +++ b/tests/docker/dockerfiles/debian-bootstrap.pre
> @@ -3,6 +3,8 @@
>  # Simple wrapper for debootstrap, run in the docker build context
>  #
>  FAKEROOT=`which fakeroot 2> /dev/null`
> +# debootstrap < 1.0.67 generates empty sources.list, see Debian#732255
> +MIN_DEBOOTSTRAP_VERSION=1.0.67
>  
>  exit_and_skip()
>  {
> @@ -40,9 +42,16 @@ fi
>  #
>  
>  if [ -z $DEBOOTSTRAP_DIR ]; then
> +    NEED_DEBOOTSTRAP=false
>      DEBOOTSTRAP=`which debootstrap 2> /dev/null`
>      if [ -z $DEBOOTSTRAP ]; then
>          echo "No debootstrap installed, attempting to install from SCM"
> +        NEED_DEBOOTSTRAP=true
> +    elif ! (echo "${MIN_DEBOOTSTRAP_VERSION}" ; "${DEBOOTSTRAP}" --version |cut -d ' ' -f 2) |sort -VC; then

Unbalanced whitespaces around '|'?

"sort -VC" seems to be unavalable on OSX. Is there another way to write the
check?

Fam

> +        echo "debootstrap too old, attempting to install from SCM"
> +        NEED_DEBOOTSTRAP=true
> +    fi
> +    if $NEED_DEBOOTSTRAP; then
>          DEBOOTSTRAP_SOURCE=https://anonscm.debian.org/git/d-i/debootstrap.git
>          git clone ${DEBOOTSTRAP_SOURCE} ./debootstrap.git
>          export DEBOOTSTRAP_DIR=./debootstrap.git
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 3/7] docker: debian-bootstrap.pre: print helpful message if DEB_ARCH/DEB_TYPE unset
  2016-08-24 18:30 ` [Qemu-devel] [PATCH v2 3/7] docker: debian-bootstrap.pre: print helpful message if DEB_ARCH/DEB_TYPE unset Sascha Silbe
@ 2016-09-06  1:47   ` Fam Zheng
  2016-09-06 18:45     ` Sascha Silbe
  0 siblings, 1 reply; 16+ messages in thread
From: Fam Zheng @ 2016-09-06  1:47 UTC (permalink / raw)
  To: Sascha Silbe; +Cc: qemu-devel, Alex Bennée

On Wed, 08/24 20:30, Sascha Silbe wrote:
> The debian-bootstrap image doesn't choose a default architecture and
> distribution version, instead the user has to set both DEB_ARCH and
> DEB_TYPE in the environment. Print a reasonably helpful message if
> either of them isn't set instead of complaining about "qemu-" being
> missing or erroring out because we cannot cd to the mirror URL.
> 
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> ---
>  tests/docker/dockerfiles/debian-bootstrap.pre | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/tests/docker/dockerfiles/debian-bootstrap.pre b/tests/docker/dockerfiles/debian-bootstrap.pre
> index 5d9c8d5..2ae363f 100755
> --- a/tests/docker/dockerfiles/debian-bootstrap.pre
> +++ b/tests/docker/dockerfiles/debian-bootstrap.pre
> @@ -15,6 +15,19 @@ exit_and_skip()
>  if [ -z $FAKEROOT ]; then
>      echo "Please install fakeroot to enable bootstraping"
>      exit_and_skip
> +
> +fi
> +
> +if [ -z "${DEB_ARCH}" ]; then
> +    echo "Please set DEB_ARCH to choose an architecture (e.g. armhf)"
> +    exit_and_skip
> +
> +fi
> +
> +if [ -z "${DEB_TYPE}" ]; then
> +    echo "Please set DEB_TYPE to a Debian archive name (e.g. testing)"
> +    exit_and_skip
> +
>  fi
>  
>  # We check in order for
> -- 
> 1.9.1
> 

Can you also update the three 'echo' commands to output the message to stderr?

Fam

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/7] docker.py: don't hang on large docker output
  2016-09-06  1:30   ` Fam Zheng
@ 2016-09-06 18:31     ` Sascha Silbe
  2016-09-07  1:53       ` Fam Zheng
  0 siblings, 1 reply; 16+ messages in thread
From: Sascha Silbe @ 2016-09-06 18:31 UTC (permalink / raw)
  To: Fam Zheng; +Cc: Alex Bennée, qemu-devel

Dear Fam,

Fam Zheng <famz@redhat.com> writes:

> On Wed, 08/24 20:30, Sascha Silbe wrote:
[tests/docker/docker.py]
>> @@ -25,6 +25,10 @@ from tarfile import TarFile, TarInfo
>>  from StringIO import StringIO
>>  from shutil import copy, rmtree
>>  
>> +
>> +DEVNULL = open(os.devnull, 'wb')
>> +
>> +
>
> Too many blank lines? Otherwise looks good.

You caught me sneaking in a PEP-8 compliance fix. ;)

Is qemu using a custom code style for Python sources? If so, is it
documented somewhere?

Thanks for reviewing!

Sascha
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 3/7] docker: debian-bootstrap.pre: print helpful message if DEB_ARCH/DEB_TYPE unset
  2016-09-06  1:47   ` Fam Zheng
@ 2016-09-06 18:45     ` Sascha Silbe
  0 siblings, 0 replies; 16+ messages in thread
From: Sascha Silbe @ 2016-09-06 18:45 UTC (permalink / raw)
  To: Fam Zheng; +Cc: Alex Bennée, qemu-devel

Dear Fam,

Fam Zheng <famz@redhat.com> writes:

[tests/docker/dockerfiles/debian-bootstrap.pre]
[...]
>> +if [ -z "${DEB_ARCH}" ]; then
>> +    echo "Please set DEB_ARCH to choose an architecture (e.g. armhf)"
>> +    exit_and_skip
>> +
>> +fi
>> +
>> +if [ -z "${DEB_TYPE}" ]; then
>> +    echo "Please set DEB_TYPE to a Debian archive name (e.g. testing)"
>> +    exit_and_skip
>> +
>
> Can you also update the three 'echo' commands to output the message to stderr?

I've updated my patch to send output to stderr and added a new patch
that fixes up the existing messages that are "fatal" (i.e. those
directly prior to an exit_and_skip).

Sascha
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 5/7] docker: make sure debootstrap is at least 1.0.67
  2016-09-06  1:43   ` Fam Zheng
@ 2016-09-06 18:54     ` Sascha Silbe
  0 siblings, 0 replies; 16+ messages in thread
From: Sascha Silbe @ 2016-09-06 18:54 UTC (permalink / raw)
  To: Fam Zheng; +Cc: Alex Bennée, qemu-devel

Dear Fam,

Fam Zheng <famz@redhat.com> writes:

> On Wed, 08/24 20:31, Sascha Silbe wrote:
[tests/docker/dockerfiles/debian-bootstrap.pre]
>> +# debootstrap < 1.0.67 generates empty sources.list, see Debian#732255
>> +MIN_DEBOOTSTRAP_VERSION=1.0.67
[...]
>>  if [ -z $DEBOOTSTRAP_DIR ]; then
>> +    NEED_DEBOOTSTRAP=false
>>      DEBOOTSTRAP=`which debootstrap 2> /dev/null`
>>      if [ -z $DEBOOTSTRAP ]; then
>>          echo "No debootstrap installed, attempting to install from SCM"
>> +        NEED_DEBOOTSTRAP=true
>> +    elif ! (echo "${MIN_DEBOOTSTRAP_VERSION}" ; "${DEBOOTSTRAP}" --version |cut -d ' ' -f 2) |sort -VC; then
>
> Unbalanced whitespaces around '|'?

Sorry, habit from interactive shell usage. This way Ctrl+W will erase
the command and the pipe. Will fix this up.


> "sort -VC" seems to be unavalable on OSX. Is there another way to write the
> check?

Ah, right, BSD user space. sort -VC was such an elegant solution but
unfortunately it's a GNU extension. I'll try to come it up with some sed
or cut based hack instead.

Sascha
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/7] docker.py: don't hang on large docker output
  2016-09-06 18:31     ` Sascha Silbe
@ 2016-09-07  1:53       ` Fam Zheng
  2016-09-07  1:59         ` Eric Blake
  0 siblings, 1 reply; 16+ messages in thread
From: Fam Zheng @ 2016-09-07  1:53 UTC (permalink / raw)
  To: Sascha Silbe; +Cc: Alex Bennée, qemu-devel

On Tue, 09/06 20:31, Sascha Silbe wrote:
> Dear Fam,
> 
> Fam Zheng <famz@redhat.com> writes:
> 
> > On Wed, 08/24 20:30, Sascha Silbe wrote:
> [tests/docker/docker.py]
> >> @@ -25,6 +25,10 @@ from tarfile import TarFile, TarInfo
> >>  from StringIO import StringIO
> >>  from shutil import copy, rmtree
> >>  
> >> +
> >> +DEVNULL = open(os.devnull, 'wb')
> >> +
> >> +
> >
> > Too many blank lines? Otherwise looks good.
> 
> You caught me sneaking in a PEP-8 compliance fix. ;)

Ah, no problem. I just didn't know.

> 
> Is qemu using a custom code style for Python sources? If so, is it
> documented somewhere?

No, and following PEP-8 is completely fine.

Fam

> 
> Thanks for reviewing!
> 
> Sascha
> -- 
> Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
> https://se-silbe.de/
> USt-IdNr. DE281696641
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/7] docker.py: don't hang on large docker output
  2016-09-07  1:53       ` Fam Zheng
@ 2016-09-07  1:59         ` Eric Blake
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2016-09-07  1:59 UTC (permalink / raw)
  To: Fam Zheng, Sascha Silbe; +Cc: Alex Bennée, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]

On 09/06/2016 08:53 PM, Fam Zheng wrote:
>>>
>>> Too many blank lines? Otherwise looks good.
>>
>> You caught me sneaking in a PEP-8 compliance fix. ;)
> 
> Ah, no problem. I just didn't know.
> 
>>
>> Is qemu using a custom code style for Python sources? If so, is it
>> documented somewhere?
> 
> No, and following PEP-8 is completely fine.

Although it never hurts to mention stuff like this in the commit
message, to make it obvious that it was intentional :)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-09-07  1:59 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-24 18:30 [Qemu-devel] [PATCH v2 0/7] docker tests fixes Sascha Silbe
2016-08-24 18:30 ` [Qemu-devel] [PATCH v2 1/7] docker.py: don't hang on large docker output Sascha Silbe
2016-09-06  1:30   ` Fam Zheng
2016-09-06 18:31     ` Sascha Silbe
2016-09-07  1:53       ` Fam Zheng
2016-09-07  1:59         ` Eric Blake
2016-08-24 18:30 ` [Qemu-devel] [PATCH v2 2/7] docker: avoid dependency on 'realpath' package Sascha Silbe
2016-08-24 18:30 ` [Qemu-devel] [PATCH v2 3/7] docker: debian-bootstrap.pre: print helpful message if DEB_ARCH/DEB_TYPE unset Sascha Silbe
2016-09-06  1:47   ` Fam Zheng
2016-09-06 18:45     ` Sascha Silbe
2016-08-24 18:31 ` [Qemu-devel] [PATCH v2 4/7] docker: print warning if EXECUTABLE is not set when building debootstrap image Sascha Silbe
2016-08-24 18:31 ` [Qemu-devel] [PATCH v2 5/7] docker: make sure debootstrap is at least 1.0.67 Sascha Silbe
2016-09-06  1:43   ` Fam Zheng
2016-09-06 18:54     ` Sascha Silbe
2016-08-24 18:31 ` [Qemu-devel] [PATCH v2 6/7] docker: build debootstrap after cloning Sascha Silbe
2016-08-24 18:31 ` [Qemu-devel] [PATCH v2 7/7] docker: silence debootstrap when --quiet is given Sascha Silbe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.