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

NodeJS: 7.6.0, new V8: 5.5.372.43 #315

Closed
wants to merge 1 commit into from

Conversation

matiwinnetou
Copy link
Contributor

@matiwinnetou matiwinnetou commented Jul 23, 2017

Locally works, on 7.6.0:

[INFO] --- maven-bundle-plugin:3.3.0:bundle (default-bundle) @ j2v8_macos_x86_64 ---
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 4.409 s
[INFO] Finished at: 2017-07-23T15:52:30+02:00
[INFO] Final Memory: 24M/417M

@matiwinnetou matiwinnetou force-pushed the new-nodejs branch 2 times, most recently from 74a84cb to 8b2775c Compare July 23, 2017 15:54
@matiwinnetou
Copy link
Contributor Author

matiwinnetou commented Jul 23, 2017

I also tried with latest NodeJS but it failed: 8.x

I needed to add two registers:

    _register_inspector();
    _register_serdes();

but then it failed here:

SHELL building macos@x64 => j2v8jni
Scanning dependencies of target j2v8
[ 50%] Building CXX object CMakeFiles/j2v8.dir/jni/com_eclipsesource_v8_V8Impl.cpp.o
[100%] Linking CXX shared library libj2v8_macosx_x86_64.dylib
Undefined symbols for architecture x86_64:
  "typeinfo for v8::ArrayBuffer::Allocator", referenced from:
      typeinfo for ShellArrayBufferAllocator in com_eclipsesource_v8_V8Impl.cpp.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [libj2v8_macosx_x86_64.dylib] Error 1
make[1]: *** [CMakeFiles/j2v8.dir/all] Error 2

@matiwinnetou matiwinnetou force-pushed the new-nodejs branch 7 times, most recently from ed37be3 to 84ccd16 Compare July 23, 2017 23:30
@matiwinnetou
Copy link
Contributor Author

7.6.0 was a success build on linux with V8 version: 5.5.372.40

@matiwinnetou matiwinnetou force-pushed the new-nodejs branch 2 times, most recently from 634cc3c to 7a89aa1 Compare July 25, 2017 19:52
@matiwinnetou matiwinnetou changed the title New NodeJS: 7.10.1, new V8: 5.5.372.43 New NodeJS: 7.6.0, new V8: 5.5.372.43 Jul 25, 2017
@matiwinnetou matiwinnetou changed the title New NodeJS: 7.6.0, new V8: 5.5.372.43 NodeJS: 7.6.0, new V8: 5.5.372.43 Jul 25, 2017
@matiwinnetou
Copy link
Contributor Author

matiwinnetou commented Aug 1, 2017

@irbull
I am working on enabling NodeJS 8.1.x + V8: 5.8 to make it a reality:

https://github.com/eclipsesource/J2V8/compare/master...matiwinnetou:nodejs8?expand=1

This already compiles on OSX.

@matiwinnetou
Copy link
Contributor Author

matiwinnetou commented Aug 1, 2017

8.1.x fails on linux only:

j2v8/node/out/Release/obj.target/node/src/tracing/trace_event.o: In function `node::tracing::TraceEventHelper::GetCurrentPlatform()':
trace_event.cc:(.text+0x10): multiple definition of `node::tracing::TraceEventHelper::GetCurrentPlatform()'
/j2v8/node/out/Release/obj.target/node/src/tracing/trace_event.o:trace_event.cc:(.text+0x10): first defined here
collect2: error: ld returned 1 exit status
cctest.target.mk:188: recipe for target '/j2v8/node/out/Release/cctest' failed
make[1]: *** [/j2v8/node/out/Release/cctest] Error 1
Makefile:76: recipe for target 'node' failed
make: *** [node] Error 2
SHELL building linux@x64 => nodejs
Traceback (most recent call last):
  File "./build.py", line 311, in <module>
    execute_build(args)
  File "./build.py", line 307, in execute_build
    execute_build_step(target_compiler, target_step)
  File "./build.py", line 266, in execute_build_step
    compiler.build(build_step)
  File "/j2v8/build_system/cross_build.py", line 60, in build
    self.exec_build(config)
  File "/j2v8/build_system/shell_build.py", line 26, in exec_build
    self.exec_cmd(shell_str, config)
  File "/j2v8/build_system/cross_build.py", line 79, in exec_cmd
    utils.execute(cmd, dir)
  File "/j2v8/build_system/build_utils.py", line 37, in execute
    raise subprocess.CalledProcessError(return_code, cmd)
subprocess.CalledProcessError: Command 'cd /j2v8 && cd ./node && ./configure                          --without-intl                      --without-inspector                 --dest-cpu=x64                    --without-snapshot                  --enable-static && CFLAGS=-fPIC CXXFLAGS=-fPIC make -j4' returned non-zero exit status 2
Traceback (most recent call last):
  File "build.py", line 311, in <module>
    execute_build(args)
  File "build.py", line 279, in execute_build
    execute_build_step(x_compiler, x_step)
  File "build.py", line 266, in execute_build_step
    compiler.build(build_step)
  File "/home/mati/Devel/J2V8/build_system/cross_build.py", line 60, in build
    self.exec_build(config)
  File "/home/mati/Devel/J2V8/build_system/docker_build.py", line 74, in exec_build
    self.exec_host_cmd(docker_run_str, config)
  File "/home/mati/Devel/J2V8/build_system/cross_build.py", line 70, in exec_host_cmd
    utils.execute(cmd, dir)
  File "/home/mati/Devel/J2V8/build_system/build_utils.py", line 37, in execute
    raise subprocess.CalledProcessError(return_code, cmd)
subprocess.CalledProcessError: Command 'docker run  --privileged -P -v /home/mati/Devel/J2V8:/j2v8 --name j2v8.linux.x64 j2v8-linux /bin/bash -c "cd /j2v8; python ./build.py --build-agent -t linux -a x64  nodejs j2v8cmake j2v8jni j2v8java"' returned non-zero exit status 1

Corresponding NodeJS issue that looks similar:
nodejs/node#13500

@matiwinnetou
Copy link
Contributor Author

matiwinnetou commented Aug 2, 2017

@drywolf maybe you can help here a bit in terms of our build system. Even though I do not specify --enable-node as a config param it still tries to build nodejs and runs tests for us. Is this ok, or this is a bug? I was thinking I could avoid this bug by not building NodeJS. This is still then not ready for PR but at least I could build a package.

python build.py -t linux -a x64

see: nodejs/node#13500

@drywolf
Copy link
Contributor

drywolf commented Aug 2, 2017

@matiwinnetou This was a bug/unfinished feature in the build-system code.
Yesterday I wrapped up the PR for submission that I was already mentioning some days ago. So today after work it will be submitted, and this bug should be fixed with this PR also. During testing I also noticed that CMake didn't pick up the "node-enabled" command line switch consistently.

I'll mention you in the PR once I submit it (later today), it would be great if you could then confirm if the problem went away 😄

EDIT: also I think I missread your question slightly ... the "node-enabled" CLI switch is just for the compiler to decide if the Node.js features should be linked into J2V8 ... the build-system will try and build Node.js regardless, since only the "build-steps" arguments list of the CLI decides over which steps are built and which are not. And by default if you don't specify any steps then it assumes that you want to build "all" steps.

Also with the upcoming PR you will be able to specify build-steps like so:
python build.py -t linux -a x64 all ~nodejs ~test
which means to build all steps, except for nodejs and not run the tests.

For even more convenience I also added an alias that does exactly the same build-steps as specified manually above:
python build.py -t linux -a x64 j2v8
(The j2v8 build-step alias builds everything, except nodejs and also doesn't run tests)

@matiwinnetou
Copy link
Contributor Author

@drywolf top notch stuff, it must lots of work on your side. Thank you.

@drywolf
Copy link
Contributor

drywolf commented Aug 2, 2017

@matiwinnetou Thanks
I had some rainy days during my holiday break last week, which helped to push through my J2V8 roadmap a lot quicker 😉
Also I very much appreciate your encouragement & feedback.

I saw that the last week or two you were also busy working on some native concurrency issues. I had some trouble with some of the concurrency unit tests running on Android. It will be interesting to see if some of them are already fixed by this. Keep up the great work 😄 👍

@ewized
Copy link

ewized commented Aug 3, 2017

I think there should only be one place to put the version and url downloads.

@matiwinnetou
Copy link
Contributor Author

matiwinnetou commented Aug 3, 2017

@ewized yes, but we have a mixture of old an new build system... for the moment.

@matiwinnetou
Copy link
Contributor Author

Obsoleted I guess due to: #327

@matiwinnetou matiwinnetou deleted the new-nodejs branch August 16, 2017 16:43
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