687225 - configure does not detect MSVC 2011 compiler

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

I'm trying to build mozilla-central on Windows 8 with the Visual Studio 2011 toolchain and am getting the following error when running configure:

checking for strip... no
checking for windres... no
checking for midl... midl
configure: error: This version (17.00.40825.2) of the MSVC compiler is unsupport
ed. See https://developer.mozilla.org/en/Windows_Build_Prerequisites.
*** Fix above errors and then restart with               "c:/mozilla-build/pytho
n/python.exe c:/dev/mozilla-central/build/pymake/pymake/../make.py -f client.mk
build"
c:\dev\mozilla-central\client.mk:315:0: command 'cd obj-i686-pc-mingw32 &&   c:/
dev/mozilla-central/configure  \
  || ( echo "*** Fix above errors and then restart with\
               \"c:/mozilla-build/python/python.exe c:/dev/mozilla-central/build
/pymake/pymake/../make.py -f client.mk build\"" && exit 1 )' failed, return code
 1

I'll likely have a patch in a few minutes...

Comment 1

[Gregory Szorc [:gps]]

Attachment: Detect MSVC2011 Compiler in configure

This (trivial) patch detects the MSVC 2011 compiler in configure.

When applied, `make.py -f client.mk configure` works! (Although, there still might be a few warnings related to Windows 8/MSVC 2011 in there.) But, at least this patch unblocks on Windows 8 compiling.

Comment 2

[Mike Hommey [:glandium]]

You also need to fix
*/installer/package-manifest.in
and
*/installer/removed-files.in
for the corresponding CRT.

Comment 3

[Mike Hommey [:glandium]]

And I forgot toolkit/mozapps/installer/packager.mk, too

Comment 4

[Gregory Szorc [:gps]]

Ahh, good catch. I didn't realize those existed.

My compilation hasn't yet reached the installer phase. I could incorporate those into the patch easily enough or I can create a part 2 once compilation gets that far.

Comment 5

[Mike Hommey [:glandium]]

Oh, and you made me remember of yet another one: build/win32/Makefile.in. Man, we really need to group that.

Comment 6

[Gregory Szorc [:gps]]

OK, I'll upload a new patch shortly.

In the meantime, the output from `make.py -f client.mk configure` without an existing objdir is at http://gps.pastebin.mozilla.org/1333021. I don't know enough about our build system (yet) to determine if there is anything fishy.

Comment 7

[Mike Hommey [:glandium]]

Nothing seems wrong at first (quick) glance.
Note that for the CRT to be installed, you need to manually set the WIN32_REDIST_DIR variable.

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 560677 [details] [diff] [review]
Detect MSVC2011 Compiler in configure

Review of attachment 560677 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +731,5 @@
>              AC_DEFINE(_CRT_NONSTDC_NO_WARNINGS)
> +        elif test "$_CC_MAJOR_VERSION" = "17"; then
> +            _CC_SUITE=11
> +            AC_DEFINE(_CRT_SECURE_NO_WARNINGS)
> +            AC_DEFINE(_CRT_NONSTDC_NO_WARNINGS)

I wonder if we shouldn't just fold these three blocks together now, like:
elif test $_CC_MAJOR_VERSION -ge 15; then
  _CC_SUITE=$(( $_CC_MAJOR_VERSION - 6 ))
  ...

What do you think?

Comment 9

[Gregory Szorc [:gps]]

Actually, I should have canceled the review since there were some missing files per comment #2.

As for rolling everything together, it is very tempting. We run the risk of something in the future not following convention. But, I suppose we can cross that bridge if it comes to that.

I'll upload a new patch when I have access to the computer it is on.

Comment 10

[Gregory Szorc [:gps]]

Attachment: Detect MSVC2011 Compiler in configure, v2

This version adds checks to files that were missed in the first iteration. I confirmed that detection and compilation works in both x86 and x64 modes.

Comment 11

[(not currently active) Ted Mielczarek]

Comment on attachment 564093 [details] [diff] [review]
Detect MSVC2011 Compiler in configure, v2

Review of attachment 564093 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/win32/Makefile.in
@@ +97,5 @@
> +ifeq (1700,$(_MSC_VER))
> +REDIST_FILES = \
> +	msvcp110.dll \
> +	msvcr110.dll \
> +	$(NULL)

Please use two-space indent here. Apparently I've been bad about enforcing that in this file. Feel free to fix the rest of them to match if you'd like.

Comment 12

[Gregory Szorc [:gps]]

Attachment: Detect MSVC2011 in configure, v3

This is the same as the last patch (which received r+) except it corrects whitespace in build/win32/Makefile.in, as suggested by reviewer.

I don't have checkin privileges, so on r+, please check it in for me.

Comment 13

[Justin Wood (:Callek)]

Comment on attachment 564458 [details] [diff] [review]
Detect MSVC2011 in configure, v3

I verified this has no other changes, and meets ted's review requirement. Saving him the trouble and setting checkin-needed. Someone will get this into the tree shortly.

Comment 14

[Justin Wood (:Callek)]

r+=ted for posterity for checkin

Comment 15

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/projects/build-system/rev/fe2e10601158

Comment 16

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

https://hg.mozilla.org/mozilla-central/rev/fe2e10601158