Repository navigation
Don't include common.gypi from node source tree #1118
Description
node-gyp includes $nodedir/common.gypi when creating build files for an add-on.
Per nodejs/CTC#62 (comment), that probably isn't future-proof and it's a bad idea in general: it makes add-ons pick up compiler and linker settings that aren't intended for them and may be actively harmful. People have filed bug reports about it.
Plan of action:
- Merge
common.gypiintoaddon.gypi. - Strip everything that isn't needed by add-ons.
- Test the bejebus out of it with citgm.
Activity
murgatroid99 commentedon Jul 6, 2017
I would support this change. I got burned by the fact that
common.gypiexplicitly disables-Werrorrichardlau commentedon May 23, 2018
Node.js 10's
common.gypirequires C++14:https://github.com/nodejs/node/blob/cf41627411886000429bde058a6594fb7f6d6d47/common.gypi#L324
which may expose anyone using older compilers to compile addons.
bnoordhuis commentedon May 23, 2018
Yep, but that flows on down from V8. We'd still carry that change with a separate common.gypi.
rvagg commentedon Jun 20, 2019
still planning on making this happen @bnoordhuis? I fear this is just going to linger as a perpetually open issue unless you make it happen, I suspect there's too many dragons for most mortals to want to attempt this
bnoordhuis commentedon Jun 20, 2019
@rvagg It's one of those projects that I do want to get around to someday but I don't know when. If you want to close it, that's fine by me.
Good job on the cleanup action, by the way. :-)
rvagg commentedon Jun 20, 2019
I think I got throttled along the way with my cleanup, I stopped being able to close issues, a red error box from GitHub as if I was blocked. Seems to be working again now.
I don't mind this being left open if it's genuinely a TODO, your call. I'm just aiming to get rid of the clutter so this starts being manageable again.