-
Notifications
You must be signed in to change notification settings - Fork 27
Port fixes from node-16 branch to node 17 #41
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
Conversation
Build the symbol table after adding each file instead of continually rebuilding it.
This fixes an issue where functions that require ICU data segfault the process due to an unexpected lack of ICU data. The actual ICU data is provided by the icudt71_dat object.
Linking on Linux currently errors with `undefined reference to `'dlsym'`
(including our codespace)
Alpine Linux's `ar` program doesn't support having objects with the same name in the archive twice, so we have to use the GNU extension for including paths. This commit *should* honestly be split into multiple separate commits, but unfortunately pretty much all of these changes have to be applied as a unit or a build step fails.
Let node figure out the snake business.
@lloeki @SamSaffron
|
2.7 failures look like ye olde bundler libc detection problem. |
@v8: If you use `std::exchange`, include the header for it?
a30b0a6
to
482ac9c
Compare
482ac9c fixes the Ruby test in 3.1 musl. I expect that change to be specific to this version of node. |
@lloeki @SamSaffron we are green. (except for the two 2.7 tests which are failing the same way as the 16 branch) |
this should be fixable by updating rubygems+bundler. if not then there may be an upstream regression. |
Are you doing this? Where do we update this? Maybe you can just make the change so we can see where that is controlled. |
Yeah I can make the change later today |
I pushed a bunch of commits to |
I've dropped the change in |
CI is almost entirely green, with a failure on darwin test due to the GC test: https://github.com/seanmakesgames/libv8-node/actions/runs/4009651664/jobs/6887267190 Looks like a transient failure, though. Probably something to do with how the V8 GC was feeling that run. |
@SamSaffron @lloeki Can we get a merge? |
Done! |
wow looking amazing! how far are we from a new release here? Should we move 17 branch to main ? it is weird that the cutting edge is tucked away in a branch. |
Completely agree @SamSaffron - though are we stopping at 17 or going to 18? |
would love a go at 18 ❤️ |
Looks like there's already a branch for 18; we'll do another rebase |
Was just about to make a PR from 18 to master, but I'd love for us to change master to |
On this project I don't merge into
This is a separate matter, please open a dedicated issue for that (one on |
No description provided.