| Message ID | 1342383972-11640-1-git-send-email-vapier@gentoo.org |
|---|---|
| State | New |
| Headers | show |
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
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
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.
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.
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
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
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
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.
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
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
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
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(-)