Python libs and interpreter version.
This adds a fix to make sure Python interpreter and libraries have the same version.
Merge request reports
Activity
"but I'm not sure if the Python interpreter should determine with library version you want, or the other way around... Sounds more like the local configuration is messed up."
@tkoch: But you do agree that both interpreter and library should be of the same version, right?
I'm certain that my system config is not messed up, because having multiple instances of the same program installed at the same is essentially working well on gentoo and Python is a central part of this system including multiple Python targets, so no, certainly not the case.
In addition, the newer and more appropriate cmake test, FindPython, i.e. find_package(Python ...) works as it should and the DUNE cmake should transfer to that.
of course this is foremost a CMake bug... The doc even states
If also calling find_package(PythonLibs), call find_package(PythonInterp) first to get the currently active Python version by default with a consistent version of PYTHON_LIBRARIES.
This was added in the CMake 3.1 documentation. @robert.kloefkorn What CMake version do you have? Edit: well 3.1 is required any in Dune...
Edited by Timo Koch
added 1 commit
- 4175d536 - [bugfix][FindPythonLibs] Python libs and interpreter should be of the same
@tkoch: And btw, your suggested patch did not work.
I didn't try it. Was just a suggestion. But I wonder why it doesn't work. You can specify a version for
find_package
and also expect an exact match. Maybe it wasn't the correct syntax.Edited by Timo Kochnot in this case. The interpreter version is already found. The challenge here is to only accept the library version that exactly matches the version of the Python interpreter found.
Edited by Timo Koch
Disclaimer: just saw that Robert already referred to this but had already written this and do not want to waste it...
CMake (apparently from 3.12 onwards) offers a single approach for finding interpreter/library and other parts of Python in one go which seems to be aimed at solving the issues mentioned here: https://cmake.org/cmake/help/latest/module/FindPython.html From the documentation
To ensure consistent versions between components Interpreter, Compiler, Development and NumPy, specify all components at the same time:
So as @robert.kloefkorn we should be using that if available - perhaps we don't need to fix anything for older versions of CMake?
I would just check if the new FindPython is available and if not either (1) leave the old behaviour (2) use Robert's fix. I think leaving the old behaviour is fine because (a) it's not that common an issue (b) can be solved by using a virtual env (c) if someone already installed multiple Python3 version they can probably also manage to upgrade their CMake
mentioned in merge request !790 (merged)
@andreas.dedner: Certainly not a bad idea.
@andreas.dedner: This problem also occurs when a virtual environment ist used.
-- Found PythonInterp: .../Dune/testing/dune-fem-dg/dune-pip/bin/python3 (found suitable version "3.6.10", minimum required is "3")
-- Found PythonLibs: /usr/lib/libpython3.7m.so (found version "3.7.7")
-- Found pip .../Dune/testing/dune-fem-dg/dune-pip/bin/python3: TRUE
The virtual environment was created with:
python3 -m venv $WORKDIR/dune-pip
WORKDIR in this case is .../Dune/testing/dune-fem-dg/
Edited by Robert Kadded 336 commits
-
4175d536...649d7caa - 335 commits from branch
master
- a51b9d48 - [bugfix][FindPythonLibs] Python libs and interpreter should be of the same
-
4175d536...649d7caa - 335 commits from branch
I suggest to merge this because it effectively fixes the problem I have described, and switching to FindPython at this point is not an option since it would force cmake 3.12 as minimum required version.
FindPython has only been introduced in cmake 3.12 (not available in cmake 3.11: https://cmake.org/cmake/help/v3.11/manual/cmake-modules.7.html).
Also, all other alternative suggestions from above have not worked.
@robert.kloefkorn I think the idea is not to require cmake 3.12 but to ship the module on our own and make the implementation work with cmake 3.1. In case that doesn't work, we should still use the new
FindPython3.cmake
as soon as a newer version of cmake is detected and only fall back to current implementation+yourfix if cmake is too old.
@robert.kloefkorn Looking at the doc again, I think your fix sounds like the most reasonable thing to do. I was a bit irritated by the variable name
Python_ADDITIONAL_VERSIONS
but it apparent just refers the versions it will look for.@tkoch: Thanks for double checking.
I guess then this can be merged. I'll do so if there are no objections during the week.
added 10 commits
-
a51b9d48...cc52d788 - 9 commits from branch
master
- f2f2ac3e - [bugfix][FindPythonLibs] Python libs and interpreter should be of the same
-
a51b9d48...cc52d788 - 9 commits from branch
Hey guys, sorry for jumping in late, I was on leave for a month.
@andreas.dedner has pointed out my preferred solution: Switching to the newer
FindPython.cmake
and ship that module with dune-common if possible to fix up older CMake versions. Note, that I have not tried that myself, but just from reading the docs it sounds very much like they learned from their past mistakes regarding Python detection.Also, I think that the originally proposed solution here (using the additional variables thing) does not work. The semantics of this additional variables are weird: They do not enforce any versions, but instead extend the list of possible versions. Specifying additional versions may lead to your desired version being found, but there is absolutely no guarantee about that.
As I understood this small fix that @robert.kloefkorn wants to merge here wasn't supposed to be the final solution. I also support trying to ship a compatible version of
FindPython3.cmake
in Dune. Maybe we should open a new issue with a possible task description? It sounds like a bit of works at least to make sure the check runs also with older versions of CMake.This MR is more like a quick remedy until the more involved solution has been implemented and tested. It worked for @robert.kloefkorn's case and should do no harm. Or am I wrong?
@tkoch: Thanks, that is correct.
@dominic: I think the only person that did testing on this matter is me, and then the CI checked that everything works too, since the Python stuff is now tested there.
I would prefer to merge this fix and then, in a different MR, consider switching to the newer version, which as I said, means one has to have at least cmake 3.12 which I think is rather new.
So long story short: I'll merge this later today.
Edited by Robert KA more general comment: This problems only shows, when more than one Python3 version is installed and available. I guess this is not the case of a standard debian/ubuntu installation. It is common on gentoo, though.
That said, the newer FindPython3 version also fails to find the correct Python3 version, at least in my case, as it finds the newest version, 3.7 in my case, when Python3 (python3 --version: Python 3.6.10) is clearly not that version. Again, this also shows when using a virtual environment and I think this is a deficiency of the cmake macros. Nevertheless, at least interp and lib version fit together with the newer macros.
Edited by Robert Kmentioned in commit 8d364f29
@gruenich, @tkoch, @andreas.dedner, @dominic: Just FYI: I don't think that the new version will fix this problem. I think the new version is broken, because, it finds the wrong Python interpreter and lib, although, as pointed out, at least the version of interpreter and lib match.
Inside a venv created with python3 -m venv dune-pip
ls ./dune-pip/bin ./dune-pip/lib
dune-pip/bin/: activate activate.csh activate.fish easy_install* easy_install-3.6* pip* pip3* pip3.6* python@ python3@
dune-pip/lib: python3.6/
The links point to the correct location in /usr.
I change the cmake stuff in DunePythonCommonMacros:
find_package(Python COMPONENTS Interpreter Development)
I run dune-control on a pristine dune-common (with the above change) and I get:
-- Found Python: /usr/bin/python3.7 (found version "3.7.7") found components: Interpreter Development
In other words: Importing the newer version will make things worse.