MSVC version detection fails

UNCONFIRMED
Unassigned

Status

NSPR
NSPR
UNCONFIRMED
2 years ago
9 months ago

People

(Reporter: Nicolas Auvray, Unassigned)

Tracking

4.11

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
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.
(Reporter)

Comment 1

2 years ago
Hi Jacek, apparently you touched the code before, would you have any input on this bug? Thanks in advance :-)
Flags: needinfo?(jacek)

Comment 2

2 years ago
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").
Flags: needinfo?(jacek)
(Reporter)

Comment 3

2 years ago
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.
(Reporter)

Comment 4

2 years ago
Created attachment 8739464 [details]
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

2 years ago
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?
(Reporter)

Comment 6

2 years ago
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

2 years ago
Created attachment 8740051 [details] [diff] [review]
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?
(Reporter)

Comment 8

2 years ago
Yes, it works with this patch!

Comment 9

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

Updated

2 years ago
Attachment #8740051 - Flags: review?(ted)
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
Attachment #8740051 - Flags: review?(ted)
Flags: needinfo?(jacek)

Comment 11

a year ago
Created attachment 8792526 [details] [diff] [review]
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 :/).
Attachment #8740051 - Attachment is obsolete: true
Flags: needinfo?(jacek)
Attachment #8792526 - Flags: review?(ted)
Attachment #8792526 - Flags: review?(ted) → review+
Kaie is trying to ship an NSPR release so I'm going to wait to land this for a little bit.
Flags: needinfo?(ted)
I have resigned as NSPR module owner. Sorry for the inconvenience.
Flags: needinfo?(ted)
You need to log in before you can comment on or make changes to this bug.