The default bug view has changed. See this FAQ.

Add _MSC_VER (the cl version) to coreconf.

RESOLVED FIXED in 3.12.3

Status

NSS
Build
P2
enhancement
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Wan-Teh Chang, Assigned: Wan-Teh Chang)

Tracking

trunk
3.12.3
x86
Windows XP

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
Created attachment 362412 [details] [diff] [review]
Work in progress

We should define the make variable _MSC_VER as
the cl version in coreconf so that we can use it
to enable or disable cl features that only exist
in certain versions of cl.

This patch is work in progress.  Note that I
name the variable _MSC_VER (used in mozilla/configure.in)
rather than MSC_VER (used in mozilla/nsprpub/configure.in)
because I'd like to try to align with mozilla's build
system to make future maintenance easier.  Also
note that I don't define the variables _CC_MAJOR_VERSION
and _CC_MINOR_VERSION because I think they're just
intermediate variables, and we can save one shell call
by defining _MSC_VER directly with one shell call.

Finally, I don't escape the '.' in "awk -F." because '.'
is not a special character in shell.  '.' only needs to
be escaped in a regular expression.

I turn off -PDB:NONE if _MSC_VER != 1200 (VC++ 6.0).
Ideally the test should be _MSC_VER <= 1200, but it's
cumbersome to implement <= in makefiles.  As long as
none of us is using MSVC 5.0, the simple ifneq test
is sufficient.
(Assignee)

Comment 1

8 years ago
My last paragraph has some errors.  It should read:

  I turn on -PDB:NONE if _MSC_VER == 1200 (VC++ 6.0).
  Ideally the test should be _MSC_VER <= 1200, but it's
  cumbersome to implement <= in makefiles.  As long as
  none of us is using MSVC 5.0, the simple ifeq test
  is sufficient.
This can be further consolidated into a single $(shell command, e.g. 

+_MSC_VER    := $(shell $(CC) -v 2>&1 | grep Version | sed -e 's|.* Version ||' -e 's| .*||' | awk -F. '{ print $$1$$2 }')

Comment 3

8 years ago
Wan-Teh, what about saving some pipes in the shell call as well...? Like so:

_MSC_VER    := $(shell $(CC) 2>&1 | sed -ne 's/.* Version \([0-9]\{1,\}\)\.\([0-9]\{1,\}\).*/\1\2/p')
Kaspar inspired me.  :)

_MSC_VER := $(shell $(CC) 2>&1 | sed -ne \
's/.*[^0-9.]\([0-9]\{1,\}\)\.\([0-9]\{1,\}\).*/\1\2/p')
Created attachment 370043 [details] [diff] [review]
Patch v2 - brought forward to tip, incorporating suggested changes

Julien, please see the preceding comments in this bug before reviewing.
Thanks.
Attachment #362412 - Attachment is obsolete: true
Attachment #370043 - Flags: review?(julien.pierre.boogz)
Attachment #370043 - Attachment description: Patch v2 - brought forward to tip, incprorating suggested changes → Patch v2 - brought forward to tip, incorporating suggested changes
Attachment #370043 - Flags: superreview?(wtc)
(Assignee)

Updated

8 years ago
Attachment #370043 - Flags: superreview?(wtc) → superreview+
(Assignee)

Comment 6

8 years ago
Comment on attachment 370043 [details] [diff] [review]
Patch v2 - brought forward to tip, incorporating suggested changes

r=wtc.

> 	BSDECHO      = echo
> 	RC           = rc.exe
> 	MT           = mt.exe
>+	# Determine compiler version
>+	_MSC_VER_6   = 1200	# MSVC 6
>+	_MSC_VER := $(shell $(CC) 2>&1 | sed -ne \
>+		's/.*[^0-9.]\([0-9]\{1,\}\)\.\([0-9]\{1,\}\).*/\1\2/p' )

Nit: you may want to align := with the ='s above.
Thanks, Wan-Teh.  I cleaned up that alignment.
Checking in WIN32.mk; new revision: 1.35; previous revision: 1.34
Severity: normal → enhancement
Priority: -- → P2
Target Milestone: --- → 3.12.3
Version: unspecified → trunk
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Comment 8

8 years ago
Note that the VC6 build is broken on the trunk in freebl, even after that checkin, in the following way :

/WINNT_SINGLE_SHLIB/ -Od -W3 -nologo -D_CRT_SECURE_NO_WARNINGS -MD -we4002 -we40
03 -we4004 -we4006 -we4009 -we4013 -we4015 -we4028 -we4033 -we4035 -we4045 -we40
47 -we4053 -we4054 -we4063 -we4064 -we4078 -we4087 -we4098 -we4390 -we4551 -we45
53 -we4715 -GT -DXP_PC -DSHLIB_SUFFIX=\"dll\" -DSHLIB_PREFIX=\"\" -DSHLIB_VERSIO
N=\"3\" -DSOFTOKEN_SHLIB_VERSION=\"3\" -DRIJNDAEL_INCLUDE_TABLES -DDEBUG -D_DEBU
G -UNDEBUG -DDEBUG_Administrator -DWIN32 -D_X86_ -D_WINDOWS -DWINNT -DNSS_ENABLE
_ECC -DNSS_ECC_MORE_THAN_SUITE_B -DUSE_UTIL_DIRECTLY -D_X86_ -DMP_ASSEMBLY_MULTI
PLY -DMP_ASSEMBLY_SQUARE  -DMP_ASSEMBLY_DIV_2DX1D -DMP_USE_UINT_DIGIT -DMP_NO_MP
_WORD -DMP_API_COMPATIBLE -I../../../../dist/WINNT5.2_DBG.OBJ/include -I../../..
/../dist/public/nss -I../../../../dist/private/nss -Impi -Iecl  /c/NSS/tip/mozil
la/security/nss/lib/freebl/sysrand.c
Command line warning D4002 : ignoring unknown option '-we4002'
Command line warning D4002 : ignoring unknown option '-we4003'
Command line warning D4002 : ignoring unknown option '-we4004'
Command line warning D4002 : ignoring unknown option '-we4006'
Command line warning D4002 : ignoring unknown option '-we4009'
Command line warning D4002 : ignoring unknown option '-we4013'
Command line warning D4002 : ignoring unknown option '-we4015'
Command line warning D4002 : ignoring unknown option '-we4028'
Command line warning D4002 : ignoring unknown option '-we4033'
Command line warning D4002 : ignoring unknown option '-we4035'
Command line warning D4002 : ignoring unknown option '-we4045'
Command line warning D4002 : ignoring unknown option '-we4047'
Command line warning D4002 : ignoring unknown option '-we4053'
Command line warning D4002 : ignoring unknown option '-we4054'
Command line warning D4002 : ignoring unknown option '-we4063'
Command line warning D4002 : ignoring unknown option '-we4064'
Command line warning D4002 : ignoring unknown option '-we4078'
Command line warning D4002 : ignoring unknown option '-we4087'
Command line warning D4002 : ignoring unknown option '-we4098'
Command line warning D4002 : ignoring unknown option '-we4390'
Command line warning D4002 : ignoring unknown option '-we4551'
Command line warning D4002 : ignoring unknown option '-we4553'
Command line warning D4002 : ignoring unknown option '-we4715'
sysrand.c
c:/NSS/tip/mozilla/security/nss/lib/freebl/win_rand.c(194) : error C2065: 'CSIDL
_COMPUTERSNEARME' : undeclared identifier
c:/NSS/tip/mozilla/security/nss/lib/freebl/win_rand.c(194) : error C2099: initia
lizer is not a constant
make[1]: *** [WINNT5.2_DBG.OBJ/WINNT_SINGLE_SHLIB/sysrand.obj] Error 2
make[1]: Leaving directory `/c/NSS/tip/mozilla/security/nss/lib/freebl'
make: *** [libs] Error 2

The 3.11 branch still builds .

Comment 9

8 years ago
Comment on attachment 370043 [details] [diff] [review]
Patch v2 - brought forward to tip, incorporating suggested changes

Already checked in, cancelling review. This looks fine, but the tip VC6 build is broken in the first place, I don't know if we should bother with _MSC_VER_6.

We could use the _MSC_VER in the future to deal with other incompatibilities in new releases of MSVC.
Attachment #370043 - Flags: review?(julien.pierre.boogz)
Created attachment 370329 [details] [diff] [review]
patch - don't use -weNNNN with MSVC6 (checked in)

This patch addresses one of Julien's observations.
Julien, please review.
Attachment #370329 - Flags: review?(julien.pierre.boogz)

Updated

8 years ago
Attachment #370329 - Flags: review?(julien.pierre.boogz) → review+
The problem reported in the MSVC6 build with the error message:

> win_rand.c(194): error C2065: 'CSIDL_COMPUTERSNEARME': undeclared identifier

Isn't really an issue with the compiler version.  It's an issue with the 
header files.  The solution is to use header files from a newer Microsoft
"Platform SDK" with the older compiler.
Comment on attachment 370329 [details] [diff] [review]
patch - don't use -weNNNN with MSVC6 (checked in)

Checking in WIN32.mk; new revision: 1.36; previous revision: 1.35
Attachment #370329 - Attachment description: patch - don't use -weNNNN with MSVC6 → patch - don't use -weNNNN with MSVC6 (checked in)

Comment 13

8 years ago
Nelson,

With the same VC6 build environment, the NSS_3_11_BRANCH builds, but the trunk build fails, so it can't just be the platform SDK .
Julien, do you see the symbol CSIDL_COMPUTERSNEARME in the 3.11 branch code?

Comment 15

8 years ago
No. Hmm, I think I need to get more sleep, because I had checked that yesterday and I thought that code existed in both the branch and trunk. I guess I should be able to fix this in my VC6 environment in this case by pointing to the Windows 2003 SDK.
You need to log in before you can comment on or make changes to this bug.