Electron-packager: Request: `electron-rebuild` plugin to use with afterCopy

[MarshallOfSound]

Currently when packaging cross-arch native node modules I am having to wrap my electron-packager calls with electron-rebuild spawn promise magic :sparkles:

An optimum solution to this issue would be for electron-packager to support a rebuildNativeModules option that would call electron-rebuild while packaging and pass in the correct arch and version parameters.

Example of how to use electron-rebuild can be found in their README and in the cli.js

I'm happy to attempt a PR for this if it is a feature that you guys feel should be part of electron-packager

[develar]

BTW, electron-builder handle native deps (if two-package.json structure is used).

Also, it is not good to depends on electron-rebuild (see nslog and babel-runtime).

[MarshallOfSound]

BTW, electron-builder does it (if two-package.json structure is used).

@develar Yeah, I saw that electron-builder supports it but I have a fundamental issue with the two-package.json structure. It's a discussion for another place, but personally it complicates the development of electron apps and makes introducing new developers very tricky. The concept that instead of having two sections in a package.json dependencies and devDependencies you have two completely separate files in different directories that call npm install on each is just weird _IMO_ Especially when you can achieve the same results with the standard package.json structure.

But yeah, that's a discussion for a different place :laughing:

Back on topic though

Also, it is not good to depends on electron-rebuild (see nslog and babel-runtime).

Yeah, nslog causes a LOT of issues, maybe this would be better served as functionality implemented in a hook such as the one I implemented here #354 and just put in this repo as a documented code sample. But I still feel it would be a great option to add assuming we can document or handle nslog tricks internally.

[malept]

This doesn't cross-compile native modules, right? I'm worried that users will imply that feature exists.

[MarshallOfSound]

@malept No it doesn't, I don't think that is fundamentally possible at all. It would definitely have to throw errors or at least warnings on an attempt to cross-compile

[develar]

on each is just weird IMO

That's why electron-builder provides install-app-deps bin script :) "postinstall": "install-app-dept", in the scripts. So, you don't need to call install in both places.

Anyway – you don't need to use electron-rebuild if you decide that this feature should be a part of electron-packager (well, prune is here) — see https://github.com/electron-userland/electron-builder/blob/master/src/util.ts#L26 method installDependencies. Code is very easy and you don't need to depends on electron-rebuild.

[develar]

This doesn't cross-compile native modules, right

No, it is not possible, AFAIK.

[MarshallOfSound]

@develar Yeah, I was just reading through the way the electron-builder does it. Do you know of any limitations of that technique. There must be a reason why electron-rebuild does things the way it does :confused:

[develar]

@MarshallOfSound electron-rebuild does the same — call npm rebuild (https://github.com/electron/electron-rebuild/blob/master/src/main.js#L105 command='rebuild').

No magic here. Plain old npm rebuild (BE AWARE — npm install is broken in this aspect — native module will be not rebuild if arch changed) and special environment.

Why electron-rebuild looks bloated? Because it is a CLI tool ;)

[malept]

Just to be clear, I am against reimplementing electron-rebuild inside electron-packager just to avoid dependencies on nslog and babel-runtime - on the other hand, they are free to reimplement in their own (maintained) module and then submit a PR here to integrate with that.

[reconbot]

If you take a project like node-serialport, I precompile binaries for all platforms and arch's. I use node-pre-gyp to distribute them in the preinstall hook, and save them in a common location. The other major prebuilt binary package manager prebuild will do the same. Each platform would need the file replaced (not the default behavior) before the package is rebuilt.

I'd gladly work to make the electron precompiled binaries available if they could be used with electron packager.

[develar]

@reconbot Yep! It is what user expected, not rebuild, but precompiled binary. Unfortunately, most of the native modules doesn't provide it (e.g. https://github.com/atom/node-keytar/issues/27).

[reconbot]

We've got an issue where the location of the precompiled binary is the same
for each platform. We "upgraded" from keeping it in a different location
for each platform and it caused electron users trouble.
https://github.com/voodootikigod/node-serialport/issues/538#issuecomment-207714692

---

Francis Gulotta
[email protected]

On Tue, May 17, 2016 at 2:15 AM, Vladimir Krivosheev <
[email protected]> wrote:

@reconbot https://github.com/reconbot Yep! It is what user expected,
not rebuild, but precompiled binary. Unfortunately, most of the native
modules doesn't provide it (e.g. atom/node-keytar#27
https://github.com/atom/node-keytar/issues/27).

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/electron-userland/electron-packager/issues/359#issuecomment-219628747

[malept]

@MarshallOfSound is this now "Request: plugin for electron-rebuild support", now that #448 is merged?

[MarshallOfSound]

@malept Yep, thanks for merging. I'll update the title of the issue and start work on the magical electron-rebuild plugin 👍

Tbh, it should be relatively straightforward 👍

[MarshallOfSound]

@malept Do you have a timeline on the next electron-packager release, using npm link is depressing 😆

[malept]

Releases are feature-driven. Here's the plan for the next release: https://github.com/electron-userland/electron-packager/milestones/7.6.0

Hopefully by the end of the week? (As always, no promises.)

[MarshallOfSound]

Progress currently blocked by: https://github.com/electron/electron-rebuild/pull/98

[MarshallOfSound]

Unblocked 👍

PR is up to electron-rebuild. https://github.com/electron/electron-rebuild/pull/120

[malept]

For reference, this should be closed when the electron-rebuild pull request has been merged & released, and a link added under "Related" in the README, preferably directly to a description of the plugin.

[deepak]

Also, it is not good to depends on electron-rebuild (see nslog and babel-runtime).

@develar electron noob here. what is the problem with babel-runtime ?

[develar]

@deepak Unnecessary dependency. Also, now, I see that it also depends on npm (heavyweight dependency). In any case you can use electron-builder — native production (only production) deps will be automatically rebuild.

[malept]

@deepak FYI, @MarshallOfSound has an active branch to refactor electron-rebuild to minimize the number of dependencies that is required. This code is based on Electron Forge's implementation, which also only rebuilds the native modules that are required by dependencies in package.json. We recommend using Electron Forge, particularly if you're currently using Electron Packager (we are also working on a migration path from other Electron build tools).

[deepak]

thanks @malept and @MarshallOfSound :-)

[malept]

Update: electron-rebuild 1.5.0 has been released with an afterCopy hook for Electron Packager. I'll close this issue once there's instructions on how to use it in the rebuild README.

[unindented]

@malept could you add instructions on how to make the two packages work together nicely?

[malept]

@unindented I was hoping @MarshallOfSound would write it 😄

[malept]

@unindented I didn't realize he'd already made a PR, here's my update of it: https://github.com/electron/electron-rebuild/pull/152

[malept]

Merged! https://github.com/electron/electron-rebuild#how-can-i-integrate-this-into-electron-packager