Skip to content

ci: ios #5705

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

Merged
merged 3 commits into from
Jun 2, 2025
Merged

ci: ios #5705

merged 3 commits into from
Jun 2, 2025

Conversation

henryiii
Copy link
Collaborator

@henryiii henryiii commented May 30, 2025

Description

Builds on #5708.

Suggested changelog entry:

  • Test on iOS in CI.

@henryiii henryiii force-pushed the henryiii/fix/ios branch from 4fe7197 to bf663be Compare May 30, 2025 06:15
@henryiii henryiii changed the title fix: ios ci: ios May 30, 2025
@henryiii henryiii force-pushed the henryiii/fix/ios branch 2 times, most recently from 6c19acd to bcbab64 Compare May 30, 2025 15:48
@henryiii henryiii force-pushed the henryiii/fix/ios branch from bcbab64 to 23e1a72 Compare May 30, 2025 16:09
@henryiii
Copy link
Collaborator Author

@b-pass how hard would it be to work around thread_local, possibly via an opt-in mechanism? iOS requires an increased target to get thread_local, but you can't require it currently due to python/cpython#133183 not being in a release yet. It would also be nice to have a way to target iOS 13 (I think 17 is required for thread-local). Ideally we could detect the the target iOS version and select the correct mechanism.

@henryiii henryiii force-pushed the henryiii/fix/ios branch 2 times, most recently from 5b568e8 to 9875955 Compare May 30, 2025 23:04
@b-pass
Copy link
Contributor

b-pass commented May 30, 2025

@b-pass how hard would it be to work around thread_local, possibly via an opt-in mechanism?

The easiest thing would be to just exclude the platform from PYBIND11_HAS_SUBINTERPRETER_SUPPORT.

Does CPython's PyThread_tss_set (and friends) work on this target? pybind11 is using that else where so I am guessing yes?

If so, then I can probably make something that uses that....

@henryiii
Copy link
Collaborator Author

This might just be a problem with it defaulting to iOS 1, hopefully a new build with the upstream fix will be out soon. What I did for now was allow this to be manually overridden. 13 is was CPython itself is compiled against. If 13+ turns out to be fine, it's okay. If it does require 17+, we can document it at least, and develop a workaround if it wasn't too hard.

@henryiii
Copy link
Collaborator Author

iOS test pass! (Minus the sub interpreter ones which I disabled when building)

@henryiii
Copy link
Collaborator Author

henryiii commented May 31, 2025

I think it must handle PYBIND11_TLS_*, happy to test something in the iOS job if you have an idea.

@henryiii henryiii force-pushed the henryiii/fix/ios branch 6 times, most recently from eec1d51 to d5ef814 Compare May 31, 2025 03:49
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii henryiii force-pushed the henryiii/fix/ios branch from d5ef814 to cb6ef95 Compare May 31, 2025 19:45
@henryiii henryiii marked this pull request as ready for review June 1, 2025 04:00
@henryiii
Copy link
Collaborator Author

henryiii commented Jun 1, 2025

It would probably be easier to set up a PYBIND11_TLS_* version if this was in so it would be part of the CI, so taking out of draft.

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii henryiii merged commit 7da1d53 into pybind:master Jun 2, 2025
83 checks passed
@henryiii henryiii deleted the henryiii/fix/ios branch June 2, 2025 04:29
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Jun 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs changelog Possibly needs a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants