openzfs / zfs

OpenZFS on Linux and FreeBSD
https://openzfs.github.io/openzfs-docs
Other
10.68k stars 1.75k forks source link

Python version detection is broken #15121

Open JiiPee74 opened 1 year ago

JiiPee74 commented 1 year ago

System information

Type Version/Name
Distribution Name Fedora
Distribution Version 38
Kernel Version 6.4.0
Architecture x86_64
OpenZFS Version 2.2.0-rc3

Describe the problem you're observing

Trying to build latest rc3 and it fails because configure detects distro python version wrong, what is 3.11.4

python --version
Python 3.11.4

This python version detection seems to be quite wide spread issue. It's not a 1st time when I have seen this and when you are not a real coder, fixing it myself is sometimes easy, other times like in this case not. I took a look of it and seems to be quite complex stuff you guys have build for simple version detection. Maybe it could be made simpler. Issue here seems to be that configure script makes poopoo because it cannot handle minor version higher than 9 or it makes poopoo because this .4 after minor version.. Not really sure.

Describe how to reproduce the problem

Include any warning/errors/backtraces from the system logs

./configure
--- SNIP ---
checking for python3... python3
checking for python version... 3.1
checking for python platform... linux
checking for python script directory... ${prefix}/lib/python3.11/site-packages
checking for python extension module directory... ${exec_prefix}/lib64/python3.11/site-packages
configure: error: "Python >= 3.6 is required"
rincebrain commented 1 year ago

I would suggest looking at #14297 for people saying they should add an override flag for this case, rather than keeping the fixed logic that fixed this bug a while ago, and then just not implementing it.

JiiPee74 commented 1 year ago

I would suggest looking at #14297 for people saying they should add an override flag for this case, rather than keeping the fixed logic that fixed this bug a while ago, and then just not implementing it.

Well I dont know what they are talking there. Some legacy stuff. Like I said my distro python is so new that configure script cant handle it.

But I got it working with this change:

--- zfs-2.2.0-rc3/configure     2023-07-28 02:18:03.000000000 +0300
+++ zfs-2.2.0/configure 2023-07-30 06:07:10.356131386 +0300
@@ -17099,7 +17099,7 @@
 if ${am_cv_python_version+:} false; then :
   $as_echo_n "(cached) " >&6
 else
-  am_cv_python_version=`$PYTHON -c "import sys; sys.stdout.write(sys.version[:3])"`
+  am_cv_python_version=`$PYTHON -c "import sys; py_ver = sys.version; sys.stdout.write(py_ver.split(' ')[0])"`
 fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $am_cv_python_version" >&5
 $as_echo "$am_cv_python_version" >&6; }
@@ -17243,6 +17243,7 @@
                PYTHON_VERSION=$(echo ${PYTHON##*/} | tr -cd 0-9.)

 fi
+       PYTHON_VERSION=$(echo $PYTHON_VERSION | awk -F '.' '{printf $1"."$2}')
        PYTHON_MINOR=${PYTHON_VERSION#*\.}

        case $PYTHON_VERSION in #(

Most likely not best way to do it but hey it works for me.

cjd commented 1 year ago

I just hit this as well for 2.2.0-rc3 The core issue is within the version of automake used to generate the configure script. You can workaround/fix by going to the extracted source dir and re-runing ./autogen.sh (as long as you have automake 1.16.4 or later installed) After that ./configure worked without isse for python 3.11 (for me at least)

rnickle commented 4 months ago

I just hit this as well for 2.2.0-rc3 The core issue is within the version of automake used to generate the configure script. You can workaround/fix by going to the extracted source dir and re-runing ./autogen.sh (as long as you have automake 1.16.4 or later installed) After that ./configure worked without isse for python 3.11 (for me at least)

I agree that this isn't a defect in OpenZFS, it is an issue with automake, and the issue manifests in the configuration in the local shared data for aclocal (on my distro it is /usr/share/aclocal-1.16).

The older distro I was using for builds (Ubuntu 20.04) has automake 1.16.1, which still has this problem.

I used autogen.sh to build the local version of aclocal.m4 and then patched the use of 'sys.stdout.write(sys.version[:3])' with this variation of JiiPee74's workaround:

*** aclocal.m4  2024-07-18 20:29:47.856111110 +0000
--- aclocal.m4.patched  2024-07-18 20:29:25.119976156 +0000
***************
*** 976,982 ****
    dnl library.

    AC_CACHE_CHECK([for $am_display_PYTHON version], [am_cv_python_version],
!     [am_cv_python_version=`$PYTHON -c "import sys; sys.stdout.write(sys.version[[:3]])"`])
    AC_SUBST([PYTHON_VERSION], [$am_cv_python_version])

    dnl Use the values of $prefix and $exec_prefix for the corresponding
--- 976,982 ----
    dnl library.

    AC_CACHE_CHECK([for $am_display_PYTHON version], [am_cv_python_version],
!     [am_cv_python_version=`$PYTHON -c "import sys; sys.stdout.write('.'.join(sys.version.split('.')[[:2]]))"` ])
    AC_SUBST([PYTHON_VERSION], [$am_cv_python_version])

    dnl Use the values of $prefix and $exec_prefix for the corresponding

Generate the context diff and pipe it to patch:

$ diff -c aclocal.m4 aclocal.m4.patched | patch -c 
patching file aclocal.m4

If you have a similar aclocal.m4 you should be able to copy the above context diff and feed it to patch with a hereis document:

$ patch -c << "_EOD_"
<< PASTE CONTEXT DIFF HERE >>
_EOD_
patching file aclocal.m4

After patching, I did not commit the change because it'll get regenerated by the next run of autogen.sh .

The aclocal.m4 will be consumed by autoconf, so run that now to evaluate the patched aclocal.m4:

$ autoconf
$ ./configure

And that enabled my build to adopt Python 3.12 (the one I had configured):

$ ./configure
...
checking for python3... python3
checking for python version... 3.12
checking for python platform... linux
checking for python script directory... ${prefix}/lib/python3.12/site-packages
checking for python extension module directory... ${exec_prefix}/lib/python3.12/site-packages
checking for python3 module: packaging... yes
...

Just for example, here's the same fragment run directly (no escaped brackets since not in m4) using pyenv against different Python versions to show it handles versions greater than 10.

$ echo `python3.12  -c "import sys; sys.stdout.write('.'.join(sys.version.split('.')[:2]))"`
3.12
$ echo `python3.6  -c "import sys; sys.stdout.write('.'.join(sys.version.split('.')[:2]))"`
3.6

The real fix will be upstream from automake of course!

Thank you!