kliment / Printrun

Pronterface, Pronsole, and Printcore - Pure Python 3d printing host software
GNU General Public License v3.0
2.36k stars 995 forks source link

ci: Add more macOS runners #1317

Closed rockstorm101 closed 1 year ago

rockstorm101 commented 1 year ago

Dear @dubninja, would you please be so kind to try out the macOS 12 binary produced by this PR and check if it works as expected?

@a2k-hanlon, could you please once more test whether the binary for macOS 10.15 works OK now?

a2k-hanlon commented 1 year ago

Yes, this 10.15 binary does run successfully on my 10.15.7 machine.

rockstorm101 commented 1 year ago

I managed to run the binary on an Intel MacBook with macOS 11.6. So I guess that one works as well.

However I tried running the macOS 12 binary on an M1 iMac with macOS 13.2 and it wouldn't start. As expected I guess?

a2k-hanlon commented 1 year ago

Maybe this is worth a try for M1 machine support?

https://apple.stackexchange.com/questions/438044/how-do-i-make-my-python-app-default-to-run-in-rosetta/438056#438056

Or maybe it’s a macOS 12 build to OS 13 incompatibility in which case this post probably doesn’t have the answer.

You can execute the binary file within a packaged macOS “.app” from the terminal to see some console warning messages to potentially get more information about the failure.

On Sat, Feb 18, 2023 at 7:34 AM Rock Storm @.***> wrote:

I managed to run the binary on an Intel MacBook with macOS 11.6. So I guess that one works as well.

However I tried running the macOS 12 binary on an M1 iMac with macOS 13.2 and it wouldn't start. As expected I guess?

— Reply to this email directly, view it on GitHub https://github.com/kliment/Printrun/pull/1317#issuecomment-1435701220, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANGEW7OHWCN3QEHNPJWM7K3WYDTYPANCNFSM6AAAAAAU6ZB5J4 . You are receiving this because you were mentioned.Message ID: @.***>

rockstorm101 commented 1 year ago

Hi all, I'm still unable to test the suggested trick. I'm thinking of giving this PR until the end of this month. If by then we haven't managed to test the macOS 12 binary then:

Thoughts?

DivingDuck commented 1 year ago

I just took a look on the log files: There are two entries, the first "pip install --use-pep517". I recognize this in my repository as well. It looks like we should change this in future and add --use-pep517 as mentioned. But for now not the problem.

The second one seems to be a problem - at least it do not happen with the two other runners for macOS: build (macos-12, x64, 3.10), "Build Cython ext" line 16 ff. There is an error building the gcoder_line extension.

For the question: Solution A seems to be a good compromise. The interim version for macOS12 is still available for those who want to help and test.

rockstorm101 commented 1 year ago

the first "pip install --use-pep517". I recognize this in my repository as well. It looks like we should change this in future and add --use-pep517 as mentioned.

I don't fully understand what is going on in there. I can see the deprecation warning in the logs but, is it a problem with cairocffi not following the standard practices? Is it because the wheel package is not installed? Or do we really need to enforce legacy install method? Anyway, good catch! I've created #1318 to discuss this at a later point.

at least it do not happen with the two other runners for macOS: build (macos-12, x64, 3.10), "Build Cython ext" line 16 ff. There is an error building the gcoder_line extension.

I don't see a true error there, aren't they just warnings? Interestingly, on the build for 12 the generated binary has "11.7" on its path. The one for 11 has "11.7" as well (that's fine) and the one for 10 has "10.15" (as expected).

rockstorm101 commented 1 year ago

As per this comment, the binaries produced by this change were tested with satisfactory (enough) results. So I'm merging this PR.