Update binding for electron 4.x #34

trodi · 2019-01-29T14:28:09Z

Attempting to address #32

According to the documentation already linked in the above issue, this change appears to be the fix. However, I'm unsure of the steps to build electron-edge-js so I haven't tested. I'm happy to build and test, if you can point me to build steps. Thanks.

agracio · 2019-01-29T15:33:15Z

tools\build.bat release 10.11.0

trodi · 2019-01-29T17:41:20Z

I'm running into issues attempting to run build.bat. I attempted to build on a win7 VM which I use to build other native addons (e.g., node-sqlite3). Here's what I've done.

npm i (to grab deps defined there)
npm i node-gyp -g (as build failed asking for it)
Attempted to build (I had nodejs v8.9.4 x86 installed) -> Failed
npm i windows-build-tools to grab latest in case something significant was missing
Attempted to build -> Failed
Installed nodejs v10.15.0 x86 then x64 -> Attempted to build -> Failed

I have the entire build output if helpful. The bits that stuck out to me were:
LINK : fatal error LNK1104: cannot open file 'MSCOREE.lib' [C:\devel\electron-edge-js\build\edge_nativeclr.vcxproj]

gyp ERR! stack Error: `C:\Program Files (x86)\Microsoft Visual Studio\2017\Build
Tools\MSBuild\15.0\Bin\MSBuild.exe` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onExit (C:\Users\Admin\AppData\Roaming\npm\no
de_modules\node-gyp\lib\build.js:262:23)
gyp ERR! stack     at ChildProcess.emit (events.js:182:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_proces
s.js:240:12)
gyp ERR! System Windows_NT 6.1.7601
gyp ERR! command "C:\\devel\\electron-edge-js\\lib\\native\\win32\\x64\\10.11.0\
\node.exe" "C:\\Users\\Admin\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\bin
\\node-gyp.js" "configure" "build" "--target=v4.0.0-beta.7" "--dist-url=https://
atom.io/download/electron" "--msvs_version=2017" "-release"
gyp ERR! cwd C:\devel\electron-edge-js
gyp ERR! node -v v10.11.0
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok
Error building edge.node release for node.js x64 v10.11.0

buettner123 · 2019-01-30T00:04:29Z

Just tried it out on my side and it still fails.

What I did:

npm i
tools\build.bat release 10.11.0 (went through well)
copied over the lib folder to my projects node_modules electron-edge-js folder
tried to start the app

=> Error from linked issue persists:

App threw an error during load
Error: The specified procedure could not be found.
\\?\...\dist\node_modules\electron-edge-js\lib\native\win32\x64\10.11.0\edge_nativeclr.node

Edit:

Tested with electron version v4.0.3

allisonshaw · 2019-02-18T21:57:01Z

With the updates from #32 I was able to rebuild and everything works now.

This pull request is ready to merge, @agracio . Then a new version of electron-edge-js can be released that will support Electron 4.0.0.

stereokai · 2019-02-22T11:04:47Z

@allisonshaw which updates in particular?

Hammster · 2019-02-22T12:11:22Z

@stereokai it is up to @agracio now, and since the pull is still open, there are no updates yet.

stereokai · 2019-02-22T12:51:31Z

@Hammster: @allisonshaw wrote "With the updates from #32 I was able to rebuild and everything works now.", so which updates?

I'm trying to get this PR to work on my machine, but I'm getting a node_modules\electron-edge-js\lib\native\win32\x64\10.11.0\edge_nativeclr error. (I installed directly from the latest commit in the PR).

That's why I asked :)

Update: I switched to trodi/electron-edge-js and now I'm getting this error (as reported here):

The module electron-webpack-quick-start\node_modules\electron-edge-js\lib\native\win32\x64\10.11.0\edge_nativeclr.node'
  was compiled against a different Node.js version using
  NODE_MODULE_VERSION 64. This version of Node.js requires
  NODE_MODULE_VERSION 69. Please try re-compiling or re-installing

So that happens because of this PR merged in 4.0.4:

@alexmoon This is being caused by electron/electron#16687 in electron 4.0.4.

@phhoef not sure what your issue is but 4.0.4 changes the NODE_MODULE_VERSION to 69 so any previous version works for me

Currently, it seems the pre-compiled binaries in this PR support Electron only up to version 4.0.3.

agracio · 2019-02-22T22:02:29Z

Tried rebuilding using this PR changes but so far it made no difference. Are there any more changes that are missing?

absalonv · 2019-02-23T02:39:03Z

@allisonshaw were you running Electron version 4.0.0 exactly? or newer?

stereokai · 2019-02-23T07:21:56Z

@agracio The pre-compiled binaries in this PR are working for me with Electron 4.0.3. From v4.0.4 they will not work, and will need recompilation on the user machine (see reason in my previous comment). You can try this for yourself by using npm i git://github.com/agracio/electron-edge-js.git#bb83c09ea7b01426055878fa651cdd57b9388479

agracio · 2019-02-24T13:14:34Z

I would prefer to have a concrete build process that does not rely on 'hacked' pre-compiled binaries. I will merge binaries if they are updated to 4.0.4 and will mark in README that only 4.0.4 and above are supported. But I would only do it as a one off, and would need a concrete build process steps going forward.

stereokai · 2019-02-24T15:22:16Z

These PR compiles fine in repos using Electron 4.0.0-4.0.3.

For 4.0.4+ there is a workaround until Electron devs fix electron-builder

agracio · 2019-02-24T15:54:58Z

I have used the following approach: use new byndings.gyp and build.bat from this PR, build using build.bat release 10.11.0. Then used produces binaries in https://github.com/agracio/electron-edge-js-quick-start on Electron 4.0.0. Results are exactly the same Error: The specified procedure could not be found.
I understand that if I used the binaries it would have worked, but I am talking about a standalone build process.
Am I missing something here?

allisonshaw · 2019-02-24T19:34:43Z

@stereokai updates from [this comment] (#32 (comment))
File win_delay_load_hook.cc needs a #pragma unmanaged
I made this change locally which enabled these pre-compiled binaries to be generated.
I need to go make a pull-request with this change on the node-gyp repo so building electron-edge-js doesn't require this additional step. For the moment, anyone can rebuild the binaries after adding the required pragma to.

The pre-compiled binaries on this branch were targeting 4.0.0. In 4.0.4 the version of node must have been updated (yet again) so the binaries will need to be re-compiled I;m assuming.

@agracio I will rebuild the binaries for 4.0.4 and make the change in README as requested. A future version of node-gyp will have the pragma added and the extra step won't be necessary - build process should remain as it is now.

agracio · 2019-02-24T19:54:29Z

@allisonshaw No need to change README, I meant I will do it myself, just add 4.0.4 binaries. Also make sure that changes in build.bat and binding.gyp are correct in this PR. Reading #32 (comment) it appears that this https://github.com/agracio/electron-edge-js/pull/34/files#diff-21e418851ae48f9b11907837add9e39aR22 is not needed?

allisonshaw · 2019-02-24T20:46:40Z

The changes to bindings.gyp should still be required, but I will confirm before adding the new built binaries.
I'll add the binaries tomorrow when I can rebuild and test.

stereokai · 2019-02-25T06:28:54Z

Awesome 😁

- binaries built - update bindings.gyp win_delay_load_hook true - build script targets Node 10.11.0 Electron 4.0.4

allisonshaw · 2019-02-25T19:48:22Z

All changes are complete. Ready for merge, @agracio .

agracio · 2019-03-05T20:26:07Z

Merged and published to npm.

Update binding for electron 4.x

4a31922

allisonshaw force-pushed the electron-4.x branch from 044519c to bb83c09 Compare February 18, 2019 21:53

allisonshaw force-pushed the electron-4.x branch from bb83c09 to 8b75682 Compare February 25, 2019 19:21

Support Electron 4.0.4

8e6d81d

- binaries built - update bindings.gyp win_delay_load_hook true - build script targets Node 10.11.0 Electron 4.0.4

allisonshaw force-pushed the electron-4.x branch from 8b75682 to 8e6d81d Compare February 25, 2019 19:25

agracio merged commit 3b1c1be into agracio:master Mar 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update binding for electron 4.x #34

Update binding for electron 4.x #34

trodi commented Jan 29, 2019

agracio commented Jan 29, 2019 •

edited

trodi commented Jan 29, 2019

buettner123 commented Jan 30, 2019 •

edited

allisonshaw commented Feb 18, 2019

stereokai commented Feb 22, 2019

Hammster commented Feb 22, 2019

stereokai commented Feb 22, 2019 •

edited

agracio commented Feb 22, 2019

absalonv commented Feb 23, 2019 •

edited

stereokai commented Feb 23, 2019

agracio commented Feb 24, 2019

stereokai commented Feb 24, 2019

agracio commented Feb 24, 2019 •

edited

allisonshaw commented Feb 24, 2019

agracio commented Feb 24, 2019

allisonshaw commented Feb 24, 2019

stereokai commented Feb 25, 2019

allisonshaw commented Feb 25, 2019

agracio commented Mar 5, 2019

Update binding for electron 4.x #34

Update binding for electron 4.x #34

Conversation

trodi commented Jan 29, 2019

agracio commented Jan 29, 2019 • edited

trodi commented Jan 29, 2019

buettner123 commented Jan 30, 2019 • edited

allisonshaw commented Feb 18, 2019

stereokai commented Feb 22, 2019

Hammster commented Feb 22, 2019

stereokai commented Feb 22, 2019 • edited

agracio commented Feb 22, 2019

absalonv commented Feb 23, 2019 • edited

stereokai commented Feb 23, 2019

agracio commented Feb 24, 2019

stereokai commented Feb 24, 2019

agracio commented Feb 24, 2019 • edited

allisonshaw commented Feb 24, 2019

agracio commented Feb 24, 2019

allisonshaw commented Feb 24, 2019

stereokai commented Feb 25, 2019

allisonshaw commented Feb 25, 2019

agracio commented Mar 5, 2019

agracio commented Jan 29, 2019 •

edited

buettner123 commented Jan 30, 2019 •

edited

stereokai commented Feb 22, 2019 •

edited

absalonv commented Feb 23, 2019 •

edited

agracio commented Feb 24, 2019 •

edited