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

ci: enable Win ARM #27

Closed
wants to merge 20 commits into from
Closed

Conversation

henryiii
Copy link
Contributor

@henryiii henryiii commented Jun 1, 2023

Will un-draft when I download the produced wheel and test locally.

@henryiii henryiii force-pushed the henryiii/ci/winarm branch from fb59530 to 46ad940 Compare June 1, 2023 16:24
Signed-off-by: Henry Schreiner <[email protected]>
@henryiii henryiii closed this Jun 2, 2023
@henryiii henryiii reopened this Jun 2, 2023
@henryiii henryiii closed this Jun 2, 2023
@henryiii henryiii reopened this Jun 2, 2023
@henryiii henryiii closed this Jun 2, 2023
@henryiii henryiii reopened this Jun 2, 2023
@henryiii henryiii closed this Jun 10, 2023
@henryiii henryiii reopened this Jun 10, 2023
@henryiii henryiii closed this Jun 10, 2023
@henryiii henryiii reopened this Jun 10, 2023
@henryiii henryiii closed this Jun 10, 2023
@henryiii henryiii reopened this Jun 10, 2023
@henryiii henryiii closed this Jun 10, 2023
@henryiii henryiii reopened this Jun 10, 2023
@henryiii henryiii closed this Jun 10, 2023
@henryiii henryiii reopened this Jun 10, 2023
@henryiii henryiii closed this Jun 10, 2023
@henryiii henryiii reopened this Jun 10, 2023
@henryiii henryiii closed this Jun 10, 2023
@henryiii henryiii reopened this Jun 10, 2023
Signed-off-by: Henry Schreiner <[email protected]>
@wjakob
Copy link
Owner

wjakob commented Aug 10, 2023

What's the status on this PR? Are we waiting for a future cibuildwheel release?

@henryiii
Copy link
Contributor Author

The problem is https://gitlab.kitware.com/cmake/cmake/-/issues/25001. Since nanobind sets the extension separately (like pybind11), and since scikit-build-core does make the correct extension available, we could fix it in nanobind but not generally; maybe that's the best way to go for now.

@wjakob wjakob force-pushed the master branch 2 times, most recently from 7c16b54 to 503b25d Compare February 14, 2024 22:08
@wjakob
Copy link
Owner

wjakob commented Mar 18, 2024

Checking in on the status of this PR as the CMake blocker issue sounds like it's unlikely to be fixed.

@henryiii
Copy link
Contributor Author

Yeah, there's two options. One would be to check and see if SKBUILD_SOABI is defined, then use that. That's always correct, and nanobind could just look for that and override if present. The other would be for scikit-build-core to cause Python to report the cross-compiled values so that FindPython picks it up, and that was hard and the PR is on hold. There continue to be good reasons to allow us to set the SOABI from scikit-build-core, so I'm still holding out hope that there might be an option someday.

But the best course of action for nanobind specifically is to check for SKBUILD_SOABI and use that if set.

@wjakob
Copy link
Owner

wjakob commented Mar 18, 2024

Like this? wjakob/nanobind#483. I am not quite following the subtleties as to where SKBUILD_SO[S]ABI and/or Python_SO[S]ABI is defined. My assumption would have been that they are equivalent when run via scikit-build-core.

@henryiii
Copy link
Contributor Author

henryiii commented Mar 18, 2024

That looks correct. They are usually equivalent when running in scikit-build-core, but if they differ, scikit-build-core is pulling the value from the running Python using the standard mechanisms (including respecting cross-compiling) while FindPython is setting Python_SOABI from a subprocess call and it's getting it incorrectly when cross-compiling.

scikit-build/scikit-build-core#355 would fix FindPython by getting the Python subprocess it calls to report the cross-compiling values, but it's tricky to get right.

@wjakob
Copy link
Owner

wjakob commented Mar 19, 2024

closing in favor of #43.

@wjakob wjakob closed this Mar 19, 2024
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