From: Antonio Ospite <antonio.ospite@collabora.com>
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".
To avoid this issue, refuse to run the configure script if there are
spaces or colons in the source path, this is also what kbuild from linux
does.
Buglink: https://bugs.launchpad.net/qemu/+bug/1817345
Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>
---
configure | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/configure b/configure
index 7071f52584..fbd70a0f51 100755
--- a/configure
+++ b/configure
@@ -295,6 +295,11 @@ libs_qga=""
debug_info="yes"
stack_protector=""
+if printf "%s" "$(realpath "$source_path")" | grep -q "[[:space:]:]";
+then
+ error_exit "main directory cannot contain spaces nor colons"
+fi
+
if test -e "$source_path/.git"
then
git_update=yes
--
2.20.1
On Fri, 15 Mar 2019 at 18:26, Antonio Ospite <ao2@ao2.it> wrote: > > From: Antonio Ospite <antonio.ospite@collabora.com> > > 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". > > To avoid this issue, refuse to run the configure script if there are > spaces or colons in the source path, this is also what kbuild from linux > does. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1817345 > > Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com> Hi Antonio; thanks for this patch. > --- > configure | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/configure b/configure > index 7071f52584..fbd70a0f51 100755 > --- a/configure > +++ b/configure > @@ -295,6 +295,11 @@ libs_qga="" > debug_info="yes" > stack_protector="" > > +if printf "%s" "$(realpath "$source_path")" | grep -q "[[:space:]:]"; > +then > + error_exit "main directory cannot contain spaces nor colons" > +fi > + If you do this after the point where we make the source path absolute, you can skip the realpath (which avoids the problem that 'realpath' doesn't exist on OSX by default). It will also then be after the handling of the --source-path option argument. Do we also need to check for spaces in the path of the build directory (which is always the current working directory of the script) ? thanks -- PMM
On 3/15/19 1:40 PM, Peter Maydell wrote: > If you do this after the point where we make the source path absolute, you > can skip the realpath (which avoids the problem that 'realpath' doesn't exist > on OSX by default). It will also then be after the handling of the > --source-path option argument. > > Do we also need to check for spaces in the path of the build directory > (which is always the current working directory of the script) ? I wasn't thinking about VPATH builds, but yes, in general, we should ensure that both srcdir and builddir are sane names, while still allowing symlinks to work around otherwise problematic canonical names. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Hi, thanks for the comments.
On Fri, 15 Mar 2019 13:48:25 -0500
Eric Blake <eblake@redhat.com> wrote:
> On 3/15/19 1:40 PM, Peter Maydell wrote:
>
> > If you do this after the point where we make the source path absolute, you
> > can skip the realpath (which avoids the problem that 'realpath' doesn't exist
> > on OSX by default). It will also then be after the handling of the
> > --source-path option argument.
> >
> > Do we also need to check for spaces in the path of the build directory
> > (which is always the current working directory of the script) ?
>
> I wasn't thinking about VPATH builds, but yes, in general, we should
> ensure that both srcdir and builddir are sane names, while still
> allowing symlinks to work around otherwise problematic canonical names.
>
[...]
On Fri, 15 Mar 2019 13:46:58 -0500
Eric Blake <eblake@redhat.com> wrote:
>
> Why realpath? If my sources live in "/home/me/bad dir" but I have a
> symlink "/home/me/good", this prevents me from building even though I
> won't trip the problem.
>
The rationale behind the current patch was that the check should be
done as soon as possible to avoid unneeded work, and since $source_path
is a relative path early on in the script I thought about realpath.
TBH I used realpath unconditionally because I saw it in the Makefile
but I overlooked the fact that it is an internal function in make.
I will move the check after the expansion of $source_path.
After Eric's comment I also tried building from a sane symlink, and
while configure is fine with it "make" still does not like it for
in-tree builds: apparently CURDIR (used to set BUILD_PATH in the
Makefile) resolves symlinks and brings back the bad path.
I do get your point tho, do I did some testing to see the current status
and base the changes on that.
Assuming this setup:
├── no_spaces
│ ├── build
│ ├── qemu
│ └── qemulink -> ../with spaces/qemu
└── with spaces
├── build
├── qemu
└── qemulink -> ../no_spaces/qemu
This are the results with the current master branch:
in-tree build:
no_spaces/qemu $ ./configure # OK
no_spaces/qemu $ make # OK
no_spaces/qemulink $ ./configure # OK
no_spaces/qemulink $ make # FAILS because of CURDIR
with spaces/qemu $ ./configure # FAILS because of source_path
with spaces/qemu $ make # FAILS because of SRC_PATH
with spaces/qemulink $ ./configure # FAILS because of source_path
with spaces/qemulink $ make # FAILS because of SRC_PATH
out-of-tree builds:
no_spaces/build $ ../qemu/configure # OK
no_spaces/build $ make # OK
no_spaces/build $ ../qemulink/configure # OK
no_spaces/build $ make # OK
no_spaces/build $ ../../with\ spaces/qemu/configure # FAILS
no_spaces/build $ make # FAILS no Makefile
no_spaces/build $ ../../with\ spaces/qemulink/configure # FAILS
no_spaces/build $ make # FAILS ^
with spaces/build $ ../qemu/configure # FAILS
with spaces/build $ make # FAILS no Makefile
with spaces/build $ ../qemulink/configure # FAILS
with spaces/build $ make # FAILS no Makefile
with spaces/build $ ../../no_spaces/qemu/configure # OK
with spaces/build $ make # FAILS (CURDIR)
with spaces/build $ ../../no_spaces/qemulink/configure # OK
with spaces/build $ make # FAILS (CURDIR)
So checking both source_path and PWD is a probably a good thing.
I'd add the check in the Makefile too, to be on the safe side and cover
the case of: no_spaces/qemulink $ make
Yeah, it is a slow Saturday. :)
Ciao,
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?
On 3/15/19 12:50 PM, Antonio Ospite wrote: > From: Antonio Ospite <antonio.ospite@collabora.com> > > 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". > > To avoid this issue, refuse to run the configure script if there are > spaces or colons in the source path, this is also what kbuild from linux > does. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1817345 > > Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com> > --- > configure | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/configure b/configure > index 7071f52584..fbd70a0f51 100755 > --- a/configure > +++ b/configure > @@ -295,6 +295,11 @@ libs_qga="" > debug_info="yes" > stack_protector="" > > +if printf "%s" "$(realpath "$source_path")" | grep -q "[[:space:]:]"; Why realpath? If my sources live in "/home/me/bad dir" but I have a symlink "/home/me/good", this prevents me from building even though I won't trip the problem. Also, grep is not required to operate on non-text files (the POSIX definition states that if your input does not end in a newline, it is not a text file, and grep can skip that line) - better is to use "%s\n" in some form. So I'd rather see this just use: if printf %s\\n "$PWD" | grep -q "[[:space:]:]" -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
© 2016 - 2026 Red Hat, Inc.