Fix CI Again

[DeeDeeG]

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):

• Install older Python (Python 3.10) for the Node 14 jobs
  • Rationale: Node 14 comes with npm 6, which comes with node-gyp 5, which is incompatible with Python 3.11+.
  • The issue this works around is fixed in gyp-next 0.7.0, which is included in node-gyp 8.0.0, which is included in npm 8.0.0. Newer npm than 8.0.0 shouldn't require this workaround.
  • Latest Node 16 (Node 16.20.2) comes with npm 8.19.4, so the workaround isn't needed other than on older Node 14
• Install Python package 'setuptools', which provides the 'distutils' module, which is needed with Python 3.12+
  • GitHub Actions' GitHub-hosted macOS runner base images started using Python 3.12 by default recently.
  • Presumably, the runner images for Ubuntu and Windows will also switch to this newer Python version eventually.
  • Pretty much a no-op on older Python, so it's very low-to-no-risk on older Python. The step python3 -m pip install setuptools simply says the 'setuptools' requirement is already satisfied, since 'setuptools' used to be bundled out of the box with these older versions of Python. It was only deprecated in Python 3.10 (if I recall correctly), and then removed in Python 3.12.
• Update all actions/xyz actions to the latest semver-major versions
  • This change is optional, CI works without it.
  • These actions run on newer NodeJS -- GitHub Actions occasionally raises the minimum NodeJS version actions can run on, so this is keeping ahead of the curve for our CI dependencies becoming deprecated or disabled in the future, forcing the upgrade then. Better to keep up now, IMO and avoid being surprised, or being stuck in a corner if the upgrade wouldn't go smoothly then.
  • These newer actions may have desirable bug-fixes I suppose??
  • To be honest, I just thought it was weird we were still on actions/setup-node@v2-beta. That action has had stable v2 and even v3 for a long time now.
• Tweak the syntax of the 'path' to not include quotation marks for actions/cache
  • Optional change, this is only useful/needed if keeping the change above that upgrades the semver major versions of actions/xyz actions.
  • This change is required as of the actions/cache@v3 update I just did in this PR branch. Otherwise, it fails to find a path literally named 'node_modules', (as if the single quote characters were literally part of the path name, not semantic quotes) and doesn't upload/restore any cache.
  • Caching is kind of pointless the way it's set up in this workflow right now, since it does npm install regardless of if the cache was restored or not, and most of the time spent during npm install is rebuilding the native C/C++ code anyway -- It's unclear to me if caching saves any time whatsoever, as CI runtime is much more dependent on how quickly the CI VMs compile the native code twice throughout the run. Which appears to be extremely variable, probably dependent on how hard other VMs running on the same hardware use the CPU/storage/network, etc. concurrently with this repo's jobs.
  • We might want to avoid doing npm install if cache restored successfully, but I didn't go for that optimization yet.
  • We might just as well disable caching, as installing the dependencies doesn't take that long, and caching in CI can cause surprising yet obscure and hard-to-notice-and-debug issues, if not done carefully. Is it worth saving a couple minutes (Windows) or ~20 seconds (Ubuntu) per run, if it might cause obscure issues down the line? Hmm. By the way, some of the runs with restored caches took longer than ones with no restored cache. So yeah, the cache is not a straight-forward benefit for this repo's CI, at least not the way it's set up right now.

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]

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]

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

[DeeDeeG]

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.

[confused-Techie]

Apologies this PR hasn't been addressed, thanks for pinging me on it @savetheclocktower.

@DeeDeeG if you'd rather try to fix this one that sounds fine by me, otherwise as a reviewer I see great reasoning and passing CI, even if I know that's sorta a lie, I'd be more than willing to get this merged

[DeeDeeG]

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.