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

add node-pre-gyp support #170

Closed
wants to merge 2 commits into from
Closed

add node-pre-gyp support #170

wants to merge 2 commits into from

Conversation

b-ono
Copy link

@b-ono b-ono commented May 9, 2017

This PR modifies electron-rebuild to use node-pre-gyp if the binary section is present. It uses the --fallback-to-build option to build the dependency if no prebuild binary is available.

add node-pre-gyp as dependency

check if module is node-pre-gyp ready otherwise fallback to node-gyp

check if module is node-pre-gyp ready

use the module path from package json correctly
@MarshallOfSound
Copy link
Member

Can you explain exactly what this achieves, as far as I am aware node-pre-gyp is 100% incompatible with Electron binaries and this fallback-to-build is semantically equivalent to our existing node-gyp logic?

@b-ono
Copy link
Author

b-ono commented May 9, 2017

node-pre-gyp can build the packages for electronif the runtime and disturl and so on is specified correctly. It can also download the prebuild packages from the specified host.

fallback-to-build should be equivalent to the existing node-gyp logic.

@MarshallOfSound
Copy link
Member

As far as I am aware node-pre-gyp --fallback-to-build is just a complicated wrapper around node-gyp and this module actually exists to fix numerous issues with using node-pre-gyp so I'd like to avoid using it.

If this PR actually fixes a bug please raise that as an issue and we can look at solving it without bring in the pre-gyp dependency

@roytan883
Copy link

@MarshallOfSound node-pre-gyp is very useful when the module already has binary pre-build. for example, use electron with realm-js which has:

  "binary": {
    "module_name": "realm",
    "module_path": "./compiled/{node_abi}_{platform}_{arch}/",
    "host": "https://static.realm.io",
    "remote_path": "/node-pre-gyp/{version}/"
  }

so node-pre-gyp will download it from
https://static.realm.io/node-pre-gyp/2.29.2/realm-v2.29.2-electron-v4.2-win32-x64.tar.gz.
This tar.gz only include a single file realm.node already pre-build.

please consider add node-pre-gyp support. Because electron-forge depend on electron-rebuild but electron-rebuild not support node-pre-gyp.

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