Switch from find_package(PythonInterp) to find_package(Python3)
CC: @sebrowne, @ccober6, @rppawlo
Parent Epic:
- #411
Description
The TriBITS module TribitsFindPythonInterp.cmake (created by Bill Spotz back in the day) currently calls find_package(PythonInterp) which uses the module FindPythonInterp.cmake. According to:
- https://cmake.org/cmake/help/v3.23/module/FindPythonInterp.html
the FindPythonInterp.cmake module has been deprecated since CMake 3.12! This should be updated to use FindPython.cmake or FindPython3.cmake.
In fact, it seems that the old FindPythonInterp.cmake module can't find versions of Python 3.0 and greater (i.e. with the name python3). This causes problems (e.g. see #607).
The new FindPython3.cmake module returns a variable called Python3_EXECUTABLE while the old FindPythonInterp.cmake module returned a variable called PYTHON_EXECUTABLE. Therefore, you can't just swap calling find_package(PythonInterp ...) with find_package(Python3 ...). Doing so would break all of the existing configure scripts for TriBITS projects that are currently setting -DPYTHON_EXECUTABLE=$(which python3) and it would break all of the internal TriBITS code that keys off of PYTHON_EXECUTABLE. So this would be a major break in backward compatibility (but the changes would be easy to absorb on both ends).
NOTE: We could make it easier for users of TriBITS projects by putting in a check for a cache var PYTHON_EXECUTABLE and error out telling users that they must instead set Python3_EXECUTABLE. Then, we could make it easier for TriBITS projects using PYTHON_EXECUTABLE in internal CMake code by setting it to an error value like PYTHON_EXECUTABLE is no longer supported! Please use Python3_EXECTUABLE instead!. And we would put in a variable_watch(PYTHON_EXECUTABLE) to and execute and error if the project tried to set PYTHON_EXECUTABLE.
Workarounds
The current workaround is to just have users manually set:
-DPYTHON_EXECUTABLE=$(which python3)
But that is not great.
NOTE: It appears that all of the Trilinos GenConfig PR builds are explicitly setting -DPYTHON_EXECUTABLE=$(which python3) (which is computed in a more verbose way). See:
https://github.com/trilinos/Trilinos/blob/1eab15637f2998d1e86fe127b78200f2c9687cb5/packages/framework/ini-files/config-specs.ini#L1805
I think it's a good thing to make the switch a soon as we can. Looks like pretty easy fixes on the trilinos side. We'll bring it up at the next leaders meeting to see if there are any worries. Thanks @bartlettroscoe !
Related to this, I am already switching over all of the TriBITS Python scripts from:
#!/usr/bin/env python
to
#!/usr/bin/env python3
That should be 100% robust on all newer machines where 'python3' is the expected standard.
This was discussed at the last Trilinos leaders meeting. No one had any worries. We would just like to send out an email to the application teams and trilinos lists warning them of the change to make sure they are ready for it.
Is there a desire to do this before the Trilinos 16.0 release? That would be a good time to do this since you are allowed to break backward compatibility with major releases.
Update: Wait, we are too late. Trilinos 16.0 was released on 7/22/2024?
Yeah, the release just went through. :(
@rppawlo, @ccober6
Wait, we are too late. Trilinos 16.0 was released on 7/22/2024?
Yeah, the release just went through. :(
Then making this change now would technically break backward compatibility and violate sematic versioning.
However, it would not be too hard to change to use find_package(Python3) internally and then provide some backward compatibility for those users who are manually setting:
-D PTYHON_EXECUTAGBLE=<some-path>
where we would set that to Python3_EXECUTABLE before calling find_package(Python3).
So we would change from find_package(PythonInterp) to find_package(Python3) and update all of the internal TriBITS and Trilinos code to switch from PYTHON_EXECUTABLE to Python3_EXECUTABLE. (And other TriBITS projects like Charon2 and Drekar would have to make this change as well.) This would break backward compatibility for TriBITS but not for external CMake project users.
So here is what I propose:
- If
PTYHON_EXECUTAGBLEis set in the cache by the user, then set it to the cache varPython3_EXECUTABLEand callmessage(WARNING "PYTHON_EXECUTABLE is set to <some-path> which is deprecated! Please set Python3_EXECUTABLE instead!") - Switch TriBITS to call
find_package(Python3)instead offind_package(PythonInterp) - Refactor all TriBITS CMake code to use
Python3_EXECUTABLEinstead ofPYTHON_EXECUTABLE - Refactor all Trilinos CMake code to use
Python3_EXECUTABLEinstead ofPYTHON_EXECUTABLE - All other TriBITS-based projects (e.g. Charon2, Drekar, etc.) would need to change to use
Python3_EXECUTABLEinstead ofPYTHON_EXECUTABLE - Customers configuring with
-D PTYHON_EXECUTAGBLE=<some-path>with updated Trilinosdevelopwould see the warning could change to set-D Python3_EXECUTABLE=<some-path>to make the warning go away. - When Trilinos 17.0 comes out, you change
message(WARNING ...)tomessage(FATAL_ERROR ...)when a user sets-D PYTHON_EXECUTABLE=<some-path>.
Does that sound like a good plan?
Sounds good to me @bartlettroscoe ! Thanks for working on this!
This was addressed with the merge of PR #616 and this was merged to Trilinos with https://github.com/trilinos/Trilinos/pull/13523. That was merged to Trilinos in Oct 16, 2024 and no complaints. Closing as complete.