Created attachment 366032 [details] [diff] [review] Force logging in build rather than in code We need to have logging forced on, even in release builds. It's impossible to debug problems users have without it.
Attachment #366032 - Flags: review?(vladimir)
Comment on attachment 366032 [details] [diff] [review] Force logging in build rather than in code I don't think you even need the ifneq bit.. no reason not to just -DFORCE_PR_LOG unconditionally
Attachment #366032 - Flags: review?(vladimir) → review+
Created attachment 366333 [details] [diff] [review] Force logging everywhere in build Turns out we need to add the logging bit to just about every Makefile. Are you sure about -DFORCE_PRLOG everywhere, regardless of whether MOZ_LOGGING is on? Are we going to run into build problems in a build where logging is forced off?
Comment on attachment 366333 [details] [diff] [review] Force logging everywhere in build I don't see how it would, but ask ted?
Attachment #366333 - Flags: review?(vladimir) → review+
The only point of MOZ_LOGGING, it would appear, is to control whether various modules define FORCE_PR_LOG. It looks like everywhere else in the tree just does: #if defined(MOZ_LOGGING) #define FORCE_PR_LOG #endif http://mxr.mozilla.org/mozilla-central/search?string=MOZ_LOGGING Do you not have a central header you could do this in? If not, maybe instead you could add MOZ_LOGGING=1 here: http://mxr.mozilla.org/mozilla-central/source/configure.in#6461 and then AC_SUBST(MOZ_LOGGING) down with the other AC_SUBST bits, add it to autoconf.mk.in, and just use it as a makefile var: ifdef MOZ_LOGGING ... endif
Is this still something we want? I'd kind of like it for bug 666352 (although there, I really only care about having it on during my custom release builds, not official builds).
Yes, we definitely still want this.
Assignee: joe → justin.lebar+bug
OS: Mac OS X → All
Hardware: x86 → All
Created attachment 544848 [details] [diff] [review] Patch v1 This forces logging on within ImageLogging.h. This is similar to how nsHttp.h works.
Attachment #544848 - Flags: review?(joe)
Attachment #366333 - Attachment is obsolete: true
Comment on attachment 544848 [details] [diff] [review] Patch v1 If'n this passes try, hooray!
Attachment #544848 - Flags: review?(joe) → review+
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.