[Qemu-devel] [PATCH] configure: preserve various environment variables in config.status

[Qemu-devel] [PATCH] configure: preserve various environment variables in config.status

[Daniel P. Berrange]

Suggested in

  https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03298.html

The config.status script is auto-generated by configure upon
completion. The intention is that config.status can be later
invoked by the developer to re-detect the same environment
that configure originally used. The current config.status
script, however, only contains a record of the command line
arguments to configure. Various environment variables have
an effect on what configure will find. In particular the
PKG_CONFIG_LIBDIR & PKG_CONFIG_PATH vars will affect what
libraries pkg-config finds. The PATH var will affect what
toolchain binaries and XXXX-config scripts are found. The
LD_LIBRARY_PATH var will affect what libraries are found.
All these key env variables should be recorded in the
config.status script.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---

Open question: are there more env vars we should preserve ?

 configure | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/configure b/configure
index d7472d7..9c9f6ac 100755
--- a/configure
+++ b/configure
@@ -5925,6 +5925,24 @@ cat <<EOD >config.status
 # Compiler output produced by configure, useful for debugging
 # configure, is in config.log if it exists.
 EOD
+
+preserve_env() {
+    envname=$1
+
+    if test -n "${!envname}"
+    then
+	echo "$envname=\"${!envname}\"" >> config.status
+	echo "export $envname" >> config.status
+    fi
+}
+
+# Preserve various env variables that influence what
+# features/build target configure will detect
+preserve_env PATH
+preserve_env LD_LIBRARY_PATH
+preserve_env PKG_CONFIG_LIBDIR
+preserve_env PKG_CONFIG_PATH
+
 printf "exec" >>config.status
 printf " '%s'" "$0" "$@" >>config.status
 echo >>config.status
-- 
2.5.0

Re: [Qemu-devel] [PATCH] configure: preserve various environment variables in config.status

[Stefan Weil]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Am 17.11.2015 um 18:59 schrieb Daniel P. Berrange:
> Suggested in
>
>   https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03298.html
>
> The config.status script is auto-generated by configure upon
> completion. The intention is that config.status can be later
> invoked by the developer to re-detect the same environment
> that configure originally used. The current config.status
> script, however, only contains a record of the command line
> arguments to configure. Various environment variables have
> an effect on what configure will find. In particular the
> PKG_CONFIG_LIBDIR & PKG_CONFIG_PATH vars will affect what
> libraries pkg-config finds. The PATH var will affect what
> toolchain binaries and XXXX-config scripts are found. The
> LD_LIBRARY_PATH var will affect what libraries are found.
> All these key env variables should be recorded in the
> config.status script.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>
> Open question: are there more env vars we should preserve ?

Probably yes. They can be added as soon as we discover them.

>
>  configure | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/configure b/configure
> index d7472d7..9c9f6ac 100755
> --- a/configure
> +++ b/configure
> @@ -5925,6 +5925,24 @@ cat <<EOD >config.status
>  # Compiler output produced by configure, useful for debugging
>  # configure, is in config.log if it exists.
>  EOD
> +
> +preserve_env() {
> +    envname=$1
> +
> +    if test -n "${!envname}"
> +    then
> +    echo "$envname=\"${!envname}\"" >> config.status
> +    echo "export $envname" >> config.status

else
    echo "unset $envname" >>config.status

>
> +    fi
> +}
> +
> +# Preserve various env variables that influence what
> +# features/build target configure will detect
> +preserve_env PATH
> +preserve_env LD_LIBRARY_PATH
> +preserve_env PKG_CONFIG_LIBDIR
> +preserve_env PKG_CONFIG_PATH
> +
>  printf "exec" >>config.status
>  printf " '%s'" "$0" "$@" >>config.status
>  echo >>config.status

With the additional code for unset variables this
patch is nearly perfect. Even without it, it is a
reasonable improvement.

Reviewed-by: Stefan Weil <sw@weilnetz.de>

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCAAGBQJWS3P1AAoJEOCMIdVndFCttNMP/jhmKG7da1uAKk5+9YPWRrPr
l+kpWW9CFYMvddTL1jdfjdgu3d+Pb8EMPcGRxdU01W8TNj6+4UuP9t1BxqG/TRNi
kENa6czzJayY0c8axtgS24jGQfFo6b5zboH3TN+XOtK89SpwB+SXXGwglU/3/UNS
cU/yrFpWrCDrsH4XwyTXlJb5870VcfFtO1584yLR1noaytYFz4tI1ihvyMdMc8pJ
rTARuJkYRez+3oZGfF/GDfPZkfZeuxiYTdKgIfjLxUSRf5NO8+SMHY4jD3G/6eow
iJWOjjB2A42mTcRenomuj0XoSt143UQOj67hsFvJt/3FMlBR7z4feUxydzHsIx34
LGDazNUo+eAxx0wWqP2/RtSRdU9rdzZR0pdkMwLJQd8zGlgXB9sIwcDggJE8CWIZ
Mf05qCWGLkHbujFo3pkzJz71isOtBi8kCu+n09iK5DjIvSa4RwqPHyY5qGVrExmM
lQe+GA9NGq23ceNaZDoTbpZhxzNcjvgKyiV19YkR971IQa7opR7BOn40iDD7upqL
Nylw87TUPDAzqsc8i8fZaKo4bxUERqBJMsBzUZMGQMaUjAL5XC3TAWKMiT3boyZ3
+UlsC9yuSwGPGA7g/+mwxtB7aFvUwglt56JzCU1uWeWUEnjX4qrXpXwhB66RuyY+
lgt4QOEeUPwKAxvuv0dK
=SenJ
-----END PGP SIGNATURE-----

Re: [Qemu-devel] [PATCH] configure: preserve various environment variables in config.status

[Eric Blake]

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

On 11/17/2015 10:59 AM, Daniel P. Berrange wrote:
> Suggested in
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03298.html
> 
> The config.status script is auto-generated by configure upon
> completion. The intention is that config.status can be later
> invoked by the developer to re-detect the same environment
> that configure originally used. The current config.status
> script, however, only contains a record of the command line
> arguments to configure. Various environment variables have
> an effect on what configure will find. In particular the
> PKG_CONFIG_LIBDIR & PKG_CONFIG_PATH vars will affect what
> libraries pkg-config finds. The PATH var will affect what
> toolchain binaries and XXXX-config scripts are found. The
> LD_LIBRARY_PATH var will affect what libraries are found.
> All these key env variables should be recorded in the
> config.status script.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> 
> Open question: are there more env vars we should preserve ?

Yes - anything that autoconf would mark as precious.  See below.

> +++ b/configure
> @@ -5925,6 +5925,24 @@ cat <<EOD >config.status
>  # Compiler output produced by configure, useful for debugging
>  # configure, is in config.log if it exists.
>  EOD
> +
> +preserve_env() {
> +    envname=$1
> +
> +    if test -n "${!envname}"

Bashism, but configure is /bin/sh.  This won't work on dash :(

I think you'll have to use eval, and we'll just have to audit that
preserve_env can never be called with suspicious text where eval would
open a security hole.

> +    then
> +	echo "$envname=\"${!envname}\"" >> config.status

Another use of the bashism.

> +	echo "export $envname" >> config.status
> +    fi
> +}
> +
> +# Preserve various env variables that influence what
> +# features/build target configure will detect
> +preserve_env PATH
> +preserve_env LD_LIBRARY_PATH
> +preserve_env PKG_CONFIG_LIBDIR
> +preserve_env PKG_CONFIG_PATH
> +

Autoconf preserves CC, CFLAGS, LDFLAGS, LIBS, CPPFLAGS, and CPP by
default.  Also, PKG_CONFIG is typically preserved.  If you run libvirt's
'./configure --help', you'll also notice a bunch of *_CFLAGS and *_LIBS
in the precious list starting under the label "Some influential
environment variables".

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

Re: [Qemu-devel] [PATCH] configure: preserve various environment variables in config.status

[Daniel P. Berrange]

On Tue, Nov 17, 2015 at 12:06:55PM -0700, Eric Blake wrote:
> On 11/17/2015 10:59 AM, Daniel P. Berrange wrote:
> > Suggested in
> > 
> >   https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03298.html
> > 
> > The config.status script is auto-generated by configure upon
> > completion. The intention is that config.status can be later
> > invoked by the developer to re-detect the same environment
> > that configure originally used. The current config.status
> > script, however, only contains a record of the command line
> > arguments to configure. Various environment variables have
> > an effect on what configure will find. In particular the
> > PKG_CONFIG_LIBDIR & PKG_CONFIG_PATH vars will affect what
> > libraries pkg-config finds. The PATH var will affect what
> > toolchain binaries and XXXX-config scripts are found. The
> > LD_LIBRARY_PATH var will affect what libraries are found.
> > All these key env variables should be recorded in the
> > config.status script.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > 
> > Open question: are there more env vars we should preserve ?
> 
> Yes - anything that autoconf would mark as precious.  See below.
> 
> > +++ b/configure
> > @@ -5925,6 +5925,24 @@ cat <<EOD >config.status
> >  # Compiler output produced by configure, useful for debugging
> >  # configure, is in config.log if it exists.
> >  EOD
> > +
> > +preserve_env() {
> > +    envname=$1
> > +
> > +    if test -n "${!envname}"
> 
> Bashism, but configure is /bin/sh.  This won't work on dash :(
> 
> I think you'll have to use eval, and we'll just have to audit that
> preserve_env can never be called with suspicious text where eval would
> open a security hole.

Ok, shouldn't be a big deal

> > +    then
> > +	echo "$envname=\"${!envname}\"" >> config.status
> 
> Another use of the bashism.
> 
> > +	echo "export $envname" >> config.status
> > +    fi
> > +}
> > +
> > +# Preserve various env variables that influence what
> > +# features/build target configure will detect
> > +preserve_env PATH
> > +preserve_env LD_LIBRARY_PATH
> > +preserve_env PKG_CONFIG_LIBDIR
> > +preserve_env PKG_CONFIG_PATH
> > +
> 
> Autoconf preserves CC, CFLAGS, LDFLAGS, LIBS, CPPFLAGS, and CPP by
> default.  Also, PKG_CONFIG is typically preserved.  If you run libvirt's
> './configure --help', you'll also notice a bunch of *_CFLAGS and *_LIBS
> in the precious list starting under the label "Some influential
> environment variables".

I'll add in env vars for all the commands like CC, CPP, MAKE, etc that
QEMU's configure uses.  I've tried preserving various *FLAGS vars but
the problem here is that configure will modify/augment those variables
while it is running, so when we get to preserve the original flags we
only have the munged version. In any case recommendation is to use
--extra-cflags rather than CFLAGS, so I figure its not a big deal to
skip preserving CFLAGS/LDFLAGS

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] configure: preserve various environment variables in config.status

[Eric Blake]

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

On 11/18/2015 03:03 AM, Daniel P. Berrange wrote:

>>
>> Autoconf preserves CC, CFLAGS, LDFLAGS, LIBS, CPPFLAGS, and CPP by
>> default.  Also, PKG_CONFIG is typically preserved.  If you run libvirt's
>> './configure --help', you'll also notice a bunch of *_CFLAGS and *_LIBS
>> in the precious list starting under the label "Some influential
>> environment variables".
> 
> I'll add in env vars for all the commands like CC, CPP, MAKE, etc that
> QEMU's configure uses.  I've tried preserving various *FLAGS vars but
> the problem here is that configure will modify/augment those variables
> while it is running, so when we get to preserve the original flags we
> only have the munged version. In any case recommendation is to use
> --extra-cflags rather than CFLAGS, so I figure its not a big deal to
> skip preserving CFLAGS/LDFLAGS

Just one more case where we (perhaps needlessly) diverge from autotools
and make distro's life harder.  Oh well, it's not your fault that we
aren't using CFLAGS like other projects, and not preserving CFLAGS does
not change that.

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

Re: [Qemu-devel] [PATCH] configure: preserve various environment variables in config.status

[Stefan Weil]

Am 17.11.2015 um 20:06 schrieb Eric Blake:
> On 11/17/2015 10:59 AM, Daniel P. Berrange wrote:
[...]
>> +++ b/configure
>> @@ -5925,6 +5925,24 @@ cat <<EOD >config.status
>>  # Compiler output produced by configure, useful for debugging
>>  # configure, is in config.log if it exists.
>>  EOD
>> +
>> +preserve_env() {
>> +    envname=$1
>> +
>> +    if test -n "${!envname}"
> Bashism, but configure is /bin/sh.  This won't work on dash :(
>
> I think you'll have to use eval, and we'll just have to audit that
> preserve_env can never be called with suspicious text where eval would
> open a security hole.

You could also call the function with two arguments ...
preserve_env PATH "$PATH"
... and use the 2nd argument in the test.