Re: [Qemu-devel] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path
On Mon, 6 May 2019 at 18:27, Eric Blake wrote: > > On 5/3/19 3:27 AM, Antonio Ospite wrote: > > From: Antonio Ospite > > > > The configure script breaks when the qemu source directory is in a path > > containing white spaces, in particular the list of targets is not > > correctly generated when calling "./configure --help" because of how the > > default_target_list variable is built. > > > > In addition to that, *building* qemu from a directory with spaces breaks > > some assumptions in the Makefiles, even if the original source path does > > not contain spaces like in the case of an out-of-tree build, or when > > symlinks are involved. > > > > To avoid these issues, refuse to run the configure script and the > > Makefile if there are spaces or colons in the source path or the build > > path, taking as inspiration what the kbuild system in linux does. > > > > Buglink: https://bugs.launchpad.net/qemu/+bug/1817345 > > > > Signed-off-by: Antonio Ospite > > --- > > Makefile | 4 > > configure | 6 ++ > > 2 files changed, 10 insertions(+) > > > > > +++ b/Makefile > > @@ -1,5 +1,9 @@ > > # Makefile for QEMU. > > > > +ifneq ($(words $(subst :, ,$(CURDIR))), 1) > > + $(error main directory cannot contain spaces nor colons) > > +endif > > + > > # Always point to the root of the build tree (needs GNU make). > > BUILD_DIR=$(CURDIR) > > > > diff --git a/configure b/configure > > index 9832cbca5c..f7ad4381bd 100755 > > --- a/configure > > +++ b/configure > > @@ -279,6 +279,12 @@ ld_has() { > > # make source path absolute > > source_path=$(cd "$(dirname -- "$0")"; pwd) > > > > +if printf "%s\n" "$source_path" | grep -q "[[:space:]:]" || > > + printf "%s\n" "$PWD" | grep -q "[[:space:]:]"; > > For less typing and fewer processes, you could shorten this to: > > if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]"; > > but that's trivial enough for a maintainer to fold in if desired. > > Reviewed-by: Eric Blake What tree is this going to go in via? I suggest the -trivial tree. thanks -- PMM
Re: [Qemu-devel] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path
On Mon, 6 May 2019 12:27:46 -0500 Eric Blake wrote: > On 5/3/19 3:27 AM, Antonio Ospite wrote: > > From: Antonio Ospite > > > > The configure script breaks when the qemu source directory is in a path > > containing white spaces, in particular the list of targets is not > > correctly generated when calling "./configure --help" because of how the > > default_target_list variable is built. > > > > In addition to that, *building* qemu from a directory with spaces breaks > > some assumptions in the Makefiles, even if the original source path does > > not contain spaces like in the case of an out-of-tree build, or when > > symlinks are involved. > > > > To avoid these issues, refuse to run the configure script and the > > Makefile if there are spaces or colons in the source path or the build > > path, taking as inspiration what the kbuild system in linux does. > > > > Buglink: https://bugs.launchpad.net/qemu/+bug/1817345 > > > > Signed-off-by: Antonio Ospite > > --- > > Makefile | 4 > > configure | 6 ++ > > 2 files changed, 10 insertions(+) > > > > > +++ b/Makefile > > @@ -1,5 +1,9 @@ > > # Makefile for QEMU. > > > > +ifneq ($(words $(subst :, ,$(CURDIR))), 1) > > + $(error main directory cannot contain spaces nor colons) > > +endif > > + > > # Always point to the root of the build tree (needs GNU make). > > BUILD_DIR=$(CURDIR) > > > > diff --git a/configure b/configure > > index 9832cbca5c..f7ad4381bd 100755 > > --- a/configure > > +++ b/configure > > @@ -279,6 +279,12 @@ ld_has() { > > # make source path absolute > > source_path=$(cd "$(dirname -- "$0")"; pwd) > > > > +if printf "%s\n" "$source_path" | grep -q "[[:space:]:]" || > > + printf "%s\n" "$PWD" | grep -q "[[:space:]:]"; > > For less typing and fewer processes, you could shorten this to: > > if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]"; > I always forget about this printf feature :) > but that's trivial enough for a maintainer to fold in if desired. > > Reviewed-by: Eric Blake Thanks, Antonio -- Antonio Ospite https://ao2.it https://twitter.com/ao2it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing?
Re: [Qemu-devel] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path
On 5/3/19 3:27 AM, Antonio Ospite wrote: > From: Antonio Ospite > > The configure script breaks when the qemu source directory is in a path > containing white spaces, in particular the list of targets is not > correctly generated when calling "./configure --help" because of how the > default_target_list variable is built. > > In addition to that, *building* qemu from a directory with spaces breaks > some assumptions in the Makefiles, even if the original source path does > not contain spaces like in the case of an out-of-tree build, or when > symlinks are involved. > > To avoid these issues, refuse to run the configure script and the > Makefile if there are spaces or colons in the source path or the build > path, taking as inspiration what the kbuild system in linux does. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1817345 > > Signed-off-by: Antonio Ospite > --- > Makefile | 4 > configure | 6 ++ > 2 files changed, 10 insertions(+) > > +++ b/Makefile > @@ -1,5 +1,9 @@ > # Makefile for QEMU. > > +ifneq ($(words $(subst :, ,$(CURDIR))), 1) > + $(error main directory cannot contain spaces nor colons) > +endif > + > # Always point to the root of the build tree (needs GNU make). > BUILD_DIR=$(CURDIR) > > diff --git a/configure b/configure > index 9832cbca5c..f7ad4381bd 100755 > --- a/configure > +++ b/configure > @@ -279,6 +279,12 @@ ld_has() { > # make source path absolute > source_path=$(cd "$(dirname -- "$0")"; pwd) > > +if printf "%s\n" "$source_path" | grep -q "[[:space:]:]" || > + printf "%s\n" "$PWD" | grep -q "[[:space:]:]"; For less typing and fewer processes, you could shorten this to: if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]"; but that's trivial enough for a maintainer to fold in if desired. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature