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

Build fails for Electron 32 / Node 20.16.0 #163

Open
Antelope-IT opened this issue Aug 21, 2024 · 7 comments
Open

Build fails for Electron 32 / Node 20.16.0 #163

Antelope-IT opened this issue Aug 21, 2024 · 7 comments

Comments

@Antelope-IT
Copy link

Antelope-IT commented Aug 21, 2024

I've cloned a latest version of the project, installed the node_modules and modified the build.bat file to build for the latest version of Electron (v32) with associated version of NodeJS (v20.16.0) according to the ElectronJS release notes.

If I then try and build the project using the command \agracio_electron_edge_js> ./tools/build.bat release 32.0.0 the build fails with the error
electron_edge_build_error

for both the i32 and x64 architectures.

If I build for a previous version <=v31 then the build succeeds as you would expect.

I'm building with NodeJS version 20.16.0, node-gyp version 10.2.0.

The change to build.bat is shown trivially below
electron_edge_build_change

I've done some analysis in order to try and resolve the issue but my knowledge of node-gyp is limited.

As far as I can see the initial problem is that despite the binding.gyp file referencing std:c++20 and LanguageStandard stdcpp20 when it generates the 3 vcxproj files in the build directory for the build_managed, edge_coreclr and edge_native targets it sets flags and options for C++17 not C++20 as below.
electron_edge_vcxproj

I've tried some experiments; by commenting out the node-gyp configure statement in build.bat this then allows me to edit the vcxproj files by hand.

Adding <LanguageStandard>stdcpp20</LanguageStandard> to the ClCompile section doesn't work because although the build recognises the request to use C++20, the C++17 shown above overrides it.

Editing the binding.gyp file so that it includes -std:c++20 in the AdditionalOptions section doesn't work because instead of replacing the C++17 flag it simply adds the c++20 flag to the end of the additional options, so both the c++17 and the c++20 flags are inserted into the additional options. This does result in the build using C++20 because the output shows a warning
cl : command line warning D9025: overriding '/std:c++17' with '/std:c++20'

However the build fails with multiple C2039 errors generated from nan.h "SetAccessor is not a memeber of.." and a C2440 error.

I've attached my ammended binding.gyp so that you can see where I've gone wrong and also the full transcript of the build output in case you are unable to reproduce it. I had to rename the gyp file to upload it.
electron_edge_js_out.txt
binding.gyp

Just to add I looked at the difference between the version 31 node/v8config.h and the version 32 of the same file - which is raising the the original >= C++20 error.and the file has changed. I'm not convinced is for the better.

From my reading the v31 file allows the use of C++17 or greater but then doesn't have the include v8-gn.h then it fails but the v32 version of the file excludes the use of C++17 or lower - forcing you to use C++20 or 23 even but then that contradicts the TODO comment above "once we only support compilers with full C++20 support. The code seems to have already implemented the requirement to use C++20 or greater but the added comment suggests its a work in progress and not done yet
v8config h_changes
I confess I don't fully understand

@agracio
Copy link
Owner

agracio commented Aug 21, 2024

From a very brief investigation the issue appears to be related to Node.js version and how it is treated in binding.gyp and node-gyp.
Building edge-js for Node.js v20.16.0 will set additional options to -std:c++17 but when building Node.js v22.6.0 it sets -std:c++20.

Just like in your build adding -std:c++20 to bindings.gyp will result in cl : command line warning D9025: overriding '/std:c++17' with '/std:c++20' for Node.js v20 but there will be no warning or force overwrite for Node.js v22.

The build itself runs using a specific version of Node.js and not a version that you have on your system so I suspect that where the problem is. I need to investigate further whether it is required for building electron-edge-js since the headers for Node.js version are specified in build command --target=%3

@agracio
Copy link
Owner

agracio commented Aug 21, 2024

Have a simple workaround in mind implementing it now.

@agracio
Copy link
Owner

agracio commented Aug 21, 2024

Unfortunately while my workaround does work the project still does not build.
When building Electron v31 there is long list of warning that come from nan.h that were not present in v30. In v32 those warning become errors.

EDIT: This appears to be an issue with nan rather than Edge.js nodejs/nan#973

v30 - no warnings

v31

utils.cpp

C:\path\electron-edge-js\node_modules\nan\nan.h(700,39): warning C4996: 'v8::Isolate::IdleNotificationDeadline' : Use MemoryPressureNotification() to influence the GC schedule. [C:\path\electron-edge-js\build\edge_nativeclr .vcxproj]
C:\path\electron-edge-js\node_modules\nan\nan.h(2654,15): warning C4996: 'v8::Object::SetAccessor': Use SetNati veDataProperty instead [C:\path\electron-edge-js\build\edge_nativeclr.vcxproj]
C:\path\electron-edge-js\node_modules\nan\nan.h(2730,56): warning C4996: 'v8::NamedPropertyHandlerConfiguration ::NamedPropertyHandlerConfiguration': Provide interceptor callbacks with new signatures instead (NamedPropertyXxxCallback)  [C:\path\electron-edge-js\build\edge_nativeclr.vcxproj]
C:\path\electron-edge-js\node_modules\nan\nan_scriptorigin.h(19,23): warning C4996: 'v8::ScriptOrigin::ScriptOr igin': Use constructor without the isolate. [C:\path\electron-edge-js\build\edge_nativeclr.vcxproj]
C:\path\electron-edge-js\node_modules\nan\nan_scriptorigin.h(23,23): warning C4996: 'v8::ScriptOrigin::ScriptOr igin': Use constructor without the isolate. [C:\path\electron-edge-js\build\edge_nativeclr.vcxproj]
C:\path\electron-edge-js\node_modules\nan\nan_scriptorigin.h(30,23): warning C4996: 'v8::ScriptOrigin::ScriptOr igin': Use constructor without the isolate. [C:\path\electron-edge-js\build\edge_nativeclr.vcxproj]

v32

utils.cpp

C:\path\electron-edge-js\node_modules\nan\nan.h(700,39): error C2039: 'IdleNotificationDeadline': is not a memb er of 'v8::Isolate' [C:\path\electron-edge-js\build\edge_nativeclr.vcxproj]
C:\path\electron-edge-js\node_modules\nan\nan.h(2560,8): error C2039: 'SetAccessor': is not a member of 'v8::Ob jectTemplate' [C:\path\electron-edge-js\build\edge_nativeclr.vcxproj]
C:\path\electron-edge-js\node_modules\nan\nan.h(2608,8): error C2039: 'SetAccessor': is not a member of 'v8::Ob jectTemplate' [C:\path\electron-edge-js\build\edge_nativeclr.vcxproj]
C:\path\electron-edge-js\node_modules\nan\nan.h(2654,15): error C2039: 'SetAccessor': is not a member of 'v8::O bject' [C:\path\electron-edge-js\build\edge_nativeclr.vcxproj]
C:\path\electron-edge-js\node_modules\nan\nan.h(2730,56): error C2440: '<function-style-cast>': cannot convert
from 'initializer list' to 'v8::NamedPropertyHandlerConfiguration' [C:\path\electron-edge-js\build\edge_nativec lr.vcxproj]
C:\path\electron-edge-js\node_modules\nan\nan.h(2800,58): error C2440: '<function-style-cast>': cannot convert
from 'initializer list' to 'v8::IndexedPropertyHandlerConfiguration' [C:\path\electron-edge-js\build\edge_nativ eclr.vcxproj]
C:\path\electron-edge-js\node_modules\nan\nan_scriptorigin.h(19,23): error C2664: 'v8::ScriptOrigin::ScriptOrig in(v8::Local<v8::Value>,int,int,bool,int,v8::Local<v8::Value>,bool,bool,bool,v8::Local<v8::Data>)': cannot convert argumen t 1 from 'v8::Isolate *' to 'v8::Local<v8::Value>' [C:\path\electron-edge-js\build\edge_nativeclr.vcxproj]
C:\path\electron-edge-js\node_modules\nan\nan_scriptorigin.h(23,23): error C2664: 'v8::ScriptOrigin::ScriptOrig in(v8::Local<v8::Value>,int,int,bool,int,v8::Local<v8::Value>,bool,bool,bool,v8::Local<v8::Data>)': cannot convert argumen t 1 from 'v8::Isolate *' to 'v8::Local<v8::Value>' [C:\path\electron-edge-js\build\edge_nativeclr.vcxproj]
C:\path\electron-edge-js\node_modules\nan\nan_scriptorigin.h(30,23): error C2664: 'v8::ScriptOrigin::ScriptOrig in(v8::Local<v8::Value>,int,int,bool,int,v8::Local<v8::Value>,bool,bool,bool,v8::Local<v8::Data>)': cannot convert argumen t 1 from 'v8::Isolate *' to 'v8::Local<v8::Value>' [C:\path\electron-edge-js\build\edge_nativeclr.vcxproj]

@agracio
Copy link
Owner

agracio commented Aug 22, 2024

build.bat updated with workaround for Electron v32 awaiting new nan release.

@agracio
Copy link
Owner

agracio commented Aug 26, 2024

Unfortunately nan project has no plans to support newer version of v8 that ships with Electron v32 or future newer versions.
It is likely (fingers crossed) that sooner or later someone will create a fork of nan with support for Electron 32.

@ivan-romanchuk-axon
Copy link

@agracio hello! I have the same problem with v31 and nan. The problem is, I have an electron-react-boilerplate and rebuild script fails on this stage. So, I can't launch an app.

@agracio
Copy link
Owner

agracio commented Sep 4, 2024

Is that related to electron-edge-js build, it should be excluded from rebuild scripts. Electron 31 builds fine using build.bat but with warnings.

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

3 participants