1265460 - Require configure flag to use VS2013

Status: RESOLVED DUPLICATE of bug 1186064

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

Our automation is using VS2015 now. We still soft support VS2013 but there isn't any automation coverage to ensure it still builds on mozilla-central.

Let's add a configure check that aborts when <VS2015u1 is used. Let's create a backdoor so people can define an environment variable or flag to override the check. This allows people to still build on VS2013 and to upgrade on their terms (instead of getting blocked when they pull down the tree).

Comment 1

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1265460 - Don't allow building with VS2013 unless a configure argument is present; r?ted

We don't have automation coverage of VS2013 any more. We want developers
to be using the same toolchain we use in automation.

This patch prods people towards VS2015 by requiring the newly-introduced
--enable-legacy-toolchain flag to build with VS2013.

Yes, the version checking should live in toolchain.configure. However,
the existing code is in old-configure.in and I didn't want to bloat
scope by porting the code to toolchain.configure.

Review commit: https://reviewboard.mozilla.org/r/47201/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/47201/

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 8742430 [details]
MozReview Request: Bug 1265460 - Don't allow building with VS2013 unless a configure argument is present; r?ted

https://reviewboard.mozilla.org/r/47201/#review43887

I'm not sure I really like this idea. I understand the motivation, to let people know they're building with something that we're not building in CI, but there are lots of other ways to do a non-standard build that might break, and I don't know that I want to chase that long tail. People are much more likely to wind up with a broken build from `--disable-crashreporter`, which is wildly popular. Should we warn on that?

::: build/moz.configure/toolchain.configure:154
(Diff revision 1)
>  # Compilers
>  # ==============================================================
> +option('--enable-legacy-toolchain', env='MOZ_ENABLE_LEGACY_TOOLCHAIN',
> +       help='Allow building with legacy toolchains')
> +
> +@depends('--enable-legacy-toolchain')

I don't think you actually need this @depends here.

::: old-configure.in:367
(Diff revision 1)
>          AC_DEFINE(_CRT_SECURE_NO_WARNINGS)
>          AC_DEFINE(_CRT_NONSTDC_NO_WARNINGS)
>          AC_DEFINE(_USE_MATH_DEFINES) # Otherwise MSVC's math.h doesn't #define M_PI.
>  
>          if test "$_CC_MAJOR_VERSION" = "18" -a "$_CC_BUILD_VERSION" -ge "30723"; then
> +            if [ -z "$MOZ_ENABLE_LEGACY_TOOLCHAIN" ]; then

You're not using `add_old_configure_assignment` on this value, so this won't work.

Comment 3

[Mike Hommey [:glandium]]

I agree with Ted. I understand the sentiment, but this is not doing anything useful IMHO. People who want to upgrade on their terms will add the flag, and have their build broken regularly exactly the same way they would have without having to add the flag.

The only thing this might achieve is having more people upgrade to 2015, but the builds breaking regularly on 2013 can do that just the same.

Seeing how regularly those builds break, we *really* should have 2013 tested on automation. Why not make e.g. the debug builds use 2013?

Comment 4

[Masatoshi Kimura [:emk]]

Why don't you just fix bug 1186064? VC2013 builds already start breaking (bug 1265154, bug 1265158).

Comment 5

[Gregory Szorc [:gps]]

(In reply to Masatoshi Kimura [:emk] from comment #4)
> Why don't you just fix bug 1186064? VC2013 builds already start breaking
> (bug 1265154, bug 1265158).

Some combination of {ehsan, froydnj, glandium, ted} insisted we maintain support for VC2013 for a few cycles. I believe this was a #build conversation. Their reasons sounded legit.

Comment 6

[Gregory Szorc [:gps]]

It is a common theme that we want to prod/notify people about things that make their build/dev experience sub-optimal only we don't have a good mechanism to do this. It is really frustrating because I can't help but feel that a lot of people get tripped up by preventable issues. I understand people don't want to get interrupted by tools insisting they do things. But at the same time, if we can inform people something is wrong, I think that will save more time in the end.

Perhaps we can add a warning to the post build output. We've done this before for things like high Finder CPU usage. Although, as Ted says, there is a long tail of non-standard builds. Do we want to prompt for them?

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Tip doesn't build on 2013 anymore, which just wasted 70 minutes of compiling time for me today.

Comment 8

[Masatoshi Kimura [:emk]]

Then this patch will waste even more time. VS2013 won't build anyway even if the flag is set.
If people have local patches to build Firefox on VS2013 locally, they can just add old-configure.in (or moz.configure) changes to their local patches.
I don't understand how this patch is useful at all.

Comment 9

[Gregory Szorc [:gps]]

This isn't needed since we now require VS2015 to build.