qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	qemu-devel@nongnu.org,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] configure: preserve various environment variables in config.status
Date: Wed, 18 Nov 2015 10:03:32 +0000	[thread overview]
Message-ID: <20151118100332.GA27591@redhat.com> (raw)
In-Reply-To: <564B7ACF.30701@redhat.com>

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 :|

  reply	other threads:[~2015-11-18 10:03 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-17 17:59 [Qemu-devel] [PATCH] configure: preserve various environment variables in config.status Daniel P. Berrange
2015-11-17 18:37 ` Stefan Weil
2015-11-17 19:06 ` Eric Blake
2015-11-18 10:03   ` Daniel P. Berrange [this message]
2015-11-18 15:40     ` Eric Blake
2015-11-18 15:52   ` Stefan Weil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151118100332.GA27591@redhat.com \
    --to=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).