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

fix: support pybind11 2.10 #45

Closed
wants to merge 17 commits into from
Closed

fix: support pybind11 2.10 #45

wants to merge 17 commits into from

Conversation

henryiii
Copy link
Collaborator

@henryiii henryiii commented Aug 1, 2022

No description provided.

@henryiii
Copy link
Collaborator Author

henryiii commented Aug 8, 2022

To be clear, this is just showing that it's broken. Don't know what the fix is yet. :)

@henryiii henryiii marked this pull request as draft August 8, 2022 19:12
@henryiii
Copy link
Collaborator Author

This has stopped picking up PyPy and is getting CPython instead: adding 'scikit_build_example/_core.cpython-37m-x86_64-linux-gnu.so'.

CMakeLists.txt Outdated Show resolved Hide resolved
@henryiii
Copy link
Collaborator Author

Checking to see if setting PythonLibsNew_FIND_VERSION in pybind11 2.10 was a problem when looking for PyPy. Couldn't mask it out so just setting it with pybind11 2.9 instead.

@henryiii
Copy link
Collaborator Author

Nope, that wasn’t it

@henryiii
Copy link
Collaborator Author

Oh, wait, it might not have this var. Will try something else later.

henryiii and others added 3 commits September 22, 2022 08:58
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@henryiii
Copy link
Collaborator Author

Okay, I know what's wrong. The problem is the cache dir _skbuild is being persisted between runs. Now that pybind11 is respecting the "old" cache value for these settings, it's getting the last non-PyPy values filled in when it starts building for PyPy. This is the first place we notice a break (though it could happen elsewhere, values from a previous run are not always going to be "safe" to use!)

The core problem is we can't tell if someone passed -D..., in which case we want to respect it, and if it is just cached from a previous run. A few ideas for fixes:

  • We delete the cached directory between cibuildwheel runs. I did that for linux above. Has to be done by every user, though.
  • We have an opt-in flag for overriding the values. Not horrible, but introduces a flag (and only needed for the "classic" FindPythonIntep form).
  • We find some way to tell a previous run from a user-set value. This is ideal if I can come up with a way to do it.
  • We change scikit-build to include the interpreter path in the cache dir, like setuptools does. Longer term solution, but is a generally good idea, I think - though ideally configurable. More likely to be integrated into scikit-build-core long-term than scikit-build short-term.

henryiii and others added 2 commits October 19, 2022 22:41
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii henryiii deleted the henryiii-patch-1 branch August 10, 2023 03:02
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.

2 participants