Skip to content

Conversation

@DeeDeeG
Copy link
Member

@DeeDeeG DeeDeeG commented Dec 3, 2022

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

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/
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
Copy link
Member Author

DeeDeeG commented Dec 3, 2022

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

See #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.)

Copy link
Member

@confused-Techie confused-Techie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for lack of approval, lets get this merged

@DeeDeeG DeeDeeG merged commit e09cc63 into master Dec 4, 2022
@DeeDeeG
Copy link
Member Author

DeeDeeG commented Dec 4, 2022

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.

DeeDeeG added a commit to pulsar-edit/pulsar that referenced this pull request Dec 4, 2022
Includes updated Electron headers download URL.
(See: pulsar-edit/ppm#43)
DeeDeeG added a commit to pulsar-edit/pulsar that referenced this pull request Dec 5, 2022
Includes updated Electron headers download URL.
(See: pulsar-edit/ppm#43)
@DeeDeeG DeeDeeG deleted the migrate-to-new-electron-headers-URL branch January 30, 2023 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants