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

Install app deps always trigger rebuild #787

Closed
0181532686cf4a31163be0bf3e6bb6732bf opened this issue Sep 29, 2016 · 12 comments · Fixed by #790 · May be fixed by qcif/data-curator#563
Closed

Install app deps always trigger rebuild #787

0181532686cf4a31163be0bf3e6bb6732bf opened this issue Sep 29, 2016 · 12 comments · Fixed by #790 · May be fixed by qcif/data-curator#563
Labels

Comments

@0181532686cf4a31163be0bf3e6bb6732bf
Copy link
Contributor

  • Version: 7.10.0
  • Target: Mac and Linux

Hello, I'm using electron-builder with native module (https://github.com/lyssdod/node-libtorrent). The module itself uses node-pre-gyp and has precompiled binaries. It installs flawlessly if I'm not using electron-builder directly (running npm install from app directory works). But if I do, it fails with

Error output:
module.js:457
    throw err;
    ^

Error: Cannot find module 'adm-zip'
    at Function.Module._resolveFilename (module.js:455:15)
    at Function.Module._load (module.js:403:25)
    at Module.require (module.js:483:17)
    at require (internal/module.js:20:19)

The fun fact is, this module (adm-zip) is specified in node-libtorrent-ngs devDependencies and is handled by regular npm install just fine.

So my questions are:

  1. Why electron-builder ignores default action of node-gyp-rebuild despite binaries are available?
  2. Is there a way to debug 'install-app-deps' step?

Thanks!

@develar
Copy link
Member

develar commented Sep 30, 2016

Why electron-builder ignores default action of node-gyp-rebuild despite binaries are available?

Binaries available for nodejs, but not for electron. Electron headers must be used.

The fun fact is, this module (adm-zip) is specified in node-libtorrent-ngs devDependencies and is handled by regular npm install just fine.

We use flag --production. And adm-zip is used in the https://github.com/lyssdod/node-libtorrent/blob/master/compile.js file. @lyssdod It seems it is your bug ;)?

Is there a way to debug 'install-app-deps' step?

As general nodejs app :)

There are flags to disable rebuild, but your app will not work correctly (native dep must be compiled for Electron, not for NodeJS).

@0181532686cf4a31163be0bf3e6bb6732bf
Copy link
Contributor Author

@develar If I'll provide binaries for electron, is it possible to skip build step? What should I do to accomplish this? Handle --production flag?

@develar
Copy link
Member

develar commented Sep 30, 2016

You can set npmRebuild to false. In this case it is your responsibility to provide correct native binaries (electron version and arch).

I suggest you to investigate why adm-zip is required to build and, if it is really required, just add it to production deps.

@0181532686cf4a31163be0bf3e6bb6732bf
Copy link
Contributor Author

Thank you!

@0181532686cf4a31163be0bf3e6bb6732bf
Copy link
Contributor Author

0181532686cf4a31163be0bf3e6bb6732bf commented Oct 1, 2016

Hi, I'm still stuck here. Despite I've made separate binaries for electron (https://github.com/lyssdod/node-libtorrent/releases/tag/0.3.2), I still can't use 'em with electron-builder.

I've found out that:

  1. Setting npmRebuild to false just skips module install:
    if (this.devMetadata.build.npmRebuild === false) {

    Whole application build itself goes fine though, but if I look inside build/Release dir of my module inside app's node_modules, it's empty — looks like package was just fetched, but no npm install (which will trigger node-pre-gyp, which in turn will download binaries) was called.
  2. Somewhere npm option --build-from-source appears, which overrides default node-pre-gyp behaviour (to provide binaries in case they're available) — https://github.com/mapbox/node-pre-gyp/blob/2d57e8f33a319165c29daa0f6a396473b6b0f0b3/lib/install.js#L125
    If I try to install my module with npm install --production, everything goes smooth and node-pre-gyp fetches prebuilt binaries, but if I do npm install --production --build-from-source it fails with the same error as in the first post, because of trying to run compilation directly (without devDependencies which are ignored because of --production flag).

The obvious workarounds here are to fork node-pre-gyp to silence the --build-from-source option (so-so) or download binaries by myself (reinventing node-pre-gyp). However, I would like to find a more elegant solution. Can you point me in the right direction?

@darkshades42
Copy link

+1

@develar
Copy link
Member

develar commented Oct 2, 2016

Again — are you sure that you will be able to provide correct binaries for ALL electron versions and for ALL archs? Your module is public, not only for you, right?

Yes — we force build from sources, please see #647

@develar
Copy link
Member

develar commented Oct 2, 2016

Yes — if npmRebuild is false, it is your responsibility to install/compile/rebuild node modules.

@develar
Copy link
Member

develar commented Oct 2, 2016

To be clear — I am not against and +100 to avoid native compilation. But npm/node-pre-gyp issues make this task not easy. For example, second call of npm install will not recompile binaries if arch/headers were changed.

So, as a robust solution, electron-builder should call node-pre-gyp directly to download correct binaries or, maybe, even do it yourself. PR welcome.

@0181532686cf4a31163be0bf3e6bb6732bf
Copy link
Contributor Author

I'm pretty sure I will provide required binaries, since I'm bound to a specific electron version. When I'll switch to another version, I'll make binaries for it (though I already did that, I have binaries for 0.37.8 which's I'm currently using and for 1.2.3, which I hope to switch to). Is it possible to make --build-from-source option (#647) controlled by flag? For example, make a package.json's build.npmBuildFromSource setting which defaults to true and triggers current behavior, but when set to false it allows node-pre-gyp to behave as usual (download binaries if they're possible) ?

@develar
Copy link
Member

develar commented Oct 2, 2016

@lyssdod Could please try it — I am not sure that binaries will be downloaded correctly. I mean — fork project and investigate.

@0181532686cf4a31163be0bf3e6bb6732bf
Copy link
Contributor Author

@develar Sure! Will provide a result soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants