[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version
From: |
Cleber Rosa |
Subject: |
Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version |
Date: |
Thu, 22 Aug 2019 17:19:26 -0400 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
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.