1238154 - MSVC version detection fails

Status: UNCONFIRMED

(NSPR :: NSPR, defect)

Description

[Nicolas Auvray [:Itms]]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20160105164030

Steps to reproduce:

I tried building NSPR 4.11 on Windows using the latest MozillaBuild (as of 01-08-2016), my compiler being VisualStudio Express 2013 for desktop.


Actual results:

I got the error "Could not determine MSC version." Building earlier versions of NSPR (namely 4.10.7) works fine with the same toolchain. As I understand it it comes from the fix to #1160125 which, on my system, doesn't output a valid string to $CC_VERSION.

As a workaround, I removed the `if test -z "$CC_VERSION";` check. After that configuring worked as expected. However the compilation failed because MSC_VER wasn't defined at line 86 of config/rules.mk, resulting in an extra "lib" in some filenames.


Expected results:

The $CC_VERSION and $MSC_VER variables should be properly defined by the ./configure script.

Comment 1

[Nicolas Auvray [:Itms]]

Hi Jacek, apparently you touched the code before, would you have any input on this bug? Thanks in advance :-)

Comment 2

[Jacek Caban]

I'm sorry for the delay. Is there anything interesting in config.log? Also, what's the output of ${CC} -v (where ${CC} is probably "cl").

Comment 3

[Nicolas Auvray [:Itms]]

I'm attaching the config.log, I don't see anything interesting in it but I may be missing something.

${CC} is indeed 'cl'.

'-v' is not a valid option for cl (I found this question on StackOverflow - https://stackoverflow.com/questions/1233312/ - could it cause the problem?)
I still get an output before the "unknown option" error message, which tells me that cl is at 18.00.40629.

Comment 4

[Nicolas Auvray [:Itms]]

Attachment: config.log for a tentative NSPR 4.12 build

Here I tried with NSPR 4.12 because it's out now, but the bug is the same.

Comment 5

[Jacek Caban]

Yes, it's an unknown option and the check depends on its side effect of printing the version in  error message. However, that's generally true for all versions, so I don't think that's the reason it fails for you.

What configure script does may be done manually with:

_MSVC_VER_FILTER='s|.* \([0-9]\+\.[0-9]\+\.[0-9]\+\(\.[0-9]\+\)\?\).*|\1|p' cl -v 2>&1 | sed -ne "$_MSVC_VER_FILTER"

Could you check what's its result if you exec it manually?

Comment 6

[Nicolas Auvray [:Itms]]

The output of this command is empty.

By running 
cl -v > test.txt 2>&1
I see in that file that there is a special character just before the version number (I get "versionÿ18.00.40629 for x86"). Could it cause the issue?

Comment 7

[Jacek Caban]

Attachment: patch.diff

Yes, it may be a problem. As far as I can see we require a space before version numbers. Does the attached patch help?

Comment 8

[Nicolas Auvray [:Itms]]

Yes, it works with this patch!

Comment 9

[Jacek Caban]

Cool. That may actually be the right fix, but I'm not an expert.

Comment 10

[(not currently active) Ted Mielczarek]

Comment on attachment 8740051 [details] [diff] [review]
patch.diff

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

::: configure.in
@@ +1886,5 @@
>          DLL_SUFFIX=dll
>  
>          # Determine compiler version
>          changequote(,)
> +        _MSVC_VER_FILTER='s|.*\([0-9]\+\.[0-9]\+\.[0-9]\+\(\.[0-9]\+\)\?\).*|\1|p'

I think I'd rather that you make this match the check in the top-level configure:
https://dxr.mozilla.org/mozilla-central/rev/1da1937a9e03154ae7c60089f2dcf5ad9ee20fa3/old-configure.in#347

Comment 11

[Jacek Caban]

Attachment: fix

Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0eec4a885ed


My apologize for the delay. Recent python configure rewrite broke totally cross compiling for me, so I'm unable to do any development until I find enough time to fix if (and finding it is tricky those days :/).

Comment 12

[(not currently active) Ted Mielczarek]

Kaie is trying to ship an NSPR release so I'm going to wait to land this for a little bit.

Comment 13

[(not currently active) Ted Mielczarek]

I have resigned as NSPR module owner. Sorry for the inconvenience.

Comment 14

[serval]

On Windows 7 with Visual Studio 2017, I encounter a building problem on LibreOffice (which uses nss and nspr as third party modules) related to regexp provided to sed.
I use French UI and indeed cl.exe doesn't give plain spaces in my cases why makes regexp fail.

Considering the patch provided, I see that regexp has been replaced by:
's|.^!-~.|\1|p'

shouldn't it be:
s|.[^!-~]([0-9]+.[0-9]+.[0-9]+(.[0-9]+)?).|\1|p
?

See http://document-foundation-mail-archive.969070.n3.nabble.com/Could-not-determine-MSC-version-Visual-Studio-2017-td4256538.html for all the details.

Comment 15

[serval]

Sorry, automatic reformat changed the regexp:

patch proposed:
s|.*[^!-~]([0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?).*|\1|p

I'd rather suggest:
s|.*[^!-~]\([0-9]\+\.[0-9]\+\.[0-9]\+\(\.[0-9]\+\)\?\).*|\1|p