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

Can't build for electron on Windows #132

Closed
artch opened this issue Oct 26, 2019 · 7 comments
Closed

Can't build for electron on Windows #132

artch opened this issue Oct 26, 2019 · 7 comments

Comments

@artch
Copy link
Contributor

artch commented Oct 26, 2019

We're having issues with building isolated-vm for electron on Windows. We're trying electron 4.x branch since it ships nodejs 10. This is what I get when trying to build isolated-vm 2.0.1 against electron 4.2.12 using MSBuild 2017:

C:\Users\chivc\isolated-vm>node-gyp rebuild --target=4.2.12 --arch=x64 --dist-url=https://atom.io/electron/headers
gyp info it worked if it ends with ok
gyp info using node-gyp@6.0.0
gyp info using node@10.16.3 | win32 | x64
gyp info find Python using Python version 2.7.17 found at "C:\Python27\python.exe"
gyp info find VS using VS2017 (15.9.28307.905) found at:
gyp info find VS "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community"
gyp info find VS run with --verbose for detailed information
gyp info spawn C:\Python27\python.exe
gyp info spawn args [ 'C:\\Users\\chivc\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\gyp\\gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'msvs',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\chivc\\isolated-vm\\build\\config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\chivc\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\chivc\\AppData\\Local\\node-gyp\\Cache\\4.2.12\\include\\node\\common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=C:\\Users\\chivc\\AppData\\Local\\node-gyp\\Cache\\4.2.12',
gyp info spawn args   '-Dnode_gyp_dir=C:\\Users\\chivc\\AppData\\Roaming\\npm\\node_modules\\node-gyp',
gyp info spawn args   '-Dnode_lib_file=C:\\\\Users\\\\chivc\\\\AppData\\\\Local\\\\node-gyp\\\\Cache\\\\4.2.12\\\\<(target_arch)\\\\node.lib',
gyp info spawn args   '-Dmodule_root_dir=C:\\Users\\chivc\\isolated-vm',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'C:\\Users\\chivc\\isolated-vm\\build',
gyp info spawn args   '-Goutput_dir=.' ]
gyp info spawn C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe
gyp info spawn args [ 'build/binding.sln',
gyp info spawn args   '/clp:Verbosity=minimal',
gyp info spawn args   '/nologo',
gyp info spawn args   '/p:Configuration=Release;Platform=x64' ]
Building the projects in this solution one at a time. To enable parallel build, please add the "/m" switch.
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\nortti.vcxproj]
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\nortti.vcxproj]
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\nortti.vcxproj]
  external_copy_nortti.cc
  win_delay_load_hook.cc
  nortti.vcxproj -> C:\Users\chivc\isolated-vm\build\Release\\nortti.lib
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
  allocator.cc
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
  class_handle.cc
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
  environment.cc
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
  holder.cc
  inspector.cc
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
cl : Command line warning D9025: overriding '/GR-' with '/GR' [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
  stack_trace.cc
  three_phase_task.cc
  context_handle.cc
  external_copy.cc
  external_copy_handle.cc
  isolate.cc
  isolate_handle.cc
  lib_handle.cc
  native_module_handle.cc
  reference_handle.cc
  script_handle.cc
  module_handle.cc
  session_handle.cc
  transferable.cc
  win_delay_load_hook.cc
     Creating library C:\Users\chivc\isolated-vm\build\Release\isolated_vm.lib and object C:\Users\chivc\isolated-vm\bu
  ild\Release\isolated_vm.exp
environment.obj : error LNK2001: unresolved external symbol "public: static class v8::Platform * __cdecl v8::internal::
V8::GetCurrentPlatform(void)" (?GetCurrentPlatform@V8@internal@v8@@SAPEAVPlatform@3@XZ) [C:\Users\chivc\isolated-vm\bui
ld\isolated_vm.vcxproj]
inspector.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) public: static class std::unique_ptr<c
lass v8_inspector::V8Inspector,struct std::default_delete<class v8_inspector::V8Inspector> > __cdecl v8_inspector::V8In
spector::create(class v8::Isolate *,class v8_inspector::V8InspectorClient *)" (__imp_?create@V8Inspector@v8_inspector@@
SA?AV?$unique_ptr@VV8Inspector@v8_inspector@@U?$default_delete@VV8Inspector@v8_inspector@@@std@@@std@@PEAVIsolate@v8@@P
EAVV8InspectorClient@2@@Z) [C:\Users\chivc\isolated-vm\build\isolated_vm.vcxproj]
C:\Users\chivc\isolated-vm\build\Release\isolated_vm.node : fatal error LNK1120: 2 unresolved externals [C:\Users\chivc
\isolated-vm\build\isolated_vm.vcxproj]
gyp ERR! build error
gyp ERR! stack Error: `C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onExit (C:\Users\chivc\AppData\Roaming\npm\node_modules\node-gyp\lib\build.js:194:23)
gyp ERR! stack     at ChildProcess.emit (events.js:198:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:248:12)
gyp ERR! System Windows_NT 10.0.18362
gyp ERR! command "C:\\Program Files\\nodejs\\node.exe" "C:\\Users\\chivc\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\bin\\node-gyp.js" "rebuild" "--target=4.2.12" "--arch=x64" "--dist-url=https://atom.io/electron/headers"
gyp ERR! cwd C:\Users\chivc\isolated-vm
gyp ERR! node -v v10.16.3
gyp ERR! node-gyp -v v6.0.0
gyp ERR! not ok

Building with local Node.js 10.16.3 works fine. Building for electron on Linux also works fine.

Note: there is a small unrelated problem with node-gyp 6.0.0 which we had to overcome using this workaround.

@laverdet
Copy link
Owner

Did this work before on an earlier version of Electron or is it possible this has always been broken?

Could you give this diff a shot?

diff --git a/src/isolate/platform_delegate.h b/src/isolate/platform_delegate.h
index d6c2a97..ab031dd 100644
--- a/src/isolate/platform_delegate.h
+++ b/src/isolate/platform_delegate.h
@@ -86,7 +86,7 @@ class PlatformDelegate : public v8::Platform {
                PlatformDelegate(v8::Isolate* node_isolate, v8::Platform* node_platform) : node_isolate(node_isolate), node_platform(node_platform) {}
 
                static PlatformDelegate& DelegateInstance() {
-                       static PlatformDelegate delegate(v8::Isolate::GetCurrent(), v8::internal::V8::GetCurrentPlatform());
+                       static PlatformDelegate delegate(v8::Isolate::GetCurrent(), node::GetMainThreadMultiIsolatePlatform());
                        return delegate;
                }
 

@artch
Copy link
Contributor Author

artch commented Oct 26, 2019

With this diff now there is only one error:

inspector.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) public: static class std::unique_ptr<class v8_inspector::V8Inspector,struct std::default_delete<class v8_inspector::V8Inspector> > __cdecl v8_inspector::V8Inspector::create(class v8::Isolate *,class v8_inspector::V8InspectorClient
 *)" (__imp_?create@V8Inspector@v8_inspector@@SA?AV?$unique_ptr@VV8Inspector@v8_inspector@@U?$default_delete@VV8Inspector@v8_inspector@@@std@@@std@@PEAVIsolate@v8@@PEAVV8InspectorClient@2@@Z) [C:\Home\screeps\driver\node_modules\isolated-vm\build\isolated_vm.vcxproj]
C:\Home\screeps\driver\node_modules\isolated-vm\build\Release\isolated_vm.node : fatal error LNK1120: 1 unresolved externals [C:\Home\screeps\driver\node_modules\isolated-vm\build\isolated_vm.vcxproj]

The last working version was isolated-vm@1.5.2 with electron@2.0.0-beta.7, but we didn't try to build any intermediates.

@laverdet
Copy link
Owner

Ok I created a new electron branch for you which removes the inspector support that is bothering electron. I don't have Windows set up at the moment so it's a bit of a guessing game, but I hope that one will work.

@artch
Copy link
Contributor Author

artch commented Oct 27, 2019

That worked, thanks for the super quick response!

@artch artch closed this as completed Oct 27, 2019
@artch
Copy link
Contributor Author

artch commented Oct 27, 2019

Well no. Build worked, but the module doesn't load. This code hangs up and doesn't output anything after loading:

console.log('loading');
try {
  require('isolated-vm');
}
catch(e) {
  console.error(e);
}
console.log('loaded');

@artch artch reopened this Oct 27, 2019
@laverdet
Copy link
Owner

I took a look at this for a while today and I think it's going to be tough to get this into Electron with the changes that they've made to their build process. I need access to an internal v8 API that they've stripped from their binaries.

There's two options for now:
a) Include a plain node binary with screeps that runner will run under.
b) Ship a custom build of Electron with the extra feature reenabled.

I'm also going to reach out the nodejs team on #133 about the issue because I think it's a more general problem than one that just affects isolated-vm.

@artch
Copy link
Contributor Author

artch commented Oct 28, 2019

OK, thanks, we'll have to consider it. Meanwhile, it looks like electron 2.0.18 builds and works well with isolated-vm 1.7.5, so we'll continue shipping it.

@artch artch closed this as completed Oct 28, 2019
laverdet added a commit that referenced this issue Dec 7, 2019
This is the solution for the issue raised in #132 and discussed in #133.
Implements the API which landed in nodejs c712fb7c. We should be able to
take advantage of this starting in nodejs 14.x
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

2 participants