-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Build macOS releases #2198
Build macOS releases #2198
Conversation
Hi @disrupted, Thank you for the PR, it's real nice to improve the ease of using Black on various platforms. I'll review but expect this to be reviewed by someone else as well. I'll also approve the workflows to run, although it's really the lint job that matters. Welcome to the community! Also FYI you'll need to add a changelog entry for this enhancement. This is definitely something we want to advertise a little bit. You can read more here. Use the PR number already assigned, you don't need to use Next PR Number. @cooperlees can you test and review this PR? I can check that the GHA configuration makes sense but ultimately I don't have a MacOS running machine to test on. Thank you in advance! |
@ichard26 thanks for your feedback and warm welcome! I'll have a look at addressing the changelog tomorrow. |
My main point here before testing is this is only going to be an x86_64 binary right. So should we name it that? Does GitHub Action have the ability for a arm64 build for Apple Silicon yet? Will Apple Silicon just run the x86_64 binary with some emulator? I'll try build one and test. Luckily I have x86_64 + a arm64 Mac. |
The executable is by default is x86_64, even when building on my M1 Mac:
The Action by default also seems to use PyInstaller 4.3 and all the talk in pyinstaller/pyinstaller#5315 has people using 5.0 dev versions + some settings. This command from a member has the |
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.
Lets support arm64 + x86_64 for the binary making a universal2
MacOS Binary.
The macOS 11 runner seems to be down at the moment. |
Bummer. Sorry to waste your time. I didn't mean to do that! If this is too bleeding and does not work, I am happy to start with x86_64 only. Let's just state that somewhere in the docs and once we can lets move to a Universal binary rather than letting this great PR sit here and rot. Sound good? |
Just found this lovely /s bit of information:
So yeah, I agree with Cooper, let's get x86-64 working for now. I guess while at it, let's rename the binaries to indicate the arch and os. Old me thought the file extensions would've been enough but sadly not. |
Agreed! For now the process is rather hacky anyways and I am not too keen about adding a whole bunch of custom logic for the universal Mac build to the CI script. So I believe it's for the best to wait until PyInstaller 5.0 is stable and released. Hopefully by then the process of building universal x86/ARM binary will be a bit more straightforward. |
@disrupted just for clarity, what's the plan here? Are we waiting for PyInstaller 5 and the macOS 11.0 virtual environment to land so we can built universal binaries? Or are we going down the route Cooper suggested of just producing x86-64 macOS binaries for the time being? |
@ichard26 I'd say we can follow through as suggested to build x86 binaries for now and add ARM/universal binary support at a later point. This is almost finished btw, just gotta do the part about renaming the release artifacts and update the changelog. I was a bit distracted with some other stuff recently but hope I can finish it up soon. (Marked as a draft in the meantime) |
Ubuntu 16.04 runner environment is deprecated https://github.blog/changelog/2021-04-29-github-actions-ubuntu-16-04-lts-virtual-environment-will-be-removed-on-september-20-2021/
This reverts commit 77e8a59.
1fe7857
to
7e94b84
Compare
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.
Thanks.
I'll open 2 issues here. Build a universal2 Mac Binary + see if we can move this exe builds to Python 3.9
@@ -13,6 +13,10 @@ | |||
- Add a lower bound for the `aiohttp-cors` dependency. Only 0.4.0 or higher is | |||
supported. (#2231) | |||
|
|||
### _Packaging_ | |||
|
|||
- Release self-contained macOS binaries as part of the GitHub release pipeline (#2198) |
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.
I'll edit when I release - Will state it's only x86_64 arch.
Thank you so much for your contribution! This project is only possible by contributions like these 🖤. You're awesome, @disrupted! I couldn't do this myself when I first added this workflow due to me not owning an machine to test on. This will definitely make it easier for our users to use Black. Also congrats on your first PR here and for the PSF! Finally, I'm actually in the process of improving the contributing process so I'd love to hear your feedback, if you can spare some time checkout GH-2238 for details. Thank you once again! |
This is an addition to the existing CI releases for Linux and Windows, adding support for self-contained macOS builds.
The pipeline run can be inspected here: https://github.com/disrupted/black/runs/2504846870
The result was uploaded here: https://github.com/disrupted/black/releases/tag/21.5b2
With those three targets in place I believe it would make sense to add the target names to the binaries to avoid ambiguity. I was thinking something like this:
black_macos
black_ubuntu.elf
black_windows.exe