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

Workaround for node-pre-gyp's lack of electron runtime detection (and a little bugfixing) #44

Merged
merged 5 commits into from
Jan 9, 2016

Conversation

abread
Copy link
Contributor

@abread abread commented Jan 4, 2016

node-pre-gyp is used by some popular projects (like serialport) to provide precompiled binaries to most users. In electron's case it falls back to compilation and does so successfully.
Unfortunately, node-pre-gyp doesn't detect electron during runtime and tries to load native addons through the "normal" node path (...../node-v1234-os-arch.... instead of electron-v1234-os-arch).
Some pull requests have been submitted that fix the issue: mapbox/node-pre-gyp#177 and mapbox/node-pre-gyp#187 but they have been open for some months without any activity.

So I created a command-line switch (-p or --pre-gyp-fix) that copies the bindings' folder to the path current node-pre-gyp versions expect it to be in, when a fix is released in node-pre-gyp compatibility should not be broken and, since it's disabled by default it won't break anything to those not using it.

While developing this patch I noticed a couple of things

  • electronPath in src/cli.js is defined without taking into account the path where electron-prebuilt resides, generating an ENOENT whenever getElectronModuleVersion() is called
  • Calling toString(versionAsString) at the end of getElectronModuleVersion() turns the version string (it is returned as a string already) into a useless weird object

...so I fixed them in the process! :)

@@ -50,7 +53,7 @@ let nodeModuleVersion = null;
if (!argv.n) {
try {
let pathDotText = path.join(argv.e, 'path.txt');
electronPath = fs.readFileSync(pathDotText, 'utf8');
electronPath = path.join(argv.e, fs.readFileSync(pathDotText, 'utf8'));
Copy link
Contributor

Choose a reason for hiding this comment

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

You should use path.resolve instead of path.join, so it can handles both cases, when path.txt contains absolute or relative path.

anaisbetts added a commit that referenced this pull request Jan 9, 2016
Workaround for node-pre-gyp's lack of electron runtime detection (and a little bugfixing)
@anaisbetts anaisbetts merged commit 5867e23 into electron:master Jan 9, 2016
@anaisbetts
Copy link
Contributor

Thanks!

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