[Qemu-devel] [PATCH] configure: do not quote $PKG_CONFIG

[Qemu-devel] [PATCH] configure: do not quote $PKG_CONFIG

[Mike Frysinger]

We should not quote the PKG_CONFIG setting as this deviates from the
canonical upstream behavior that gets integrated with all other build
systems, and deviates from how we treat all other toolchain variables
that we get from the environment.

Ultimately, the point is that it breaks passing custom flags directly
to pkg-config via the env var where this normally works elsewhere,
and it used to work in the past.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 configure |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 83fa1ca..bd12ed3 100755
--- a/configure
+++ b/configure
@@ -245,7 +245,7 @@ strip="${STRIP-${cross_prefix}strip}"
 windres="${WINDRES-${cross_prefix}windres}"
 pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
 query_pkg_config() {
-    "${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
+    ${pkg_config_exe} ${QEMU_PKG_CONFIG_FLAGS} "$@"
 }
 pkg_config=query_pkg_config
 sdl_config="${SDL_CONFIG-${cross_prefix}sdl-config}"
@@ -1511,7 +1511,7 @@ fi
 ##########################################
 # pkg-config probe
 
-if ! has "$pkg_config_exe"; then
+if ! has $pkg_config_exe; then
   echo "Error: pkg-config binary '$pkg_config_exe' not found"
   exit 1
 fi
-- 
1.7.9.7

Re: [Qemu-devel] [PATCH] configure: do not quote $PKG_CONFIG

[Stefan Weil]

Am 15.07.2012 22:26, schrieb Mike Frysinger:
> We should not quote the PKG_CONFIG setting as this deviates from the
> canonical upstream behavior that gets integrated with all other build
> systems, and deviates from how we treat all other toolchain variables
> that we get from the environment.
>
> Ultimately, the point is that it breaks passing custom flags directly
> to pkg-config via the env var where this normally works elsewhere,
> and it used to work in the past.


What about passing custom flags with QEMU_PKG_CONFIG_FLAGS?

Removing the quotes will not allow paths containing spaces,
so that's not a good idea.

Regards,

Stefan Weil



>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>   configure |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index 83fa1ca..bd12ed3 100755
> --- a/configure
> +++ b/configure
> @@ -245,7 +245,7 @@ strip="${STRIP-${cross_prefix}strip}"
>   windres="${WINDRES-${cross_prefix}windres}"
>   pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
>   query_pkg_config() {
> -    "${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
> +    ${pkg_config_exe} ${QEMU_PKG_CONFIG_FLAGS} "$@"
>   }
>   pkg_config=query_pkg_config
>   sdl_config="${SDL_CONFIG-${cross_prefix}sdl-config}"
> @@ -1511,7 +1511,7 @@ fi
>   ##########################################
>   # pkg-config probe
>   
> -if ! has "$pkg_config_exe"; then
> +if ! has $pkg_config_exe; then
>     echo "Error: pkg-config binary '$pkg_config_exe' not found"
>     exit 1
>   fi

Re: [Qemu-devel] [PATCH] configure: do not quote $PKG_CONFIG

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 954 bytes --]

On Sunday 15 July 2012 15:54:51 Stefan Weil wrote:
> Am 15.07.2012 22:26, schrieb Mike Frysinger:
> > We should not quote the PKG_CONFIG setting as this deviates from the
> > canonical upstream behavior that gets integrated with all other build
> > systems, and deviates from how we treat all other toolchain variables
> > that we get from the environment.
> > 
> > Ultimately, the point is that it breaks passing custom flags directly
> > to pkg-config via the env var where this normally works elsewhere,
> > and it used to work in the past.
> 
> What about passing custom flags with QEMU_PKG_CONFIG_FLAGS?
> 
> Removing the quotes will not allow paths containing spaces,
> so that's not a good idea.

except that doesn't work with other build variables (like CC/etc...), nor does 
it work with the standard pkg-config build environments.  making qemu deviate 
from the standard to support non-existent setups makes no sense.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH] configure: do not quote $PKG_CONFIG

[Eric Blake]

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

On 07/15/2012 01:54 PM, Stefan Weil wrote:
> Am 15.07.2012 22:26, schrieb Mike Frysinger:
>> We should not quote the PKG_CONFIG setting as this deviates from the
>> canonical upstream behavior that gets integrated with all other build
>> systems, and deviates from how we treat all other toolchain variables
>> that we get from the environment.
>>
>> Ultimately, the point is that it breaks passing custom flags directly
>> to pkg-config via the env var where this normally works elsewhere,
>> and it used to work in the past.
> 
> 
> What about passing custom flags with QEMU_PKG_CONFIG_FLAGS?
> 
> Removing the quotes will not allow paths containing spaces,
> so that's not a good idea.

Actually, it IS a good idea.  The de facto standard build environment
requires that pkg-config is not allowed to live in a path containing
spaces, precisely so that you can override the variable to pass options
to your preferred location of pkg-config; and if your build setup is
truly so messed up as to have pkg-config installed in a canonical
location with spaces, then you can also tweak your unusual environment
to provide a symlink to pkg-config that does not contain spaces as the
workaround.

-- 
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: 620 bytes --]

Re: [Qemu-devel] [PATCH] configure: do not quote $PKG_CONFIG

[Stefan Weil]

Am 16.07.2012 17:39, schrieb Eric Blake:
> On 07/15/2012 01:54 PM, Stefan Weil wrote:
>> Am 15.07.2012 22:26, schrieb Mike Frysinger:
>>> We should not quote the PKG_CONFIG setting as this deviates from the
>>> canonical upstream behavior that gets integrated with all other build
>>> systems, and deviates from how we treat all other toolchain variables
>>> that we get from the environment.
>>>
>>> Ultimately, the point is that it breaks passing custom flags directly
>>> to pkg-config via the env var where this normally works elsewhere,
>>> and it used to work in the past.
>>
>>
>> What about passing custom flags with QEMU_PKG_CONFIG_FLAGS?
>>
>> Removing the quotes will not allow paths containing spaces,
>> so that's not a good idea.
>
> Actually, it IS a good idea.  The de facto standard build environment
> requires that pkg-config is not allowed to live in a path containing
> spaces, precisely so that you can override the variable to pass options
> to your preferred location of pkg-config; and if your build setup is
> truly so messed up as to have pkg-config installed in a canonical
> location with spaces, then you can also tweak your unusual environment
> to provide a symlink to pkg-config that does not contain spaces as the
> workaround.

That sounds reasonable. Then the following patch was at least partially
unnecessary:

commit 17884d7b6462b0fe497f08fec6091ffbe04caa8d
Author: Sergei Trofimovich <slyfox@gentoo.org>
Date:   Tue Jan 31 22:03:45 2012 +0300

     ./configure: request pkg-config to provide private libs when static 
linking

     Added wrapper around pkg-config to allow:
     - safe options injection via ${QEMU_PKG_CONFIG_FLAGS}
     - spaces in path to pkg-config

     Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
     CC: Peter Maydell <peter.maydell@linaro.org>
     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

With Mike's new patch, QEMU_PKG_CONFIG_FLAGS is no longer needed
because options can be passed using the pkg-config macro.
I suggest to remove it.

Regards,

Stefan W.

Re: [Qemu-devel] [PATCH] configure: do not quote $PKG_CONFIG

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 2440 bytes --]

On Monday 16 July 2012 11:58:55 Stefan Weil wrote:
> Am 16.07.2012 17:39, schrieb Eric Blake:
> > On 07/15/2012 01:54 PM, Stefan Weil wrote:
> >> Am 15.07.2012 22:26, schrieb Mike Frysinger:
> >>> We should not quote the PKG_CONFIG setting as this deviates from the
> >>> canonical upstream behavior that gets integrated with all other build
> >>> systems, and deviates from how we treat all other toolchain variables
> >>> that we get from the environment.
> >>> 
> >>> Ultimately, the point is that it breaks passing custom flags directly
> >>> to pkg-config via the env var where this normally works elsewhere,
> >>> and it used to work in the past.
> >> 
> >> What about passing custom flags with QEMU_PKG_CONFIG_FLAGS?
> >> 
> >> Removing the quotes will not allow paths containing spaces,
> >> so that's not a good idea.
> > 
> > Actually, it IS a good idea.  The de facto standard build environment
> > requires that pkg-config is not allowed to live in a path containing
> > spaces, precisely so that you can override the variable to pass options
> > to your preferred location of pkg-config; and if your build setup is
> > truly so messed up as to have pkg-config installed in a canonical
> > location with spaces, then you can also tweak your unusual environment
> > to provide a symlink to pkg-config that does not contain spaces as the
> > workaround.
> 
> That sounds reasonable. Then the following patch was at least partially
> unnecessary:
> 
> commit 17884d7b6462b0fe497f08fec6091ffbe04caa8d
> Author: Sergei Trofimovich <slyfox@gentoo.org>
> Date:   Tue Jan 31 22:03:45 2012 +0300
> 
>      ./configure: request pkg-config to provide private libs when static
> linking
> 
>      Added wrapper around pkg-config to allow:
>      - safe options injection via ${QEMU_PKG_CONFIG_FLAGS}
>      - spaces in path to pkg-config
> 
>      Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
>      CC: Peter Maydell <peter.maydell@linaro.org>
>      Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> With Mike's new patch, QEMU_PKG_CONFIG_FLAGS is no longer needed
> because options can be passed using the pkg-config macro.
> I suggest to remove it.

i'm ambivalent on the additional functionality that qemu provides in its build 
system -- i'm just concerned with the baseline being the same as all other 
build systems.  some people probably find this handy.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH] configure: do not quote $PKG_CONFIG

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 573 bytes --]

On Sunday 15 July 2012 15:54:51 Stefan Weil wrote:
> Am 15.07.2012 22:26, schrieb Mike Frysinger:
> > We should not quote the PKG_CONFIG setting as this deviates from the
> > canonical upstream behavior that gets integrated with all other build
> > systems, and deviates from how we treat all other toolchain variables
> > that we get from the environment.
> > 
> > Ultimately, the point is that it breaks passing custom flags directly
> > to pkg-config via the env var where this normally works elsewhere,
> > and it used to work in the past.

ping ...
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH] configure: do not quote $PKG_CONFIG

[Stefan Weil]

Am 16.09.2012 21:52, schrieb Mike Frysinger:
> On Sunday 15 July 2012 15:54:51 Stefan Weil wrote:
>> Am 15.07.2012 22:26, schrieb Mike Frysinger:
>>> We should not quote the PKG_CONFIG setting as this deviates from the
>>> canonical upstream behavior that gets integrated with all other build
>>> systems, and deviates from how we treat all other toolchain variables
>>> that we get from the environment.
>>>
>>> Ultimately, the point is that it breaks passing custom flags directly
>>> to pkg-config via the env var where this normally works elsewhere,
>>> and it used to work in the past.
> ping ...
> -mike

The complete discussion is here: http://patchwork.ozlabs.org/patch/171087/.

I suggested using Mike's patch and removing commit
17884d7b6462b0fe497f08fec6091ffbe04caa8d.
(./configure: request pkg-config to provide private
libs when static linking).

Anthony, Sergei, any comments?

Regards
Stefan

Re: [Qemu-devel] [PATCH] configure: do not quote $PKG_CONFIG

[Sergei Trofimovich]

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

On Sun, 16 Sep 2012 22:21:31 +0200
Stefan Weil <sw@weilnetz.de> wrote:

> Am 16.09.2012 21:52, schrieb Mike Frysinger:
> > On Sunday 15 July 2012 15:54:51 Stefan Weil wrote:
> >> Am 15.07.2012 22:26, schrieb Mike Frysinger:
> >>> We should not quote the PKG_CONFIG setting as this deviates from the
> >>> canonical upstream behavior that gets integrated with all other build
> >>> systems, and deviates from how we treat all other toolchain variables
> >>> that we get from the environment.
> >>>
> >>> Ultimately, the point is that it breaks passing custom flags directly
> >>> to pkg-config via the env var where this normally works elsewhere,
> >>> and it used to work in the past.
> > ping ...
> > -mike
> 
> The complete discussion is here: http://patchwork.ozlabs.org/patch/171087/.
> 
> I suggested using Mike's patch and removing commit
> 17884d7b6462b0fe497f08fec6091ffbe04caa8d.
> (./configure: request pkg-config to provide private
> libs when static linking).
> 
> Anthony, Sergei, any comments?

I'm fine with Mke's patch.

-- 

  Sergei

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [Qemu-devel] [PATCH] configure: do not quote $PKG_CONFIG

[Anthony Liguori]

Mike Frysinger <vapier@gentoo.org> writes:

> We should not quote the PKG_CONFIG setting as this deviates from the
> canonical upstream behavior that gets integrated with all other build
> systems, and deviates from how we treat all other toolchain variables
> that we get from the environment.
>
> Ultimately, the point is that it breaks passing custom flags directly
> to pkg-config via the env var where this normally works elsewhere,
> and it used to work in the past.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

This doesn't make any sense to me.   What is the command line that
you're trying to execute?

This obviously would introduce a bug if there was a space in the
filename of the pkg-config binary.

Regards,

Anthony Liguori

> ---
>  configure |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index 83fa1ca..bd12ed3 100755
> --- a/configure
> +++ b/configure
> @@ -245,7 +245,7 @@ strip="${STRIP-${cross_prefix}strip}"
>  windres="${WINDRES-${cross_prefix}windres}"
>  pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
>  query_pkg_config() {
> -    "${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
> +    ${pkg_config_exe} ${QEMU_PKG_CONFIG_FLAGS} "$@"
>  }
>  pkg_config=query_pkg_config
>  sdl_config="${SDL_CONFIG-${cross_prefix}sdl-config}"
> @@ -1511,7 +1511,7 @@ fi
>  ##########################################
>  # pkg-config probe
>  
> -if ! has "$pkg_config_exe"; then
> +if ! has $pkg_config_exe; then
>    echo "Error: pkg-config binary '$pkg_config_exe' not found"
>    exit 1
>  fi
> -- 
> 1.7.9.7

Re: [Qemu-devel] [PATCH] configure: do not quote $PKG_CONFIG

[Eric Blake]

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

On 09/17/2012 01:38 PM, Anthony Liguori wrote:
> Mike Frysinger <vapier@gentoo.org> writes:
> 
>> We should not quote the PKG_CONFIG setting as this deviates from the
>> canonical upstream behavior that gets integrated with all other build
>> systems, and deviates from how we treat all other toolchain variables
>> that we get from the environment.
>>
>> Ultimately, the point is that it breaks passing custom flags directly
>> to pkg-config via the env var where this normally works elsewhere,
>> and it used to work in the past.
>>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> 
> This doesn't make any sense to me.   What is the command line that
> you're trying to execute?
> 
> This obviously would introduce a bug if there was a space in the
> filename of the pkg-config binary.

Except that the de facto standard is that the pkg-config binary already
is prohibited from having a space in the filename.

https://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02015.html

-- 
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: 617 bytes --]

[Qemu-devel] [PATCH v2] configure: do not quote $PKG_CONFIG

[Mike Frysinger]

We should not quote the PKG_CONFIG setting as this deviates from the
canonical upstream behavior that gets integrated with all other build
systems, and deviates from how we treat all other toolchain variables
that we get from the environment.

Ultimately, the point is that it breaks passing custom flags directly
to pkg-config via the env var where this normally works elsewhere,
and it used to work in the past.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- rebased onto latest master
	- people were happy with this in the previous thread:
		http://permalink.gmane.org/gmane.comp.emulators.qemu/160198

 configure | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 95dc66b..9c14ab6 100755
--- a/configure
+++ b/configure
@@ -291,7 +291,7 @@ strip="${STRIP-${cross_prefix}strip}"
 windres="${WINDRES-${cross_prefix}windres}"
 pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
 query_pkg_config() {
-    "${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
+    ${pkg_config_exe} ${QEMU_PKG_CONFIG_FLAGS} "$@"
 }
 pkg_config=query_pkg_config
 sdl_config="${SDL_CONFIG-${cross_prefix}sdl-config}"
@@ -1424,7 +1424,7 @@ fi
 ##########################################
 # pkg-config probe
 
-if ! has "$pkg_config_exe"; then
+if ! has $pkg_config_exe; then
   error_exit "pkg-config binary '$pkg_config_exe' not found"
 fi
 
-- 
1.8.2.1