search

thelfer / MFrontGenericInterfaceSupport

This project aims at providing support for MFront generic behaviours. This project can be embedded in open-source and propriary sofware
37 stars 35 forks source link

Some CMake fixes for Python bindings. #111

Closed bilke closed 1 year ago

bilke commented 1 year ago

See commit messages for details.

Related to https://gitlab.opengeosys.org/bilke/ogs/-/commits/mfront-python.

thelfer commented 1 year ago

Hi @bilke, would you comment on the change in the detection of the python interpreter ? The current version tries to ensure that the python libraries are consistent with the interpreter. In your version, this check is removed. I don't remember the details, but the current version was done on purpose and does not seem to be a problem on all configurations tested so far.

bilke commented 1 year ago

Hi Thomas,

the CMake docs state:

If calling both find_package(PythonInterp) and find_package(PythonLibs), call find_package(PythonInterp) first to get the currently active Python version by default with a consistent version of PYTHON_LIBRARIES.

And with my fix I got consistent behaviour per default (not specifying something Python related) and also with specifying a specific python version with e.g. -DPYTHON_EXECUTABLE=python3.11. The latter did not work before as PythonLibs picked up the default Python (e.g. 3.10) and the PythonInterp did then complain that a different version (the one I provided via the CMake variable, e.g. 3.11) was found than requested.

bilke commented 1 year ago

I need to specify the PYTHON_EXECUTABLE as we integrate MGIS and TFEL into our builds and we need to make them consistent with the Python version used by the OGS build.

thelfer commented 1 year ago

@bilke Things changed over time concerning python detection in cmake. Both PythonInterp and PythonLibs are now deprecated and I am waiting to upgrade to the "new" Python module.

Anyway, many scripts that we use reliy on the definition of PYTHON_ADDITIONAL_VERSIONS to select a proper python version. Wouldn't that work for you ?

bilke commented 1 year ago

Ah thanks! I was not aware of that variable. I will let you know if that works...

bilke commented 1 year ago

Yes it seems to work! I will update this PR tomorrow and remove the python commit. I will also close the other PR.. Thanks!

bilke commented 1 year ago

I have removed the Python commit.

Regarding the test properties commit: for me I got a CMake error that set_tests_properties() was not called with right number of arguments. I guess the environment variables did not form a CMake list before.