Closed
Bug 938505
Opened 9 years ago
Closed 9 years ago
--enable-trace-malloc and --enable-dmd should fail at configure time
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla28
People
(Reporter: cbook, Assigned: sylvestre)
Details
Attachments
(1 file, 3 obsolete files)
2.11 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
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
The patch replaces the transparent desactivation of DMD by trace-malloc by a failure.
Attachment #832275 -
Flags: review?(mh+mozilla)
Comment 2•9 years ago
|
||
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.)
Attachment #832275 -
Flags: review?(mh+mozilla) → review-
Taking in account Mike's comments.
Attachment #832275 -
Attachment is obsolete: true
Attachment #832843 -
Flags: review?(mh+mozilla)
Comment 4•9 years ago
|
||
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?
Attachment #832843 -
Flags: review?(mh+mozilla) → review+
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.
Attachment #832843 -
Attachment is obsolete: true
Attachment #833319 -
Flags: review?(mh+mozilla)
Attachment #833319 -
Flags: checkin?
Comment 6•9 years ago
|
||
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.
Attachment #833319 -
Flags: review?(mh+mozilla) → review+
Sorry, fixed.
Attachment #833319 -
Attachment is obsolete: true
Attachment #833319 -
Flags: checkin?
Attachment #833322 -
Flags: review?(mh+mozilla)
Comment 8•9 years ago
|
||
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.
Attachment #833322 -
Flags: review?(mh+mozilla) → review+
Attachment #833322 -
Flags: checkin?
Comment 9•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/01c2d7fb5e71
Assignee: nobody → sylvestre
Flags: in-testsuite-
Comment 10•9 years ago
|
||
Comment on attachment 833322 [details] [diff] [review] fix-bug-938505.diff Please just use the checkin-needed bug keyword in the future :)
Attachment #833322 -
Flags: checkin? → checkin+
Comment 11•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/01c2d7fb5e71
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Updated•5 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•