Release build should not print to console

RESOLVED WONTFIX

Status

SeaMonkey
Build Config
--
major
RESOLVED WONTFIX
17 years ago
7 years ago

People

(Reporter: Phil Peterson, Assigned: Daniel (Leaf) Nunes)

Tracking

(Depends on: 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
One of our embedding customers observed that the release build still prints
stuff out to the console. Even if you redirect to /dev/null, it uses some memory
and looks unprofessional. 

mdunn, can someone in your group generate a list of things which get printed out
in a release build? If it can be fixed in a straightforward way, we'll just do
that. If not, we'll create more dependent bugs, assigned to various people.
(Reporter)

Updated

17 years ago
OS: Windows NT → All
Hardware: PC → All

Comment 1

17 years ago
seawood,  and quick and dirty ways to turn this off in the build system
or do we need to sweep the tree with a lot of fixes to do this?

I think there is a bug open on this already.
mdunn, can you search for the dupe?
(Reporter)

Comment 3

17 years ago
Jon, I think bug 53226 is a component of this, but there are probably other
abusers of log output besides PR_LOG, e.g. naked printfs. Mareking it a dependency.
Depends on: 53226

Comment 4

17 years ago
Created attachment 31504 [details] [diff] [review]
quick-n-dirty silence patch for unix

Comment 5

17 years ago
There's the quick-n-dirty fix that should turn off all output.  It doesn't
address the following concerns:

Wasted footprint from fprintf/PR_LOG usage
QA output needed from PR_LOG
win32/mac issues

Someone will probably have to scour the tree to remove the naked *printfs or
convert them over to PR_LOG.  I believe warren tried the conversion once before
but we backed him out because it broke various build configurations. 
(Reporter)

Comment 6

17 years ago
I'm pretty sure than in a memory-constrained environment, we'd want to address
the footprint taken up by /dev/null. With respect to the QA issue, I think we
want to parameterize that for each product. Some products will want to be
debuggable that way, and some won't. 

Comment 7

17 years ago
we need more guidance on the ultimate objective of this bug.  ccing lchiang.

do we want to disable all output to the console for al *nix builds?  Just for
embedding or for the whole product?  Just for mozilla milestones and Netscape
betas, or for all official builds?

setting 0.9.1.
Priority: -- → P2
Target Milestone: --- → mozilla0.9.1

Comment 8

17 years ago
I really like this "console print" feature in Mozilla.

If there is a need to disable this I hope that there will be a command line
option to enable it.
(Reporter)

Comment 9

17 years ago
My must-have is no console output (or memory use from redirected output) on
Linux optimized builds. 

I don't *need* to solve the problem of whether other platforms have console
output, although ideally they would be fixed by the same fixes as Linux. 

If QA wants to have FORCE_PR_LOG output in release builds (and I would want that
if I were them), suppressing all console output could be a build-time option,
based on environment variable, or something like that. Runtime option does not
work for me.

Comment 10

17 years ago
I'd prefer something we check in rather than something in the automation so that 
we can check that change in on every branch to disable console output so that if 
we have to respin that branch at some point in the future, it's already off and 
we don't have to remember to turn it off again.

It sounds like we need to change FORCE_PR_LOG to be global and only set at the 
top level, and then turn that off on the branch and see what other output we 
get and have to get turned off.
(Reporter)

Comment 11

17 years ago
> I'd prefer something we check in rather than something in the automation

Ok with me. I think these guys have their own top level makefile, so if a flag
in client.mk could do this, that's probley ok.

> It sounds like we need to change FORCE_PR_LOG to be global and only set at the 
> top level, and then turn that off on the branch and see what other output we 
> get and have to get turned off.

As I think about it, FORCE_PR_LOG is a red herring, and I'm sorry I brought it
up. Output from PR_LOG goes to a file, not to the console. And it only outputs
when the right set of PR_LOG environment variable bitfields are set. It's
console output I'm interested in.

Comment 12

17 years ago
FORCE_PR_LOG issue should be discussed in bug 53226.

I think for QA purposes for writing to the console, we can default to no writing
to the console and have a switch to enable doing so for debugging purposes for
development as needed.  Is that possible?

Comment 13

17 years ago
I don't get the "memory usage" comment for the /dev/null redirect.
In the linux kernel, at least, the write operation for /dev/null
is a NOP.  There's no memory allocation involved.  Redirecting
to /dev/null *only* involves formatting the strings for printing.

Comment 14

17 years ago
phil is talking about people doing "mozilla >& /dev/null" to avoid the console
output.  that leaves all the printfs, etc. in the code which takes up run time
memory.  if we ifdef them out, they won't be in the release builds which means
slightly less memory footprint, and barely faster runtimes.

current plan is to check in the temporary hack for 0.9 on the branch, and for
0.9.1 make sure all printfs that are currently writing to the console are
wrapped in an #ifdef DEBUG.

Comment 15

17 years ago
almost forgot, reassigning to cls.  mdunn on cc for qa input.

Comment 16

17 years ago
Taking since granrose wants me to have it.
Assignee: mdunn → cls
Component: Browser-General → Build Config
QA Contact: doronr → granrose

Comment 17

17 years ago
The quick-n-dirty fix has been checked into the 0.9 branch.

*commencing scour now*
Status: NEW → ASSIGNED

Updated

17 years ago
Depends on: 78627

Updated

17 years ago
Depends on: 78629

Updated

17 years ago
Depends on: 78631

Updated

17 years ago
Depends on: 78633

Updated

17 years ago
Depends on: 78634

Updated

17 years ago
Depends on: 78637

Updated

17 years ago
Depends on: 78638

Updated

17 years ago
Depends on: 78639

Updated

17 years ago
Depends on: 78641

Updated

17 years ago
Depends on: 78644

Updated

17 years ago
Depends on: 78645

Updated

17 years ago
Depends on: 78646

Updated

17 years ago
Depends on: 78647

Updated

17 years ago
Depends on: 78648

Updated

17 years ago
Depends on: 78649

Updated

17 years ago
Depends on: 78650

Updated

17 years ago
Depends on: 78651

Updated

17 years ago
Depends on: 78652

Updated

17 years ago
Depends on: 78653

Updated

17 years ago
Depends on: 78654

Updated

17 years ago
Depends on: 78655

Updated

17 years ago
Depends on: 78656

Updated

17 years ago
Depends on: 78658

Updated

17 years ago
Depends on: 78660

Updated

17 years ago
Depends on: 78661

Updated

17 years ago
Depends on: 78662

Updated

17 years ago
Depends on: 78663

Updated

17 years ago
Depends on: 78664

Updated

17 years ago
Depends on: 78665

Updated

17 years ago
Depends on: 78666

Updated

17 years ago
Depends on: 78667

Updated

17 years ago
Depends on: 78668

Updated

17 years ago
Depends on: 78669

Updated

17 years ago
Depends on: 78672

Updated

17 years ago
Depends on: 78674

Updated

17 years ago
Severity: normal → major

Comment 18

17 years ago
There are many bugs which have been triaged or fixed faster because of useful
information from the console in optimized builds.  I don't think we should
remove this for the mozilla nightly and release builds.  It sounds like you're
removing useful information from the 150,000 or so builds that get downloaded
every release cycle.  This seems unwise to me.  Am I missing something?
(Reporter)

Comment 19

17 years ago
asa, that's fine with me, as long as there's a way for embedding customers
remove printfs etc. at build time. For example:

#if defined (MOZ_ALLOW_PRINTF)
#define MOZ_PRINTF(_astring) printf(_astring)
#else
#define MOZ_PRINTF(_astring)
#endif

Comment 20

17 years ago
so what, now cls has to go back and repatch all 33 dependent bugs?  We can have
this fast, right, and cheap.  pick two.  right now, we're doing it fast and
cheap.  If we go back and try to do this so we still have console output in
nightly and release builds, I fail to see the point of doing this at all since
the cost outweighs the benefit.
Please don´t kill the console output.
Make a pref for that or something else but don´t kill it.
This output is very helpfull for many people who reporting bugs !
(Reporter)

Comment 22

17 years ago
> If we go back and try to do this so we still have console output in
> nightly and release builds, I fail to see the point of doing this at all

Two reasons to care about this, even if nightly release builds allow console output:
1. Embedding customers do not use our nightly release builds. 
2. We still want to disable console output when we release to customers, nightly
release builds notwithstanding.
i agree with Asa. having the console output is basically bread'n'butter for
those who use or work on mozilla/netscape6.x: it has helped me far more often in
narrowing down issues than ever hindering me.

Comment 24

17 years ago
> Two reasons to care about this, even if nightly release builds allow console
output:
> 1. Embedding customers do not use our nightly release builds. 
> 2. We still want to disable console output when we release to customers,
nightly
> release builds notwithstanding.

Ok, so after spending far too long on what is really a trivial fix (*150 places
to fix it + another 500 places to see if it needed to be fixed), I have to ask:
If we aren't going to turn it off for release builds and it's embedding only,
why can't they use the quick-n-dirty hack that I came up with earlier?  It would
really take approximately 60 sec to turn that previous patch into one that used
a configure option.  And from the sounds of it, we already do something similar
on win32.  While the extra printfs do cause a fraction of a percent of bloat and
a fraction of a percent of slowdown (swag), I don't see going back and tweaking
all those dependent patches to s/fprintf/DPRINTF/ as an effective use of the
remaining 0.9.1 timeframe.  And none of these patches touch any xul/js code
which is sure to be the product of some dump()s.

Comment 25

17 years ago
setting to 0.9.2 since the dependent bugs were set that by PDT.
Target Milestone: mozilla0.9.1 → mozilla0.9.2

Comment 26

17 years ago
I can very much understand that you want to remove the excessive status msgs on
the console for final customer builds.

I hope you do not remove error printfs. Sometimes, we don't get to show a normal
error msg and printf is the only option to output anything. I'd argue that this
"bloat" is worth it.

Making printf depend on DEBUG does not work for me. We seem(ed?) to have some
very slow DEBUG code, so I always run non-debug, O2, even for development. If
you remove these printfs, I have not much chance to debug. Compare bug 81400.

Comment 27

17 years ago
it seems like there will always be some cases where leaving in those printouts in 
a release build could be of value.  so is it possible to do something like phil's 
suggested mozprintf that is wrapped in a configure option (and default it to 
keeping the printouts), then when someone makes a deliverable, they have the 
option of cleanly killing the code and output if they don't want it.

btw-what was the answer to cls's question of if win32 does something like his 
patch?

Comment 28

17 years ago
not a blocker for 0.9.2, moving to 1.0.
Target Milestone: mozilla0.9.2 → mozilla1.0
Error messages should be output to stderr only. stdout should stay totally 
clean in non-debug builds. Among other things, this will allow the program to 
be invoked cleanly within scripts, pipelines, etc.
see bug 47207 for last year's model of this bug.

Comment 31

17 years ago
*** Bug 93647 has been marked as a duplicate of this bug. ***

Comment 32

17 years ago
See bug 93647: I need a way to still get the old information in release builds.
Keywords: mozilla1.0
Target Milestone: mozilla1.0 → Future

Comment 33

15 years ago
Mass reassign to default build config owner
Assignee: cls → mozbugs-build
Status: ASSIGNED → NEW
Priority: P2 → --
Mass reassign of Build/Config bugs to Leaf.
Assignee: mozbugs-build → leaf
Target Milestone: Future → ---
(Assignee)

Comment 35

14 years ago
wontfix this.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → WONTFIX
Depends on: 243870
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.