Bug 272457 - graphics/qgis multiple installed pythons break build, latest version is always selected by cmake
Summary: graphics/qgis multiple installed pythons break build, latest version is alway...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Rainer Hurling
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-07-12 00:04 UTC by alt2600
Modified: 2023-07-28 05:23 UTC (History)
1 user (show)

See Also:
bugzilla: maintainer-feedback? (rhurlin)


Attachments
git-qgis-forcepython.diff (1.06 KB, patch)
2023-07-12 02:14 UTC, alt2600
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description alt2600 2023-07-12 00:04:03 UTC
So i recently had to add python 3.10 so I could install blender, but now qgis complains that doesn't have python 3.10 sip installed, which is true in my case, I have it for 3.9 . even if I figure how to fix this, it seems like maybe multi-python may be a problem as it will seemingly default to using the newest installed python. Still trying to work out how the detection routines work. I could certain install a more complete python 3.10 to get all the dependencies needed, but don't see as a fix. I tried amending USES python:3.8+ but this is a cmake routine finding the newer installed version. I'm sure this could be resolved once the right Cmake variable is forced, but when I find it that would seemingly force the python version to support. Not sure what you may want to do, either drop multi python, or just target the default python version. I think if targeting default once cmake is taked all that is needed is swapping PY_FLAVOR and PYTHON_PKGNAMEPREFIX with like a PYVER forcing one. But this is the kind of thing that would be your call as maintainer. I could certainly try to work a patch that does that, but don't want to do so if this is not the direction you'd want to take. I know of no trick to find a more suitable python except some evile Makefile magic to check the depends ahead of the dependency checks to pick a python version to pass to cmake, but this just feels wrong to me. I'll keep checking for a cleaner way to trick cmake into using my preferred python, but still looking for which cmake routine is doing the search. I didn't notice in the source, so I'm assuming its the system cmake routines.

PR 168159 seems to suggest cmake should respect PYTHON_VER set in python.mk, but not sure if a flag needs to be set or a possible regression in cmake???

I'm going to dig some more, but not clear totally what is wrong

-- Found Python: /usr/local/bin/python3.10 (found suitable version "3.10.12", minimum required is "3.7") found components: Interpreter Development Development.Module Development.Embed 
-- Found Python executable: /usr/local/bin/python3.10 (version 3.10.12)
-- Python library: /usr/local/lib/libpython3.10.so
-- Python site-packages: /usr/local/lib/python3.10/site-packages
Traceback (most recent call last):
  File "/usr/ports/graphics/qgis/work/qgis-3.32.0/cmake/FindSIP.py", line 34, in <module>
    import sipbuild
ModuleNotFoundError: No module named 'sipbuild'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/ports/graphics/qgis/work/qgis-3.32.0/cmake/FindSIP.py", line 47, in <module>
    import sipconfig
ModuleNotFoundError: No module named 'sipconfig'
CMake Error at cmake/FindSIP.cmake:58 (MESSAGE):
  Could not find SIP
Call Stack (most recent call first):
  CMakeLists.txt:983 (find_package)


-- Configuring incomplete, errors occurred!
*** Error code 1
Comment 1 alt2600 2023-07-12 02:14:17 UTC
Created attachment 243346 [details]
git-qgis-forcepython.diff

ok so I figured out how to force a version that should respect python.mk using the default python or allowing folks to force a version in make.conf if thats how they roll.

Base Cmake FindPython.cmake in cmake-core has no respect for PYTHON_VER, but we can pass a hint to it to effectively do the same. this patch appends USES python with python:3.8+ to make sure at least the minimum for the port of 3.7 is presented. and adds the cmake argument -DPython_EXECUTABLE=${PYTHON_CMD} to allow python.mk to provide the version to use. 

just finished building, took a while but wanted to make sure before posting the patch. it also seems to run fine as well. not sure how to score if this is a cmake-core regression for freebsd or not, but it seems we now need to use Python_EXECUTABLE=${PYTHON_CMD} if this comes up with the current cmake in ports. else the latest version is auto selected, unless the port being build opts for possibly LOCATION instead of VERSION which is the default, for its Python_FIND_STRATEGY when calling the find python routine.
Comment 2 Rainer Hurling freebsd_committer freebsd_triage 2023-07-12 10:11:21 UTC
Many thanks for your patch. I wasn't aware of it before ;)

While I am trying to understand the underlying problem and test your patch on Poudriere (this will take some time), I found the following in our ports tree:


#ag Python_ADDITIONAL_VERSIONS
Mk/Uses/python.mk
773:CMAKE_ARGS+=        -DPython_ADDITIONAL_VERSIONS=${PYTHON_VER}

devel/pam_wrapper/files/python-detection.patch
33:-set(Python_ADDITIONAL_VERSIONS 2.7 2.6)
86:-set(Python_ADDITIONAL_VERSIONS 3.8 3.7 3.6)

graphics/ogre3d/Makefile
88:     ${REINPLACE_CMD} -e 's/\(Python_ADDITIONAL_VERSIONS\) [0-9\.]*/\$1 ${PYTHON_VER}/' \

java/openjfx14/files/patch-modules_javafx.web_src_main_native_Source_cmake_WebKitCommon.cmake
7:-    set(Python_ADDITIONAL_VERSIONS 3)

math/kig/Makefile
26:     ${REINPLACE_CMD} '/Python_ADDITIONAL_VERSIONS/s|2.7|${PYTHON_VER};2.7|' \

math/py-gau2grid/files/patch-CMakeLists.txt
6: set(Python_ADDITIONAL_VERSIONS 3.9 3.8 3.7 3.6 3.5)  # adjust with CMake minimum FindPythonInterp

math/py-ambit/files/patch-CMakeLists.txt
6:     set(Python_ADDITIONAL_VERSIONS 3.11 3.10 3.9 3.8 3.7)  # adjust with CMake minimum FindPythonInterp

science/psi4/files/patch-CMakeLists.txt
6: set(Python_ADDITIONAL_VERSIONS 3.11 3.10 3.9 3.8)  # adjust with CMake minimum FindPythonInterp

sysutils/bareos19-server/files/patch-core-cmake_BareosFindAllLibraries.cmake
9:-    Python_ADDITIONAL_VERSIONS
21:+      Python_ADDITIONAL_VERSIONS


Apparently there is a similar problem with some other ports as well?

BTW: Because there is no Python 3.7 in the ports tree anymore, USES=python is the same as USES=python:3.8+ ATM, see lines 20 and 23 in Mk/Uses/python.mk.
Comment 3 alt2600 2023-07-12 22:09:00 UTC
(In reply to Rainer Hurling from comment #2)

oops, didn't realize python implies only 3.8+ these days. Literally went in to confirm 3.8 was the bottom these days and didn't read about two more lines into python.mk 2.7 is the release that just won't go away, wrongly assumed it was still in that list. feel free to strip that out if you want, that was early attempt that was left in the Makefile, but not a problem, just overly pedantic. 

I would imagine lots of ports are having these issues yes. Cmake can make you want to pull your hair out at times, least for me. Given this port relies on the system cmake routines patching that would be tough. Just glad it can be forced with the executable. Its beyond frustrating how hard Cmake can make just setting it to use specific prefixes or versions is, just because it wants to find everything itself based so often on linux convention, where things like multiple system pythons would be relatively rare for instance from my experience. hope it makes it through poudriere ok, i would imagine that would take a while to run for sure. Not sure when PYTHON_VER went out of our system cmake scripts, but that PR I found came up as I typed this bug description and it was about a decade old I think. I'm just glad cmake could be tamed without patching .cmake files.
Comment 4 Rainer Hurling freebsd_committer freebsd_triage 2023-07-14 07:16:26 UTC
Just committed your patch (without) USES=compiler:py38+ in 
https://cgit.freebsd.org/ports/commit/?id=c3a413762d4f057b69e689df9ceeaf17a64419ec

Many thanks for your investigations into this! 

And I am very sorry I forgot to link this PR in the commit :(
Comment 5 commit-hook freebsd_committer freebsd_triage 2023-07-14 07:26:56 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=347cd7d336121850e3b6234d4d4ebea96ab42588

commit 347cd7d336121850e3b6234d4d4ebea96ab42588
Author:     alt2600@icloud.com <alt2600@icloud.com>
AuthorDate: 2023-07-14 07:22:33 +0000
Commit:     Rainer Hurling <rhurlin@FreeBSD.org>
CommitDate: 2023-07-14 07:24:52 +0000

    graphics/qgis-ltr: Use default Python version

    cmake's FindPython.cmake always returns the highest found Python version,
    although the default version is needed here, since only for this usually
    further tools like SIP etc. are installed.

    Passing the path of the default version to Python_EXECUTABLE causes the
    cmake configuration to use it.

    Co-authored-by: Rainer Hurling <rhurlin@gwdg.de>

    PR:             272457
    MFH:            2023Q3

 graphics/qgis-ltr/Makefile | 2 ++
 1 file changed, 2 insertions(+)
Comment 6 commit-hook freebsd_committer freebsd_triage 2023-07-14 07:27:57 UTC
A commit in branch 2023Q3 references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=37513514834bc0a27a792f458f2dbcd5ae3d7143

commit 37513514834bc0a27a792f458f2dbcd5ae3d7143
Author:     alt2600@icloud.com <alt2600@icloud.com>
AuthorDate: 2023-07-14 07:22:33 +0000
Commit:     Rainer Hurling <rhurlin@FreeBSD.org>
CommitDate: 2023-07-14 07:26:39 +0000

    graphics/qgis-ltr: Use default Python version

    cmake's FindPython.cmake always returns the highest found Python version,
    although the default version is needed here, since only for this usually
    further tools like SIP etc. are installed.

    Passing the path of the default version to Python_EXECUTABLE causes the
    cmake configuration to use it.

    Co-authored-by: Rainer Hurling <rhurlin@gwdg.de>

    PR:             272457
    MFH:            2023Q3
    (cherry picked from commit 347cd7d336121850e3b6234d4d4ebea96ab42588)

 graphics/qgis-ltr/Makefile | 2 ++
 1 file changed, 2 insertions(+)
Comment 7 Rainer Hurling freebsd_committer freebsd_triage 2023-07-28 05:23:56 UTC
Thanks for the pointer!

However, I've been watching the bug #262109 for some time now. My plan is to remove our patch for QGIS again once the general patch is committed ;)