1411156 - Set MOZ_DEBUG globally as both a CONFIG and a #define

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1411156 - Set MOZ_DEBUG globally as both a CONFIG and a #define.

Bug 1365460 introduced code paths behind MOZ_DEBUG #ifdefs, but
MOZ_DEBUG is never defined, while it is available in CONFIG in
moz.builds. This is kind of a confusing situation, but the fact that
we've been able to avoid those problems for so long would tend to
put the blame on mozjemalloc, and fixes should go there.

Except that bug 1261161 explains that the only existing alternative
(the DEBUG #define), as used in MFBT, is not working for spidermonkey,
so it actually makes sense to converge to MOZ_DEBUG rather than DEBUG.

So start defining MOZ_DEBUG globally, fixing the mozjemalloc issues of
not having the debug code enabled. Bug 1261161 can then take care of
changing the DEBUG #ifdefs.

Review commit: https://reviewboard.mozilla.org/r/192370/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/192370/

Comment 2

[Michael Shal [:mshal]]

Comment on attachment 8921345 [details]
Bug 1411156 - Set MOZ_DEBUG globally as both a CONFIG and a #define.

https://reviewboard.mozilla.org/r/192370/#review197666

::: moz.configure:106
(Diff revision 1)
>            help='Enable building with developer debug info '
>                 '(using the given compiler flags).')
>  
> -add_old_configure_assignment('MOZ_DEBUG',
> -                             depends('--enable-debug')(lambda v: bool(v)))
> +@depends('--enable-debug')
> +def moz_debug(debug):
> +    if debug:

Why do you need this if statement? It seems to work fine without it (just return bool(debug) always).

Comment 3

[Mike Hommey [:glandium]]

(In reply to Michael Shal [:mshal] from comment #2)
> Comment on attachment 8921345 [details]
> Bug 1411156 - Set MOZ_DEBUG globally as both a CONFIG and a #define.
> 
> https://reviewboard.mozilla.org/r/192370/#review197666
> 
> ::: moz.configure:106
> (Diff revision 1)
> >            help='Enable building with developer debug info '
> >                 '(using the given compiler flags).')
> >  
> > -add_old_configure_assignment('MOZ_DEBUG',
> > -                             depends('--enable-debug')(lambda v: bool(v)))
> > +@depends('--enable-debug')
> > +def moz_debug(debug):
> > +    if debug:
> 
> Why do you need this if statement? It seems to work fine without it (just
> return bool(debug) always).

Because, especially for defines, there's a difference between "not defined" and "no/false value". Without the if, we end up with -DMOZ_DEBUG="" in ACDEFINES, and #define MOZ_DEBUG in mozilla-config.h. (which is weird, I was expecting something different, actually ; comes from bug 1258618, apparently)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/396203b98e35
Set MOZ_DEBUG globally as both a CONFIG and a #define. r=mshal

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/396203b98e35