Re: [Qemu-devel] [PATCH v3 0/2] configure: disallow spaces and colons in source path and build path
On 26/06/19 17:50, Laurent Vivier wrote: Le 26/06/2019 à 17:16, Antonio Ospite a écrit : On 26/05/19 16:47, Antonio Ospite wrote: Hi, Here is a v3 set to address https://bugs.launchpad.net/qemu/+bug/1817345 CCing Laurent Vivier as the patch is going through the trivial-patches branch. Ping. I cannot see this in the trivial-patches repository nor in mainline qemu. I missed it because you didn't cc: qemu-trivial ML (I use this list to pick up the patches). Ah yes, I had read that on https://wiki.qemu.org/Contribute/TrivialPatches but then forgot about it. I'm going to add them to the next pull request. Thanks, Laurent Thank you, Antonio
Re: [Qemu-devel] [PATCH v3 0/2] configure: disallow spaces and colons in source path and build path
On 26/05/19 16:47, Antonio Ospite wrote: Hi, Here is a v3 set to address https://bugs.launchpad.net/qemu/+bug/1817345 CCing Laurent Vivier as the patch is going through the trivial-patches branch. Ping. I cannot see this in the trivial-patches repository nor in mainline qemu. Thanks, Antonio The series follows up to: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00562.html Changes since v2: - Shorten 'if' check as suggested by Eric Blake in https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg01190.html - Added Reviewed-by tags by Eric Blake. Changes since v1: - Add a preparatory patch to set source_path only once and in a more robust way. - Move the checks in configure after the source_path definition to avoid using realpath which is not available everywhere. - Cover also the build path in Makefile. An extensive explanation of why I think this is needed is here: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg05398.html Thank you, Antonio Antonio Ospite (2): configure: set source_path only once and make its definition more robust configure: disallow spaces and colons in source path and build path Makefile | 4 configure | 11 --- 2 files changed, 12 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH v3 2/2] configure: disallow spaces and colons in source path and build path
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
Reviewed-by: Eric Blake
Signed-off-by: Antonio Ospite
---
Makefile | 4
configure | 5 +
2 files changed, 9 insertions(+)
diff --git a/Makefile b/Makefile
index e02b88bcb1..bed1323a45 100644
--- a/Makefile
+++ 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 9f12120ad9..2833d73ccc 100755
--- a/configure
+++ b/configure
@@ -279,6 +279,11 @@ ld_has() {
# make source path absolute
source_path=$(cd "$(dirname -- "$0")"; pwd)
+if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
+then
+ error_exit "main directory cannot contain spaces nor colons"
+fi
+
# default parameters
cpu=""
iasl="iasl"
--
2.20.1
[Qemu-devel] [PATCH v3 1/2] configure: set source_path only once and make its definition more robust
From: Antonio Ospite
Since commit 79d77bcd36 (configure: Remove --source-path option,
2019-04-29) source_path cannot be overridden anymore, move it out of the
"default parameters" block since the word "default" may suggest that the
value can change, while in fact it does not.
While at it, only set source_path once and separate the positional
argument of basename with "--" to more robustly cover the case of path
names starting with a dash.
Reviewed-by: Eric Blake
Signed-off-by: Antonio Ospite
---
configure | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index 528b9ff705..9f12120ad9 100755
--- a/configure
+++ b/configure
@@ -276,10 +276,10 @@ ld_has() {
$ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
}
-# default parameters
-source_path=$(dirname "$0")
# make source path absolute
-source_path=$(cd "$source_path"; pwd)
+source_path=$(cd "$(dirname -- "$0")"; pwd)
+
+# default parameters
cpu=""
iasl="iasl"
interp_prefix="/usr/gnemul/qemu-%M"
--
2.20.1
[Qemu-devel] [PATCH v3 0/2] configure: disallow spaces and colons in source path and build path
Hi, Here is a v3 set to address https://bugs.launchpad.net/qemu/+bug/1817345 CCing Laurent Vivier as the patch is going through the trivial-patches branch. The series follows up to: https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00562.html Changes since v2: - Shorten 'if' check as suggested by Eric Blake in https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg01190.html - Added Reviewed-by tags by Eric Blake. Changes since v1: - Add a preparatory patch to set source_path only once and in a more robust way. - Move the checks in configure after the source_path definition to avoid using realpath which is not available everywhere. - Cover also the build path in Makefile. An extensive explanation of why I think this is needed is here: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg05398.html Thank you, Antonio Antonio Ospite (2): configure: set source_path only once and make its definition more robust configure: disallow spaces and colons in source path and build path Makefile | 4 configure | 11 --- 2 files changed, 12 insertions(+), 3 deletions(-) -- 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] [Qemu-trivial] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path
On 22/05/19 17:21, Laurent Vivier wrote:
On 22/05/2019 17:01, Antonio Ospite wrote:
On 22/05/19 15:57, Laurent Vivier wrote:
On 09/05/2019 16:42, Peter Maydell wrote:
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.
Applied (unchanged) to my trivial-patches branch.
Thank you Laurent.
I'll think about sending a followup patch with the changes proposed by
Eric and I'll CC you if I do.
If you want to send a v3 of this patch to update it, I can wait.
That works too, I was waiting for the maintainers to decide what to do.
I'll try to send a v3 before Monday then.
Thanks,
Antonio
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path
On 22/05/19 15:57, Laurent Vivier wrote:
On 09/05/2019 16:42, Peter Maydell wrote:
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.
Applied (unchanged) to my trivial-patches branch.
Thank you Laurent.
I'll think about sending a followup patch with the changes proposed by
Eric and I'll CC you if I do.
Ciao,
Antonio
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?
[Qemu-devel] [PATCH v2 0/2] configure: disallow spaces and colons in source path and build path
Hi, Here is a v2 patch set to address https://bugs.launchpad.net/qemu/+bug/1817345 The series follows up to: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg05290.html https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg05573.html Changes since v1: - Add a preparatory patch to set source_path only once and in a more robust way. - Move the checks in configure after the source_path definition to avoid using realpath which is not available everywhere. - Cover also the build path in Makefile. An extensive explanation of why I think this is needed is here: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg05398.html Thanks, Antonio Antonio Ospite (2): configure: set source_path only once and make its definition more robust configure: disallow spaces and colons in source path and build path Makefile | 4 configure | 12 +--- 2 files changed, 13 insertions(+), 3 deletions(-) -- 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?
[Qemu-devel] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path
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(+)
diff --git a/Makefile b/Makefile
index 1211e78c91..7f3f7a7fef 100644
--- a/Makefile
+++ 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:]:]";
+then
+ error_exit "main directory cannot contain spaces nor colons"
+fi
+
# default parameters
cpu=""
iasl="iasl"
--
2.20.1
[Qemu-devel] [PATCH v2 1/2] configure: set source_path only once and make its definition more robust
From: Antonio Ospite
Since commit 79d77bcd36 (configure: Remove --source-path option,
2019-04-29) source_path cannot be overridden anymore, move it out of the
"default parameters" block since the word "default" may suggest that the
value can change, while in fact it does not.
While at it, only set source_path once and separate the positional
argument of basename with "--" to more robustly cover the case of path
names starting with a dash.
Signed-off-by: Antonio Ospite
---
configure | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index 60719ddcc5..9832cbca5c 100755
--- a/configure
+++ b/configure
@@ -276,10 +276,10 @@ ld_has() {
$ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
}
-# default parameters
-source_path=$(dirname "$0")
# make source path absolute
-source_path=$(cd "$source_path"; pwd)
+source_path=$(cd "$(dirname -- "$0")"; pwd)
+
+# default parameters
cpu=""
iasl="iasl"
interp_prefix="/usr/gnemul/qemu-%M"
--
2.20.1
Re: [Qemu-devel] [PATCH] configure: Remove --source-path option
On 19/03/19 09:41, Antonio Ospite wrote:
On 18/03/19 14:40, Peter Maydell wrote:
Normally configure identifies the source path by looking
at the location where the configure script itself exists.
We also provide a --source-path option which lets the user
manually override this.
There isn't really an obvious use case for the --source-path
option, and in commit 927128222b0a91f56c13a in 2017 we
accidentally added some logic that looks at $source_path
before the command line option that overrides it has been
processed.
The fact that nobody complained suggests that there isn't
any use of this option and we aren't testing it either;
remove it. This allows us to move the "make $source_path
absolute" logic up so that there is no window in the script
where $source_path is set but not yet absolute.
Signed-off-by: Peter Maydell
---
Since this is a "noticed while reading code" issue rather than
one that's actually causing a problem, it's probably 4.1
material at this point.
Cc'ing Antonio since they also have a patch to configure
which this will affect.
Thanks for CC-ing me, I will send a v2 of my patch after this one gets in.
A minor comment below.
---
configure | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/configure b/configure
index 7071f525843..bc2953e6114 100755
--- a/configure
+++ b/configure
@@ -278,6 +278,8 @@ ld_has() {
# default parameters
source_path=$(dirname "$0")
+# make source path absolute
+source_path=$(cd "$source_path"; pwd)
While we are at it, can't source_path be set only once?
And probably $(dirname -- "$0") is a little more robust, it covers the
case of directories starting with a dash, so maybe:
# default parameters
# make source path absolute
source_path=$(cd "$(dirname -- "$0")"; pwd)
...
Ping.
Now that 4.0 has been released, maybe we can move on with this minor change.
I will send a fix for https://bugs.launchpad.net/qemu/+bug/1817345 after
this patch lands.
Thank you,
Antonio
cpu=""
iasl="iasl"
interp_prefix="/usr/gnemul/qemu-%M"
@@ -519,8 +521,6 @@ for opt do
;;
--cxx=*) CXX="$optarg"
;;
- --source-path=*) source_path="$optarg"
- ;;
--cpu=*) cpu="$optarg"
;;
--extra-cflags=*) QEMU_CFLAGS="$QEMU_CFLAGS $optarg"
@@ -599,9 +599,6 @@ if test "$debug_info" = "yes"; then
LDFLAGS="-g $LDFLAGS"
fi
-# make source path absolute
-source_path=$(cd "$source_path"; pwd)
-
# running configure in the source tree?
# we know that's the case if configure is there.
if test -f "./configure"; then
@@ -945,8 +942,6 @@ for opt do
;;
--interp-prefix=*) interp_prefix="$optarg"
;;
- --source-path=*)
- ;;
--cross-prefix=*)
;;
--cc=*)
@@ -1624,7 +1619,6 @@ $(echo Available targets: $default_target_list | \
fold -s -w 53 | sed -e 's/^/ /')
Advanced options (experts only):
- --source-path=PATH path of source code [$source_path]
--cross-prefix=PREFIX use PREFIX for compile tools [$cross_prefix]
--cc=CC use C compiler CC [$cc]
--iasl=IASL use ACPI compiler IASL [$iasl]
Re: [Qemu-devel] [PATCH] configure: Remove --source-path option
On 18/03/19 14:40, Peter Maydell wrote:
Normally configure identifies the source path by looking
at the location where the configure script itself exists.
We also provide a --source-path option which lets the user
manually override this.
There isn't really an obvious use case for the --source-path
option, and in commit 927128222b0a91f56c13a in 2017 we
accidentally added some logic that looks at $source_path
before the command line option that overrides it has been
processed.
The fact that nobody complained suggests that there isn't
any use of this option and we aren't testing it either;
remove it. This allows us to move the "make $source_path
absolute" logic up so that there is no window in the script
where $source_path is set but not yet absolute.
Signed-off-by: Peter Maydell
---
Since this is a "noticed while reading code" issue rather than
one that's actually causing a problem, it's probably 4.1
material at this point.
Cc'ing Antonio since they also have a patch to configure
which this will affect.
Thanks for CC-ing me, I will send a v2 of my patch after this one gets in.
A minor comment below.
---
configure | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/configure b/configure
index 7071f525843..bc2953e6114 100755
--- a/configure
+++ b/configure
@@ -278,6 +278,8 @@ ld_has() {
# default parameters
source_path=$(dirname "$0")
+# make source path absolute
+source_path=$(cd "$source_path"; pwd)
While we are at it, can't source_path be set only once?
And probably $(dirname -- "$0") is a little more robust, it covers the
case of directories starting with a dash, so maybe:
# default parameters
# make source path absolute
source_path=$(cd "$(dirname -- "$0")"; pwd)
...
cpu=""
iasl="iasl"
interp_prefix="/usr/gnemul/qemu-%M"
@@ -519,8 +521,6 @@ for opt do
;;
--cxx=*) CXX="$optarg"
;;
- --source-path=*) source_path="$optarg"
- ;;
--cpu=*) cpu="$optarg"
;;
--extra-cflags=*) QEMU_CFLAGS="$QEMU_CFLAGS $optarg"
@@ -599,9 +599,6 @@ if test "$debug_info" = "yes"; then
LDFLAGS="-g $LDFLAGS"
fi
-# make source path absolute
-source_path=$(cd "$source_path"; pwd)
-
# running configure in the source tree?
# we know that's the case if configure is there.
if test -f "./configure"; then
@@ -945,8 +942,6 @@ for opt do
;;
--interp-prefix=*) interp_prefix="$optarg"
;;
- --source-path=*)
- ;;
--cross-prefix=*)
;;
--cc=*)
@@ -1624,7 +1619,6 @@ $(echo Available targets: $default_target_list | \
fold -s -w 53 | sed -e 's/^/ /')
Advanced options (experts only):
- --source-path=PATH path of source code [$source_path]
--cross-prefix=PREFIXuse PREFIX for compile tools [$cross_prefix]
--cc=CC use C compiler CC [$cc]
--iasl=IASL use ACPI compiler IASL [$iasl]
Re: [Qemu-devel] [PATCH] configure: disallow spaces and colons in source path
Hi, thanks for the comments. On Fri, 15 Mar 2019 13:48:25 -0500 Eric Blake 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 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?
[Qemu-devel] [PATCH] configure: disallow spaces and colons in source path
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". 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 --- 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
[Qemu-devel] [Bug 1817345] Re: configure script breaks when $source_path contains white spaces
I am OK with just checking and complaining. Linux too solves the problem in this way: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Makefile?id=51193b76bfff5027cf96ba63effae808ad67cca7 A general "shellcheck" pass wouldn't hurt, tho. Thank you, Antonio -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1817345 Title: configure script breaks when $source_path contains white spaces Status in QEMU: New Bug description: Hi, I noticed that 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". Steps to reproduce the problem: $ mkdir "dir with spaces" $ cd dir\ with\ spaces/ $ git clone https://git.qemu.org/git/qemu.git $ cd qemu/ $ ./configure --help | grep -A3 target-list Actual result: --target-list=LIST set target list (default: build everything) Available targets: dir with *-softmmu dir with *-linux-user Expected result: --target-list=LIST set target list (default: build everything) Available targets: aarch64-softmmu alpha-softmmu arm-softmmu cris-softmmu hppa-softmmu i386-softmmu lm32-softmmu m68k-softmmu microblaze-softmmu This happens because the $mak_wilds variable uses spaces to separate different paths, maybe newlines may be used, which are less likely to be in directory names. BTW "shellcheck" may help finding some other problems. Qemu version: $ git describe v3.1.0-1960-ga05838cb2a Thanks, Antonio To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1817345/+subscriptions
[Qemu-devel] [Bug 1817345] [NEW] configure script breaks when $source_path contains white spaces
Public bug reported: Hi, I noticed that 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". Steps to reproduce the problem: $ mkdir "dir with spaces" $ cd dir\ with\ spaces/ $ git clone https://git.qemu.org/git/qemu.git $ cd qemu/ $ ./configure --help | grep -A3 target-list Actual result: --target-list=LIST set target list (default: build everything) Available targets: dir with *-softmmu dir with *-linux-user Expected result: --target-list=LIST set target list (default: build everything) Available targets: aarch64-softmmu alpha-softmmu arm-softmmu cris-softmmu hppa-softmmu i386-softmmu lm32-softmmu m68k-softmmu microblaze-softmmu This happens because the $mak_wilds variable uses spaces to separate different paths, maybe newlines may be used, which are less likely to be in directory names. BTW "shellcheck" may help finding some other problems. Qemu version: $ git describe v3.1.0-1960-ga05838cb2a Thanks, Antonio ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1817345 Title: configure script breaks when $source_path contains white spaces Status in QEMU: New Bug description: Hi, I noticed that 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". Steps to reproduce the problem: $ mkdir "dir with spaces" $ cd dir\ with\ spaces/ $ git clone https://git.qemu.org/git/qemu.git $ cd qemu/ $ ./configure --help | grep -A3 target-list Actual result: --target-list=LIST set target list (default: build everything) Available targets: dir with *-softmmu dir with *-linux-user Expected result: --target-list=LIST set target list (default: build everything) Available targets: aarch64-softmmu alpha-softmmu arm-softmmu cris-softmmu hppa-softmmu i386-softmmu lm32-softmmu m68k-softmmu microblaze-softmmu This happens because the $mak_wilds variable uses spaces to separate different paths, maybe newlines may be used, which are less likely to be in directory names. BTW "shellcheck" may help finding some other problems. Qemu version: $ git describe v3.1.0-1960-ga05838cb2a Thanks, Antonio To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1817345/+subscriptions
