448573 - configure fails - can't parse "$GCC -v" and retrieve version number with French VC8

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Francis Robichaud]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.1) Gecko/2008073015 Minefield/3.0.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.1) Gecko/2008073015 Minefield/3.0.1

cl.exe -v outputs this:
"Compilateur d'optimisation Microsoft (R) 32 bits C/C++ version 14.00.50727.762 pour 80x86
Copyright (C) Microsoft Corporation. Tous droits réservés."

_CC_MAJOR_VERSION variable is empty and configure exits with message "configure: error: This version of the MSVC compiler, $CC_VERSION , is unsupported."
The installed version of Visual Studio is "Microsoft Visual Studio 2005 - Français"

Workaround:
replacing lines 3006 and 3013 respectively with:
CC_VERSION=14.00.50727.762
CXX_VERSION=14.00.50727.762

Possible fix:
modify the _MSVC_VER_FILTER regex to support any language

Reproducible: Always



Expected Results:  
The right version number should be set for the CC_VERSION and CXX_VERSION variables.

Comment 1

[(not currently active) Ted Mielczarek]

I've seen this mentioned in the forums before, but I don't think it's ever been reported. I don't know what's actually failing here. Maybe you could figure out why that regex *doesn't* match the output you pasted there?

Comment 2

[Francis Robichaud]

Attachment: fix - supports any char instead of ' ' in the regex

I changed two regex used to parse the cl.exe, link.exe and mt.exe version numbers. They now rely on the word "version" (without the v) preceding the numbers.

On a french system, one or many 'ÿ' chars were printed between "version" and "14.00..." instead of an empty space. We can assume the "ersion" string won't be different on systems with other encodings/languages.

Comment 3

[Francis Robichaud]

Attachment: generated with autoconf2.13 on a linux system

@see attachment 331858 [details] [diff] [review]

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 331859 [details] [diff] [review]
generated with autoconf2.13 on a linux system

You don't need to submit patches to configure, configure.in is enough.

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 331858 [details] [diff] [review]
fix - supports any char instead of ' ' in the regex

I don't see how this regex can possibly work. You're trying to match "version 14.00.50727.762" with the regex ".*ersion[^0-9]+((?[0-9]+\.)*[0-9]+).*", which looks very wrong.

Comment 6

[Francis Robichaud]

Attachment: tested regex

Regex works perfectly, it has been tested by building Firefox on a Windows machine. I also tested it didn't impact my Linux build.
You can test the regex at http://www.rexv.org/
The '?' char might be optional but i placed it there to ignore the grouping create by first set of parenthesis.

The regex works, you might want to try it out yourself at rexv.org.

Comment 7

[Cameron McCormack (:heycam)]

I've had someone come to me with build problems with a French version of MSVC 2010 that is exactly this problem -- a ÿ character used just before the version number in the compiler's banner rather than a space.  ("ÿ" of course being Unicode character 255, which if interpreted in CP437 is a non-breaking space.)

I am having trouble exactly matching the ÿ, but how about we change the regular expression to allow a space or any non-ASCII character as the delimiter just before the version number?

Comment 8

[Cameron McCormack (:heycam)]

Attachment: patch

Comment 9

[(not currently active) Ted Mielczarek]

Comment on attachment 632163 [details] [diff] [review]
patch

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

::: configure.in
@@ +678,4 @@
>          AC_LANG_RESTORE
>  
>          changequote(,)
> +        _MSVC_VER_FILTER='s|.*[^!-~]([0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?).*|\1|p'

I guess this is fine. Honestly, we probably don't need the space check at all. Matching 3-4 groups of numbers separated by periods is probably sufficient.

You'll need to make this change in js/src/configure.in as well. It looks like NSPR's configure has this same check, but it doesn't explicitly error out for unrecognized versions, so you might not need to change that.

Comment 10

[Cameron McCormack (:heycam)]

Attachment: patch v1.1

OK.  There is some behaviour that switches depending on what the detected version is in nsprpub, so let's fix it there too.  I'll take your r+ as applying to the nsprpub/configure.in too, and r?dmandelin for js/src/configure.in.

Comment 11

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cce693a7edca

Comment 12

[(not currently active) Ted Mielczarek]

Oh, I'm sorry, I wasn't clear. NSPR still lives in CVS, so we have to land the patch there and sync it back to m-c with a tagged revision. If you attach a separate NSPR patch I'll land it in CVS for you.

Comment 13

[(not currently active) Ted Mielczarek]

(Also my review is fine for js/src/configure.in as well, it's all build system.)

Comment 14

[Cameron McCormack (:heycam)]

Attachment: patch to nspr

Oops, I think I knew that but forgot.  Good thing too, as I made a stupid c/p error in the original patch to nsprpub/configure.in.  This one should be good.

Comment 15

[Ed Morley [:emorley]]

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

Comment 16

[Alphat-PC]

Attachment: nsprpub-typo

(In reply to Ed Morley [:edmorley] from comment #15)
> https://hg.mozilla.org/mozilla-central/rev/cce693a7edca

There is a typographical error in nsprpub/configure.in.

Comment 17

[Cameron McCormack (:heycam)]

Ted, will the NSPR update and relanding in m-c happen soon, or should I fix up this nsprpub/configure.in problem first?  (Not sure whether it is causing a problem at the moment.)

Comment 18

[Alphat-PC]

nsprpub/config/autoconf.mk:
MSC_VER is undefined!

MSC_VER is used in nsprpub/pr/src/misc/Makefile.in:
$(OBJDIR)/prdtoa.$(OBJ_SUFFIX): prdtoa.c
	@$(MAKE_OBJDIR)
ifeq (,$(filter-out 1100 1200 1300 1310,$(MSC_VER)))
	$(CC) -Fo$@ -c $(CFLAGS) -Op $(call pr_abspath,$<)
else
	$(CC) -Fo$@ -c $(CFLAGS) -fp:precise $(call pr_abspath,$<)
endif

Comment 19

[Cameron McCormack (:heycam)]

Yeah, fair enough.  Filed bug 767848 to backout the nsprpub/configure.in change.