qemu-trivial.nongnu.org archive mirror
 help / color / mirror / Atom feed
* Re: [Qemu-trivial] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path
       [not found]   ` <541bfc5c-0e45-58e6-f0b1-81e9b0c8881d@redhat.com>
@ 2019-05-09 14:42     ` Peter Maydell
  2019-05-22 13:57       ` Laurent Vivier
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2019-05-09 14:42 UTC (permalink / raw)
  To: Eric Blake; +Cc: Antonio Ospite, QEMU Developers, Antonio Ospite, QEMU Trivial

On Mon, 6 May 2019 at 18:27, Eric Blake <eblake@redhat.com> wrote:
>
> On 5/3/19 3:27 AM, 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" 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 <antonio.ospite@collabora.com>
> > ---
> >  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 <eblake@redhat.com>

What tree is this going to go in via? I suggest the
-trivial tree.

thanks
-- PMM


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

* Re: [Qemu-trivial] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path
  2019-05-09 14:42     ` [Qemu-trivial] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path Peter Maydell
@ 2019-05-22 13:57       ` Laurent Vivier
  2019-05-22 15:01         ` Antonio Ospite
  0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2019-05-22 13:57 UTC (permalink / raw)
  To: Peter Maydell, Eric Blake
  Cc: QEMU Trivial, Antonio Ospite, Antonio Ospite, QEMU Developers

On 09/05/2019 16:42, Peter Maydell wrote:
> On Mon, 6 May 2019 at 18:27, Eric Blake <eblake@redhat.com> wrote:
>>
>> On 5/3/19 3:27 AM, 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" 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 <antonio.ospite@collabora.com>
>>> ---
>>>   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 <eblake@redhat.com>
> 
> What tree is this going to go in via? I suggest the
> -trivial tree.


Applied (unchanged) to my trivial-patches branch.

Thanks,
Laurent



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

* Re: [Qemu-trivial] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path
  2019-05-22 13:57       ` Laurent Vivier
@ 2019-05-22 15:01         ` Antonio Ospite
  2019-05-22 15:21           ` Laurent Vivier
  0 siblings, 1 reply; 6+ messages in thread
From: Antonio Ospite @ 2019-05-22 15:01 UTC (permalink / raw)
  To: Laurent Vivier, Peter Maydell, Eric Blake
  Cc: QEMU Trivial, Antonio Ospite, QEMU Developers

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 <eblake@redhat.com> wrote:
>>>
>>> On 5/3/19 3:27 AM, 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" 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 <antonio.ospite@collabora.com>
>>>> ---
>>>>   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 <eblake@redhat.com>
>>
>> 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


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

* Re: [Qemu-trivial] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path
  2019-05-22 15:01         ` Antonio Ospite
@ 2019-05-22 15:21           ` Laurent Vivier
  2019-05-22 15:26             ` Antonio Ospite
  0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2019-05-22 15:21 UTC (permalink / raw)
  To: Antonio Ospite, Peter Maydell, Eric Blake
  Cc: QEMU Trivial, Antonio Ospite, QEMU Developers

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 <eblake@redhat.com> wrote:
>>>>
>>>> On 5/3/19 3:27 AM, 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" 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 <antonio.ospite@collabora.com>
>>>>> ---
>>>>>   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 <eblake@redhat.com>
>>>
>>> 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.

Thanks,
Laurent


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

* Re: [Qemu-trivial] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path
  2019-05-22 15:21           ` Laurent Vivier
@ 2019-05-22 15:26             ` Antonio Ospite
  2019-05-22 15:37               ` Laurent Vivier
  0 siblings, 1 reply; 6+ messages in thread
From: Antonio Ospite @ 2019-05-22 15:26 UTC (permalink / raw)
  To: Laurent Vivier, Peter Maydell, Eric Blake
  Cc: QEMU Trivial, Antonio Ospite, QEMU Developers

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 <eblake@redhat.com> wrote:
>>>>>
>>>>> On 5/3/19 3:27 AM, 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" 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 <antonio.ospite@collabora.com>
>>>>>> ---
>>>>>>   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 <eblake@redhat.com>
>>>>
>>>> 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


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

* Re: [Qemu-trivial] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path
  2019-05-22 15:26             ` Antonio Ospite
@ 2019-05-22 15:37               ` Laurent Vivier
  0 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2019-05-22 15:37 UTC (permalink / raw)
  To: Antonio Ospite, Peter Maydell, Eric Blake
  Cc: QEMU Trivial, Antonio Ospite, QEMU Developers

On 22/05/2019 17:26, Antonio Ospite wrote:
> 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 <eblake@redhat.com> wrote:
>>>>>>
>>>>>> On 5/3/19 3:27 AM, 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" 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 <antonio.ospite@collabora.com>
>>>>>>> ---
>>>>>>>   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 <eblake@redhat.com>
>>>>>
>>>>> 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.

OK, I will queue your v3 in the pull-request of the next week.

Thanks,
Laurent


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

end of thread, other threads:[~2019-05-22 15:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20190503082728.16485-1-ao2@ao2.it>
     [not found] ` <20190503082728.16485-3-ao2@ao2.it>
     [not found]   ` <541bfc5c-0e45-58e6-f0b1-81e9b0c8881d@redhat.com>
2019-05-09 14:42     ` [Qemu-trivial] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path Peter Maydell
2019-05-22 13:57       ` Laurent Vivier
2019-05-22 15:01         ` Antonio Ospite
2019-05-22 15:21           ` Laurent Vivier
2019-05-22 15:26             ` Antonio Ospite
2019-05-22 15:37               ` Laurent Vivier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).