flux-framework / flux-sched

Fluxion Graph-based Scheduler
GNU Lesser General Public License v3.0
87 stars 41 forks source link

configure fails to accept Python 3.10.0 #880

Closed andre-merzky closed 3 years ago

andre-merzky commented 3 years ago

I see flux deployment fail in a conda env with the Python 3.10.0:

...
checking for python3... /home/developer/ce_tutorial/bin/python3
checking for a version of Python >= '2.1.0'... yes
checking for a version of Python >='3.6'... no
configure: error: this package requires Python >='3.6'.
If you have it installed, but it isn't the default Python
interpreter in your system path, please pass the PYTHON_VERSION
variable to configure. See ``configure --help'' for reference.
$ /home/developer/ce_tutorial/bin/python3 -V
Python 3.10.0

It seems like configure has an incorrect lexical check in one place:

>>> ver '3.6.9'
>>> ver >='3.6'
True
>>> ver = '3.10.0'
>>> ver >='3.6'
False

which presumably comes from config/ax_python_devel.m4:

                ac_supports_python_ver=`$PYTHON -c "import sys; \
                        ver = sys.version.split ()[0]; \
                        print (ver >='3.6')"`

FWIW, aclocal.m4 defines an AM_PYTHON_CHECK_VERSION macro which does the right thing (comparing hexversion instead of version string).

Happy to prepare a PR but am not sure what your preferred way of resolving this would be. I would suggest to use the hexversion check also in config/ax_python_devel.m4 - that would result in a very small diff.

garlick commented 3 years ago

Ah thanks for running this down! A PR with your proposed change would be welcome.

dongahn commented 3 years ago

@andre-merzky: I know you are busy with ExaWorks SDK E4S coordination. I think I can throw this into a quick PR sometime today if you want. Thanks

andre-merzky commented 3 years ago

@dongahn - thank you, that's appreciated! :-)

dongahn commented 3 years ago

I think the patch used in ZFS can be reused:

https://cgit.freebsd.org/src/commit/?id=08cd0717359b1a18693e3c8e6d6e5a2819b35a48

This way we don't have to reinvent the wheel and by having a comment where the patch comes from, we can refer back to it if something else creeps up?

grondo commented 3 years ago

I don't like the idea of adding more build requirements (e.g. the approach taken by zfs seems to require the packaging module? not sure how standard that is)

We should just try pulling the latest version of ax_python_devel.m4 from autoconf-archive and see if that works?

dongahn commented 3 years ago

Ah do we already have a fix from the upstream? That'd be the best.

dongahn commented 3 years ago

And yes I was looking into packaging or distlib module used in the check and having the similar thought.

grondo commented 3 years ago

Also, does flux-sched actually use PYTHON_LIBS and PYTHON_CFLAGS? This macro might not even be needed in projects which don't build python bindings...

Ah do we already have a fix from the upstream? That'd be the best.

I haven't checked yet. I'll have to find a system or container with Python 3.10.x to verify.

dongahn commented 3 years ago

Also, does flux-sched actually use PYTHON_LIBS and PYTHON_CFLAGS? This macro might not even be needed in projects which don't build python bindings...

That's a great question. My guess is not. This probably came to flux-sched through flux-core and this could be an overkill. Perhaps solution is to just check the python version with AM_PYTHON_CHECK_VERSION since it correctly handles the version comparison. Let me try this.

jameshcorbett commented 3 years ago

In case it's helpful you can create a Python 3.10 conda environment on LC with conda create --name PY310 --no-default-packages python=3.10 && conda activate PY310. You can then install Flux's python dependencies with a regular pip install (if you try to use conda, conda will try to downgrade Python to 3.9 since one of the packages isn't listed with conda as supporting 3.10).

We should just try pulling the latest version of ax_python_devel.m4 from autoconf-archive and see if that works?

I tried it and ran into the same problem. Although it does fix another problem I ran into which I got after downgrading the version >= 3.6 check to version >= 3.1:

configure: error: cannot import Python module "distutils".
Please check your Python installation. The error was:
<string>:1: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives
grondo commented 3 years ago

Thanks @jameshcorbett!

We've got a proposed fix over in flux-core as flux-framework/flux-core#3939. It would be great if you can comment on the solution (it is a little kludgy)

I think for flux-sched, we should proceed with an attempt to remove ax_python_devel.m4, assuming it isn't really required.

garlick commented 3 years ago

@jameshcorbett - see the proposed solution over in flux-framework/flux-core#3939

As for what to do in flux-sched and flux-accounting. Hmm, removing ax_python_devel.m4 and applying this diff seems to work. I'll propose something.

diff --git a/configure.ac b/configure.ac
index 7500c198a..b353e955a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -65,18 +65,12 @@ PKG_CHECK_MODULES(LIBEDIT, libedit)

 #  Set PYTHON_VERSION to FLUX_PYTHON_VERSION here
 PYTHON_VERSION=${PYTHON_VERSION:-$FLUX_PYTHON_VERSION}
-AX_PYTHON_DEVEL([>='3.6'])

-AM_PATH_PYTHON([$ac_python_version])
+AM_PATH_PYTHON([$PYTHON_VERSION])
 if test "X$PYTHON" = "X"; then
   AC_MSG_ERROR([could not find python])
 fi

-# Flag for PYTHON_LDFLAGS workaround below.
-if test -n "$PYTHON_LDFLAGS"; then
-  ac_python_ldflags_set_by_user=true
-fi
-
 AM_CHECK_PYMOD(yaml,
                [StrictVersion(yaml.__version__) >= StrictVersion('3.10')], [],
                [AC_MSG_ERROR([[could not find python module yaml, version 3.10+ required]])])