From: Eduardo Habkost <ehabkost@redhat.com>
To: Cleber Rosa <crosa@redhat.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>,
"QEMU Developers" <qemu-devel@nongnu.org>,
"Wainer dos Santos Moschetta" <wainersm@redhat.com>,
"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
"Alex Bennée" <alex.bennee@linaro.org>
Subject: Re: [Qemu-devel] [PATCH 1/4] configure: keep track of Python version
Date: Thu, 22 Aug 2019 18:54:20 -0300 [thread overview]
Message-ID: <20190822215420.GO7077@habkost.net> (raw)
In-Reply-To: <20190822211636.GA23027@dhcp-17-173.bos.redhat.com>
On Thu, Aug 22, 2019 at 05:19:26PM -0400, Cleber Rosa wrote:
> On Thu, Aug 22, 2019 at 05:48:46PM +0100, Peter Maydell wrote:
> > On Fri, 9 Nov 2018 at 15:09, Cleber Rosa <crosa@redhat.com> 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 <crosa@redhat.com>
> > > ---
> > > 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.
Agreed.
> >
> > 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])')
Sounds much better, but why sys.version().split() instead of
sys.version_info?
python_version=$($python -c 'import sys; print(sys.version_info[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.
We have only one place where $(PYTHON_VERSION) is used, and that
code will be removed once we stop supporting Python 2. I don't
see the point of trying to store extra information that is not
used anywhere in our makefiles.
--
Eduardo
next prev parent reply other threads:[~2019-08-22 21:55 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-09 15:07 [Qemu-devel] [PATCH 0/4] Record Python version and misc test/CI fixes Cleber Rosa
2018-11-09 15:07 ` [Qemu-devel] [PATCH 1/4] configure: keep track of Python version Cleber Rosa
2018-11-09 15:49 ` Eduardo Habkost
2018-11-09 16:39 ` Cleber Rosa
2018-11-09 18:25 ` Eduardo Habkost
2018-11-09 19:09 ` Cleber Rosa
2018-11-09 19:51 ` Cleber Rosa
2018-11-09 21:25 ` Eduardo Habkost
2018-11-09 21:26 ` Eduardo Habkost
2019-08-22 16:48 ` Peter Maydell
2019-08-22 21:19 ` Cleber Rosa
2019-08-22 21:54 ` Eduardo Habkost [this message]
2019-08-23 13:40 ` Cleber Rosa
2019-08-23 13:44 ` Peter Maydell
2019-08-23 17:42 ` Cleber Rosa
2019-08-23 15:04 ` Eduardo Habkost
2019-08-23 17:44 ` Cleber Rosa
2018-11-09 15:07 ` [Qemu-devel] [PATCH 2/4] check-venv: use recorded " Cleber Rosa
2018-11-09 21:27 ` Eduardo Habkost
2018-11-09 15:07 ` [Qemu-devel] [PATCH 3/4] Travis CI: make specified Python versions usable on jobs Cleber Rosa
2018-11-09 15:52 ` Alex Bennée
2018-11-12 16:25 ` Eduardo Habkost
2018-11-09 19:34 ` Philippe Mathieu-Daudé
2018-11-09 19:39 ` Cleber Rosa
2018-11-12 16:23 ` Eduardo Habkost
2018-11-12 17:38 ` Cleber Rosa
2018-11-09 15:07 ` [Qemu-devel] [PATCH 4/4] check-help: visual and content improvements Cleber Rosa
2018-11-09 16:43 ` Eric Blake
2018-11-09 19:32 ` Philippe Mathieu-Daudé
2018-11-09 21:29 ` Eduardo Habkost
2018-11-12 17:51 ` Wainer dos Santos Moschetta
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=20190822215420.GO7077@habkost.net \
--to=ehabkost@redhat.com \
--cc=alex.bennee@linaro.org \
--cc=crosa@redhat.com \
--cc=f4bug@amsat.org \
--cc=peter.maydell@linaro.org \
--cc=philmd@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=wainersm@redhat.com \
/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).