deps: Bump node-gyp to latest ^10.2.0 for Python 3.12 compat

[DeeDeeG]

Includes an edition of nodejs/gyp-next which vendors Python packaging library and moves away from distutils, which used to be shipped with Python, but was deprecated starting in Python 3.10, and then dropped starting in Python 3.12.

---

This solves an error -- Python 3.12 users without distutils (or else setuptools which provides distutils) on their systems would get an error like the following when trying to build native C/C++ code:

    from distutils.version import StrictVersion
ModuleNotFoundError: No module named 'distutils'

Newer gyp-next 0.16.0+ (and by extension node-gyp 10+) avoids the error by not depending on the library that was dropped from being included "out of the box" with Python as of Python 3.12. (It vendors packaging module to do its version string parsing, rather than relying on the now-missing distutils.)

[savetheclocktower]

As I understand it, the main benefit of this bump is that end users shouldn't need to install a setuptools package, but I'm having trouble verifying that it's working that way.

Here are the steps I've used:

• Check out this branch
• Rebase it against master (since I'm on arm64 and want to get the recent fix)
• yarn install
• I did an ln -s into a folder on my PATH to alias the full path to bin/ppm as ppm-test
• Temporarily removed my setuptools installation with brew uninstall python-setuptools
• ppm-test install --check fails with the following error:

> module-with-native-code@ install /Users/andrew/Code/JavaScript/ppm/native-module
> node-gyp rebuild
undefined
Traceback (most recent call last):

File "/Users/andrew/Code/JavaScript/ppm/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py", line 42, in 

import gyp  # noqa: E402

^^^^^^^^^^

File "/Users/andrew/Code/JavaScript/ppm/node_modules/npm/node_modules/node-gyp/gyp/pylib/gyp/init.py", line 9, in 

import gyp.input

File "/Users/andrew/Code/JavaScript/ppm/node_modules/npm/node_modules/node-gyp/gyp/pylib/gyp/input.py", line 19, in 

from distutils.version import StrictVersion

ModuleNotFoundError: No module named 'distutils'

gyp ERR! configure error

gyp ERR! stack Error: gyp failed with exit code: 1

gyp ERR! stack at ChildProcess.onCpExit (/Users/andrew/Code/JavaScript/ppm/node_modules/npm/node_modules/node-gyp/lib/configure.js:305:16)

gyp ERR! stack at ChildProcess.emit (node:events:365:28)

gyp ERR! stack at Process.ChildProcess._handle.onexit (node:internal/child_process:290:12)

gyp ERR! System Darwin 23.4.0

gyp ERR! command "/Users/andrew/Code/JavaScript/ppm/bin/node" "/Users/andrew/Code/JavaScript/ppm/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"

gyp ERR! cwd /Users/andrew/Code/JavaScript/ppm/native-module

gyp ERR! node -v v16.0.0

gyp ERR! node-gyp -v v9.4.0

gyp ERR! not ok

npm ERR! code ELIFECYCLE

npm ERR! errno 1

npm ERR! module-with-native-code@ install: node-gyp rebuild

npm ERR! Exit status 1

npm ERR!

npm ERR! Failed at the module-with-native-code@ install script.

npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:

npm ERR!     /Users/andrew/.pulsar/.apm/_logs/2024-09-15T20_47_14_872Z-debug.log

I had initially used npm instead of yarn and figured that was the problem, since yarn supports the resolutions field in package.json and npm doesn't. But no matter what I did, I couldn't convince npm/node-gyp to resolve to 10.2.0.

Things I've tried:

• Deleting package-lock.json before running yarn install
• Verifying the string 9.4.0 appears nowhere in yarn.lock
• Nuking node_modules and reinstalling
• Adding a "npm/node-gyp": "^10.2.0", line into the resolutions field (was just trying random stuff at this point)

I can't seem to convince yarn that npm/node-gyp should be using 10.2.0. This might need some more voodoo — or maybe we just have to update our npm-cli fork to point to 10.2.0.

[DeeDeeG]

Thank you for the review, I can confirm now, after extra effort to ensure node-gyp uses Python 3.12 on this syetm, that this "fix" does not fix the issue. (Must have been using Python 3.11 inadvertently. Which of course "just works" and doesn't even need this intervention. For best practice, I should have confirmed I could reproduce the issue without the fix in place first before pushing this PR.)

I believe the node-gyp dependency's files may be directly checked in to our forked npm repo's checked in node_modules dir, following the lead of how upstream npm did it. It's weird that upstream did it, presumably for bootstrappability and CI purposes, but I probably shouldn't have followed their lead.

It's also possible git repo

I think dropping our forked npm would fix this and make node-gyp actually resolve to the intended version. Well nope, as I just mentioned but didn't fully think through the implication when writing it out above, official npm@6.14.18 also directly checks in its node-gyp dependency as plain files directly in its repo, they are in the tarball you get from the npm registry and not overrideable by yarn overrides... Only they are even older (node-gyp 5.1.1).

CONCLUSION: Okay, so this needs some messing around with our fork of npm I guess, probably to just have a branch of it that stops directly checking in deps as plain files, because dang is that approach unintuitive and way too inflexible/weird to work with.

[DeeDeeG]

I'm pushing a rebased version on top of the latest master branch commits, then will proceed to try and use an update copy of our npm fork, https://github.com/pulsar-edit/npm-cli/releases/tag/v6.14.19-pulsar2.

[savetheclocktower]

OK! I can confirm this much: the steps that me-from-September detailed above (for which which me-from-February is forever grateful) work swimmingly this time around.

I also went into a package directory (~/.pulsar/packages/x-terminal-reloaded) and was able to do a ppm rebuild without error. I don't know if that actually does anything when the package is already properly built for a certain version of Electron, but it's a positive sign.

I admit I'm a bit curious how this will go for PulsarNext — and honestly a bit surprised that package installation works on PulsarNext at all. It's bizarre to me that, while Pulsar 1.125 runs Electron 12.2.3 and Node 14, our copy of ppm runs on Node 16, and that's fine. It's more bizarre to me that PulsarNext can run Electron 30 and Node 20, whereas ppm-next is still on Node 16 and working fine. I don't have incontrovertible proof that everything Just Works™ on PulsarNext, but I distinctly remember being able to build a version of x-terminal-reloaded for PulsarNext, so that bodes well.

If there aren't any scary implications of this version bump — like if everything still works the way it worked before — and the only benefit is that we don't have to make people install setuptools, it'll be like the earliest Christmas present. I will go out and buy a blue ribbon and affix it to nothing in particular.

[DeeDeeG]

Thank you much for taking another look at this!

The only thing I was worried about with the node-gyp bump is that they claim to stop supporting NodeJS 14 as of node-gyp 10.0.0+, but that's just because they don't want to commit to supporting an EOL version of NodeJS (understandable) and some of their deps don't promise such support either (also understandable). But apparently me from August was so gung-ho about this node-gyp bump that I decided to keep testing against NodeJS 14 anyway, engines field and support guarantees bedamned. ("I'll see to it that it's compatible, thank you very much" ??).

Brazen, bold, apparently it works.

Anyhow: Given that CI is still running against NodeJS 14, I'm not particularly worried.

---

As for verifying the fix to a high degree of certainty: Maybe I will try to reproduce the issue with Python 3.12 on master branch, then change nothing but switching to this branch to see if it fixes things. That would be a fairly strong standard of proof as I alluded to in my prior "oops" reply from September.

---

Yes, we can stop worrying about distutils, and yes, it is kind of magical. The upstream solution in gyp-next for this was so elegant, I felt like we dodged a bullet, or maybe something more potent than just a bullet. I dunno. It's late and I'm waxing poetic.

---

(Long-winded tangent time)

And yeah, as for what version of NodeJS ppm runs on... AFAIK it hasn't mattered in a long time. ppm tries to find a copy of Pulsar on-disk, determine what Electron version it is using, and download headers for/build against that Electron version as a target. It never builds for NodeJS as a target. (Outside of some test/debug scenario perhaps.) There was allegedly some package at one point which snooped on the current running version of Node during its postinstall scripts (or something like that?) but this is a rare thing that we apparently no-longer encounter, perhaps after that lone dev realized it was a bad thing to do ™️, I dunno but it hasn't been a problem or particularly relevant that I know of in a long time.

Why ppm is on 16.0.0 exactly, I'm not honestly entirely sure at this point. The justification for it was sort of thin at the time and has been even less examined since. But it's arbitrary.

We could easily change it, and I have been half-meaning to at least bump to the latest patch release of NodeJS 16, even considering that 16 is fully EOL at this point! We test on 18 as well, so that is an option. I recall being reluctant to update because they keep ballooning NodeJS in size each major version, but oh, well. They want it to include everything and the kitchen sink, to obsolete some of the most essential third-party libraries, pave the cow-paths, make it batteries-included. Makes sense, I guess. Just a bit heavy as a runtime for ppm the plucky little CLI app.

(Last I recall, some of ppm's native deps needed work to be compatible with nodeJS 20. Dunno if that bridge has since been crossed yet or not.)

[savetheclocktower]

(Last I recall, some of ppm's native deps needed work to be compatible with nodeJS 20. Dunno if that bridge has since been crossed yet or not.)

Luckily, it sounds like that's not a blocker for PulsarNext, and we can bump our npm fork to a newer version whenever it makes sense instead of being arbitrarily forced to do it.

[savetheclocktower]

So the next steps would be something like:

1. Land this PR
2. Create a PR off of master that bumps the ppm submodule to the latest
3. If CI builds a working binary from that PR, we're good to land the bump; otherwise we have to go back to the drawing board

Does that sound right, @DeeDeeG?

[savetheclocktower]

By the way: I still don't know what changed in the CI environment that explains why this stopped working… but now that I've looked into it, I'm more puzzled about how it ever worked in the first place.

When we run yarn build:apm, it does a yarn install in the ppm directory, implicitly triggering its postinstall script. That script delegates some work to either a shell script or a Windows cmd script, depending on platform. This code seems right, superficially, but it wouldn't surprise me if there were some bug here that meant a non-zero error code didn't propagate upward, or if yarn just doesn't handle that properly internally. That's seemingly why a failure in this step doesn't stop the CI job altogether or even trigger the retry logic for that step.

The shell script downloads the correct version of node, then rebuilds ppm’s native dependencies against it. It calls ./bin/npm rebuild, which itself is a wrapper script that dives into the guts of ppm. I would find it quite plausible that my attempts to set the right version of Python via npm config set python or yarn config set python were simply not getting seen by ppm… but ppm should still be able to see a PYTHON environment variable, plus there's no evidence that it's using the “wrong” Python to begin with.

(There should only be one version installed on the Windows CI image, right? I'll do a couple more quick experiments with this before I give up. I know this problem is likely going to be made irrelevant, but it would still be satisfying to be able to identify the root cause here.)

[DeeDeeG]

Copy-pasting/paraphrasing from Discord...:

I can happily report this fix works on my machine with Python 3.12.

• Before: ModuleNotFoundError: No module named 'distutils' without the fix ❌
• After: Successful yarn install (with ppm self-rebuilding during the postinstall script) with the fix. ✅

[DeeDeeG]

I would find it quite plausible that my attempts to set the right version of Python via npm config set python or yarn config set python were simply not getting seen by ppm… but ppm should still be able to see a PYTHON environment variable, plus there's no evidence that it's using the “wrong” Python to begin with.

Well, ppm has its own config files separate from npm. (And separate from yarn maybe?). Its config file is ~/.pulsar/.apm/.apmrc (global file, regenerated every ppm command from scratch, not meant to be user-editable) and ~/.pulsar/.apmrc (user config).

So, it should be ppm config set python SOME_VALUE I think.

[savetheclocktower]

I would find it quite plausible that my attempts to set the right version of Python via npm config set python or yarn config set python were simply not getting seen by ppm… but ppm should still be able to see a PYTHON environment variable, plus there's no evidence that it's using the “wrong” Python to begin with.

Well, ppm has its own config files separate from npm. (And separate from yarn maybe?). Its config file is ~/.pulsar/.apm/.apmrc (global file, regenerated every ppm command from scratch, not meant to be user-editable) and ~/.pulsar/.apmrc (user config).

So, it should be ppm config set python SOME_VALUE I think.

Right, until I dug into the code I didn't realize that ppm built itself using its own npm fork, so I figured yarn was the operative place to set it, and I think I just threw npm in there out of paranoia.