Skip to content
Snippets Groups Projects

Python libs and interpreter version.

Merged Robert K requested to merge bugfix/python-version-mixup into master

This adds a fix to make sure Python interpreter and libraries have the same version.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
    • "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.

    • Yes I agree they should be the same.

      That different versions can be found is apparently a known issue that many people have. This is fixed in newer CMake versions with FindPython3 which finds interpreter and library all in once..

    • I had a lot of issues with multiple Python versions for many different things. That's why virtualenv is so good.

    • 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
    • Please register or sign in to reply
  • Robert K added 1 commit

    added 1 commit

    • 4175d536 - [bugfix][FindPythonLibs] Python libs and interpreter should be of the same

    Compare with previous version

    • 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?

    • sounds good to use this if the detected CMake version allows. FindPython3 would probably be the right one.

    • We should try tocopy the affect Find-cmake files into dune-common. If it works, it would be the easiest and can be written in a forward-compatible manner.

    • We could try but they most probably internally use CMake features that are not available in 3.1 yet.

    • 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

    • Please register or sign in to reply
  • Andreas Dedner mentioned in merge request !790 (merged)

    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 K
  • Robert K added 336 commits

    added 336 commits

    • 4175d536...649d7caa - 335 commits from branch master
    • a51b9d48 - [bugfix][FindPythonLibs] Python libs and interpreter should be of the same

    Compare with previous version

    • 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.

    • I'll prepare an according merge request.

    • Please register or sign in to reply
  • @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.

  • Robert K added 10 commits

    added 10 commits

    • a51b9d48...cc52d788 - 9 commits from branch master
    • f2f2ac3e - [bugfix][FindPythonLibs] Python libs and interpreter should be of the same

    Compare with previous version

  • 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 K
  • A 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 K
  • Robert, go ahead with this MR.

    I will prepare another MR as suggest by Timo and myself.

  • merged

  • Robert K mentioned in commit 8d364f29

    mentioned 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.

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading