http://lxr.mozilla.org/seamonkey/search?string=FORCE_PR_LOG This generates bloat, we need to make sure we don't build w/ this flag in final builds.

do you mean mozilla final or netscape final?

-> chofmann. this is a final build flag.

over to granrose. jj leaf to cc

this looks nasty. what exactly does FORCE_PR_LOG do? And what exactly does disabling it give us? It looks like we have to check in on the branch to remove all the -DFORCE_PR_LOG entries since I'd expect we don't want this turned off in anything but the Netscape and/or mozilla official releases (does mozilla care about this possible bloat?). If that's the case this may be moved to bugscape. ccing cls, lpham, and kysmith.

it's not as nasty as it appears. force_log was put in to enable PR_LOGGING in release builds so we could provide logging to QA in optimized bits. It's been argued that it should be removed altogether, I'm easily convinced of this :-). It's not global, each module that wants logging in opt bits has to define it. We want it removed in a "final" build so all those logging statements get boiled out and we reduce code bloat.

cc lisa for her input from the QA perspective about turning off logging in the release builds and when we want to do this. Personally, I would think this wouldn't make much difference to QA, but a world of difference to engineers who might have a harder time debugging a problem without the log info in the release builds.

Actually, this may affect QA as well. Mail uses a lot of logging to figure out problems with IMAP, POP, SMTP, NNTP protocols. See http://www.mozilla.org/quality/mailnews/mail-troubleshoot.html#imap. Does turning this flag off prevent those protocols from being generated?

retargeting for 0.9.1 since we still don't know when we want to turn this off. Regardless, we'll need to turn this off for a milestone or beta to test it before we turn it off for any RTM releases.

I'm not going to have time to do this, reassigning to cls. setting to moz 0.9 since we want to get this done asap. cls - can you put together a patch to make FORCE_PR_LOG a global and get that reviewed and in? Then once moz 0.9 is cut, turn it off on the branch so the release builds don't have it.

cc: mscott. Scott, can you see my question on 3-29? We rely on the protocol logs a lot on release builds.

Sorry guys, you can't turn off FORCE_PR_LOG globally. Our RTM imap builds have to have logging capability. We work through most of our interop issues via imap logs companies submit to us from our RTM bits. I keep having to remind Judson about this every time he gets it in his head to disable logging for everyone in rtm builds =).....

so can I just WONTFIX this?

this flag is abused. the whole foundation of it was so that RTM folks who *needed* this logging could have it. now people are using it all over the place. we should determine which modules have the *real* needs (right now, only mail/news AFAIK), and those modules should have to opt-in at build time for NS/Mozilla browser RTMs), default is to not log. I need the ability to ensure that *no* logging is on in *my* RTM build though (thus build time flags). I'm emebedding gecko, and I don't want *any* logging to occur, _anywhere_. The whole point was that this would be a build flag, *not* a #define (which is what people are doing) in source code. The interested modules would build themselves w/ this flag in the cmd line, *not* the .h/.c*.

oooook. Waterson, any objections to overriding the --disable-logging option to handle this PR_Log case as well or do I have to do something lame like add a --disable-pr-logging ?

Naw, that should be fine.

- equiv on windows is what? just setting build time flags as usual? - this looks right to me, but we need to ensure that mail/news is built (for moz/NS) w/ this flag on. how do we ensure that?

Windows? *sigh* You don't ensure that this is on for mailnews. This is the all or nothing option that you requested. It's on by default (for unix anyway) and you turn it off by specifying the --disable-logging option. I'll cobble togther a NO_LOGGING equiv for windows and I'll have to hand off the mac changes to someone else.

>> This is the all or nothing option that you requested. fair enough. thanks!

this may help jud but won't help the build team since we package the embedding bits based on the mozilla bits. so we'd either have to turn off all logging for the browser and embedding, or build twice: browser w/logging, embedding w/o logging. since embedding has priority, if we turn this on that means no logging in release builds for mailnews until we straighten this out. cc'ing phil and selmer for executive oversight.

currently embedding customers don't use our packaged bits for their dists. so let's leave this off for now (that will keep QA/mail/news happy). The biggest thing is to allow this build flag to "work" so embedding customers can use it. Thanks!

do early in 0.9.1

Agree with Jud. We add the build flag which makes PR_LOG statements compile to nothing and embedding customers run builds with that flag enabled. If Netscape-branded builds don't run with the flag (in order to meet the mail team's needs) that's fine with us.

r=sfraser on mac build changes.

so, for rtm mozilla and netscape bits we are going to force PR_Logging, right? but not for the embedding bits. (is that something release will be doing? building twice, or having two trees, one for mozilla and one for embedding?) as lchiang and mscott pointed out, we in mailnews land needs to ship rtm bits with logging. please confirm. if yes, then r=sspitzer for the mailnews changes.

client release won't be flipping this on for anything we produce, at first. I'm sure a friendly embedding testing customer will file an RFE for us to produce separate builds of embedding packages with it on, but the idea of *this* bug is to allow other people (outside of netscape client development) to produce embeddable stuff without logging on.

> I'm sure a friendly embedding testing customer will file an RFE for us to > produce separate builds of embedding packages with it on I don't expect that. Our embedding customers don't take CPD-generated binaries -- they do their own builds from source.

but if qa is doing performance/footprint testing of the embedding tarfiles we spit out every day, the numbers are not going to match what is actually seem by customers using source. Isn't that the point of the embedding tarfiles?

Leaf, the embedding customers already don't see precisely the same behavior as the embedding tarfiles would show -- they're not running gtkEmbed or mfcEmbed -- they're running their own apps. In some cases, the build settings they use even need to be different than ours (i.e. they use settings we wouldn't want, like this PR_LOG thing). Could that make bug reproducibility hard? Sure. Welcome to our hell :-)

our embedding test apps just help us to see what kinds of behavior and estimated performance and size characteristics a wide variety of customers might see. Individual customer's mileage may vary, additional Taxes and License Fees may be appilicable to Residents of New Jersey and Massachusetts and Oregon.

r=leaf, though this is probably going to slow down the regular app, as PR_LOGGING is going to be turned on for everything now, right?

y. I bet this is a non-trivial perf hit.

Yep, that's the downside. An alternative is to have configure set MOZ_LOGGING or something and then have each module have a: #ifdef MOZ_LOGGING #define FORCE_PR_LOG #endif if they want to force logging on. Aieee...back to the random patch maker (damn you leaf :-)).

Is this really what we want? I think we want a way to force it off in the few places which try to force it on. This patch appears to be a way to force it on for all code everywhere.

r=leaf for 32673

Patch has been checked in. Notice about wrapping FORCE_PR_LOG defines in a MOZ_LOGGING ifdef has been sent out. Marking fixed.