794757 - add build config option to enable logging on release build

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

some test failures happens on release builds and never happen on debug builds. It'd be nice if we have build option to enable logging capabilities on release and have mochitest-other machines built with this option.

Does it sound reasonable? Trev/David?

Comment 1

[David Bolter [:davidb] (NeedInfo me for attention)]

If you think it is useful into the future rather than just on a case by case basis then this sounds okay to me. It would never go into an actual release right?

Comment 2

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #0)
> some test failures happens on release builds and never happen on debug
> builds. It'd be nice if we have build option to enable logging capabilities
> on release and have mochitest-other machines built with this option.
> 
> Does it sound reasonable? Trev/David?

I suspect it'll be a good bit easier to just a A11Y_LOGGING define checks instead of the DEBUG ones, and then just define it in Logging.h which I believe you need to include every where you care about anyway.

Comment 3

[alexander :surkov (:asurkov)]

(In reply to David Bolter [:davidb] from comment #1)
> If you think it is useful into the future rather than just on a case by case
> basis then this sounds okay to me. It would never go into an actual release
> right?

I'm not sure I get you. I need this feature on case by case basis, for example, for to log bug 782991 since debug builds don't fail

Comment 4

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #2)

> I suspect it'll be a good bit easier to just a A11Y_LOGGING define checks
> instead of the DEBUG ones, and then just define it in Logging.h which I
> believe you need to include every where you care about anyway.

Yes, that's what you suggested when I implemented that initially. It doesn't seem we really need this feature on any release build (only those who's running the mochitests) so I wouldn't want to do this.

But I guess we have 'ifdef' to detect nightlies and thus we can keep this feature enabled on nightlies/debug only. How does it sound?

Comment 5

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #4)
> (In reply to Trevor Saunders (:tbsaunde) from comment #2)
> 
> > I suspect it'll be a good bit easier to just a A11Y_LOGGING define checks
> > instead of the DEBUG ones, and then just define it in Logging.h which I
> > believe you need to include every where you care about anyway.
> 
> Yes, that's what you suggested when I implemented that initially. It doesn't
> seem we really need this feature on any release build (only those who's
> running the mochitests) so I wouldn't want to do this.
> 
> But I guess we have 'ifdef' to detect nightlies and thus we can keep this
> feature enabled on nightlies/debug only. How does it sound?

is there an ifdef for nightly as oposed to on change opt or onchange pgo opt build?

If not it seems like atleast in the short term it'll be faster to just land the patch just after one nightly and back it out before the next.

Comment 6

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #5)

> is there an ifdef for nightly as oposed to on change opt or onchange pgo opt
> build?

I don't know that. Btw, what's difference between them?

> If not it seems like atleast in the short term it'll be faster to just land
> the patch just after one nightly and back it out before the next.

not in long term at least until you write crazy regexp to generate patches

Comment 7

[alexander :surkov (:asurkov)]

example of nightly detection (if I read things right) http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/Makefile.in#13

Comment 8

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #6)
> (In reply to Trevor Saunders (:tbsaunde) from comment #5)
> 
> > is there an ifdef for nightly as oposed to on change opt or onchange pgo opt
> > build?
> 
> I don't know that. Btw, what's difference between them?

not exactly sure, I think only nightly's are forced to be full builds in clean objdir, but I'm not if there's anything else.

> > If not it seems like atleast in the short term it'll be faster to just land
> > the patch just after one nightly and back it out before the next.
> 
> not in long term at least until you write crazy regexp to generate patches

why? its a one line patch to Logging.h

-#undef A11Y_LOGGING
+#define A11Y_LOGGING


(In reply to alexander :surkov from comment #7)
> example of nightly detection (if I read things right)
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> Makefile.in#13

I'm not really familiar with that make goo, but it looks to me like that says nightlys as well as the stuff built on every commit is a nightly, I guess if your fine with that it works.

Comment 9

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to alexander :surkov from comment #3)
> (In reply to David Bolter [:davidb] from comment #1)
> > If you think it is useful into the future rather than just on a case by case
> > basis then this sounds okay to me. It would never go into an actual release
> > right?
> 
> I'm not sure I get you. I need this feature on case by case basis, for
> example, for to log bug 782991 since debug builds don't fail

Another way to put that was: as long as the additional debugging spew is useful into the future (and not just for one particular case). Anyways, whatever you and Trevor conclude here is fine with me.

Comment 10

[Trevor Saunders (:tbsaunde)]

Attachment: patch

if your fine with this I don't mind if you make sure it works on windows and land it.

Comment 11

[alexander :surkov (:asurkov)]

Comment on attachment 667787 [details] [diff] [review]
patch

r=me, I'll land it with nit fixed as we discussed per irc. thank you for fixing it!

Comment 12

[alexander :surkov (:asurkov)]

landed with couple fixes: http://hg.mozilla.org/integration/mozilla-inbound/rev/38b9f2f85de0

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/38b9f2f85de0