Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version

[Cleber Rosa]

On Thu, Aug 22, 2019 at 05:48:46PM +0100, Peter Maydell wrote:
> On Fri, 9 Nov 2018 at 15:09, Cleber Rosa <address@hidden> wrote:
> >
> > Some functionality is dependent on the Python version
> > detected/configured on configure.  While it's possible to run the
> > Python version later and check for the version, doing it once is
> > preferable.  Also, it's a relevant information to keep in build logs,
> > as the overall behavior of the build can be affected by it.
> >
> > Signed-off-by: Cleber Rosa <address@hidden>
> > ---
> >  configure | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/configure b/configure
> > index 74e313a810..67fff0290d 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1740,6 +1740,9 @@ if ! $python -c 'import sys; 
> > sys.exit(sys.version_info < (2,7))'; then
> >        "Use --python=/path/to/python to specify a supported Python."
> >  fi
> >
> > +# Preserve python version since some functionality is dependent on it
> > +python_version=$($python -V 2>&1 | sed -e 's/Python\ //')
> > +
> 
> Hi. Somebody on IRC has just fallen over a problem where
> their python's "-V" output prints multiple lines, which
> means that "$python_version" here is multiple lines, which
> means that the eventual config-host.mak has invalid syntax
> because we assume here:
>

We've tried a number of things, and just when I thought we wouldn't be
able to make any sense out of it, I arrived at a still senseless but
precise reproducer.  TL;DR: it has to do with interactive shells and
that exact Python build.

Reproducer (docker may also do the trick here):

  $ podman run --rm -ti fedora:29 /bin/bash -c 'dnf -y install 
http://mirror.siena.edu/fedora/linux/releases/29/Everything/x86_64/os/Packages/p/python3-3.7.0-9.fc29.x86_64.rpm;
 python3 -V'
  Python 3.7.0 (default, Aug 30 2018, 14:32:33) 
  [GCC 8.2.1 20180801 (Red Hat 8.2.1-2)]

With an interactive shell instead:

  $ podman run --rm -ti fedora:29 /bin/bash -i -c 'dnf -y install 
http://mirror.siena.edu/fedora/linux/releases/29/Everything/x86_64/os/Packages/p/python3-3.7.0-9.fc29.x86_64.rpm;
 python3 -V'
  Python 3.7.0

How this behavior came to be, baffles me.  But, it seems to be fixed
on newer versions.

> > @@ -6823,6 +6826,7 @@ echo "INSTALL_DATA=$install -c -m 0644" >> 
> > $config_host_mak
> >  echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
> >  echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
> >  echo "PYTHON=$python" >> $config_host_mak
> > +echo "PYTHON_VERSION=$python_version" >> $config_host_mak
> >  echo "CC=$cc" >> $config_host_mak
> >  if $iasl -h > /dev/null 2>&1; then
> >    echo "IASL=$iasl" >> $config_host_mak
> 
> that it's only one line, and will generate bogus makefile
> syntax if it's got an embedded newline. (Problem system
> seems to be Fedora 29.)
>

The assumption could be guaranteed by a "head -1", and while
it's not a failproof solution, it would at least not corrupt
the makefile and the whole build system.

> I've reread this thread, where there seems to have been
> some discussion about just running Python itself to
> get the sys.version value (which is how we check for
> "is this python too old" earlier in the configure script).
> But I'm not really clear why trying to parse -V output is better:
> it's definitely less reliable, as demonstrated by this bug.
>
> Given that the only thing as far as I can tell that we
> do with PYTHON_VERSION is use it in tests/Makefile.inc
> to suppress a bit of test functionality if we don't have
> Python 3, could we stop trying to parse -V output and run
> python to print sys.version_info instead, and/or just
> have the makefile variable track "is this python 2",
> since that's what we really care about and would mean we
> don't have to then search the string for "v2"  ?

Because I've been bitten way too many times with differences in Python
minor versions, I see a lot of value in keeping the version
information in the build system.  But, the same information can
certainly be obtained in a more resilient way.  Would you object something
like:

  python_version=$($python -c 'import sys; print(sys.version().split()[0])')

Or an even more paranoid version?  On my side, I understand the
fragility of the current approach, but I also appreciate the
information it stores.

> 
> thanks
> -- PMM

Thanks!
- Cleber.