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: use dev version of FindPython #102

Merged
merged 3 commits into from
Feb 10, 2023
Merged

fix: use dev version of FindPython #102

merged 3 commits into from
Feb 10, 2023

Conversation

henryiii
Copy link
Collaborator

@henryiii henryiii commented Nov 17, 2022

This is now a port, see https://gitlab.kitware.com/cmake/cmake/-/issues/24185. Based on VTK's port at https://gitlab.kitware.com/vtk/vtk/-/blob/master/CMake/patches/3.23/FindPython/Support.cmake.

Support for the correct SOABI on PyPy and the Stable ABI are included.

Closes #148.

@henryiii henryiii closed this Nov 17, 2022
@henryiii henryiii reopened this Nov 17, 2022
@henryiii
Copy link
Collaborator Author

henryiii commented Nov 18, 2022

CMake Error at C:/Users/runneradmin/AppData/Local/Temp/tmpr2c79_me/build/CMakeFiles/CMakeScratch/TryCompile-1a28id/CMakeLists.txt:2 (set):
  Syntax error in cmake code at

    C:/Users/runneradmin/AppData/Local/Temp/tmpr2c79_me/build/CMakeFiles/CMakeScratch/TryCompile-1a28id/CMakeLists.txt:2

  when parsing string

    C:\hostedtoolcache\windows\Python\3.11.0\x64\Lib\site-packages\scikit_build_core\resources\find_python

  Invalid character escape '\h'.

The slashes are backwards in try-compile. I wonder if I'm injecting the wrong sort of path?

@henryiii henryiii force-pushed the henryiii/fix/pypy branch 3 times, most recently from dad26b4 to df3b468 Compare November 23, 2022 14:45
@codecov
Copy link

codecov bot commented Nov 23, 2022

Codecov Report

Merging #102 (c142321) into main (bf86f79) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #102      +/-   ##
==========================================
+ Coverage   90.29%   90.34%   +0.05%     
==========================================
  Files          44       44              
  Lines        1844     1844              
==========================================
+ Hits         1665     1666       +1     
+ Misses        179      178       -1     
Impacted Files Coverage Δ
src/scikit_build_core/settings/skbuild_model.py 100.00% <100.00%> (ø)
src/scikit_build_core/builder/builder.py 96.59% <0.00%> (+1.13%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@LecrisUT
Copy link
Collaborator

Maybe some cmake code needs to be passed through cmake_path or on python side?

@henryiii henryiii force-pushed the henryiii/fix/pypy branch 2 times, most recently from 5bafdf4 to 18cccd1 Compare January 26, 2023 17:27
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

tests: update
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii
Copy link
Collaborator Author

henryiii commented Jan 26, 2023

@jcfr or @thewtex would you like to check this - specifically fa987eb? This applies the one relevant change in VTK's port, and a couple of changes based on usage of new features.

@henryiii henryiii marked this pull request as ready for review January 26, 2023 19:37
@jcfr
Copy link
Contributor

jcfr commented Jan 28, 2023

Thanks for working on this 🙏

I should be able to review in the few days👌

@henryiii
Copy link
Collaborator Author

Okay, great. Let me know if you have any ideas for extra ways to test it; I'm already doing some basic testing, but it's not checking all paths through this file (and I think quite a few paths may never get hit due to the way we use this always knowing what Python interpreter we want). If it looks reasonable, it can go in - next version will be 0.2.

@henryiii
Copy link
Collaborator Author

Let's go ahead and put this in, and then it can be tweaked and adjusted later. I'd like to lean towards release early, release often, and get at least an dev release out for people to try in the next few days.

@henryiii henryiii merged commit 094ad9e into main Feb 10, 2023
@henryiii henryiii deleted the henryiii/fix/pypy branch February 10, 2023 22:15
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.

Custom FindPython backport
3 participants