Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid PythonLibs and PythonInterp due to CMP0148 #1019

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aytey
Copy link

@aytey aytey commented Aug 2, 2023

This PR aims to avoid the use of PythonLibs and PythonInterp due to CMP0148, which was introduced in CMake 3.27.

Closes #1018.

@aytey
Copy link
Author

aytey commented Aug 2, 2023

As a note for something not in the description: there are other uses of PYTHON_EXECUTABLE in the scikit-build source tree, but there are no other instances of PythonLibs/PythonInterp.

I am not sure if these other uses of PYTHON_EXECUTABLE need to be fixed (and if it is simple as simply s/PYTHON_EXECUTABLE/Python_EXECUTABLE/g).

@henryiii
Copy link
Contributor

henryiii commented Aug 2, 2023

If we just hard switch, this will break all downstream packages that use the variables set by FindPythonLibs/FindPythonInterp. It will also require a really new CMake version (scikit-build-core backports FindPython from 3.26 so that it works on 3.15+). What I'd do if we really want to save FindPythonExtensions is have it respect FindPython and not trigger FindPythonLibs/Interp if FindPython has already run. That's how pybind11 handles this.

Longer term, scikit-build-core doesn't have "FindPythonExtensions". If you use FindPython, there's very little if any need to use FindPythonExtensions. It was only needed to fill in missing parts of FindPythonLibs/FindPythonInterp.

@aytey
Copy link
Author

aytey commented Aug 2, 2023

If we just hard switch, this will break all downstream packages that use the variables set by FindPythonLibs/FindPythonInterp

Yep! I totally agree ... when I started needing to make any changes to the external visibility of PYTHON_EXECUTABLE, I knew this was going to be a "breaking change" for anyone that uses scikit-build.

What I'd do if we really want to save FindPythonExtensions

I guess this is a question for you, and not one I can answer: do you?

not trigger FindPythonLibs/Interp if FindPython has already run

Seems like a very reasonable change.

That's how pybind11 handles this.

I can take a look at this and see if I can bring this change across to scikit-build.

If you use FindPython, there's very little if any need to use FindPythonExtensions

Maybe I'm using it wrong then: I was using FindPythonExtensions to get python_extension_module and python_standalone_executable -- maybe I just don't need these?

@henryiii
Copy link
Contributor

henryiii commented Aug 2, 2023

maybe I just don't need these?

For FindPython, you can use python_add_library.

@aytey
Copy link
Author

aytey commented Aug 2, 2023

Yeah, it seems that:

    add_library(${module} MODULE ${module})
    python_extension_module(${module})

Has the same result as:

    Python_add_library(${module} MODULE WITH_SOABI ${module})

However, python_standalone_executable does add some target properties that I'd need to do manually if I didn't use it.

@henryiii
Copy link
Contributor

henryiii commented Aug 2, 2023

add_executable(myexe ...)
target_link_libraries(myexe PRIVATE Python::Python)

?

@hmaarrfk
Copy link

hmaarrfk commented Dec 4, 2023

Just chiming in as a maintainer helping a few people cross compile.

It will also require a really new CMake version

I really wonder if a newer version of cmake would be in the spirit of SPEC0. All in all, those that are using old software likely in maintenance mode, and don't really need to update any of them, let alone something like scikit build.

Depending on how far you want to go:

I think that FindPython from Cmake 3.12 is thus "not really that new" and may warrant some breaking changes and a bit of cleanup. Looking at the documentation, https://cmake.org/cmake/help/latest/module/FindPython.html, it seems that most features are available since 3.19 (with the notable exception of SOABI_MODULE...) so I think it would come as a great boom.

Ultimately, cross compilation is becoming more and more important with new architectures poping up every year.

I think some breaking changes might be necessary in order to help push things forward.

https://scientific-python.org/specs/spec-0000/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CMP0148 with CMake version >= 3.27
3 participants