-
-
Notifications
You must be signed in to change notification settings - Fork 998
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
cibuildwheel: Add macOS arm64 support #7727
Conversation
Note/Review: pypa/cibuildwheel#1926 (comment) |
b8a644f
to
2519f14
Compare
@qstokkink Can you give the created wheels a test? |
6934d07
to
bc511c5
Compare
Works on my machine. 👍 On behalf of the Tribler team, I would like to request you to keep |
bc511c5
to
9639733
Compare
|
fc34c3a
to
c8add64
Compare
Thanks 👍 and, "correct" to 2: I copied the wrong string. |
@qstokkink Just so the Tribler team are aware macOS-12 images are considered Deprecated & will be Removed by end of year! |
🤔 that's sad. It would be nice to get a PyPI release before that gets killed though. |
@xavier2k6 considering the other closed issues, it looks like you can add #7729 to your "Closes" list. |
|
@arvidn Regarding to OpenSSL 1.1.1 End Of Life
libtorrent/tools/cibuildwheel/manylinux/openssl-version.sh Lines 3 to 4 in 2ab8fed
|
6069880
to
1c08548
Compare
@arvidn I'll leave the OpenSSL 32bit installation to you, do you still want to support 32bit?? We also now have a definitive timeline for the deprecation of the macOS-12 runner |
@xavier2k6 Do you have time to experiment a bit with the win32 build? If we're lucky, it could be as simple as inserting the following at line 106 of - name: Install OpenSSL
if: ${{ endsWith(matrix.CIBW_BUILD, 'win32') }}
run: choco install openssl If all checks are green/passing, it would be easier to merge this PR, I guess. |
@qstokkink i'll be able to give that a try in |
0413058
to
1687adb
Compare
vcpkg is on the windows runners, this could be used to install openssl perhaps..... |
ca610f2
to
1c150aa
Compare
@xavier2k6 Thanks for putting some time into this. Using Lines 442 to 446 in 790b662
Lines 473 to 474 in 790b662
Using your log output, switching the four paths to this should work:
My Jam language is a bit rusty. So, this might need to be |
@qstokkink Will make some changes later, will also need to make sure we use the right package as there's 3x release options I believe: I'll have to look at vcpkg port/json files etc. vcpkg gets updated each time there's a newer windows runner image released so that will bring newer versions of OpenSSL. |
Just for motivation/verification: the |
I'm not too sure if the |
@arvidn Do you still plan/want to support 32bit? |
1c150aa
to
32bf296
Compare
@qstokkink @arvidn the 32bit OpenSSL issue is the only thing stopping this PR from being finished.....any help would be highly appreciated. I could do a |
@xavier2k6 I can help with that. Are you O.K. with me opening another PR that includes your commit? |
@qstokkink Feel free! I would suggest however to remove all other OS's from Pull Request section & just have Windows x86 & Windows x64 for testing purposes of OpenSSL....this will reduce build time/waiting on runners etc...we know the other OS's work & they can be added in via a final push. |
32bf296
to
de4f8de
Compare
@@ -101,50 +114,50 @@ jobs: | |||
id: cache-wheel | |||
with: | |||
path: wheelhouse | |||
key: wheel-${{ matrix.CIBW_BUILD }}-${{ matrix.CIBW_ARCHS }}-${{ github.sha }} | |||
key: wheel-${{ matrix.os }}-${{ matrix.CIBW_BUILD }} | |||
|
|||
- uses: docker/setup-qemu-action@v3 | |||
if: steps.cache-wheel.outputs.cache-hit != 'true' && runner.os == 'Linux' | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, maybe a separate PR is overkill. This is all you need:
- name: Install OpenSSL (win32) | |
if: ${{ endsWith(matrix.CIBW_BUILD, 'win32') }} | |
run: | | |
Remove-Item -Path "C:\Program Files\OpenSSL" -Force -Recurse | |
vcpkg install openssl:x86-windows | |
New-Item -Path "C:\Program Files\OpenSSL" -ItemType SymbolicLink -Value "C:\vcpkg\packages\openssl_x86-windows\" |
There's a mini build here: https://github.com/qstokkink/libtorrent/actions/runs/11439039359
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qstokkink Thank you!, added your change now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qstokkink Do you think we should use the same vcpkg method for x64? (consistency)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion: absolutely not. This is much slower (it adds 6~7 minutes to the job). That is way too much added time just for the sake of consistency.
de4f8de
to
01f81df
Compare
thanks everyone! |
@arvidn would it be possible before support for 1.x musl and any other support is dropped to get a pypi build for 3.12 of 1.2.19 published as well? |
Closes #7308.
Closes #7473.
Closes #7536.
Closes #7650.
Closes #7683.
Closes #7729.