gyp,build: consistent shared library location #35626
Conversation
nodejs-github-bot
added
build
|
I should clarify - I've included changes to tools/gyp that are taking the place of code I would hope to be vendored in if/when the upstream patch is accepted |
Trott
added
the
python
There was a problem hiding this comment.
Overall I'm +1 on this change, because it's more consistent and it's better to change Node than break the ecosystem. That said, this PR should not land as it is, since we need to merge the gyp PR, cut a release and upgrade instead of directly changing the files.
|
@ryzokuken @rvagg what is the timeline on the node-gyp update? We should get this updated in npm ASAP too, otherwise for most ecosystem use cases the preferred behavior is not going to happen |
|
@MylesBorins we're cutting a gyp-next release tonight and I could make PRs updating the dependency both here and in node-gyp as soon as that's done. |
|
@MylesBorins re timeline, I'll get node-gyp out asap but we have consistent windows failures now on nodejs/node-gyp#2236 so we'll have to wait to figure those out. |
|
have pulled this commit in to #35635 |
Ref: nodejs/node-gyp#2233
Ref: nodejs/gyp-next#69
Pending agreement on nodejs/gyp-next#69. Perhaps this could be argued to be a breaking change here? But I'm making the case that it's a bugfix for GYP as far as node-gyp and npm is concerned. Doing this here makes it consistent across all (and matches the behaviour node-gyp has had for many years now). This really should go out with npm 7 and node 15, but I have no idea on timing on either of those things.
@nodejs/build
@nodejs/gyp
@nodejs/node-gyp