search

nodejs / node-v8

Experimental Node.js mirror on V8 lkgr :sparkles::turtle::rocket::sparkles:
416 stars 70 forks source link

issue on warn-on-error with latest v8 #165

Closed gengjiawen closed 4 years ago

gengjiawen commented 4 years ago

After use git node v8 major with this repo, build on linux failed with. Not sure I miss something.

cc @richardlau

https://github.com/gengjiawen/node-v8/runs/864971031?check_suite_focus=true

In file included from ../src/node.h:67:0,
                 from ../src/api/async_resource.cc:1:
../deps/v8/include/v8.h:9107:75: error: ‘MicrotasksCompletedCallback’ is deprecated [-Werror=deprecated-declarations]
   void AddMicrotasksCompletedCallback(MicrotasksCompletedCallback callback);
                                                                           ^
../deps/v8/include/v8.h:9115:78: error: ‘MicrotasksCompletedCallback’ is deprecated [-Werror=deprecated-declarations]
   void RemoveMicrotasksCompletedCallback(MicrotasksCompletedCallback callback);
                                                                              ^
cc1plus: all warnings being treated as errors
make[2]: *** [/home/runner/work/node-v8/node-v8/out/Release/obj.target/libnode/src/api/async_resource.o] Error 1
libnode.target.mk:345: recipe for target '/home/runner/work/node-v8/node-v8/out/Release/obj.target/libnode/src/api/async_resource.o' failed
make[2]: *** Waiting for unfinished jobs....
rm 110225900f301988e3f09fde9f4e88426d597895.intermediate c1fc8f38ba82b74e297e1f7234f47666cd3ddc7c.intermediate 170089a54f300f40f911235080621e5a05aa0ac2.intermediate
richardlau commented 4 years ago

After use git node v8 major with this repo, build on linux failed with. Not sure I miss something.

Well that's warning that whatever version of V8 is being updated to has now deprecated an API we're currently using in core. Which is the whole point of error-on-warn -- it makes it obvious when situations like this occur. It's indicative of an issue coming down the line, as deprecated V8 APIs tend to eventually be removed.

gengjiawen commented 4 years ago

Looks like indirect use, I searched the src folder, not finding AddMicrotasksCompletedCallback or RemoveMicrotasksCompletedCallback.

targos commented 4 years ago

Did you re-apply the commits from the canary-base branch on top of the update? This error should be fixed by https://github.com/nodejs/node/commit/df80a5dde650e317f32bb5af436867bd2ed691c1

gengjiawen commented 4 years ago

Did you re-apply the commits from the canary-base branch on top of the update? This error should be fixed by nodejs/node@df80a5d

Thanks, this should be the reason.