1043082 - NSPR's build system hardcodes -MD

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P1)

Description

[(no longer active)]

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!

Comment 1

[(no longer active)]

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!

Comment 2

[(no longer active)]

ping?

Comment 3

[Wan-Teh Chang]

Ehsan: sorry about the delay. I will work on this next Monday or Tuesday.

Comment 4

[Wan-Teh Chang]

Attachment: NSPR: add the --enable-static-rtl configure option

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.

Comment 5

[Wan-Teh Chang]

Attachment: NSS: add the USE_STATIC_RTL makefile variable

Comment 6

[Wan-Teh Chang]

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.

Comment 7

[(no longer active)]

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!

Comment 8

[(no longer active)]

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?

Comment 9

[Wan-Teh Chang]

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

Comment 10

[Wan-Teh Chang]

Attachment: NSPR: align the help message for the --enable-static-rtl configure option

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

Comment 11

[Wan-Teh Chang]

Attachment: NSS: add the USE_STATIC_RTL makefile variable, v2

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.

Comment 12

[(no longer active)]

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!

Comment 13

[Wan-Teh Chang]

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

Comment 14

[(no longer active)]

NSPR patch on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/c5594b478826

Comment 15

[(no longer active)]

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!

Comment 16

[Wan-Teh Chang]

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!

Comment 17

[(no longer active)]

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...

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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

Comment 19

[(no longer active)]

ping?

Comment 20

[Wan-Teh Chang]

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

Comment 21

[(no longer active)]

Awesome, thanks so much for your help, Wan-Teh!

Comment 22

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.