Repository navigation
build: drop support for VS 2013 in v6 LTS #7989
Description
Activity
Fwiw, I'm +1 on dropping support in v6
+1 if this lands before LTS.
+1
Yes. Drop the support. Please +1.
+1 since otherwise big baggage for LTS (specifically) and mainstream development (in consequence).
/cc @nodejs/platform-windows
+1
SGTM
@nodejs/build ... what's the status of getting the CI updated to drop vs2013?
- Release CI: Machines done, finetuning the job. Will let you know when it's ready to use.
- Test CI: Already compiles only with 2015 for the correct versions, but addons testing should still use VS2013 (next in my queue).
- CITGM: Stalled for now.
@joaocgreis is there anything i can do to help with CITGM? We are not even building with windows ATM
@thealphanerd If you have any comments on the first draft I made in JaneaSystems/citgm@5d44fc2...janeasystems:joaocgreis-G7R-attempts it'd be great! I'll work more on it, but if you want to finish the work on some of those features and PR them to CITGM, feel free to! (It'd be great to have a way to test modules with both downloaded and compiled binaries, but I'm not sure how to do that, ideas?)
I started a CI job for CITGM on Windows (https://ci.nodejs.org/view/All/job/reis-citgm-native-windows-fanned/) but it is still too tied to running my commits above. I'll make it general once the main canary has all the functionality.
8 remaining items
@joaocgreis as a build member you have my +1.
@nodejs/ctc is there consensus around this? Should I activate it right away or should we set a date (like before a minor release)?
Looking at the record, I'd say we seem to have consensus to move forward on this. (Anyone who thinks otherwise should say so, of course!)
The last time this was discussed at a CTC meeting was August 3. The minutes indicate that dropping support for VS2013 in Node.js version 7 was uncontroversial. For version 6, it was desirable, but there was a wait-and-see approach:
@jasnell: Let’s get this landed in master and test it out, then make a decision on putting in v6 before October.
That conversation resulted in this issue.
And now would seem to be the time to make that decision.
Looking through the issue, it seems that the following folks would appear to be on board with doing this:
I'd be inclined to interpret that as consensus until someone expresses something contrary.
Reacted by James M Snell and Gibson FahnestockFew suggestions regarding keeping the ecosystem sane after this change:
- I think that node module binary ABI should be bumped for the C++ runtime ABI change. Probably would be better to do it on some major release version. If we decide against bumping node module ABI, the extensions that provide binary downloads should be informed to differentiate between a (48, v120) binaries and (48, v140) binaries. Can be very cumbersome so I think we should just bump for that.
- We should ensure that
PlatformToolsetparameter (set atv120which is VS2013) is passed over tonode-gypandnode-pre-gypextensions now (Windows PlatformToolset: Provide stable C++ API to addons #6045) and actually used. - Next step could be to run CI under Visual Studio 2015 with
v120enabled, which is effectively using VS2013 compiler (warning: it might not be installed by default). - Extensions providing binary extensions as a download should be ready to run their CIs in a dual-mode, i.e. VS2015 with
v120andv140. The mode should be picked up automatically bynode-gypandnode-pre-gyp. We will take care of node-sass which does not use node-pre-gyp. - Once the next version is released (with a bumped node module ABI version) it would be good to verify the parallel building of
v120binaries for older nodes andv140binaries for the new one works. - We shouldn't probably update node development trails which should be left at
v120, because updating node module ABI there is probably not feasible. - An interesting question appears how does node lifecycle support should react when we want to switch to hypothetical Visual Studio 2017 (hypothetical
v160) which no longer supportsv120ABI (and older Windows SDKs matching v120 are pulled).
I think that node module binary ABI should be bumped for the C++ runtime ABI change.
That won't happen in v6.x, if that is what you are suggesting, but it will in v7.0.0. An ABI bump forces everyone to rebuild their add-ons; that is not something we want in the stable branch.
We should ensure that PlatformToolset parameter (set at v120 which is VS2013) is passed over to node-gyp and node-pre-gyp extensions now (#6045) and actually used.
That is technically possible by setting 'msbuild_toolset' in common.gypi but I don't know, actively blocking people from building their add-ons with a particular version of VS seems anti-social. Why enforce that? As long as it works, it works.
Also, why v120? We're bumping the baseline to VS 2015 in v6 LTS so v140 would make more sense, wouldn't it?
If v6.x is going to remain stable we shouldn't be change C++ library in the mean time.
I will review some extensions and will check if
msbuild_toolsetis working.The problem is that one cannot support setup where node engine and C++addons have been compiled using different C++ runtime libraries. With most modern Unix systems an attempt to do that fails with a symbol versioning error nodegit/nodegit#1131 nodegit/nodegit#845 nodegit/nodegit#1096 nodegit/nodegit#853 nodegit/nodegit#969 - those are just from one repository.
Windows unfortunately allows to do exactly this. Setting
msbuild_toolsetonly tells you which C++ ABI your node engine is built against. I have rebuild node with two different compilers and tested the crashing add-on - vs2013 node with v120 extension worked, vs2015 node with v140 extension worked, but not when the versions where mixed.We have enough unhappy folk on node-gyp issue tracker with Windows. I think it is not very difficult to install
v120support with VS2015 today. Maybe node-gyp could provide a better diagnostic if the desired PlatformToolset is not available, that can be done for sure.Why making sure
v120works? I think that we are already passingPlatformToolsetto addons properly. I'd rather test PlatformToolset "enforcement" now and add changes needed to node-gyp and node-pre-gyp (if any needed at all) now without breaking compatibility. So we make sure the PlatformToolset machine is working smoothly.Then we can switch to v140 and extensions can test CI setup for building some binaries using v120 (for the stable node) and others are built using v140. This can be done with a properly setup Visual Studio 2015.
By the time we get completely of
v120I am sure another Visual Studio gets released and we will be playing the same game again. Once we make sure the supporting infrastructure is in place, future upgrades will much smoother.My major concern is making sure downloadable module binaries provided by many extensions are built properly for users who don't install compilers at all (big majority on Windows).
Somebody compiling modules by herself with VS2015 can be asked to install VS2013 compatibility package or (if VS2015 v140 is absolutely required) asked to rebuild node with v140 and obey its
PlatformToolset.Had Windows DLL symbol versioning scheme would have made ignoring PlatformToolset hard and this issue should have been resolved somehow anyway.
I understand that given that
msbuild_toolsetis available probably no work needs to be done with node itself. Adjustments may need to be done node-gyp, node-pre-gyp and popular extensions. But if a timeline will be agreed (for example for launching this some point in 7.x), extension authors will be given chance to prepare and catch up. If C++ interface changes in the middle of the 6.x line without bumping NODE_MODULE_VERSION projects like https://github.com/sass/node-sass would have to provide a 6.x v120 and 6.x v140 module version separately and include the C++ ABI version number in addition to the node module version number. To make this work PlatformToolset identification should be provided at runtime (not sure it's there).I don't think we can enforce a PlatformToolset, at least with the motivation we've seen so far. I don't know where a "VS2013 compatibility package" can be found, the only way I know to add v120 support to VS2015 was to install the full 8GB of VS2013 alongside (that is no longer available for easy download in the VS site). For now, we should still add the PlatformToolset information to be used by node-gyp – but only if the desired toolset is available, ignored otherwise.
Currently, we build Node with VS2013 and users can compile with 2013 or 2015. It is true that we have no complete assurance of ABI compatibility. However, compiling Node with VS2015 should not create any new problems, will just change where any possible problems happen. Given that we've been recommending users to build with VS2015 for some time, it should help more than it harms.
In the future, if we encounter any problems, we have several options: we can ask everybody to use the same version of VS (breaking scenarios where a version is imposed), release binaries built with every version, or find a way around the problem (would have been possible with the node-sass bug).
Module developers should move to VS2015 simultaneously with Node v6. I understand that distributing a binary is more serious than a local test build, but many of the most downloaded npm modules provide no binaries, so there is still a significant number of users compiling on their machines. Here is a test build of v6.7.0: https://nodejs.org/download/test/v6.7.0-test20160930d87ad0ceea/
@saper were you able to find any issue when using the test builds? I tested the update path (intall v6.6 release, install a module, test it, install v6.7-testVS2015, test again without recompiling) and it worked fine with both VS versions. The known issue with node-sass was with Windows XP, that is no longer supported.
@nodejs/ctc any objection to moving forward with this on Sunday, so that the release next week is already done with VS2015? It would be good to have one before LTS, and if I'm reading it right this will be the last one.
i need some time to respond to this re possible bugs; basically we should have an agreement what a "supported configuration" means. When I propose that everyone should be using the same compiler to compile addons as they or node foundation used to build the engine itself, this is a configuration that lets us preclude a certain class of hard to debug problems. I think I needed a week or something to troubleshoot the problem we've had with vs2015 and XP and I am not going to do this again.
I am uncomfortable with a notion "maybe it's just 1% of corner cases" when we can prepare the environment not to have defects in this area.
i fully agree that we should be moving towards VS2015. Actually instead of just saying/recommending "you need to use VS2015", you express the same with the code by adding
v140. What could be additionally done is to trap the msbuild error message if the toolkit is not installed and provide an additional info about the need to use a right compiler from node-gyp.Regarding switching in the 6.x release cycle with the ABI number unchanged can you propose a way how can I distinguish whether the user is running a vs2013 node 6.x or a vs2015 node 6.x in order to download a correct binary?
@saper I completely agree about adding info about the Platform Toolset,
v120orv140, somewhere. It should be used to select the right version (if it is available) when compiling native modules, and perhaps display a warning if it is not available. I only disagree with blocking the build completely, we have no practical reason to break users with a different version of VS installed.If we decide to move ahead now, v6.7 will be the last version built with VS2013 and v6.8 will be the first with VS2015, so you can use
process.version.@saper If this is to move forward, it should be for the release this Tuesday. LTS is scheduled for October 18 and it would be good to do this one release before so that we can take a step back if absolutely necessary. Are you still investigating this? Is there anything I can do to help? If we do not find anything by Monday, I think we should move ahead.
Landed the changes in the release jenkins and changed the test CI to not use VS2013 for v6. The next release of v6 will be built with VS2015.
- added 3 commits that reference this issue
on Jan 27, 2017
This is split off from #7484 to discuss the possibility of dropping VS 2013 in v6 LTS. From the other issue:
@joaocgreis reported in #7484 (comment):
Encouraging!