Updated modules in find_package command for Python3 - #933

https://cmake.org/cmake/help/latest/module/FindPythonInterp.html deprecated since 3.12, while we require 3.16.2.

Current change is minimal. We probably should pass Python_EXECUTABLE as opposed to PYTHON_EXECUTABLE to dependencies, but this needs a check which ones we use. Probably easiest to pass both. I think we should no longer attempt to pass include and other paths, and let CMake sort it out.

Need to update how we look for cython I guess

[100%] Generating build/timestamp
Traceback (most recent call last):
  File "/home/runner/work/SIRF-SuperBuild/SIRF-SuperBuild/build/builds/TomoPhantom/build/Wrappers/Python/setup.py", line 19, in <module>
    from Cython.Distutils import build_ext
ModuleNotFoundError: No module named 'Cython'
gmake[5]: *** [Wrappers/Python/CMakeFiles/PythonWrapper.dir/build.make:76: Wrappers/Python/build/timestamp] Error 1
gmake[4]: *** [CMakeFiles/Makefile2:157: Wrappers/Python/CMakeFiles/PythonWrapper.dir/all] Error 2
gmake[3]: *** [Makefile:136: all] Error 2
gmake[2]: *** [CMakeFiles/TomoPhantom.dir/build.make:86: builds/TomoPhantom/stamp/TomoPhantom-build] Error 2
gmake[1]: *** [CMakeFiles/Makefile2:755: CMakeFiles/TomoPhantom.dir/all] Error 2

This error is different from why I saw in #932 but it's possible that @NicoleJurjew only saw the Tomophantom error when using this branch I guess.

With the suggested changes, the PYTHONPATH isn't exported correctly.

I think some changes need to be done here, as well:
https://github.com/SyneRBI/SIRF-SuperBuild/blob/5f1a7f498bf6f41551c19eb53798b5d33af43eec/SuperBuild.cmake#L366C1-L386C8

that's a bit strange. What is the content of env_sirf.sh etc then? Can you just do a

message(STATUS "PYTHON_EXECUTABLE=${PYTHON_EXECUTABLE}")

before those statement to check what it is?

This is the content of env_sirf.sh:

Where will that message be written to then? Thanks!

• See also Need to switch to use CMake's FindPython #313

Looks like a gadgetron build error related to https://groups.google.com/g/linux.debian.bugs.dist/c/JpHAw3GY0Bk

This was all discussed in #937. I've contributed a PR to Gadgetron. However, moving on with Gadgetron created a lot of other problems. So @paskino and I decided to wait with that, and just fix Ubuntu 22.04. This is done in #943. So suggestion therefore is to finish #943, merge it, and merge that back on here.

@casperdcl I've merged #943.

Note that we'll also need to update CHANGES.md here.

There seem to be 2 different Python versions used in the CIL install, resulting in the cil not being found at import. There are 2 causes for this.

For the SB, CMake finds a surprising version (which is then passed on)

Found Python: /opt/hostedtoolcache/Python/3.10.12/x64/bin/python3.10 (found version "3.10.12") found components: Interpreter Development Development.Module Development.Embed
Found Python_EXECUTABLE=/opt/hostedtoolcache/Python/3.10.12/x64/bin/python3.10

The CMake strategy for selecting a python version is still far too complicated/error-prone. I suggest therefore to explicitly set
Python_EXECUTABLE=${which python} in the Action.

In addition, it looks like the CIL version that is being built (I only checked with DEVEL_BUILD=OFF) uses old-style:

-- Found PythonLibs: /usr/lib/x86_64-linux-gnu/libpython3.10.so (found version "3.10.12")
-- Found PYTHON_EXECUTABLE=/home/runner/virtualenv/bin/python

I think therefore we need to add PYTHON_EXECUTABLE to the PYTHONLIBS_CMAKE_ARGS variable. That's safest in any case.

ModuleNotFoundError: No module named 'Cython'

when building TomoPhantom. Not sure why this happens now? Wrong python used? TBH, We shouldn't really build TomoPhantom as part of our CI I think. @paskino ?

Run actions/upload-artifact@v4
  with:
    name: build_log_files-ubuntu-22.04-gcc9-Release-DEVEL=OFF
...
Error: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run

I guess we have a problem with the naming somewhere.

TomoPhantom v2.0.0 (and also the slightly more recent v3.0) doesn't specify build deps. Its setup.py needs Cython & numpy. Adding them to the build step works.

Current test failures due to lack of CIL deps... CIL:scripts/requirements-test.yml are not included in CIL:pyproject.toml and I think they should be. I've opened TomographicImaging/CIL#2116

For the SB, CMake finds a surprising version (which is then passed on)

Found Python: /opt/hostedtoolcache/Python/3.10.12/x64/bin/python3.10 (found version "3.10.12") found components: Interpreter Development Development.Module Development.Embed
Found Python_EXECUTABLE=/opt/hostedtoolcache/Python/3.10.12/x64/bin/python3.10

The CMake strategy for selecting a python version is still far too complicated/error-prone. I suggest therefore to explicitly set Python_EXECUTABLE=${which python} in the Action.

This is still the case, hence the failures.

Ah I finally understood; you don't want to use the system Python.

TL;DR we need to unset the cmake helpers defined by actions/setup-python.

Ah I finally understood; you don't want to use the system Python.

TL;DR we need to unset the cmake helpers defined by actions/setup-python.

pretty confusing... I don't care which python is being used here, but obviously we should always use the same. As we seem to be using setup-python with pip, and then create a venv, we should be using that one. I guess that's what you did now, so ... great.

Sadly, some zenodo download issues ATM.

Success! Thanks @casperdcl . however, can you now have a different PR to update CHANGES.md, saying that we've switched to FindPython (maybe a link), and that they should use Python_EXECUTABLE or any other variables as indicated in the CMake doc? (Sorry, I shouldn't have approved the PR :-))

Already updated the changelog; feel free to update further

Sorry. Confused. Which changelog?

https://github.com/SyneRBI/SIRF-SuperBuild/blob/master/CHANGES.md