-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
build: use --whole-archive for pthread with --fully-static #30199
base: main
Are you sure you want to change the base?
Conversation
This resolves a crash during the build of Node.js when built with `./configure --fully-static`. Fixes: nodejs#30180
Is possible to add a CI job for this ? Possible can be done via #30057 |
@gengjiawen I guess, but I think that’s something someone else might have to do :) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ping @addaleax - is this stuck because there is not a proper CI job to test this, or any other concerns w.r.t the change itself? edit - I see recent conversations on this PR, so |
@gireeshpunathil It’s stuck because it isn’t at the top of my priority list :)
Neither. CI doesn’t pass because this doesn’t work on all platforms; I assume the next step would be to figure out which platforms those are and how to turn that into a condition in the gypfile. |
8ae28ff
to
2935f72
Compare
Rerunning CI to see which platforms are failing - I've seen a report (outside of GH) of another user hitting an error when building with From a suggestion from @miladfarca, |
This comment has been minimized.
This comment has been minimized.
How is it passing without the patch? It literally won't build at all with the fully static option. |
This resolves a crash during the build of Node.js when built
with
./configure --fully-static
.Fixes: #30180
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes