Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prebuilds for Electron and Atom #61

Closed
rgbkrk opened this issue Dec 27, 2015 · 39 comments
Closed

Prebuilds for Electron and Atom #61

rgbkrk opened this issue Dec 27, 2015 · 39 comments

Comments

@rgbkrk
Copy link

rgbkrk commented Dec 27, 2015

Prebuilding across the nodejs versions as listed in targets is super easy! Thanks!

How do I handle builds against the electron headers?

@ralphtheninja
Copy link
Member

How do I handle builds against the electron headers?

Good question. I've been wanting to add some --electron flag (or similar) that would allow you to do that. No one has asked for it until now.

@max-mapper
Copy link

+1, though im curious how it would work at install time for the user. For example, if I publish a electron build to github releases for my module, when the user runs npm install, how does their process know to request the electron binary and not their require('os').platform since they are running their globally installed node + npm to install dependencies and not using electron's node to install npm dependencies.

@rgbkrk
Copy link
Author

rgbkrk commented Dec 28, 2015

Last night I was thinking about it in terms of having a given electron app have the propery binary fetch as a postinstall step to go ahead and grab the right version for the electron version.
For electron-rebuild, it shells out to detect the version of electron, assuming electron-prebuilt is a sibling package of the package - https://github.com/electronjs/electron-rebuild/blob/master/src/main.js#L42. For me, that means having electron-prebuilt be a dependency. We don't need an actual rebuild in those cases, but do require fetching the proper binary at that time.

If there's some other way to get that out of the package.json, that would be even better.

At least with node-pre-gyp, it would be a matter of asking to reinstall for that package alone:

node-pre-gyp install --target=v0.33.0 --runtime=electron

I'd like the flow to be simpler than that and you're right. When a user npm installs the package, it should just work. The hard part is knowing which runtime they're targeting.

@max-mapper
Copy link

Hmm yea I think the best way would be to use some special property in package.json that the process spawned by the install script can use to decide what binary to download.

{
  "name": "a-native-module",
  "scripts": {
    "install": "prebuild --download"
  },
  "dependencies": {
    "prebuild": "^2.3.0"
  },
  "prebuild-runtime": "electron"
}

Would that work? That way everything else with prebuild stays that same, but if prebuild-runtime is declared then it will override the default

@rgbkrk
Copy link
Author

rgbkrk commented Dec 28, 2015

I certainly like the prebuild-runtime key.

Wait a second though, wouldn't that belong in the downstream package? The native package should be agnostic of the runtime, it's the user that has a hard requirement on electron vs. raw node. What about the electronVersion key that people have been using?

@mafintosh
Copy link
Collaborator

I should be aded to the downstream package i guess. electronVersion is weird to me since we track the version using semver already in the dependency map.

@ralphtheninja
Copy link
Member

+1, though im curious how it would work at install time for the user. For example, if I publish a electron build to github releases for my module, when the user runs npm install, how does their process know to request the electron binary and not their require('os').platform since they are running their globally installed node + npm to install dependencies and not using electron's node to install npm dependencies.

You could have a postinstall step in package.json doing something like prebuild --download --electron which could try starting up electron to find out its version (https://github.com/ralphtheninja/electron-version). Using that version we can pass that on to node-gyp, like HOME=~/.electron-gyp node-gyp --target=0.XY.Z --dist-url=https://atom.io/download/atom-shell, which would download those headers, which then can be parsed to get the correct abi version, which can then map to an already prebuilt version for node, since it's the abi that's important we should be able to use already existing prebuilts for available node versions, assuming they have been built for that particular abi version.

Does that make sense? :)

@rgbkrk
Copy link
Author

rgbkrk commented Dec 28, 2015

Makes sense to me, and electron --version hands you some semver directly:

$ electron --version
v0.35.2

@havenchyk
Copy link

Guys, any proposals here?

@juliangruber
Copy link
Collaborator

small piece: how to get electron's abi version:

echo "console.log(process.versions.node);process.exit(0)" > /tmp/echo.js && ./node_modules/electron-prebuilt/cli.js --require /tmp/echo.js

@ralphtheninja
Copy link
Member

small piece: how to get electron's abi version:

Nice one!

@havenchyk
Copy link

@juliangruber wouldn't it be more simple if we ask electron (or electron-prebuilt) developers to add some way to get version with one simple command instead?

@juliangruber
Copy link
Collaborator

@havenchyk sure, but this works for now

@brett19
Copy link
Collaborator

brett19 commented Jan 14, 2016

I'm just looking at this discussion now and wondering: where is the difference between Node.js and Electron that requires custom binary packages for Electron?

@brett19
Copy link
Collaborator

brett19 commented Jan 14, 2016

So the problem is that with Electron, you have a different binary compatibility requirement, but installing a module for Electron involves using your system npm (rather than the one bundled with electron).

One possibly solution would be to add a new command-line option --electron. That way when a user does npm install XXXX --electron, we would get these values from the npm options system and know we need to do an electron install rather than basing the install parameters on the npm we happen to be installing under? This would of course require us to add a new 'target' version type to allow us to specify an electron version as well.

@juliangruber
Copy link
Collaborator

#70

@ralphtheninja
Copy link
Member

One possibly solution would be to add a new command-line option --electron. That way when a user does npm install XXXX --electron, we would get these values from the npm options system and know we need to do an electron install rather than basing the install parameters on the npm we happen to be installing under?

It's a good idea, but there's a problem doing this during the install phase since electron-prebuilt might not have been installed yet, so you have to do it in the postinstall step, afaik.

This would of course require us to add a new 'target' version type to allow us to specify an electron version as well.

If --electron is set, we can just use --target as it is, since we already know it's electron :)

@juliangruber
Copy link
Collaborator

good point about electron not yet being installed, buuuut i think the solution then is to just read the package.json and see which version is to be installed. another extra step of work this takes is managing a map between electron versions and their abi

@ralphtheninja
Copy link
Member

another extra step of work this takes is managing a map between electron versions and their abi

We could have a map for that, buuuut we could also parse out the abi version in the same way we parse it out from node's headers ;)

@ralphtheninja
Copy link
Member

If we can nail this during the install phase that would be totally awesome and save a lot of work and unnecessary downloads etc.

@rgbkrk
Copy link
Author

rgbkrk commented Jan 15, 2016

At least for me, I'm going to be packaging releases anyway instead of relying on users to install dependencies themselves if that helps. I do like making the install workflow work well for developers though, which probably means I'd put this in a prepublish step.

@brett19
Copy link
Collaborator

brett19 commented Jan 15, 2016

@ralphtheninja I started looking into this more, and I'm concerned that it may not be possible to 'safely' do it during the install step. I drew this conclusion by looking at all of the ways people use Electron, and from what I can tell there are all sorts of ways people set up their projects where you might not even be aware that they are about to attempt to use electron. For example, on windows you can download Electron to its own folder, build your whole project using Node.js and NPM, then run electron.exe to startup that app. This disconnect between the installation of an app, and the execution using electron creates a problem where we might not actually know even during postinstall...

I think we may need to solve this either through collaboration with Electron, in terms of bringing their 'standard practice' to something more electron-specific, or change prebuild's behaviour.

@brett19
Copy link
Collaborator

brett19 commented Jan 15, 2016

It does dawn on me now though, how does node-gyp know what ABI to build against when doing npm install for electron anyways?

@rgbkrk
Copy link
Author

rgbkrk commented Jan 15, 2016

how does node-gyp know what ABI to build against when doing npm install for electron anyways

It doesn't, it just relies on the current node.

node-pre-gyp takes a --runtime arg and figures out the ABI. electron-rebuild spawns electron in the same way as outlined above to get the version.

@brett19
Copy link
Collaborator

brett19 commented Jan 15, 2016

It doesn't, it just relies on the current node.

So what is the behaviour if I npm install something with the wrong node.js version, then try to startup electron?

electron-rebuild spawns electron in the same way as outlined above to get the version.

So the 'typical behaviour' for Electron is to do npm install first to install all your packages, then run electron-rebuild to go back and build everything native properly? 😮

@rgbkrk
Copy link
Author

rgbkrk commented Jan 15, 2016

That's the current state of affairs, yep!

@rgbkrk
Copy link
Author

rgbkrk commented Jan 15, 2016

So what is the behaviour if I npm install something with the wrong node.js version, then try to startup electron?

You get the ABI mismatch error at runtime. Error: Module version mismatch. Expected X, got Y.

@ralphtheninja
Copy link
Member

@brett19 Aye, this is a tough nut to crack and it will take time to get right, since as you say there are so many different ways to do this. Maybe we should try to reduce the amount of ways to do this to make it easier, like document properly and clearly state how you should do it to make it work.

Another thing that struck me, what if an electron app A depends on module B, which in turn depends on native module C (say B depends on leveldown) so it indirectly depends on a native module the A has to take measures to make sure C downloads its prebuilts (ugh). Unless C can walk upwards to find the correct version of electron.

This becomes hairy fairly quickly :)

@rgbkrk
Copy link
Author

rgbkrk commented Jan 15, 2016

That's actually the situation I'm in right now. We rely on jmp which in turn relies on the binary zmq dependency. We end up using jmp with other wrappers, which in turn are used by Electron apps.

@ralphtheninja
Copy link
Member

That's actually the situation I'm in right now. We rely on jmp which in turn relies on the binary zmq dependency. We end up using jmp with other wrappers, which in turn are used by Electron apps.

Ok, cool. So we can basically assume that this is the case, or rather we can't assume at all that A depends on B which is a native module. We have to come up with an approach that solves this generic use case, regardless of module depth, if this is going to work well.

@brett19
Copy link
Collaborator

brett19 commented Jan 15, 2016

So far, I've come to the thinking that 'enforcing' that a user specifies --electron on the npm install would be a requirement (and we would capture this downstream). Then with knowing it's an electron install, we can do our best to find the proper installation. Otherwise, we can allow the user to specify --electron 0.38.1 or --electron PATH_TO_ELECTRON, which would give us the full context necessary. Hopefully electron will come up with a less rediculous install system in the future, but this seems to be the best we can do.

P.S. We should start packaging something with prebuild that allows people to 'import' their modules in their code. For instance, when I use my native binary, I do require('bindings')('BINDING_NAME'). We should provide something to passthru lookup through prebuild. This will allow us to do internal version checking and provide a more reasonable error message to the user, along with steps to fix it. For example, instead of returning "binary module compatible with X instead of Y", we can say "Looks like you installed this module using an npm, but are running with electron, please use the --electron switch when doing npm install".

P.S.S. If we make --electron our de-factor answer, we can probably get other people on board and turn that into THE way that people solve this. Making the solution much more obvious over time.

@juliangruber
Copy link
Collaborator

The idea of passing a variable down the whole npm install process is interesting, so if we did something like ELECTRON=true npm install all the modules using prebuild would build for electron. There might be modules not using prebuild, but it's still an improvement.

This also means that an --electron option, like prebuild download --electron, won't work without manually reinstalling the module, since when you do npm install --save NATIVEMODULE there's no way to tell it to use --electron. Another reason to go with an environment variable instead.

Oh, another idea: Patch electron's require() function, so that when it requires a module that throws a version mismatch error, it recompiles it synchronously. That will work down the whole dependency tree, just a little awkward to recompile modules while your app is already starting. It could also be tricky then to reload the native module. We could move this into the npm install process by having a postinstall script that tries to require all the modules in package.json and recompiles those not working.

What do you think about the require() idea? It seems most promising to me atm.

EDIT: This can also be useful for regular node projects, when you switch node versions using a tool like nvm and need to recompile stuff. I'll start working on a POC

@ralphtheninja
Copy link
Member

@juliangruber We can use command line arguments as well with npm since npm also makes them into environment variables, which we catch here https://github.com/mafintosh/prebuild/blob/master/rc.js#L4-L17

@ralphtheninja
Copy link
Member

What do you think about the require() idea? It seems most promising to me atm.

Def interesting to explore!

@ralphtheninja
Copy link
Member

Adding this for reference Level/electron-demo#13

@rgbkrk
Copy link
Author

rgbkrk commented Sep 16, 2016

For a while, everything was awesome with Electron's ABI matching node 6 releases. Now that it's diverged quite a bit, if we want to stay updated we'll need to think back on this again.

@rgbkrk
Copy link
Author

rgbkrk commented Oct 3, 2016

As I think on this again, I think we could first support

  • Building for electron, with named releases of a similar structure to the node ones:
zmq-prebuilt-v1.4.0-electron-${effectiveABI}-darwin-x64.tar.gz

Then in userland for install they have to run electron-rebuild which would fetch the proper electron version via these prebuild semantics.

Thoughts?

@rgbkrk
Copy link
Author

rgbkrk commented Dec 5, 2016

🙇 thanks all!

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

No branches or pull requests

7 participants