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

Update files from node #79

Closed
wants to merge 3 commits into from
Closed

Conversation

kfarnung
Copy link
Contributor

  • Added node_internals.cc/h to shim missing internal functions

@kfarnung
Copy link
Contributor Author

@jasongin @mhdawson I split out the node_internals changes from #70.

@kfarnung
Copy link
Contributor Author

Looks like we finally have a signature issue with node 4:

https://travis-ci.org/kfarnung/node-addon-api/jobs/253006582

> [email protected] pretest /home/travis/build/kfarnung/node-addon-api
> node-gyp rebuild -C test

make: Entering directory `/home/travis/build/kfarnung/node-addon-api/test/build'
  CXX(target) Release/obj.target/node-api/../src/node_api.o
../../src/node_api.cc:625:3: error: no member named 'Get' in 'v8::Persistent<v8::ObjectTemplate, v8::NonCopyablePersistentTraits<v8::ObjectTemplate> >'
  ENV_OBJECT_TEMPLATE(env, function_data, otpl, v8impl::kFunctionFieldCount);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../src/node_api.cc:58:48: note: expanded from macro 'ENV_OBJECT_TEMPLATE'
      (destination) = env->prefix ## _template.Get(isolate);       \
                      ~~~~~~~~~~~~~~~~~~~~~~~~ ^
../../src/node_api.cc:651:3: error: no member named 'Get' in 'v8::Persistent<v8::ObjectTemplate, v8::NonCopyablePersistentTraits<v8::ObjectTemplate> >'
  ENV_OBJECT_TEMPLATE(env, accessor_data, otpl, v8impl::kAccessorFieldCount);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../src/node_api.cc:58:48: note: expanded from macro 'ENV_OBJECT_TEMPLATE'
      (destination) = env->prefix ## _template.Get(isolate);       \
                      ~~~~~~~~~~~~~~~~~~~~~~~~ ^
../../src/node_api.cc:2087:3: error: no member named 'Get' in 'v8::Persistent<v8::ObjectTemplate, v8::NonCopyablePersistentTraits<v8::ObjectTemplate> >'
  ENV_OBJECT_TEMPLATE(env, wrap, wrapper_template, 2);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../src/node_api.cc:58:48: note: expanded from macro 'ENV_OBJECT_TEMPLATE'
      (destination) = env->prefix ## _template.Get(isolate);       \
                      ~~~~~~~~~~~~~~~~~~~~~~~~ ^
3 errors generated.
make: *** [Release/obj.target/node-api/../src/node_api.o] Error 1
make: Leaving directory `/home/travis/build/kfarnung/node-addon-api/test/build'
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/home/travis/.nvs/node/4.8.4/x64/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:276:23)
gyp ERR! stack     at emitTwo (events.js:87:13)
gyp ERR! stack     at ChildProcess.emit (events.js:172:7)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:211:12)
gyp ERR! System Linux 4.8.12-040812-generic
gyp ERR! command "/home/travis/.nvs/node/4.8.4/x64/bin/node" "/home/travis/.nvs/node/4.8.4/x64/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild" "-C" "test"
gyp ERR! cwd /home/travis/build/kfarnung/node-addon-api/test
gyp ERR! node -v v4.8.4
gyp ERR! node-gyp -v v3.4.0
gyp ERR! not ok 
npm ERR! Test failed.  See above for more details.


The command "npm test $NPMOPT" exited with 1.

@kfarnung
Copy link
Contributor Author

Opened a PR in node: nodejs/node#14211

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

* Added `node_internals.cc/h` to shim missing internal functions
* Added `util.h` and `util-inl.h` for incoming changes to error helpers
@kfarnung
Copy link
Contributor Author

kfarnung commented Jul 13, 2017

@mhdawson I added the missing macro/functions for your change. I rebased your PR against this code and everything built successfully.

Pushed a branch here for CI run: https://github.com/kfarnung/node-addon-api/tree/n-api-errors

mhdawson pushed a commit that referenced this pull request Jul 14, 2017
* Added `node_internals.cc/h` to shim missing internal functions
* Added `util.h` and `util-inl.h` for incoming changes to error helpers

PR-URL: #79
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Jason Ginchereau <[email protected]>
@mhdawson
Copy link
Member

Landed as 92d14c0

@mhdawson mhdawson closed this Jul 14, 2017
@kfarnung kfarnung deleted the src_update branch July 14, 2017 16:12
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
* Added `node_internals.cc/h` to shim missing internal functions
* Added `util.h` and `util-inl.h` for incoming changes to error helpers

PR-URL: nodejs/node-addon-api#79
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Jason Ginchereau <[email protected]>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022
* Added `node_internals.cc/h` to shim missing internal functions
* Added `util.h` and `util-inl.h` for incoming changes to error helpers

PR-URL: nodejs/node-addon-api#79
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Jason Ginchereau <[email protected]>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022
* Added `node_internals.cc/h` to shim missing internal functions
* Added `util.h` and `util-inl.h` for incoming changes to error helpers

PR-URL: nodejs/node-addon-api#79
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Jason Ginchereau <[email protected]>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
* Added `node_internals.cc/h` to shim missing internal functions
* Added `util.h` and `util-inl.h` for incoming changes to error helpers

PR-URL: nodejs/node-addon-api#79
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Jason Ginchereau <[email protected]>
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

Successfully merging this pull request may close these issues.

3 participants