+++ This bug was initially created as a clone of Bug #1186060 +++ After bug 1186060 is fixed, we should drop support for Visual Studio 2013 so that we can use new C++ features in VS2015 that aren't available in VS2013. VS2015's C++11/14 support is much more complete than VS2013's support. Work that is waiting on C++ features that are implemented in VS2015 but not VS2013 can add a dependency on this bug to be notified when those features become usable.

FTR, this would allow to use: - constexpr - char16_t/u"string" - operator""() - unrestricted unions

(In reply to Mike Hommey [:glandium] from comment #1) > FTR, this would allow to use: > - constexpr > - char16_t/u"string" > - operator""() > - unrestricted unions These would be nice, to be sure, but none of these seem like super-desirable things.

(In reply to Nathan Froyd [:froydnj][:nfroyd] from comment #2) > (In reply to Mike Hommey [:glandium] from comment #1) > > FTR, this would allow to use: > > - constexpr > > - char16_t/u"string" > > - operator""() > > - unrestricted unions > > These would be nice, to be sure, but none of these seem like super-desirable > things. One reason that list is so short is that other platforms are holding back other features that MSVC 2015 supports--i.e. each old compiler is a long pole for one feature or another. There's a lot of circular reasoning in these discussions of the form "It's not worth upgrading our ancient GCC to get that feature because MSVC 2013 doesn't support that feature. It's not worth upgrading MSVC to 2015 because features it enable aren't available on ancient GCC." This is my attempt to help break that cycle.

(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #3) > (In reply to Nathan Froyd [:froydnj][:nfroyd] from comment #2) > > (In reply to Mike Hommey [:glandium] from comment #1) > > > FTR, this would allow to use: > > > - constexpr > > > - char16_t/u"string" > > > - operator""() > > > - unrestricted unions > > > > These would be nice, to be sure, but none of these seem like super-desirable > > things. > > One reason that list is so short is that other platforms are holding back > other features that MSVC 2015 supports--i.e. each old compiler is a long > pole for one feature or another. That's a fair point. So the list should also include, according to https://developer.mozilla.org/en-US/docs/Using_CXX_in_Mozilla_code : - ref qualifiers on methods - alignof/alignas - 0b001 - inherited constructors - generic lambdas ref qualifiers on methods excites me, as that would let us close an objection to using smart pointer return values; alignof/alignas could replace some stuff in MFBT, which is always nice. I don't have a good feel for what inherited constructors and generic lambdas buy us.

Note those are hold back by both clang and gcc. Bug 1175546 is open to possibly bump the minimum gcc version. I don't know of an open bug for clang. FWIW, for gcc, what usually blocks is distros. For clang, it's our own automation using old versions.

Looks like we can't use constexpr even with 2015, see bug 1189655.

Standard constexpr support was only finished in the RTM (it was still in development even in the release candidate), so it's still very new. I wouldn't be surprised if they fix the ICE in an update, though I guess we can't rely on that until it happens.

From #build: 1:52 PM <RyanVM> gps: how fast are you planning to drop 2013 support at the configure level? 1:52 PM <@gps> there is another patch on inbound to prevent a JS test failure too 1:52 PM <RyanVM> since I suspect it won't take long before the bustage starts 1:52 PM <@gps> RyanVM: ask ehsan or froydnj: they own that decision 1:52 PM <RyanVM> ok 1:52 PM <ehsan> RyanVM: let's first wait to actually switch to 2015 ;) 1:53 PM <ehsan> if this ends up sticking, I'd be open to dropping support in the next release or two 1:53 PM <RyanVM> ok, I'm just remember how often msvc2012 broke once we switched from 2010 to 2013 in production I'm in favor of waiting a release cycle, maybe two (to see if any weird miscompilations trigger crashes on beta, for instance) and then switching.

Example of something that should get fixed in update 2, which it sounds like we should use: https://connect.microsoft.com/VisualStudio/feedback/details/2396360/bad-structure-offset-in-32-bit-code

Are we still on the track of dropping VS2013 support ? Since the infra had moved to VS2015, which means any commits on m-c could break the build of VS2013 since we can't detect it on try. There are already couple bugs(bug 1267296, bug 1267524#c131) related to this inconsistency between infra and dev. We can either either pull vs2013 build back on infra or officially drop it to solve this problem. Thoughts?

Can't. See bug 1270664 and bug 1265615

(In reply to Astley Chen [:astley] (UTC+8) from comment #10) > Are we still on the track of dropping VS2013 support ? Since the infra had > moved to VS2015, which means any commits on m-c could break the build of > VS2013 since we can't detect it on try. We're about a week into this situation.

The latest is we'll be requiring SSE (possibly SSE2) in Firefox 49+. Firefox 48/Aurora is being reverted to VS2013 to buy us time to add SSE detection to the updater/installer since we need to do that in release N-1 and we don't want to change Beta. As for dropping VS2013 support on 49/central, I believe that's a call for the new C++ module. Last I heard, they wanted us to maintain dual support for a release or two. If we need to, we should stand up VS2013 automation on central. That would likely be my responsibility. froydnj: how do you want to proceed?

(In reply to Gregory Szorc [:gps] from comment #13) > froydnj: how do you want to proceed? After a bit of bug spelunking, it looks like we started building nightlies with 2013 in bug 914596 comment 19 and bug 1061335, for the Firefox 36 release. We then dropped 2012 support in bug 117820, for the Firefox 37 release. A strict following of this pattern means that we'd build FF49 with 2015 and drop 2013 support in FF50. But this pattern isn't the full story, because we actually started using 2015 with nightlies in FF48, and we're only building FF48 with 2013 because of the SSE scramble. How confident are we that we are absolutely going to have the necessary bits in the updater/installer/etc. for SSE detection in FF48? I don't want to have to announce 2015 support, and then scramble to back it out in FF49 because we couldn't get all the other bits in place. If we can be reasonably sure that we're not going to have to scramble again, I feel good about dropping 2013 support in FF49.

rstrong: please respond to comment #14

There's also the concern that we'll find more issues with VS2015 as it hits a wider audience (namely the beta channel) and we'll have to back it out of FF49. What concerns me most is VS2015 only officially supports XP SP3. My understanding is there are gross hacks that allow us to run on XP SP2. But this doesn't exactly make me feel warm and fuzzy about XP SP2 compatibility.

Per https://wiki.mozilla.org/RapidRelease/Calendar Firefox 48 releases on 2016-08-02. The app update and installer changes should be finished hopefully well before the end of May. I have a patch in progress for app update and if worse comes to worse there is already an NSIS plugin for the installer (I haven't tested it yet) to detect the instruction set. bhearsum, will you be able to complete the changes to balrog? Not sure if web site changes will be necessary but if they are the web dev team will need to be asked. Is there anything else not covered by the above?

(In reply to Gregory Szorc [:gps] from comment #16) > What concerns me most is VS2015 only officially supports XP SP3. My > understanding is there are gross hacks that allow us to run on XP SP2. But > this doesn't exactly make me feel warm and fuzzy about XP SP2 compatibility. Our XP SP2 hack is only used for VS2013. The hack is hard-coding the CRT name "msvcr120.dll". VS2015 works without the hack somehow (although it does not officially support XP SP2, of course).

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #17) > Per > https://wiki.mozilla.org/RapidRelease/Calendar > > Firefox 48 releases on 2016-08-02. The app update and installer changes > should be finished hopefully well before the end of May. I have a patch in > progress for app update and if worse comes to worse there is already an NSIS > plugin for the installer (I haven't tested it yet) to detect the instruction > set. > > bhearsum, will you be able to complete the changes to balrog? Are you talking about the update ping stuff in https://bugzilla.mozilla.org/show_bug.cgi?id=1271761? If not, I'm not aware of any Balrog changes needed related to this.

(In reply to Ben Hearsum (:bhearsum) from comment #19) > (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from > comment #17) > > Per > > https://wiki.mozilla.org/RapidRelease/Calendar > > > > Firefox 48 releases on 2016-08-02. The app update and installer changes > > should be finished hopefully well before the end of May. I have a patch in > > progress for app update and if worse comes to worse there is already an NSIS > > plugin for the installer (I haven't tested it yet) to detect the instruction > > set. > > > > bhearsum, will you be able to complete the changes to balrog? > > Are you talking about the update ping stuff in > https://bugzilla.mozilla.org/show_bug.cgi?id=1271761? If not, I'm not aware > of any Balrog changes needed related to this. Yes... the changes needed for balrog to support this effort.

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #20) > (In reply to Ben Hearsum (:bhearsum) from comment #19) > > (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from > > comment #17) > > > Per > > > https://wiki.mozilla.org/RapidRelease/Calendar > > > > > > Firefox 48 releases on 2016-08-02. The app update and installer changes > > > should be finished hopefully well before the end of May. I have a patch in > > > progress for app update and if worse comes to worse there is already an NSIS > > > plugin for the installer (I haven't tested it yet) to detect the instruction > > > set. > > > > > > bhearsum, will you be able to complete the changes to balrog? > > > > Are you talking about the update ping stuff in > > https://bugzilla.mozilla.org/show_bug.cgi?id=1271761? If not, I'm not aware > > of any Balrog changes needed related to this. > Yes... the changes needed for balrog to support this effort. All the options discussed there so far require either trivial or no change to Balrog. I'll be able to do them if necessary, and they shouldn't be in the critical path.

(In reply to Nathan Froyd [:froydnj] from comment #14) > (In reply to Gregory Szorc [:gps] from comment #13) > > froydnj: how do you want to proceed? > > After a bit of bug spelunking, it looks like we started building nightlies > with 2013 in bug 914596 comment 19 and bug 1061335, for the Firefox 36 > release. We then dropped 2012 support in bug 117820, for the Firefox 37 > release. > > A strict following of this pattern means that we'd build FF49 with 2015 and > drop 2013 support in FF50. But this pattern isn't the full story, because > we actually started using 2015 with nightlies in FF48, and we're only > building FF48 with 2013 because of the SSE scramble. > > How confident are we that we are absolutely going to have the necessary bits > in the updater/installer/etc. for SSE detection in FF48? I don't want to > have to announce 2015 support, and then scramble to back it out in FF49 > because we couldn't get all the other bits in place. If we can be > reasonably sure that we're not going to have to scramble again, I feel good > about dropping 2013 support in FF49. FYI: The app update patch is ready for review in bug 1271761. This will be needed on Firefox 48 and Firefox ESR 45. The installer patch is ready for review in bug 1271759 but I suspect I will need new strings. The installer changes are needed for Firefox 49 so this shouldn't be an issue.

Thanks for the update. Even if those patches take some back-and-forth, they'll be done in plenty of time for 48. (47 might be another matter, see bug 1270664 comment 28 and following...) Given bug 1270664 comment 28 (that MSVC 2015 fixes real problems), that we have the necessary infrastructure to avoid SSE problems, and that we're not even building with MSVC 2013 in automation, I think it makes sense to proceed with removing MSVC 2013 support.

Update as of 5/27 The main patch for bug 1271761 has landed and merged to m-c. The remaining patch is a Firefox preference change so it will use the new value in the update url. This will land after bug 1274374 which will add support to the server for the new value is fixed. It will also be uplifted to aurora. The patch for bug 1271759 has landed and will merge to m-c in the near future. The url provided to the user is currently for the release system requirements and Bug 1276106 will add urls to systems requirement pages for beta and aurora. I'll file a new bug for changing the urls after Bug 1276106 is fixed.

I'll code up a patch for the build system changes.

Per froydnj in bug 1186064 comment #23, "it makes sense to proceed with removing MSVC 2013 support." This commit does that. Review commit: https://reviewboard.mozilla.org/r/56608/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/56608/

Update 5/31 Bug 1274374 has been pushed to production and the remaining patch for the Firefox preference change in bug 1271761 just landed on fx-team. I also requested aurora and esr45 approval on the patches in bug 1271761. Bug 1271759 merged to m-c and is fixed. So, all is in good shape and this will be my last status update.

Comment on attachment 8758348 [details] Bug 1186064 - Require Visual Studio 2015 Update 2; https://reviewboard.mozilla.org/r/56608/#review53378 ::: old-configure.in (Diff revision 1) > dnl platforms we have special knowledge of. > case "${target}" in > *-mingw*) > export MOZ_NO_DEBUG_RTL=1 > - if test "$MSVS_VERSION" != "2015"; then > - WIN32_CRT_LIBS="msvcrt.lib msvcprt.lib" this variable is only set here, and has an AC_SUBST. You can remove the AC_SUBST. ::: old-configure.in (Diff revision 1) > - WIN32_CRT_LIBS="msvcrt.lib msvcprt.lib" > - dnl Look for a broken crtdll.obj > - WIN32_CRTDLL_FULLPATH=`lib -nologo -list $WIN32_CRT_LIBS | grep crtdll\\.obj` > - lib -NOLOGO -OUT:crtdll.obj $WIN32_CRT_LIBS -EXTRACT:$WIN32_CRTDLL_FULLPATH > - if grep -q '__imp__\{0,1\}free' crtdll.obj; then > - MOZ_CRT=1 There's an AC_SUBST and an export in this file for MOZ_CRT, and it's the only thing setting it, so they can go away. There is also an AC_SUBST for it in js/src/old-configure.in (which gets the value from the export). There are also MOZ_CRT related cases that now can't happen and can be removed.

Tracking for 49 since we're aiming this change at 49.

Comment on attachment 8758348 [details] Bug 1186064 - Require Visual Studio 2015 Update 2; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56608/diff/1-2/

Comment on attachment 8758348 [details] Bug 1186064 - Require Visual Studio 2015 Update 2; https://reviewboard.mozilla.org/r/56608/#review55604 ::: build/moz.configure/toolchain.configure:544 (Diff revision 2) > if info.type == 'clang' and not info.version: > raise FatalCheckError( > 'Only clang/llvm 3.4 or newer is supported.') > > if info.type == 'msvc': > - if info.version < '18.00.30723' or ( > + if '19' < info.version < '19.00.23506': Heh, I hadn't seen this the first time, but this test doesn't fail with MSVC < 2015.

Comment on attachment 8758348 [details] Bug 1186064 - Require Visual Studio 2015 Update 2; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56608/diff/2-3/

Comment on attachment 8758348 [details] Bug 1186064 - Require Visual Studio 2015 Update 2; https://reviewboard.mozilla.org/r/56608/#review58506 ::: build/moz.configure/toolchain.configure:548 (Diff revision 3) > - if info.version < '18.00.30723' or ( > + if info.version < '19.00.23918': > - '19' < info.version < '19.00.23506'): > raise FatalCheckError( > 'This version (%s) of the MSVC compiler is not ' > 'supported.\n' > - 'You must install Visual C++ 2013 Update 3, Visual ' > + 'You must install Visual C++ 2015 Update 2 or newer in ' I'm not convinced it's necessary to drop support for 2015u1, but meh.

Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/96b590321c21 Require Visual Studio 2015 Update 2; r=glandium

https://reviewboard.mozilla.org/r/56608/#review58506 > I'm not convinced it's necessary to drop support for 2015u1, but meh. For posterity, my reasons to justify this. 1. We never shipped a Firefox built from VS2015u1. Since we never shipped with that toolchain, let's not pretend we support it. Automation jumped from VS2013u1 to VS2015u2: our developers should too. 2. VS2015u2 introduced some new compiler warnings. I don't want developers still running VS2015u1 to push to automation only to find out they get a compiler error from warnings-as-errors: this failure should be detected and reproducible locally. 3. I'd rather have N-1 toolchain change events. We'd eventually not support VS2015u1 and there's a good chance that would be discrete from dropping VS2015u2. So let's not take the chance and just drop VS2015u1 now - at the same time we're dropping VS2013. 4. I have no intention of fixing any build system bugs reported in VS2015u1. My resolution would be "WONTFIX: upgrade your toolchain." I don't want people to waste time tracing down and reporting bugs related to VS2015u1 only to have them WONTFIX. 5. I'd rather have N-1 supported toolchains. Less build configurations. Less things to go wrong. Less to support. Simpler.

I think it's also worth noting that updates to Visual Studio also include enhanced support for C++11, C++14 and C++17 features. In particular, the release notes for update 2 say that "The most notable compiler changes are support for Variable Templates and constexpr improvements". Update 3 contains a completely new optimizer for C++ code, which might drastically change performance characteristics in some cases. So I think it makes sense to aggressively remove support for earlier updates, to prevent developers making decisions about their code based on outdated behavior.

this caused bustage on autoland even on non windows builds like https://treeherder.mozilla.org/logviewer.html#?job_id=153070&repo=autoland

Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1bed3b5f90be Backed out changeset 96b590321c21 for bustage

Comment on attachment 8758348 [details] Bug 1186064 - Require Visual Studio 2015 Update 2; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56608/diff/3-4/

Comment on attachment 8758348 [details] Bug 1186064 - Require Visual Studio 2015 Update 2; Please look at the test changes. I had no clue we had tests for configure now. That is so awesome.

Comment on attachment 8758348 [details] Bug 1186064 - Require Visual Studio 2015 Update 2; https://reviewboard.mozilla.org/r/56608/#review58938 ::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:607 (Diff revision 4) > }) > > def test_msvc_2013(self): > self.do_toolchain_test(self.PATHS, { > 'c_compiler': self.VS_2013u3_RESULT, > 'cxx_compiler': self.VS_2013u3_RESULT, I'd expect this to fail because cxx_compiler is not set at all if the test for the C compiler fails. ::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:610 (Diff revision 4) > self.do_toolchain_test(self.PATHS, { > 'c_compiler': self.VS_2013u3_RESULT, > 'cxx_compiler': self.VS_2013u3_RESULT, > }, environ={ > 'CC': '/opt/VS_2013u3/bin/cl', > + 'CXX': '/opt/VS_2013u3/bin/cl', CXX doesn't need to be set anymore. ::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py (Diff revision 4) > }, environ={ > 'CC': '/opt/VS_2013u3/bin/cl', > + 'CXX': '/opt/VS_2013u3/bin/cl', > }) > > - def test_unsupported_msvc(self): I'd rather keep the tests as test_msvc/test_unsupported_msvc, and not separate them by version.

Comment on attachment 8758348 [details] Bug 1186064 - Require Visual Studio 2015 Update 2; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56608/diff/4-5/

Comment on attachment 8758348 [details] Bug 1186064 - Require Visual Studio 2015 Update 2; https://reviewboard.mozilla.org/r/56608/#review59322 ::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:587 (Diff revision 5) > GCC_4_9_RESULT = LinuxToolchainTest.GCC_4_9_RESULT > GXX_4_9_RESULT = LinuxToolchainTest.GXX_4_9_RESULT > GCC_5_RESULT = LinuxToolchainTest.GCC_5_RESULT > GXX_5_RESULT = LinuxToolchainTest.GXX_5_RESULT > > + # VS2015u2 is greater is required. s/is/or/

Comment on attachment 8758348 [details] Bug 1186064 - Require Visual Studio 2015 Update 2; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56608/diff/5-6/

Pushed by gszorc@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d7f98208809f Require Visual Studio 2015 Update 2; r=glandium

Backed out for pkg bustage like https://queue.taskcluster.net/v1/task/VesWNSzhQfGbA3rqliAbQw/runs/0/artifacts/public%2Flogs%2Flive_backing.log I think the part it's failing at is the very end of the log: Staging source tarball in /tmp/mozjs-src-pkg/mozjs-50.0a1.0... cp: cannot stat `/home/worker/workspace/build/src/js/src/../../mozglue/crt': No such file or directory [taskcluster 2016-07-06 16:10:12.123Z] === Task Finished === [taskcluster 2016-07-06 16:10:12.223Z] Unsuccessful task run with exit code: 1 completed in 103.587 seconds

Backout by kwierso@gmail.com: https://hg.mozilla.org/integration/autoland/rev/372bb24e7acd Backed out changeset d7f98208809f for breaking SM(pkg) jobs a=backout

Comment on attachment 8758348 [details] Bug 1186064 - Require Visual Studio 2015 Update 2; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56608/diff/6-7/

Landing with the reference to mozglue/crt in js/src removed.

Pushed by gszorc@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/398f38361dc2 Require Visual Studio 2015 Update 2; r=glandium

Hi :gps, Do we have plan to uplift this to 49 beta?

I was planning on letting this ride the trains.

No need to track VS2013 for 49.