search

augustoroman / v8

A Go API for the V8 javascript engine.
MIT License
395 stars 78 forks source link

Update README.md for v8 download and installation #9

Closed flowchartsman closed 6 years ago

flowchartsman commented 6 years ago

The docs were way out of date as discussed in #8. Most notably, make is no longer the recommended method for building, v8_static_library needs to be specified directly, and sw_vers -productVersion on OSX is actually too specific and will generate annoying warnings like the following:

ld: warning: object file (foo.a(bar.c.o)) was built for newer OSX version (10.13.3) than being linked (10.13)

I also modified the build instructions to just build v8 directly from the library path, and I removed the bit about fat binaries under linux as I wasn't sure if it applied any longer. Please feel free to verify.

flowchartsman commented 6 years ago

By "linux" I'm guessing you mean "Ubuntu"? I think that's fair, but I'd rather dig into the specifics, since that information is not particularly useful to the users of other distros. I'll download a VM and dig into this, and maybe I can help you solve your Travis issues.

As for the gyp note, I'm not sure that's necessary. The official wiki instructions on building from source and building with gn don't make mention of downgrading to use make, so why should we? Changing the build instructions doesn't change the golang API, so there's no backwards compatibility to break here. Users should follow the vendor's instructions.

augustoroman commented 6 years ago

Yes, I've only been testing with Ubuntu 16.04 and 14.04. A more precise requirement would probably be g++ 5.0 or higher, but I'm guessing a little bit trying to figure out the errors from https://travis-ci.org/augustoroman/v8/jobs/356213145. I believe it's something to do with libstd++ vs libc++ or some such.

I fired up two beefy VMs on GCE using Ubuntu 14.04 and 16.04 ran the travis-install-linux.sh script. It worked with 16.04 but not with 14.04 after doing something like:

# ... install go 1.10 here ...
sudo apt-get update
sudo apt-get install build-essentials git

on each.

augustoroman commented 6 years ago

As for gn vs make, my intention was that if somebody wants to build V8 5.0 or something earlier that doesn't support gn, they would be stuck. But I think that gn has been supported for a while, so nevermind about keeping a note for make.

flowchartsman commented 6 years ago

https://stackoverflow.com/questions/33394934/converting-std-cxx11string-to-stdstring Give this a try.

augustoroman commented 6 years ago

FYI, I tried that stackoverflow link and couldn't get it work: Adding the flag on the go side didn't work, and I couldn't get gn to add the compiler flag directly without editing the .ninja files, which I don't really want to do. But I do have reliable compilation instructions for 16.04+, so I'll update those soon. Also, the official_build flag breaks the build on linux for some reason, so I'll include that in my update.

What was the conclusion with the OSX target? I've seen that linker error for other, unrelated packages and I haven't figured out why it pops up sometimes.

flowchartsman commented 6 years ago

It doesn't seem necessary. I built without it completely and it seems fine.

Also symbol_level=0 and the other two flags you suggested do seem to obviate the need for strip on OSX. Indeed, I just ran it and strip seems to have made some files bigger. If you have reliable build instructions are we just going to bail on this PR then?

augustoroman commented 6 years ago

Nah, let's commit it when you are ready (it's still an improvement) and I'll follow up, hopefully tonight. Thanks for helping!

On Wed, Apr 4, 2018 at 10:02 AM Andy Walker notifications@github.com wrote:

It doesn't seem necessary, and also symbol_level=0 and the other two flags you suggested do seem to obviate the need for strip. Indeed, I just ran it and strip seems to have made some files bigger. If you have reliable build instructions are we just going to bail on this PR then?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/augustoroman/v8/pull/9#issuecomment-378654066, or mute the thread https://github.com/notifications/unsubscribe-auth/AApS1SDsnIP5PSUtZeFchHy2JHGk3gIvks5tlO6ggaJpZM4S_Avn .

flowchartsman commented 6 years ago

Okay, I'm going to make one more commit with what I've got. There are probably a few flags in here that don't really need to be. I guess either one or both of us can winnow them down later.