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

Migration to V8 LTS 6.x / 7.x #48

Closed
jeroen opened this issue Jan 31, 2019 · 21 comments
Closed

Migration to V8 LTS 6.x / 7.x #48

jeroen opened this issue Jan 31, 2019 · 21 comments

Comments

@jeroen
Copy link
Owner

jeroen commented Jan 31, 2019

New version of V8 should work with libv8 version 6 or 7 as provided by e.g nodejs 10.15 LTS. Work is currently in the newapi branch. To test this:

  • MacOS: brew install v8
  • Debian buster: apt-get install libnode-dev
  • Fedora: yum install v8-devel

And then:

remotes::install_github("jeroen/V8")

Outstanding issues:

winbuilder

Doesn't build on winbuilder because TryAcquireSRWLockExclusive() is not available on vista.

Fixed by reverting win7 patch: https://github.com/rwinlib/libv8/tree/v6.2.414.50-vista

dagitty

Fixed: turns out this package is sensitive to locale and ICU config.

On MacOS v8::V8::InitializeICUDefaultLocation() is needed to initiate the ICU data. On Debian it works with libnode-dev if we make sure the locale is set to UTF-8:

sudo apt-get install locales
localedef -i en_US -f UTF-8 en_US.UTF-8
export LANG=en_US.UTF-8
R CMD check dagitty_0.2-2.tar.gz

Example code:

ctx <- dagitty:::.getJSContext()
ctx$eval('var y1 = "pdag{ {i j} -> x -> {a -- b -- c -- a } a -> z b -> y x[e] y[o] }"')
ctx$eval('GraphParser.parseGuess(global.y1).toString()')

dqshiny

Fix is on CRAN

jsonld

Fix is on CRAN

lawn

Fix on CRAN.

@QuLogic
Copy link
Contributor

QuLogic commented Jan 31, 2019

FYI, on Fedora, headers for v8 in nodejs are in /usr/include/node (as I see you have PKG_CFLAGS="-I/usr/include/nodejs/deps"). I haven't tried to build, so I'm not sure if linking will work.

@znmeb
Copy link

znmeb commented Jan 31, 2019

I'm available for testing on Arch Linux and Fedora Silverblue.

@jeroen
Copy link
Owner Author

jeroen commented Jan 31, 2019

@QuLogic I don't think node is built as a shared module on Fedora. I can't find any libnode.so or libv8.so to link against. You would need to make a libnode package like Debian.

@daissi
Copy link

daissi commented Feb 4, 2019

I tried to package a snapshot of the newapi branch, but it built only on amd64.

It fails to build on other architectures because the configure file tries to find the lib in the path for amd64 only.

@znmeb
Copy link

znmeb commented Feb 4, 2019

@jeroen @QuLogic Fedora 29 has libv8.so.6:

$ sudo podman run -it --rm fedora:29 /bin/bash
[root@466d2005e2a7 /]# dnf install mlocate v8-devel

[snip]

  Verifying        : libicu-62.1-3.fc29.x86_64                                                                      1/4 
  Verifying        : mlocate-0.26-22.fc29.x86_64                                                                    2/4 
  Verifying        : v8-1:6.7.17-7.fc29.x86_64                                                                      3/4 
  Verifying        : v8-devel-1:6.7.17-7.fc29.x86_64                                                                4/4 

Installed:
  mlocate-0.26-22.fc29.x86_64  v8-devel-1:6.7.17-7.fc29.x86_64  libicu-62.1-3.fc29.x86_64  v8-1:6.7.17-7.fc29.x86_64 

Complete!
[root@466d2005e2a7 /]# updatedb
[root@466d2005e2a7 /]# locate libv8
/usr/lib64/libv8.so
/usr/lib64/libv8.so.6
/usr/lib64/libv8_libbase.so
/usr/lib64/libv8_libbase.so.6
/usr/lib64/libv8_libplatform.so
/usr/lib64/libv8_libplatform.so.6
[root@466d2005e2a7 /]# 

@znmeb
Copy link

znmeb commented Feb 4, 2019

Arch Linux does not have V8 in the main repositories - you have to build it from source in the Arch User Repository with an "AUR helper". The main version is v8 7.1.302.29-1, and there's a libv8-3.14 specifically for this R package.

@jeroen
Copy link
Owner Author

jeroen commented Feb 4, 2019

@daissi thanks I'll try to improve that configure script

@QuLogic
Copy link
Contributor

QuLogic commented Feb 5, 2019

@znmeb Yes, but the biggest problem with v8 is that they like to break ABI indiscriminately. So just because it has the same soname, doesn't necessarily mean it will work fine.

@jeroen
Copy link
Owner Author

jeroen commented Feb 5, 2019

@QuLogic

Yes, but the biggest problem with v8 is that they like to break ABI indiscriminately. So just because it has the same soname, doesn't necessarily mean it will work fine.

Right, that's why it's good to use the node lts version of libv8. Those are stable for npm packages. @QuLogic @tomhughes would it possible to ask the maintainer of node-devel in fedora to build configure with -shared as in debian libnode-dev?

@jeroen
Copy link
Owner Author

jeroen commented Feb 5, 2019

@znmeb the version in the newapi branch should now work with V8 version 7. Does this work for you in arch?

@jeroen jeroen changed the title Migration to V8 6.8 (LTS) Migration to V8 LTS 6.x / 7.x Feb 5, 2019
@znmeb
Copy link

znmeb commented Feb 5, 2019

The AUR build of v8 failed - log is at https://aur.archlinux.org/packages/v8/, 👎

I've got Docker - I can test on any Linux that has libv8-devel if you wish.

@tomhughes
Copy link

Sorry? What does that do exactly? The Fedora Node.js uses the bundled v8 these days if that's what you're asking, because it's impossible to find one version of v8 that works for everything in Fedora.

@jeroen
Copy link
Owner Author

jeroen commented Feb 6, 2019

@tomhughes my question is if it would be possible to build nodejs with -shared such that we can link the R-v8 Fedora package against libnode.so instead of needing a separate v8-devel. The Fedora nodejs-devel package already contains the libv8 headers, but no library to link against unfortunately.

Debian is doing the same now in libnode-dev so that they don't need to manage a separate v8 debian package anymore.

@tomhughes
Copy link

I'm asking what -shared does? Though I don't normally deal with Node.js itself so you'd be better off asking the people that do really...

@tomhughes
Copy link

Why do you want to link to the Node.js version of v8 anyway? Fedora has a separate v8 for things that want to use v8 itself but because of the well known issues with v8 many things like Node and Chromium prefer to use their own bundled versions.

I don't think exposing those versions to third parties is going to be very helpful - there's no more guarantee that they would be compatible with your program than there is that the separate one would be.

Anyway as I don't maintain v8 or nodejs in Fedora none of this is really anything to do with me!

@jeroen
Copy link
Owner Author

jeroen commented Feb 6, 2019

Support for V8 version 6 / 7 has been merged into master

@jeroen
Copy link
Owner Author

jeroen commented Feb 7, 2019

Submitted V8 2.0 to CRAN. Fingers crossed.

@daissi
Copy link

daissi commented Feb 7, 2019

Submitted V8 2.0 to CRAN. Fingers crossed.

Uploaded in Debian/unstable.

@jeroen
Copy link
Owner Author

jeroen commented Feb 7, 2019

Thank you @daissi ! Let me know if everything works now; if there are any other issues I can do another path release in a week or so.

Closing this topic now, problems with the new V8 can go into separate issues. Thanks all!

@jeroen
Copy link
Owner Author

jeroen commented Feb 7, 2019

@QuLogic when you upload R-V8 to Fedora can you change the dependency to v8-devel ?

@ateucher
Copy link

Just want to pipe in and say a huge thank you to @jeroen for this! Using the current version of V8 seems to have solved a long-standing issue with rmapshaper on Fedora that I couldn't fix.

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

6 participants