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

Use CMake 3.12+ PYBIND11_FINDPYTHON #57

Closed
wants to merge 7 commits into from
Closed

Conversation

devw4r
Copy link
Contributor

@devw4r devw4r commented Sep 17, 2023

@devw4r devw4r changed the title Use CMake 1.12+ PYBIND11_FINDPYTHON Use CMake 3.12+ PYBIND11_FINDPYTHON Sep 17, 2023
@namreeb
Copy link
Owner

namreeb commented Sep 17, 2023

Looks fine to me. @gtker thoughts?

@devw4r
Copy link
Contributor Author

devw4r commented Sep 17, 2023

Not sure why linux build fails, any ideas?

@gtker
Copy link

gtker commented Sep 17, 2023

I'll try to investigate the Linux issue.

Why is enabling PYBIND11_FINDPYTHON necessary? Are the wheels not using the correct Python versions?

@gtker
Copy link

gtker commented Sep 17, 2023

@devw4r

It seems like this is a known issue reported in pybind/pybind11#4802, and scikit-build/scikit-build-core#472.

The problem seems to be that manylinux doesn't ship Development.Embed. There seems to be three ways to fix this on Linux:

  1. Add AND NOT UNIX to ${CMAKE_VERSION} VERSION_GREATER "3.12.0".
  2. Put find_package(Python3 REQUIRED COMPONENTS Interpreter Development.Module) inside the CMake version if statement that you added, just before add_subdirectory(pybind11).
  3. Update pybind11 to the commit in this open pull request.

I'm guessing the changes in this PR are related to Windows, if that's the case I would just add AND NOT UNIX to disable it for Apple/Linux since I haven't had any issues finding Python.

@devw4r
Copy link
Contributor Author

devw4r commented Sep 17, 2023

This PR was made for Linux, which was resolving 3.10 instead of 3.11.5.

@gtker
Copy link

gtker commented Sep 17, 2023

How do I verify that the problem has been fixed?

@devw4r
Copy link
Contributor Author

devw4r commented Sep 17, 2023

How do I verify that the problem has been fixed?

Not sure how to test, for me, adding this PR fixed python resolving locally.

Before:
image

After:
Screenshot from 2023-09-17 12-16-46

image

@devw4r devw4r marked this pull request as draft September 17, 2023 19:02
@gtker
Copy link

gtker commented Sep 17, 2023

How do I verify that the problem has been fixed?

Not sure how to test, for me, adding this PR fixed python resolving locally.

I'm not sure if that's so much a bug as it's just the CMake scripts having different priorities. For me on Ubuntu 22.04 with python3, python3-dev, python3.11, and python3.11-dev packages installed CMake consistently picks the 3.11 version over the default 3.10.

I'm actually unsure of what the intended way of choosing the Python version is, since most ways seem to just be to set the required Python3_* variables to the specific version which is a little hacky.

@devw4r devw4r closed this Sep 19, 2023
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.

3 participants