Closed Bug 1043082 Opened 10 years ago Closed 10 years ago

NSPR's build system hardcodes -MD

Categories

(NSPR :: NSPR, defect, P1)

x86
Windows 7
defect

Tracking

(Not tracked)

RESOLVED FIXED
4.10.7

People

(Reporter: ehsan.akhgari, Assigned: wtc)

References

Details

Attachments

(3 files, 1 obsolete file)

See <http://mxr.mozilla.org/mozilla-central/source/nsprpub/configure.in#2006>.

For AddressSanitizer, we need to build with static CRT.  Wan-Teh, can you please suggest a good way to override -MD with -MT in those builds from the Mozilla build system?  Thanks!
Flags: needinfo?(wtc)
The same pattern happens in NSS as well.  <http://mxr.mozilla.org/mozilla-central/source/security/nss/coreconf/WIN32.mk#155>  Any advice to fix that is also really appreciated!
ping?
Ehsan: sorry about the delay. I will work on this next Monday or Tuesday.
Flags: needinfo?(wtc)
I seem to remember the NSPR or NSS build system used to have
a USE_STATIC_RTL makefile variable to control this. This patch
adds a --enable-static-rtl option to NSPR's configure script.
Attachment #8468193 - Flags: review?(ehsan)
Flags: needinfo?(ehsan)
Attachment #8468195 - Flags: review?(ehsan)
Comment on attachment 8468193 [details] [diff] [review]
NSPR: add the --enable-static-rtl configure option

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

::: configure.in
@@ +386,5 @@
>      [  --enable-debug-rtl      Use the MSVC debug runtime library],
>      [ if test "$enableval" = "yes"; then
>  	    USE_DEBUG_RTL=1
>        else
>  	    USE_DEBUG_RTL=0

You may have noticed that the USE_DEBUG_RTL variable may
also assume the value of 0. This wasn't the case until
recently:
https://hg.mozilla.org/projects/nspr/diff/8175a7a0f186/configure.in

I modeled the USE_STATIC_RTL variable after how USE_DEBUG_RTL
was used before.
Status: NEW → ASSIGNED
OS: Mac OS X → Windows 7
Priority: -- → P1
Comment on attachment 8468193 [details] [diff] [review]
NSPR: add the --enable-static-rtl configure option

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

Looks great, thanks!
Attachment #8468193 - Flags: review?(ehsan) → review+
Comment on attachment 8468195 [details] [diff] [review]
NSS: add the USE_STATIC_RTL makefile variable

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

::: coreconf/WIN32.mk
@@ +163,5 @@
>  	else
> +		ifdef USE_STATIC_RTL
> +			OS_CFLAGS += -MT
> +		else
> +			OS_CFLAGS += -MD

-MD is also added to OS_CFLAGS around line 139 under |ifdef BUILD_OPT|, shouldn't you handle that case as well?
Attachment #8468195 - Flags: review?(ehsan) → review-
Flags: needinfo?(ehsan)
Comment on attachment 8468193 [details] [diff] [review]
NSPR: add the --enable-static-rtl configure option

NSPR patch checked in: https://hg.mozilla.org/projects/nspr/rev/74f34c743ff5
Attachment #8468193 - Flags: checked-in+
I noticed the help message of the new --enable-static-rtl configure option
was over-indented. This patch fixes that.

https://hg.mozilla.org/projects/nspr/rev/a19171d4cd48
Attachment #8469727 - Flags: checked-in+
Ehsan: good catch!

The original code ignores USE_DEBUG_RTL in optimized builds (if
BUILD_OPT is set). I decided to drop that behavior to simplify
the new code.
Attachment #8468195 - Attachment is obsolete: true
Attachment #8469736 - Flags: review?(ehsan)
Flags: needinfo?(ehsan)
Comment on attachment 8469736 [details] [diff] [review]
NSS: add the USE_STATIC_RTL makefile variable, v2

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

Looks great, thanks!
Attachment #8469736 - Flags: review?(ehsan) → review+
Flags: needinfo?(ehsan)
Comment on attachment 8469736 [details] [diff] [review]
NSS: add the USE_STATIC_RTL makefile variable, v2

NSS patch checked in: https://hg.mozilla.org/projects/nss/rev/2f2088ed8a3b
Attachment #8469736 - Flags: checked-in+
Ted, or Wan-Teh, can one of you please tag the NSS revision 2f2088ed8a3b as NSS_3_17_BETA3 so that I can land this on inbound?  Thanks!
Flags: needinfo?(wtc)
Flags: needinfo?(ted)
Ehsan: in the future please don't create NSPR or NSS tags yourself because
you may not know our release plan.

We are likely to create the NSPR_4_10_7_RTM and NSS_3_17_RTM tags tomorrow.
When they are tagged, I will push them to mozilla-inbound for you!
Flags: needinfo?(wtc)
Oh, OK, thanks!  FWIW I don't think I have commit access to either NSPR or NSS.  :-)  But Ted created the NSPR tag earlier today.  Sorry if that stepped on your toes, I asked Ted to do that...
Flags: needinfo?(ted)
ping?
Ehsan: I'm sorry I forgot to update this bug.

The NSS patch landed in mozilla-central last Wednesday as part of
the NSS_3_17_RTM update:
http://hg.mozilla.org/mozilla-central/rev/32ac8061e886
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.10.7
Awesome, thanks so much for your help, Wan-Teh!
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.