Closed Bug 794757 Opened 7 years ago Closed 7 years ago

add build config option to enable logging on release build

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: surkov, Assigned: tbsaunde)

References

Details

Attachments

(1 file)

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?
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?
(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.
(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
Blocks: 782991
(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?
(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.
(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
(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.
(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.
Attached patch patchSplinter Review
if your fine with this I don't mind if you make sure it works on windows and land it.
Attachment #667787 - Flags: review?(surkov.alexander)
Assignee: nobody → trev.saunders
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!
Attachment #667787 - Flags: review?(surkov.alexander) → review+
landed with couple fixes: http://hg.mozilla.org/integration/mozilla-inbound/rev/38b9f2f85de0
Target Milestone: --- → mozilla18
https://hg.mozilla.org/mozilla-central/rev/38b9f2f85de0
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.