Created attachment 231998 [details] [patch] suport FindPython.cmake, FindPython3.cmake, FindPython2.cmake modules cmake supports more than one way to search for python. Uses/python.mk passes -DPython_ADDITIONAL_VERSIONS=${PYTHON_VER} to help cmake "find" the version of python that a port build wants to use. The FindPython{,2,3}.cmake modules don't know anything about Python_ADDITIONAL_VERSIONS. The FindPython{Interp,Libs}.cmake modules do. The former use Python{,2,3}_EXECUTABLE as the hint. I didn't do the archeology to see which flavor of FindPython*.cmake came first, but of course it makes sense in the bizarre world of cmake that you would not make a later flavor compatible. Oh cmake, how do I hate thee? Let me count the ways. Recently a couple ports broke when a newer version of python is installed in addition to the default version of python (currently 3.8). For instance, if python3.10 was installed in addition to python3.8 (and many packages installed in the lib/python3.8/site-packages area), multimedia/onevpl broke before ports/0378719b21c61d5a5d3dc8f31be7781fd7ff11ef. See bug 261415. Now graphics/libjxl has the same problem. I did not open a bug for graphics/libjxl yet. Instead it seems that we need to appease cmake's additional python support modules by adding more knobs to tell cmake what version of python to use in case the port uses a different flavor of FindPython than the one that wants Python_ADDITIONAL_VERSIONS. Attached is a patch for Uses/python.mk that does that. Feedback please.
Help pr auto assignment - add python@ to CC.
Let's check for side-effects by testing all consumers (via exp-run). Many ports already do something similar, so one can do a cleanup later. $ rg -l 'Python[23]?_EXECUTABLE' cad/surelog/files/patch-CMakeLists.txt devel/entt/Makefile devel/llvm11/Makefile devel/llvm12/Makefile devel/llvm13/Makefile devel/llvm14/Makefile graphics/glaxnimate/Makefile graphics/openshadinglanguage/files/patch-src_doc_CMakeLists.txt math/dbcsr/Makefile math/py-dionysus/Makefile math/py-heyoka/files/patch-CMakeLists.txt math/py-or-tools/files/patch-cmake_python.cmake math/vtk9/Makefile multimedia/onevpl/Makefile multimedia/py-mlt6/Makefile net/libcharon/files/patch-CMakeLists.txt science/dakota/Makefile science/py-pygmo2/files/patch-CMakeLists.txt science/scidavis/Makefile science/siconos/files/patch-cmake_SiconosSetup.cmake science/votca/Makefile sysutils/bareos-server/files/patch-core-cmake_BareosFindAllLibraries.cmake sysutils/bareos20-server/files/patch-core-cmake_BareosFindAllLibraries.cmake x11-themes/kf5-breeze-icons/Makefile x11-themes/kf5-breeze-icons/files/patch-CMakeLists.txt x11-themes/plasma5-breeze-gtk/Makefile
Thank you for the report and patch John. Are you aware of a cmake mechanism where one can pass an explicit Python version/path using a single variable, without requiring the MAJOR_VER checks? I haven't had the opportunity to review the cmake/python ecosystem for best current practice lately. We'd appreciate any insight in that regard
(In reply to Kubilay Kocak from comment #3) No, I am not aware of anything. The FindPython{2,3,}.cmake modules use different hint variables. And the CMakeLists.txt file in a particular software package could use any of them. In the case of graphics/libjxl, for instance, it conditionally uses any of them depending on what it dynamically sees in the shebang header for a2x. In the case of multimedia/onevpl, it uses FindPython{Interp,Libs}.cmake (which wants the Python_ADDITIONAL_VERSIONS knob or FindPython.cmake (wants Python_EXECUTABLE) conditionally depending on detected cmake version. And it expresses a preference for python3 by using FindPython3.cmake as a hint before trying the others. The modules that cmake bundles do a poor job of enforcing any sort of consistency. It's just a fact of life for that tool. And it _can_ change in a later version of cmake without careful consideration for backward compatibility. Some of the main tool core features have a somewhat rigorous feature compatibility infrastructure, but many of the bundled modules are not nearly as well... considered.
Exp-run looks fine
How about: CMAKE_ARGS+= -DPython${PYTHON_MAJOR_VER}_EXECUTABLE:FILEPATH="${PYTHON_CMD}" CMAKE_ARGS+= -DPython_EXECUTABLE:FILEPATH="${PYTHON_CMD}"
(In reply to Li-Wen Hsu from comment #6) That looks nice indeed, I ended up doing something similar for py-libxml2 ( https://reviews.freebsd.org/D34384 ) For the sake of keeping the style consistent in python.mk we should use \ instead of defining CMAKE_ARGS for each line.
(In reply to Daniel Engberg from comment #7) For the sake of being correct, https://reviews.freebsd.org/D34338 not D34384 =)
Back to USES=python maintainer for approval and/or picking desired style.
(In reply to Jan Beich from comment #9) I like one in comment #6 because of obvious reason :-) I suppose we don't need another exp-run for that? BTW, will you also do the clean up for the list in comment #2 ?
See also PR 252277 .. I just landed a change to devel/cmake so that it obeys what python3 points at (if there is a python3 metapackage installed).
Adding math/Imath which recently grew a workaround based on Python_EXECUTABLE as cmake var, too. Thanks diizzy@ for the pointer.
Created attachment 238988 [details] [patch] suport FindPython.cmake, FindPython3.cmake, FindPython2.cmake modules (v2) Update with lwhsu@ version from comment 6. I've been using this with success locally now for 9-10 months. 'make -C graphics/libjxl build' is still broken (in the conditions described in comment 0) without it. The commit to onevpl to work around this issue can be removed after this is committed. I don't know if there are others (or I have forgotten them), but I would not be surprised if so.
Sorry, I'm not working on this. I already drown in my own homework, so helping teams with active committers is low priority (to avoid burnout).
*** Bug 262502 has been marked as a duplicate of this bug. ***
(In reply to John Hein from comment #13) Regarding affect on other ports, once this patch for python.mk (to work with cmake's new-ish python support) is committed, math/Imath can remove its 'post-patch' entirely (see bug 263341) - the patch doesn't hurt, but it's not necessary (confirmed by testing with and without the post-patch, and with and without the pyhon.mk patch). So add that port as a beneficiary of this patch (along with at least multimedia/onevpl and graphics/libjxl).
This python.mk patch may or may not allow the removal of the cmake patch from bug 252277. They can exist independently without conflict. I just did not test yet whether that patch from bug 252277 would be completely unnecessary if this python.mk patch is in place. That could be investigated separately, but committing this python.mk patch need not wait for such investigation, I believe.
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=272457 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=272745 applying the patch to python.mk results in resolving the bug on qgis and qt6-webengine where cmake will return the latest installed version of python, and not respect python default. Curious if this patch to python.mk will be applied or not. qgis was already patched in ports to pass the python executable hint to cmake, but removing that patch to qgis and using this patch to python.mk resolves the issue as it is setting the same cmake hint.
*** Bug 272745 has been marked as a duplicate of this bug. ***
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=aa9736e3e5f6856f5eb5e26837169b0f6022eec8 commit aa9736e3e5f6856f5eb5e26837169b0f6022eec8 Author: John Hein <jcfyecrayz@liamekaens.com> AuthorDate: 2023-09-04 16:23:06 +0000 Commit: Li-Wen Hsu <lwhsu@FreeBSD.org> CommitDate: 2023-09-04 17:21:35 +0000 python.mk: Improve CMake/Python integration Suport FindPython.cmake, FindPython3.cmake, FindPython2.cmake modules by adding Python{,2,3}_EXECUTABLE to CMAKE_ARGS in python.mk. CMake supports more than one way to search for python. Currently python.mk passes -DPython_ADDITIONAL_VERSIONS=${PYTHON_VER} to help FindPython{Interp,Libs}.cmake modules "find" the version of python that a port build wants to use. The FindPython{,2,3}.cmake modules don't know anything about Python_ADDITIONAL_VERSIONS but use Python{,2,3}_EXECUTABLE as the hint. PR: 262109 Mk/Uses/python.mk | 2 ++ 1 file changed, 2 insertions(+)
The command: $ rg -l 'Python[23]?_EXECUTABLE' Now returns 65 ports. A cleanup is wanted and help is needed.
No need to buzz ports-bugs@
(In reply to Li-Wen Hsu from comment #21) The list: cad/antimony cad/surelog cad/uhdm databases/arrow devel/catch2 devel/gdcm devel/llvm11 devel/llvm12 devel/llvm13 devel/llvm14 devel/llvm15 devel/llvm16 devel/llvm17 devel/nanopb graphics/filament graphics/glaxnimate graphics/krita graphics/openexr graphics/openshadinglanguage graphics/py-s2 graphics/qgis graphics/qgis-ltr math/clingo math/cvc5 math/dbcsr math/py-ambit math/py-dionysus math/py-faiss math/py-heyoka math/py-or-tools math/vtk9 misc/astc-encoder misc/py-openvdb multimedia/obs-studio multimedia/onevpl net/libcharon science/agrum science/arbor science/chemicalfun science/dakota science/elmerfem science/gromacs science/InsightToolkit science/lammps science/libint2-psi4 science/liggghts science/mopac science/mrchem science/openmodelica science/psi4 science/py-arbor science/py-gemmi science/py-HepMC3 science/py-mrchem science/py-pygmo2 science/salome-kernel science/scidavis science/siconos science/smoldyn science/ttk science/votca security/ca_root_nss sysutils/bareos20-server x11-themes/kf5-breeze-icons x11-themes/plasma5-breeze-gtk
(In reply to Li-Wen Hsu from comment #23) See bug 273713 - remove -DPython3_EXECUTABLE args for cmake in devel/llvm15, et. al.
(In reply to Li-Wen Hsu from comment #23) This list would be better in a separate bug. See bug 273713, comment 5. But, in any case, the llvm ports have been cleaned up - the [now] redundant cmake definitions for python in those ports has been removed. Again, see bug 273713. If we convert the list in comment 23 to a separate meta bug, we can track the cleanup better - and then just close this bug 262109 as fixed (which it is).