| Message ID | 20230210003147.1309376-4-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Python: Drop support for Python 3.6 | expand |
On 10/02/2023 01.31, John Snow wrote: > At the moment, we look for just "python3" and "python", which is good > enough almost all of the time. But ... if you are on a platform that > uses an older Python by default and only offers a newer Python as an > option, you'll have to specify --python=/usr/bin/foo every time. > > We can be kind and instead make a cursory attempt to locate a suitable > Python binary ourselves, looking for the remaining well-known binaries. > > This configure loop will prefer, in order: > > 1. Whatever is specified in $PYTHON > 2. python3 > 3. python > 4. python3.11 down through python3.6 > > Notes: > > - Python virtual environment provides binaries for "python3", "python", > and whichever version you used to create the venv, > e.g. "python3.8". If configure is invoked from inside of a venv, this > configure loop will not "break out" of that venv unless that venv is > created using an explicitly non-suitable version of Python that we > cannot use. > > - In the event that no suitable python is found, the first python found > is the version used to generate the human-readable error message. > > - The error message isn't printed right away to allow later > configuration code to pick up an explicitly configured python. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > configure | 33 +++++++++++++++++++++++++-------- > 1 file changed, 25 insertions(+), 8 deletions(-) > > diff --git a/configure b/configure > index 64960c6000f..ea8c973d13b 100755 > --- a/configure > +++ b/configure > @@ -592,20 +592,39 @@ esac > > : ${make=${MAKE-make}} > > -# We prefer python 3.x. A bare 'python' is traditionally > -# python 2.x, but some distros have it as python 3.x, so > -# we check that too > + > +check_py_version() { > + # We require python >= 3.6. > + # NB: a True python conditional creates a non-zero return code (Failure) > + "$1" -c 'import sys; sys.exit(sys.version_info < (3,6))' > +} > + > python= > +first_python= > explicit_python=no > -for binary in "${PYTHON-python3}" python > +# A bare 'python' is traditionally python 2.x, but some distros > +# have it as python 3.x, so check in both places. > +for binary in "${PYTHON-python3}" python python3.{11..6} > do > if has "$binary" > then > python=$(command -v "$binary") > - break > + if test -z "$first_python"; then > + first_python=$python > + fi > + if check_py_version "$python"; then > + # This one is good. > + first_python= > + break > + fi > fi > done > > +# If first_python is set, we didn't find a suitable binary. > +# Use this one for possible future error messages. > +if test -n "$first_python"; then > + python="$first_python" > +fi > # Check for ancillary tools used in testing > genisoimage= > @@ -1037,9 +1056,7 @@ then > error_exit "GNU make ($make) not found" > fi > > -# Note that if the Python conditional here evaluates True we will exit > -# with status 1 which is a shell 'false' value. > -if ! $python -c 'import sys; sys.exit(sys.version_info < (3,6))'; then > +if ! check_py_version "$python"; then > error_exit "Cannot use '$python', Python >= 3.6 is required." \ > "Use --python=/path/to/python to specify a supported Python." > fi I like the idea! Reviewed-by: Thomas Huth <thuth@redhat.com>
On 2/10/23 01:31, John Snow wrote: > At the moment, we look for just "python3" and "python", which is good > enough almost all of the time. But ... if you are on a platform that > uses an older Python by default and only offers a newer Python as an > option, you'll have to specify --python=/usr/bin/foo every time. > > We can be kind and instead make a cursory attempt to locate a suitable > Python binary ourselves, looking for the remaining well-known binaries. > > This configure loop will prefer, in order: > > 1. Whatever is specified in $PYTHON > 2. python3 > 3. python > 4. python3.11 down through python3.6 > > Notes: > > - Python virtual environment provides binaries for "python3", "python", > and whichever version you used to create the venv, > e.g. "python3.8". If configure is invoked from inside of a venv, this > configure loop will not "break out" of that venv unless that venv is > created using an explicitly non-suitable version of Python that we > cannot use. > > - In the event that no suitable python is found, the first python found > is the version used to generate the human-readable error message. > > - The error message isn't printed right away to allow later > configuration code to pick up an explicitly configured python. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > configure | 33 +++++++++++++++++++++++++-------- > 1 file changed, 25 insertions(+), 8 deletions(-) > > diff --git a/configure b/configure > index 64960c6000f..ea8c973d13b 100755 > --- a/configure > +++ b/configure > @@ -592,20 +592,39 @@ esac > > : ${make=${MAKE-make}} > > -# We prefer python 3.x. A bare 'python' is traditionally > -# python 2.x, but some distros have it as python 3.x, so > -# we check that too > + > +check_py_version() { > + # We require python >= 3.6. > + # NB: a True python conditional creates a non-zero return code (Failure) > + "$1" -c 'import sys; sys.exit(sys.version_info < (3,6))' > +} > + > python= > +first_python= > explicit_python=no > -for binary in "${PYTHON-python3}" python > +# A bare 'python' is traditionally python 2.x, but some distros > +# have it as python 3.x, so check in both places. > +for binary in "${PYTHON-python3}" python python3.{11..6} This is not available in e.g. dash, so we need to use {11,10,9,8,7,6}. Just a nit, I can fix it myself. Paolo
On Fri, Feb 10, 2023, 5:46 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > On 2/10/23 01:31, John Snow wrote: > > At the moment, we look for just "python3" and "python", which is good > > enough almost all of the time. But ... if you are on a platform that > > uses an older Python by default and only offers a newer Python as an > > option, you'll have to specify --python=/usr/bin/foo every time. > > > > We can be kind and instead make a cursory attempt to locate a suitable > > Python binary ourselves, looking for the remaining well-known binaries. > > > > This configure loop will prefer, in order: > > > > 1. Whatever is specified in $PYTHON > > 2. python3 > > 3. python > > 4. python3.11 down through python3.6 > > > > Notes: > > > > - Python virtual environment provides binaries for "python3", "python", > > and whichever version you used to create the venv, > > e.g. "python3.8". If configure is invoked from inside of a venv, this > > configure loop will not "break out" of that venv unless that venv is > > created using an explicitly non-suitable version of Python that we > > cannot use. > > > > - In the event that no suitable python is found, the first python found > > is the version used to generate the human-readable error message. > > > > - The error message isn't printed right away to allow later > > configuration code to pick up an explicitly configured python. > > > > Signed-off-by: John Snow <jsnow@redhat.com> > > --- > > configure | 33 +++++++++++++++++++++++++-------- > > 1 file changed, 25 insertions(+), 8 deletions(-) > > > > diff --git a/configure b/configure > > index 64960c6000f..ea8c973d13b 100755 > > --- a/configure > > +++ b/configure > > @@ -592,20 +592,39 @@ esac > > > > : ${make=${MAKE-make}} > > > > -# We prefer python 3.x. A bare 'python' is traditionally > > -# python 2.x, but some distros have it as python 3.x, so > > -# we check that too > > + > > +check_py_version() { > > + # We require python >= 3.6. > > + # NB: a True python conditional creates a non-zero return code > (Failure) > > + "$1" -c 'import sys; sys.exit(sys.version_info < (3,6))' > > +} > > + > > python= > > +first_python= > > explicit_python=no > > -for binary in "${PYTHON-python3}" python > > +# A bare 'python' is traditionally python 2.x, but some distros > > +# have it as python 3.x, so check in both places. > > +for binary in "${PYTHON-python3}" python python3.{11..6} > > This is not available in e.g. dash, so we need to use {11,10,9,8,7,6}. > Just a nit, I can fix it myself. > What platforms use dash by default? Did I not see a failure because nothing that uses dash iterated that far down in the list? Anyway, you've got my blessing to change it, of course. > Paolo > PS, while you're here, how does this new loop interfere with your "custom python specified" flag for meson? I think meson uses the version of python *it* detects and not the configure script identified one, right? Does that mean that e.g. the qapi generator gets run with the system default/meson version and not the config version? Do I need to adjust this loop to consider more binaries as "explicitly specified"? (PPS: English needs "paraphrasing quotes" as punctuation. Maybe like double quotes except they're wiggly.) --js >
On Fri, 10 Feb 2023 at 15:28, John Snow <jsnow@redhat.com> wrote: > > > > On Fri, Feb 10, 2023, 5:46 AM Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> On 2/10/23 01:31, John Snow wrote: >> > +for binary in "${PYTHON-python3}" python python3.{11..6} >> >> This is not available in e.g. dash, so we need to use {11,10,9,8,7,6}. >> Just a nit, I can fix it myself. > > > What platforms use dash by default? Ubuntu and Debian default to /bin/sh being dash. The BSDs will also have some non-bash /bin/sh I expect. shellcheck will catch this non-portability: $ cat /tmp/foo.sh #!/bin/sh echo {11..6} $ shellcheck /tmp/foo.sh In /tmp/foo.sh line 3: echo {11..6} ^-----^ SC3009 (warning): In POSIX sh, brace expansion is undefined. For more information: https://www.shellcheck.net/wiki/SC3009 -- In POSIX sh, brace expansion is u... though if you try it on configure right now there are a ton of unaddressed warnings (some of which are harmless and some of which are probably technically portability bugs) so it's a bit tricky to use to sanity-check new changes. thanks -- PMM
On Fri, Feb 10, 2023 at 4:28 PM John Snow <jsnow@redhat.com> wrote: > PS, while you're here, how does this new loop interfere with your "custom python specified" flag for meson? I think meson uses the version of python *it* detects and not the configure script identified one, right? Does that mean that e.g. the qapi generator gets run with the system default/meson version and not the config version? Yes, if neither --python nor --meson are specified, then it could happen that a different python is used during ninja's execution vs. what is used for "other stuff" (docker cross compilers and other Makefile invocations of $(PYTHON)). The meson version of Python is guaranteed to be at least 3.7 as soon as we update to 0.63.x (which will be Real Soon Now), but it's ugly. The main issue I anticipate could be a problem when running from a virtual environment, so perhaps we can force usage of the internal meson if neither --python nor --meson are specified, and VIRTUAL_ENV is set and $VIRTUAL_ENV/bin/meson does not exist? diff --git a/configure b/configure index 06bcd9031903..001a79a90170 100755 --- a/configure +++ b/configure @@ -870,8 +870,18 @@ fi # Suppress writing compiled files python="$python -B" +has_meson() { + if test "${VIRTUAL_ENV:+set}" = set; then + # Ensure that Meson and Python come from the same virtual environment + test -x "$(VIRTUAL_ENV}/bin/meson" && + test "$(command -v meson)" -ef "$(VIRTUAL_ENV}/bin/meson" + else + has meson + fi +} + if test -z "$meson"; then - if test "$explicit_python" = no && has meson && version_ge "$(meson --version)" 0.63.0; then + if test "$explicit_python" = no && has_meson && version_ge "$(meson --version)" 0.63.0; then meson=meson elif test "$git_submodules_action" != 'ignore' ; then meson=git I will include it when posting the final series. > Do I need to adjust this loop to consider more binaries as "explicitly specified"? I don't think it's a huge problem. Outside virtual environments, the most likely setting is that Meson uses python3 which in turn is the most recent python3.X, so it should be fine overall. Though part of me thinks that your new loop is slightly overengineered and we should just require /usr/bin/env python3 and call it a day. Paolo
On Fri, Feb 10, 2023, 11:17 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > On Fri, Feb 10, 2023 at 4:28 PM John Snow <jsnow@redhat.com> wrote: > > PS, while you're here, how does this new loop interfere with your > "custom python specified" flag for meson? I think meson uses the version of > python *it* detects and not the configure script identified one, right? > Does that mean that e.g. the qapi generator gets run with the system > default/meson version and not the config version? > > Yes, if neither --python nor --meson are specified, then it could > happen that a different python is used during ninja's execution vs. > what is used for "other stuff" (docker cross compilers and other > Makefile invocations of $(PYTHON)). > > The meson version of Python is guaranteed to be at least 3.7 as soon > as we update to 0.63.x (which will be Real Soon Now), but it's ugly. > The main issue I anticipate could be a problem when running from a > virtual environment, so perhaps we can force usage of the internal > meson if neither --python nor --meson are specified, and VIRTUAL_ENV > is set and $VIRTUAL_ENV/bin/meson does not exist? > > diff --git a/configure b/configure > index 06bcd9031903..001a79a90170 100755 > --- a/configure > +++ b/configure > @@ -870,8 +870,18 @@ fi > # Suppress writing compiled files > python="$python -B" > > +has_meson() { > + if test "${VIRTUAL_ENV:+set}" = set; then > + # Ensure that Meson and Python come from the same virtual environment > + test -x "$(VIRTUAL_ENV}/bin/meson" && > + test "$(command -v meson)" -ef "$(VIRTUAL_ENV}/bin/meson" > + else > + has meson > + fi > +} > + > if test -z "$meson"; then > - if test "$explicit_python" = no && has meson && version_ge > "$(meson --version)" 0.63.0; then > + if test "$explicit_python" = no && has_meson && version_ge > "$(meson --version)" 0.63.0; then > meson=meson > elif test "$git_submodules_action" != 'ignore' ; then > meson=git > > I will include it when posting the final series. > > > Do I need to adjust this loop to consider more binaries as "explicitly > specified"? > > I don't think it's a huge problem. Outside virtual environments, the > most likely setting is that Meson uses python3 which in turn is the > most recent python3.X, so it should be fine overall. > > Though part of me thinks that your new loop is slightly overengineered > and we should just require /usr/bin/env python3 and call it a day. > Well, but that'd be a problem for CentOS 8, wouldn't it? python3 is gonna resolve to python3.6. > Paolo > >
On Fri, Feb 10, 2023 at 5:22 PM John Snow <jsnow@redhat.com> wrote: >> Though part of me thinks that your new loop is slightly overengineered >> and we should just require /usr/bin/env python3 and call it a day. > > Well, but that'd be a problem for CentOS 8, wouldn't it? python3 is gonna resolve to python3.6. The user would have to specify --python=/usr/bin/python3.8, or could also set up "alternatives" so that python3 resolves to modular Python (3.8 or newer). I think it's a fair requirement for users of enterprise distributions, and it works because it forces usage of QEMU's meson submodule. My lcitool update does the former by placing ENV PYTHON "/usr/bin/python3.8" in the Dockerfile. Paolo
On Fri, Feb 10, 2023 at 10:28:42AM -0500, John Snow wrote: > > > python= > > > +first_python= > > > explicit_python=no > > > -for binary in "${PYTHON-python3}" python > > > +# A bare 'python' is traditionally python 2.x, but some distros > > > +# have it as python 3.x, so check in both places. > > > +for binary in "${PYTHON-python3}" python python3.{11..6} > > > > This is not available in e.g. dash, so we need to use {11,10,9,8,7,6}. > > Just a nit, I can fix it myself. Shoot - I didn't notice v2 before I reviewed v1, where I pointed out the same problem. But note that dash doesn't understand ANY brace expansion; {11,10,9} is no better than {11..9}. The list is also not testing python3 when $PYTHON is set. See my suggestion for fixing that in my mail on v1.
diff --git a/configure b/configure index 64960c6000f..ea8c973d13b 100755 --- a/configure +++ b/configure @@ -592,20 +592,39 @@ esac : ${make=${MAKE-make}} -# We prefer python 3.x. A bare 'python' is traditionally -# python 2.x, but some distros have it as python 3.x, so -# we check that too + +check_py_version() { + # We require python >= 3.6. + # NB: a True python conditional creates a non-zero return code (Failure) + "$1" -c 'import sys; sys.exit(sys.version_info < (3,6))' +} + python= +first_python= explicit_python=no -for binary in "${PYTHON-python3}" python +# A bare 'python' is traditionally python 2.x, but some distros +# have it as python 3.x, so check in both places. +for binary in "${PYTHON-python3}" python python3.{11..6} do if has "$binary" then python=$(command -v "$binary") - break + if test -z "$first_python"; then + first_python=$python + fi + if check_py_version "$python"; then + # This one is good. + first_python= + break + fi fi done +# If first_python is set, we didn't find a suitable binary. +# Use this one for possible future error messages. +if test -n "$first_python"; then + python="$first_python" +fi # Check for ancillary tools used in testing genisoimage= @@ -1037,9 +1056,7 @@ then error_exit "GNU make ($make) not found" fi -# Note that if the Python conditional here evaluates True we will exit -# with status 1 which is a shell 'false' value. -if ! $python -c 'import sys; sys.exit(sys.version_info < (3,6))'; then +if ! check_py_version "$python"; then error_exit "Cannot use '$python', Python >= 3.6 is required." \ "Use --python=/path/to/python to specify a supported Python." fi
At the moment, we look for just "python3" and "python", which is good enough almost all of the time. But ... if you are on a platform that uses an older Python by default and only offers a newer Python as an option, you'll have to specify --python=/usr/bin/foo every time. We can be kind and instead make a cursory attempt to locate a suitable Python binary ourselves, looking for the remaining well-known binaries. This configure loop will prefer, in order: 1. Whatever is specified in $PYTHON 2. python3 3. python 4. python3.11 down through python3.6 Notes: - Python virtual environment provides binaries for "python3", "python", and whichever version you used to create the venv, e.g. "python3.8". If configure is invoked from inside of a venv, this configure loop will not "break out" of that venv unless that venv is created using an explicitly non-suitable version of Python that we cannot use. - In the event that no suitable python is found, the first python found is the version used to generate the human-readable error message. - The error message isn't printed right away to allow later configuration code to pick up an explicitly configured python. Signed-off-by: John Snow <jsnow@redhat.com> --- configure | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-)