pulsar-edit / superstring

Native core components for Pulsar
MIT License
1 stars 5 forks source link

Fix CI Again #8

Closed DeeDeeG closed 4 months ago

DeeDeeG commented 11 months ago

Fixing CI, Again

This is working around some stuff, mostly to do with older versions of node-gyp we're running that need workarounds for newer Python versions.

Changes, and Rationale for Changes

Changes (with rationale in the nested bullet points):

Short summary

tl;dr:

Given that GitHub Actions only finally saw fit to start using the newer macOS images for this repo, with Python 3.12, these fixes were not needed until right about now. So, without us changing anything, the newer image broke this repo's CI, and this fixes it. Also some housekeeping upgrading the actions/xyz actions to their latest semver versions at the time of this PR being opened.

Yeah, it is kind of annoying that Python keeps breaking node-gyp, and that node-gyp keeps on having to change to keep up. What more can I say about that?

DeeDeeG commented 11 months ago

This isn't really urgent for me at all, since I don't have any planned changes over at this repo any time soon, but if others are working on changes here and wondering why CI isn't passing, just merge this, and unless newer changes in the GitHub-hosted runner base images have broken our CI out from under us again since this was posted, you should have passing CI again!

savetheclocktower commented 5 months ago

OK, asking for someone with write access (like @confused-Techie) to merge this PR. Maybe it'll help get #9 working.

DeeDeeG commented 5 months ago

This PR is now quite old. I think the recent "macos-latest means macos-14" situation probably re-broke things even with these fixes, as I seem to recall building libiconv was an issue somewhere in superstring on the macos-latest runs of main pulsar-edit/pulsar repo.

EDIT: That said, if we need to get CI working in this repo, I absolutely recommend starting with this PR. It would save so much time and the work's already been done. Better than starting from scratch, am I right? The macos-14 thing is just one tiny aditional fix that might be needed over this PR as it was originally done last November.

EDIT 2: Yeah, some new macOS issues, no doubt about it. See a run I just did today: https://github.com/DeeDeeG/superstring/actions/runs/9199512008. As essentially expected.

Nevertheless, I'd suggest merging this as a great head start compared to having to reverse-engineer the problems this PR solves and the new macOS ones that have cropped up since last November.

EDIT 3: Or if you really want, give me some time and I'll fix or workaround the new macOS issues for CI purposes and we can merge it along with the fixes from November, heh. Either way, though.

DeeDeeG commented 4 months ago

Merging this as-is for now, it simplifies the path forward mentally for me, and kind of democratizes/opens up the path forward for anyone willing to take a crack at fixing this.

Also, so much time has passed it feels right to open a new PR even for commit history purposes, IMO.

Anywho, kind of arbitrary, but merging as this gives us some progress toward passing CI again, even if we know it will fail as-written once merged, sadly, due to more-recent breakage.