Add ability for PPM to run electron-rebuild automatically

[savetheclocktower]

Have you checked for existing feature requests?

• Completed

Summary

Atom occasionally needed to rebuild the dependencies of community packages. For one thing, some NPM packages don't have prebuilds for some platforms. And when Atom upgraded its own version of Electron, that would require a rebuild of the native modules of previously installed community packages so that they could work with the new Electron version.

It seems that this used to be possible simply by running ppm rebuild [package]. That command delegates to npm rebuild (using ppm's internal copy of npm, of course). And Pulsar's builtin incompatible-packages package offers a GUI for performing this task — which internally just shells out to ppm rebuild.

Nowadays, however, this doesn't seem to be enough; these tasks claim to work, but don't produce modules capable of working in Pulsar. One possible fix for this problem would be to rewrite PPM's rebuild task to delegate to electron-rebuild.

This will require two things:

1. We need a way to find the correct version of Electron for the currently installed Pulsar instance. We could just shell out to pulsar the way that the test tasks seems to do, taking care to ensure we're finding the version of the command that pairs with the version of ppm that's currently running. (In other words, don't just run pulsar and rely on PATH to find it.)
2. We need electron-rebuild to be a dependency of ppm. Currently it's only a dev dependency of Pulsar itself, while PPR doesn't require or use it at all.

I'm inclined to assume control of the existing rebuild task here because I don't think that it does anything that electron-rebuild won't be able to do, but I'm happy to discuss that if anyone is concerned about it.

The short-term goal isn't to do anything magical or fancy with this — just to make it so that ppm rebuild [package] actually does what it says on the tin, instead of being seemingly useless. If this works, we can have a discussion about whether this should be a mandatory post-install task for a given package. If electron-rebuild understands when it does not need to rebuild a module, then I don't see a downside to doing this, but we don't need to decide it now.

(Reminder to myself or someone else: before working on this, I know I need to understand the installation process better, and exactly what happens when ppm installs a package that uses native modules. For instance, I've never need to run electron-rebuild on any package that uses @savetheclocktower/atom-languageclient, even though it has a dependency with a native module. Why is my experience different from that of @mjrodgers (as reported in Discord)? He's on ARM64 and I'm on Intel, but surely a native module would need a rebuild anyway for an Electron environment, right? I should try to run (e.g.) ppm install --verbose pulsar-ide-typescript-alpha in a clean environment and pore over the output to see if I can answer these questions.)

What benefits does this feature provide?

Easier package installations.

Any alternatives?

@mauricioszabo wants a fix that doesn't depend on PPM, but that can be pursued in parallel.

Other examples:

No response

[mjrodgers]

Hmm, to clarify, julia-client, which requires electron-rebuild, does NOT use atom-languageclient, and also requires rebuilding on both arm/x86. This is the one that Pulsar offers to rebuild for me.

On the other hand pulsar-ide-python, which uses languageclient, requires node-gyp rebuild on the zadeh dependency, on arm only. Pulsar gives an error for this one, but does not offer to rebuild it automatically. (I just got a new arm laptop so was finally able to see this error firsthand, but people have reported the bug on this package, as well as ide-rust and atom-ide-outline.

For example see here.

[savetheclocktower]

Yeah, you described it quite well in Discord and I made a hash of the paraphrase.

I still don't understand when/why some packages need rebuild and some need electron-rebuild, and my confusion only deepens now that I reflect on this behavior difference between those two packages of yours.

In the case of pulsar-ide-python, I'm curious if npm rebuild does the right thing — either in the root directory of ~/.pulsar/packages/pulsar-ide-python or in the ./node_modules/zadeh subdirectory. You mentioned that ppm rebuild “acted like it succeeded” (do you mean when run in the zadeh subdirectory?), so I'm curious if whatever version of npm you have installed locally would behave differently. I hope that npm rebuild does basically the same thing as node-gyp rebuild, or else I'd feel like someone was trying to play a prank on me.

The thing that ppm rebuild claims to do — and based on the needs-native-module-rebuilding detection logic in Pulsar, I would've assumed it did do — is rebuild all the dependencies of a package, no matter whether they're direct or transitive dependencies. Whatever the difference is between needing electron-rebuild and just plain rebuild should, ideally, also be detectable in ppm so that the user doesn't need to care about (or even know about) the distinction.

Complicating things is the fact that ppm used to be written in CoffeeScript and was recently rewritten head-to-toe in ordinary JavaScript. So there's a chance that a regression was introduced at that stage. Not my leading hypothesis right now, but certainly possible; I'll dig into that later.

[mjrodgers]

The next time I update something in the python-ide package I am sure I will need to rebuild - I can try that with ppm instead of node-gyp and see how it goes. I think I will try to switch it over to your version of languageclient, that gives a good excuse to look at this.

The julia-client one could be because of something I've done wrong on my end; a simple rebuild always errors because of some optional sub-sub-dependency, I'll have to remind myself which one, so I need to run npm ci --no-optional before I rebuild; and then THAT usually fails at first too, it tells me that the package.json and the lockfile aren't in sync, so I need to delete the lockfile and run npm install first... (and I have tried doing this to rebuild the lockfile, and then pushing these changes to the repo - then it will tell me I can "update" the package within pulsar, and then the whole cycle starts over...)

I'll try to dig into which dependencies are causing problems for julia-client, I would like to try to update the whole thing to use the LSP and languageclient but I think it will be a lot of work to bring to feature-parity with Juno and I need to find the time...

[mjrodgers]

One possible issue - I think that ppm rebuild is not detecting the system architecture correctly.

I was having some trouble with x-terminal-reloaded needing to be rebuild on my arm64 machine, and after trying ppm rebuild it said Rebuild Complete but after starting Pulsar I got the following:

dlopen(/Users/gek81vuf/.pulsar/packages/x-terminal-reloaded/node_modules/@homebridge/node-pty-prebuilt-multiarch/build/Release/pty.node, 0x0001): 
tried: '/Users/gek81vuf/.pulsar/packages/x-terminal-reloaded/node_modules/@homebridge/node-pty-prebuilt-multiarch/build/Release/pty.node' 
(mach-o file, but is an incompatible architecture (have 'x86_64', need 'arm64')), 
'/System/Volumes/Preboot/Cryptexes/OS/Users/gek81vuf/.pulsar/packages/x-terminal-reloaded/node_modules/@homebridge/node-pty-prebuilt-multiarch/build/Release/pty.node' 
(no such file), 
'/Users/gek81vuf/.pulsar/packages/x-terminal-reloaded/node_modules/@homebridge/node-pty-prebuilt-multiarch/build/Release/pty.node' 
(mach-o file, but is an incompatible architecture (have 'x86_64', need 'arm64'))

Running npx @electron/rebuild -v 12.2.3 returned the same message, but resulted in the package working again.

[mjrodgers]

oh nice! tested npm rebuild on pulsar-ide-python, in the main directory, it told me rebuilt dependencies successfully and then everything worked

[DeeDeeG]

I hope that npm rebuild does basically the same thing as node-gyp rebuild, or else I'd feel like someone was trying to play a prank on me.

They sort of do the same, and the difference is a potential pitfall that is indeed confusing.

node-gyp rebuild is acting like a low-level tool that's meant to be composeable for use by higher-level interfaces, I suppose. It looks for a binding.gyp in the current working dir and tries to rebuild a project's "native addons" (C/C++ code) based on that binding.gyp file, IIRC. Only ever rebuilds up to one JS module''s native addons.

npm rebuild I think iterates through all the JS modules in node_modules dir and rebuilds anything that has native add-ons (again possibly by detecting binding.gyp files or [some_filename].node files??) (Maybe npm uses info in the current dir's package.json and/or package-lock.json to help know which modules to check, IDK? Speculating blindly here.) Recursively rebuilds any native addons it can find to rebuild in node_modules dir.

[DeeDeeG]

I think this should just work as ppm rebuild as-written. If it stopped working at some point, I'd love to trace back to when this stopped working and restore it to how it was working before.

Having to bring electron-rebuild into the picture feels like mounting half of a hydraulic press sideways to press a nail into a wall, perosnally speaking, considering we already ship a full copy of npm, the use of which should be sufficient.

[savetheclocktower]

I think this should just work as ppm rebuild as-written. If it stopped working at some point, I'd love to trace back to when this stopped working and restore it to how it was working before.

Having to bring electron-rebuild into the picture feels like mounting half of a hydraulic press sideways to press a nail into a wall, perosnally speaking, considering we already ship a full copy of npm, the use of which should be sufficient.

I'm curious if we can find a way for you to experience the effects of this issue without buying you an Apple Silicon laptop. Maybe, with a bunch of logging within ppm, we can figure out why ppm rebuild is behaving differently from npm rebuild even if we can't directly observe the bug here.

[DeeDeeG]

If I had to guess when this might have broken, perhaps it was #95?? Our CI tests might not be comprehensive enough to have undertaken such an ambitious rewrite without a ton of manual testing of every command... Hmm.

EDIT: And for the record, some manual testing was done, just not comprehensively for every sub-command, as it was looking like it'd block the PR indefinitely at the rate manual testing was going. ppm rebuild may have slipped through the cracks for testing.

[mauricioszabo]

Speaking for myself, I remember having problems rebuilding native packages since the Atom era - so I'm unsure if it's something we did, actually.

[confused-Techie]

I've been looking into this one, and I wanted to start off at the beginning as well, since you also said I need to understand the installation process better so lets do that, and try and partially answer one of our other questions at the same time.

Keep in mind the data I'll layout below is not complete, and is missing many details, I've trunicated data to only focus on what we care about.

# What happens when you type 'ppm install'

ppm.install.run()
  - runs 'installPackage()'
    - runs 'getHostedGitInfo()': Gets git related data
    - runs 'installGitPackage()': Only if the package is from git, or on the local filesystem
    - runs 'installDependencies()': Only if the command was run in the directory of a package
    - runs **'installRegisteredPackage()'**: Last resort run, the one we care about here.
      - 'requestPackage()': Gets package data from the API
      - 'getLatestCompatibleVersion()': Only runs if it can't find version info from the API 
      - 'installModule()': Firstly, starts the NPM command to install the package. 
        - 'buildModuleCache()': Requires 'rebuild-module-cache.js' and runs '.rebuild()'. Which `.rebuilds()` just gets `PULSAR/module-cache.js` and runs `.create()`
          - 'moduleCache.loadDependencies()': Collects names and versions of every module of the package. Adding it to 'moduleCache.dependencies()'
          - 'moduleCache.loadFolderCompaitbility()': Ensures validity of deps, collecting data into 'moduleCache.folders'
          - 'moduleCache.loadExtensions()': Adds data to 'moduleCache.extensions'
      - 'warmCompileCache()': Requires 'PULSAR/compile-cache.js' and runs it

So a little confusing I know, but I tried to fit a lot of data about the hierarchy of how things are running once you hit install.

The jist of it is, the beyond behavior being wildly different based on where the package comes from, we take minor steps to get native module info into Pulsar as soon as we install. But beyond anything npm might do, we take no active action from PPM to rebuild modules right after installation, at all.

But from looking at the CoffeeScript on atom/apm I don't think this behavior has really changed all that much in this time, so I'm starting to think whatever triggers Pulsar to rebuild has changed, or when we bumped NPM something changed there.

This doesn't touch at all though on why ppm rebuild doesn't work. I haven't even begun to look into it. But just thought what I'd share here, and how frustrating it was to track all of this down to find that we do nothing during installation to make sure a package works. Which seems uniquely bizarre to me

---

EDIT

This may help explain why ppm rebuild does nothing. Since literally when you run that command it gets ppm.rebuild-module-cache.js runs run() which essentially finds the package info and runs PULSAR/module-cache.js and runs .create().

So we do the exact same thing in ppm rebuild that happens during a package installation. Which from what I can see above, doesn't actively do anything then collect lots of data. But I don't see where it actually does the "rebuild" part of the process.

---

EDIT EDIT

And if what you are referring to as the rebuild part of Pulsar, I think I found it.

./src/package.js: .runRebuildProcess() which just shells out to ppm rebuild. So now truly I'm wondering where the rebuild step is. Now maybe I just missed it when reviewing the steps that PPM takes, but otherwise I'm almost wondering if there's something listening to an event in Pulsar that's supposed to run

---

A final EDIT:

I've looked into package activation on Pulsar, and deeper at the module-cache.js .create() method.
.create() Seems to aim to save native module data into the package.json of the package itself, but for some reason, I can't reproduce that in practice.

But saving this data seems to be it's full purpose.

---

So a summary of my long winded ranting.

• Running ppm install does several things, one of these things is trigger ppm rebuild
• Running ppm rebuild triggers PULSAR/module-cache.create() to run with the package data
• module-cache.create() Seems to only collect data about a package's native modules and save them to the original package's package.json under the key _atomModuleCache.

But that's it, seemingly that's all that happens during the rebuild process, with my being unable to find the link to an actual rebuild process.

But what's also interesting I've taken a look at where we use _atomModuleCache and seemingly:

• During package activation after lots of other nonsense slightly detailed in my visual guide here we call .requireMainModule().
• .requireMainModule() uses .isCompatible() to check if the package is compatible.
• .isCompatible() uses .getIncompatibleNativeModules() to check compatibility.
• .getIncompatibleNativeModules() tries to collect data as a fallback, but otherwise relies on .getNativeModuleDependencyPathsMap() which basically just returns a modified copy of the package.jsons _atomModuleCache.

The package manager determining incompatibility of a package is what triggers the native module warning, so at the very least we know everything above is functional.

---

This is actually perfectly summarized by a comment left on the original nearly unchanged (in the rebuild command at least) PR.

So apm install now adds cache details to the installed package's package.json file.

You can run apm rebuild-module-cache to generate this cache information for all installed packages.

This will be available once Atom 0.138 is out since it ships with apm 0.104

Seems ppm rebuild may have not actually been intended to rebuild modules, so much as cache details.

[savetheclocktower]

One way to test this would be to run ppm install on a package with a native module dependency.

I have a theory that direct native module dependencies of packages get built just fine, but transitive dependencies don't. That would explain why

• x-terminal-reloaded installs just fine, since node-pty is its direct dependency; but
• hydrogen doesn't — it requires a rebuild of native module zeromq (depended on by hydrogen’s dependency jmp); and
• pulsar-ide-python didn’t — it required a rebuild of native module zadeh on Apple Silicon (depended on by atom-languageclient, at least until I rewrote my fork to remove the dependency).

The symptoms in the latter two cases weren't exactly the same. In hydrogen’s case, things are a bit different, since it's installed straight from GitHub and there aren't any prebuilds or anything. And I'm still confused by the zadeh issue, to the extent that I'm not sure if prebuilds were just a red herring — it should've known how to build itself on Apple Silicon whether there were prebuilds or not.

When I buy an Apple Silicon machine (later this year, perhaps) I'm going to make sure to solve all these mysteries if they haven't yet been solved.

[DeeDeeG]

I am mildly skeptical around Hydrogen and *-ide-python since there was some effort to prebuildify things, especially zeromq IIRC, in the ide-* packages in the last busy days of those packages.

So, if ppm rebuild is doing nothing, it may be because the developers of those packages over-optimistically (frankly with some hubris) expected their prebuilds to be comprehensive enough to allow not having any fallback mechanism.

I could be wrong, and removing the need for end-users to be able to compile native modules on the fly would be an amazing goal to work toward, but it's possible we're running into something ppm can't fix, if it's an over-optimistic "prebuilds" thing?? (Sorry to jump in with little context, it's a lot to read and catch up on and fully "absorb"!)

EDIT: I suppose we could work around package authors defeating the "one true blessed way" to make native dependency building work, but I'd rather bonk those package authors into submission until they relent from breaking the mechanism that causes packages to reliably install on many diverse systems. Hypothetically, until the above might be confirmed, as it's speculation. Though if memory serves, I tried but failed to dissuade one certain past collaborator from doing exactly this, for exactly this reason. And was not able to persuade them at the time.

[DeeDeeG]

I might implore to not get distracted by install calling rebuild-module-cache. (IIRC this "module cache" thing is only recording metadata about the need or not for doing the real thing.) Only install itself, orrebuild (without mention of a cache), are doing the "real thing" of rebuilding native modules, and even then only via some implicit behaviors of the bundled npm, which I suppose may not be immediately obvious.

Indeed, the main way this is being handled is in npm itself. There is the notion of lifecycle scripts. When a package has a binding.gyp file, npm infers in this situation that it should behave as if the package's install lifecycle script is node-gyp rebuild. There's the main magic where native module dependency compilation is handled during ppm install --> npm install.

Indeed, ppm rebuild has the main effect of calling npm rebuild, which recursively runs the (potentially implicit) preinstall, install, and postinstall scripts of all the packages under [current dir]/node_modules. To recap: in the case of packages with a binding.gyp file (packages with native C/C++ modules), this should invoke node-gyp rebuild. Unless perhaps the package authors have shortsightedly sabotaged this mechanism moved fast and broken things in the name of forward progress.

---

EDIT to add (a mild ramble/rant, speculating about why these packages in particular are broken -- the above info is the important background context, IMO): I might speculate these particular packages have an explicit install or postinstall lifecycle script in their package.json (perhaps a no-op) to override the default implicit node-gyp rebuild script, as having npm blindly run node-gyp rebuild at each install, prebuilds be-darned, would potentially conflict with their prebuilt native modules. Maybe necessary for their attempt at prebuilding to be respected, and yet this could be brittle if the prebuilds are insufficient. (Again, please check and confirm this, otherwise, it's half-sort-of-remembered and very speculative and inconclusive.) If this were to be confirmed, it would indicate these packages need (IMO) a more sophisticated way to have their prebuilts be used if applicable, but still provide a fallback mechanism such as node-gyp rebuild when the prebuilds can't be used. Or we brute force it somewhat by teaching ppm itself such a sophisticated fallback, but I'd really rather not, mostly on the abstract principle of the thing. If it proves pragmatic, then maybe. Fixing it in the packages themselves would be far and away my first preference, again if I'm not off on the wrong track in diagnosing the problem entirely.