search

atom / node-keytar

Native Password Node Module
https://atom.github.io/node-keytar
MIT License
1.37k stars 194 forks source link

Unable to compile for electron target: caused by recent node-addon-api 3.1.0 => 3.2.0 update #386

Open vladimiry opened 3 years ago

vladimiry commented 3 years ago

Steps to Reproduce

HOME=~/.electron-gyp node-gyp rebuild --target=13.0.0-beta.27 --arch=x64 --dist-url=https://electronjs.org/headers

Gives:

  CXX(target) Release/obj.target/keytar/src/async.o
In file included from ../node_modules/node-addon-api/napi.h:2725,
                 from ../src/async.cc:4:
../node_modules/node-addon-api/napi-inl.h: In member function ‘bool Napi::Object::Freeze()’:
../node_modules/node-addon-api/napi-inl.h:1393:24: error: ‘napi_object_freeze’ was not declared in this scope; did you mean ‘napi_object_expected’?
 1393 |   napi_status status = napi_object_freeze(_env, _value);
      |                        ^~~~~~~~~~~~~~~~~~
      |                        napi_object_expected
../node_modules/node-addon-api/napi-inl.h: In member function ‘bool Napi::Object::Seal()’:
../node_modules/node-addon-api/napi-inl.h:1399:24: error: ‘napi_object_seal’ was not declared in this scope; did you mean ‘napi_object’?
 1399 |   napi_status status = napi_object_seal(_env, _value);
      |                        ^~~~~~~~~~~~~~~~
      |                        napi_object
make: *** [keytar.target.mk:121: Release/obj.target/keytar/src/async.o] Error 1

Works fine if I downgrade node-addon-api 3.2.0 => 3.1.0.

shiftkey commented 3 years ago

Not sure I understand the issue, as we're using 3.1.0 in the lockfile:

https://github.com/atom/node-keytar/blob/12484088ce0980a903627b6fc7ddaaf02188e982/package-lock.json#L2206-L2210

And our dependency is not 3.2.0 specifically:

https://github.com/atom/node-keytar/blob/12484088ce0980a903627b6fc7ddaaf02188e982/package.json#L53

There's some extra context here about how we're prebuild-ing for N-API now https://github.com/atom/node-keytar/pull/331 so I'm not sure if that's somehow affected things

shiftkey commented 3 years ago

I was also able to do this successfully with the latest version on macOS (using our version of node-gyp, not the global one):

$ ./node_modules/.bin/node-gyp rebuild --target=13.0.0-beta.27 --arch=x64 --dist-url=https://electronjs.org/headers
gyp info it worked if it ends with ok
gyp info using node-gyp@7.1.2
gyp info using node@14.15.1 | darwin | x64
gyp info find Python using Python version 3.9.5 found at "/usr/local/opt/python@3.9/bin/python3.9"
gyp http GET https://electronjs.org/headers/v13.0.0-beta.27/node-v13.0.0-beta.27.tar.gz
gyp http 200 https://electronjs.org/headers/v13.0.0-beta.27/node-v13.0.0-beta.27.tar.gz
gyp http GET https://electronjs.org/headers/v13.0.0-beta.27/SHASUMS256.txt
gyp http 200 https://electronjs.org/headers/v13.0.0-beta.27/SHASUMS256.txt
gyp info spawn /usr/local/opt/python@3.9/bin/python3.9
gyp info spawn args [
gyp info spawn args   '/Users/shiftkey/src/node-keytar/node_modules/node-gyp/gyp/gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'make',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/shiftkey/src/node-keytar/build/config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/shiftkey/src/node-keytar/node_modules/node-gyp/addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/shiftkey/Library/Caches/node-gyp/13.0.0-beta.27/include/node/common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=/Users/shiftkey/Library/Caches/node-gyp/13.0.0-beta.27',
gyp info spawn args   '-Dnode_gyp_dir=/Users/shiftkey/src/node-keytar/node_modules/node-gyp',
gyp info spawn args   '-Dnode_lib_file=/Users/shiftkey/Library/Caches/node-gyp/13.0.0-beta.27/<(target_arch)/node.lib',
gyp info spawn args   '-Dmodule_root_dir=/Users/shiftkey/src/node-keytar',
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   'build',
gyp info spawn args   '-Goutput_dir=.'
gyp info spawn args ]
gyp info spawn make
gyp info spawn args [ 'BUILDTYPE=Release', '-C', 'build' ]
  CXX(target) Release/obj.target/keytar/src/async.o
  CXX(target) Release/obj.target/keytar/src/main.o
  CXX(target) Release/obj.target/keytar/src/keytar_mac.o
  SOLINK_MODULE(target) Release/keytar.node
gyp info ok
vladimiry commented 3 years ago

Not sure I understand the issue, as we're using 3.1.0 in the lockfile:

Lock file doesn't make sense if the module is installed from npm (since lock files don't get published to npm), so the new node-addon-api version got into the game by "node-addon-api": "^3.0.0" range. I just don't use prebuilds and this is how the issue got noticed.

I was also able to do this successfully with the latest version on macOS (using our version of node-gyp, not the global one):

I've tried both 8.0.0 (globally installed) and ./node_modules/.bin/node-gyp (7.1.2), the result is the same. Try it with node-addon-api@3.2.0.

latest version on macOS

Maybe the issue is OS-specific. I don't have macOS system to test it in my own.

PalmerAL commented 3 years ago

I'm having the same problem; I think it only fails with newer versions of node. With node 14.11.0 installed, I can build successfully; with later versions, it fails.

14.12.0 15.12.0 is the first version to support NAPI v8 (source), and the node-addon-api commit that introduced the error is conditioned on v8 being available: https://github.com/nodejs/node-addon-api/commit/77350eee98957f471a355a4cf12f6ee05262fa5b. However, I still don't really understand why it fails with NAPI v8.

@shiftkey - if you install the latest version of node and then try to build, can you see the issue?

gaetandezeiraud commented 3 years ago

I'm on Node v14.17.1 and I have the same problem @PalmerAL

vladimiry commented 3 years ago

I just pinned keytar/node-addon-api@3.1.0 as a workaround for use with node 15.

vladimiry commented 3 years ago

Btw, node-addon-api v4 was released about 2 weeks ago and it's also affected.

Wiltuk commented 3 years ago

I was having this issue on Windows with node 14.17.1 - I found the error does not occur with keytar version 7.6.0 so it seems to be something in the 7.7.0 release.

gaetandezeiraud commented 3 years ago

@Wiltuk I have the same problem with keytar 7.6.0 on my side. Event after deleting node_modules and lock file.

felipe-linares commented 3 years ago

+1 have this exact issue on macOS. Can fix by running on node 14 <= 14.16.1, but our team needs 15+ for ARM builds of our electron application. Any insight on this would be highly appreciated

EDIT: It seems that what @PalmerAL said is spot on. Using Node 15 <15.12.0 works fine, so it must be something with NAPIv8 is causing this.

gaetandezeiraud commented 3 years ago

No workaround with the lastest LTS version of node?

vladimiry commented 3 years ago

@Brouilles see the issue title, ie downgrading node-addon-api to 3.1.0 is a workaround.

felipe-linares commented 3 years ago

Any update on this? Some of our other dependencies (lodash in particular) don't support node 15, only node 14/16. But we need 15+ for ARM builds so 16 would be our only option, but because of node-addon-api issues it doesn't work on node 16...

Ranjith-Eswaran-G commented 2 years ago

@vladimiry , I am also facing the same issue. I updated the keytar version from 7.3.0 to 7.7.0. I also installed node-addon-api version 3.1.0. But still I am facing the same issue. May I know what was your work around?

vladimiry commented 2 years ago

May I know what was your work around?

The workaround was in downgrading node-addon-api to 3.1.0 but I removed downgrade in my project some time ago as the issue got fixed somewhere upstream (currently used node-addon-api is 3.2.1).

So I believe the issue can be closed now.

Ranjith-Eswaran-G commented 2 years ago

The workaround was in downgrading node-addon-api to 3.1.0 but I removed downgrade in my project some time ago as the issue got fixed somewhere upstream

May I know your current Electron and Node version?

vladimiry commented 2 years ago

Node 16.13.1 and @electron 15.3.2 & 17.0.0-alpha.4 (was fine on @electron v16 too).

Ranjith-Eswaran-G commented 2 years ago

Thanks for your response. But I am getting the error you mentioned at the top even after downgrading node-addon-api to 3.1.0. It works fine only when I use the node version 14.11.0.

vladimiry commented 2 years ago

I guess node-gyp version also might be a part of the stack, at my side it's node-gyp@8.2.0.