Repository navigation
Windows PlatformToolset: Provide stable C++ API to addons #6045
Description
Activity
/cc @nodejs/platform-windows @joaocgreis?
@saper To clear up any misunderstandings: you're suggesting to put
PlatformToolsetinprocess.config.variables? That's reasonable, I think.We don't currently set it (i.e.,
PlatformToolset- GYP calls itmsbuild_toolset) explicitly, I believe?Sounds reasonable to have this exposed.
Fixing the
PlatformToolsetwould prevent users from building with any other version of Visual Studio.I've been trying to get
node-sassand other modules to fail when they are build with a different version of VS than node, but I couldn't find any problem so far. Note that the issue innode-sasswas in Windows XP, which we no longer support.Given this, I don't see a pressing reason to force all users to use the same version of VS, since doing that would cause major pain to users. If we ever find some issue like the one in
node-sassagain, I would much rather find a way to work around it with something like sass/node-sass#1283 (comment) .@joacgreis this is not about "fixing"
PlatformToolset, it is about informing extensions about the C++ runtime ABI version used to compile. When debugging C++ ABI compatibility problems I have actually been building v120 and v140 versions of node in parallel. One can easily flip the switch, just extensions have to be compiled against the same C++ binary ABI as the node engine itself.As discussed in #7989 , we should move ahead with this and include
PlatformToolsetinformation.When a mismatch is detected when building a module, there should only be a warning instead of a failure (if possible, use the correct the correct platform if available). We can change this later if we find a pressing reason.
@nodejs/n-api ... does this need to remain open?
This won't be a problem for modules using N-API, because all N-API exported functions are "extern C", so the ABI is unaffected by the PlatformToolset version.
However I'm not sure that means this issue can be closed, because Node needs to continue supporting non-N-API modules for a long time.
This issue has been inactive for almost a year, but I suspect the issue is still valid and not yet addressed. If anyone has information to the contrary, by all means, close this or leave a comment. I'll also ping one more possibly-relevant group: @nodejs/addon-api
Let me check this. Even with N-API if the addon is using C++ for some other reason (wrapping C++ library) it still needs C++ compiler and the runtime environment, which should be the same as node's.
@saper why should it be the same as Node.js' if the N-API only ever calls
extern "C"functions from Node.js? I've always built my N-API module with the packages installed vianpm -g install windows-build-tools, and run it against an official Node.js release, and I have never had any issues even though my module uses the STL and thus needs to be built with a C++ compiler.... and I build my module against Node.js 4, 5.5.0, 6, and 8. So, unless all those Node.js versions were built using the same version of the C++ compiler, which is also the same version that my module is built with, I don't think the versions have to line up.
What is the status on this? Does this need to remain open?
I'm going to close this out since there's basically no movement here and I can't really parse the conversation very well. If someone still feels strongly about this, do feel free to reopen but it would be good to have more information so that this conversation can move forward.
This is a followup to #2365
When building nodejs addons it is very important to use the same C++ ABI as the original node engine installed. Currently node releases are done with Visual Studio 2013 so I'd propose we add
v120as thePlatformToolset.Maybe we could store the
PlatformToolsetused in theconfig.gypifile, so that all add-ons can pick it up and configure themselves accordingly?Example of a real problem caused by this issue: sass/node-sass#1326