Closed rockstorm101 closed 4 months ago
I did only a quick view and will take a closer look on this as soon I finish the translation part.
@rockstorm101, I can't see anything against implementing for windows. The protocols are looking good and the binaries are running fine on my machine so fare I can see for now.
We should ask @neofelis2X as well for a quick look.
We should ask @neofelis2X as well for a quick look.
Sure, I tried the artifacts on intel and M2 Mac and they work flawlessly. Everything looks fine!
But there is indeed one tiny thing, that I can't unsee. On macos the system sees the app as 'pronterface' and not 'Pronterface' with capital 'P'. Renaming doesn't work, it seems to be the 'internal' name of the app.
PyInstaller Docs: "...the .. script .. supplies the name for the spec file and for the executable folder or file."
I believe this can be fixed by using the --name command with pyi-makespec (line 36) and changing pronterface.spec to Pronterface.spec / Pronterface.app Sorry if this seems too nitpicky π but it just looks off to me.
LOL, I had never seen it written with an capital P. Looks like macOS users are the better linguists. :)
I tried the artifacts on intel and M2 Mac and they work flawlessly. Everything looks fine!
Am I right in thinking that both macOS runners produce exactly the same app? I can't test them but the sizes are identical. Same goes for the generated wheels, which seem to generate the same files at a first glance:
$ tree cibw-wheels-macos-11-2
cibw-wheels-macos-11-2
βββ Printrun-2.0.1-cp310-cp310-macosx_10_9_universal2.whl
βββ Printrun-2.0.1-cp310-cp310-macosx_10_9_x86_64.whl
βββ Printrun-2.0.1-cp310-cp310-macosx_11_0_arm64.whl
βββ Printrun-2.0.1-cp311-cp311-macosx_10_9_universal2.whl
βββ Printrun-2.0.1-cp311-cp311-macosx_10_9_x86_64.whl
βββ Printrun-2.0.1-cp311-cp311-macosx_11_0_arm64.whl
βββ Printrun-2.0.1-cp312-cp312-macosx_10_9_universal2.whl
βββ Printrun-2.0.1-cp312-cp312-macosx_10_9_x86_64.whl
βββ Printrun-2.0.1-cp312-cp312-macosx_11_0_arm64.whl
βββ Printrun-2.0.1-cp38-cp38-macosx_10_9_universal2.whl
βββ Printrun-2.0.1-cp38-cp38-macosx_10_9_x86_64.whl
βββ Printrun-2.0.1-cp38-cp38-macosx_11_0_arm64.whl
βββ Printrun-2.0.1-cp39-cp39-macosx_10_9_universal2.whl
βββ Printrun-2.0.1-cp39-cp39-macosx_10_9_x86_64.whl
βββ Printrun-2.0.1-cp39-cp39-macosx_11_0_arm64.whl
1 directory, 15 files
$ tree cibw-wheels-macos-12-3
cibw-wheels-macos-12-3
βββ Printrun-2.0.1-cp310-cp310-macosx_10_9_universal2.whl
βββ Printrun-2.0.1-cp310-cp310-macosx_10_9_x86_64.whl
βββ Printrun-2.0.1-cp310-cp310-macosx_11_0_arm64.whl
βββ Printrun-2.0.1-cp311-cp311-macosx_10_9_universal2.whl
βββ Printrun-2.0.1-cp311-cp311-macosx_10_9_x86_64.whl
βββ Printrun-2.0.1-cp311-cp311-macosx_11_0_arm64.whl
βββ Printrun-2.0.1-cp312-cp312-macosx_10_9_universal2.whl
βββ Printrun-2.0.1-cp312-cp312-macosx_10_9_x86_64.whl
βββ Printrun-2.0.1-cp312-cp312-macosx_11_0_arm64.whl
βββ Printrun-2.0.1-cp38-cp38-macosx_10_9_universal2.whl
βββ Printrun-2.0.1-cp38-cp38-macosx_10_9_x86_64.whl
βββ Printrun-2.0.1-cp38-cp38-macosx_11_0_arm64.whl
βββ Printrun-2.0.1-cp39-cp39-macosx_10_9_universal2.whl
βββ Printrun-2.0.1-cp39-cp39-macosx_10_9_x86_64.whl
βββ Printrun-2.0.1-cp39-cp39-macosx_11_0_arm64.whl
1 directory, 15 files
Yes they are effectively the same. I donβt have a device with macos 11, so I canβt test if it makes a difference there specifically.
Yes they are effectively the same.
Thanks. I've removed the macOS 11 runner then.
On macos the system sees the app as 'pronterface' and not 'Pronterface' with capital 'P'. [...] I believe this can be fixed by using the --name command with pyi-makespec (line 36) and changing pronterface.spec to Pronterface.spec / Pronterface.app
Could you please give a try to the latest artifact, see if it now shows up with a capital 'P' now?
Hi @rockstorm101, already did! Perfect, the latest artifact 'printrun-test_macos-12_x64_py3.10' works. And the naming is now consistent. :D
I consider this PR merge-ready now. Should anyone present know of any reason that this PR should not be merged, speak now (or next week) or forever hold your peace.
Two major changes on this PR:
cibuildwheel
. The use of the current action 'RalfG/python-wheels-manylinux-build' is deprecated in favor ofPyPA/cibuildwheel
. Hopefully this merge will ease maintenance of the PyPI builds plus it allows us to easily produce wheels to support Apple Silicon and ARM architectures.I labeled this PR as a Work In Progress as it will need testing particularly on Windows and macOS before it can be merged with confidence. I did not detect any issues on Linux so far, but I did not test wheels on architectures other than amd64. If it is too much of a change it can wait to be integrated on the next release cycle.
As always, this is open for discussion. Any and every feedback is appreciated.