pulsar-edit / ppm

Pulsar Package Manager
MIT License
35 stars 13 forks source link

src: Update Electron header download URL #43

Closed DeeDeeG closed 1 year ago

DeeDeeG commented 1 year ago

The old atom.io Electron headers URL is going down, per the updated sunset announcement.

Likewise, the old gh-contractor-zcbenz Amazon AWS/S3 bucket is no-longer guaranteed to be active, and may go away at any time.

(See: https://github.blog/2022-06-08-sunsetting-atom/#if-im-using-atom-what-changes-can-i-expect-after-the-sunset and https://www.electronjs.org/blog/s3-bucket-change)

So, this commit updates our package manager to get the Electron headers from the new 'artifacts.electronjs.org/headers/dist' location.

This is the correct, official place to get the Electron headers, according to the blog post above at electronjs.org.

Ensures we can continue to build native C/C++ code for certain Atom/Pulsar packages that use it.

(For example: several of the language packages have C/C++ addon code.)

DeeDeeG commented 1 year ago

By the way: tests are failing here for the same reason they are failing on master branch.

See https://github.com/pulsar-edit/ppm/pull/42 for the fix to that.

Getting somewhat off-topic for this present PR, to talk about #42 (click to expand): These test failures have to do with the Node 16 bump, and I think it specifically has to do with trying to use old node-gyp to build C/C++ code for older Node (spec fixture version of Node is 10.12.1) while running much newer Node 16.0.0. I think newer node-gyp has fixes for this, by the way. But we can't really take advantage of that without also bumping the bundled npm, since our bundled npm 6 specifies the older node-gyp in its own package.json.)
DeeDeeG commented 1 year ago

Copy-pasting from Discord, testing notes:

I tested, and it is working for me locally. (Also tested with other deliberately incorrect URLs to confirm that breaks it, to disprove the "null hypothesis" that my change wasn't doing anything. BTW, you can do just https://electronjs.org/headers and it will redirect, but that causes unnecessary HTTP 301 and HTTP 307 redirect messages to go back and forth across the wire and redirect to the actual final location, hurting perf and wasting some bits of network usage. So I went with this direct URL that responds with HTTP 200, no redirects needed. ✔️ )

Best way I know of to test this is to use the changed code to install a Pulsar package that includes a native C/C++ module. For example, I run ppm install --verbose language-carp since language-carp is a small package that builds a native C/C++ add-on as part of its install process. (No, I don't know anything about the Carp language. It's just a convenient package to test native module building against.)

The --verbose flag includes a lot of info, including every network request needed to fetch Electron headers. You may need to delete existing headers cached under ~/.pulsar/.node-gyp/.cache/node-gyp, because if you've already downloaded these once, node-gyp skips re-downloading. They are cached for efficient re-use. So make sure to delete those between test runs, or this URL is irrelevant since no actual requests for the header files go out over the wire for this URL, in case of an existing cached header tarball.