938505 - --enable-trace-malloc and --enable-dmd should fail at configure time

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Carsten Book [:Tomcat]]

Tried to build a with DMD enabled and failed using a mozconfig with

ac_add_options --enable-trace-malloc

According to glandium this is expected:

23:24 < glandium> Tomcat|sheriffduty: i don't think trace malloc and replace malloc work together
23:24 < glandium> we should fail at configure time


So yeah we should make 
ac_add_options --enable-trace-malloc
and
ac_add_options --enable-dmd

at the same time make failing at configure

Comment 1

[sylvestre]

Attachment: fix-bug-938505.diff

The patch replaces the transparent desactivation of DMD by trace-malloc by a failure.

Comment 2

[Mike Hommey [:glandium]]

Comment on attachment 832275 [details] [diff] [review]
fix-bug-938505.diff

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

::: configure.in
@@ +6768,5 @@
>      MOZ_DMD= )
>  
> +
> +dnl The two options are conflicting. Fails the configure to alert the user.
> +if test "$NS_TRACE_MALLOC" -a "$MOZ_DMD"; then

Make that MOZ_REPLACE_MALLOC instead of MOZ_DMD. dmd requires replace malloc, and it's replace malloc that conflicts with trace malloc.

(And in fact, configure.in already disables dmd when trace malloc is on. It would be better to AC_MSG_ERROR instead.)

Comment 3

[sylvestre]

Attachment: fix-bug-938505.diff

Taking in account Mike's comments.

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 832843 [details] [diff] [review]
fix-bug-938505.diff

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

::: configure.in
@@ +6834,5 @@
>  
> +dnl The two options are conflicting. Fails the configure to alert the user.
> +if test "$NS_TRACE_MALLOC" -a "$MOZ_REPLACE_MALLOC"; then
> +    AC_MSG_ERROR([--enable-trace-malloc and --enable-replace-malloc are conflicting options])
> +fi

While you're here, can you make --enable-trace-malloc --enable-dmd error out instead of siliently disabling dmd?

Comment 5

[sylvestre]

Attachment: fix-bug-938505.diff

Here it is! (I think it is was I have done in the first commit).

I did that in a single commit, don't hesitate if you want me to split it in two.

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 833319 [details] [diff] [review]
fix-bug-938505.diff

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

::: configure.in
@@ +6768,5 @@
>      MOZ_DMD= )
>  
> +dnl The two options are conflicting. Fails the configure to alert the user.
> +if test "$NS_TRACE_MALLOC" -a "$MOZ_DMD"; then
> +    AC_ERROR("--enable-trace-malloc and --enable-dmd are conflicting options")

AC_MSG_ERROR.

Comment 7

[sylvestre]

Attachment: fix-bug-938505.diff

Sorry, fixed.

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 833322 [details] [diff] [review]
fix-bug-938505.diff

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

No need to rerequest review when you were r+ed with trivial changed asked.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/01c2d7fb5e71

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 833322 [details] [diff] [review]
fix-bug-938505.diff

Please just use the checkin-needed bug keyword in the future :)

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/01c2d7fb5e71