481962 - Force logging on in imagelib

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Joe Drew (not getting mail)]

Attachment: 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.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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

Comment 2

[Joe Drew (not getting mail)]

Attachment: 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 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 366333 [details] [diff] [review]
Force logging everywhere in build

I don't see how it would, but ask ted?

Comment 4

[(not currently active) Ted Mielczarek]

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

Comment 5

[Justin Lebar (not reading bugmail)]

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).

Comment 6

[Joe Drew (not getting mail)]

Yes, we definitely still want this.

Comment 7

[Justin Lebar (not reading bugmail)]

Attachment: Patch v1

This forces logging on within ImageLogging.h.  This is similar to how nsHttp.h works.

Comment 8

[Joe Drew (not getting mail)]

Comment on attachment 544848 [details] [diff] [review]
Patch v1

If'n this passes try, hooray!

Comment 9

[Justin Lebar (not reading bugmail)]

Try push: http://hg.mozilla.org/try/rev/144138c0b56d

Comment 10

[Justin Lebar (not reading bugmail)]

Inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/0c69c4d8e19e

Comment 11

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/0c69c4d8e19e