Closed Bug 433701 Opened 14 years ago Closed 14 years ago

don't print non-critical messages when make is invoked with -s

Categories

(Firefox Build System :: General, defect)

defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.1a1

People

(Reporter: ynvich, Assigned: ynvich)

References

Details

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008051416 Firefox/3.0
Build Identifier: 20080514 trunk

The build system prints lots of valuable data as it cooks the program. When make is invoked with -s, the build system output is limited to the names of targets being rebuilt and errors/warnings.

However, certain command (simple scripts generally) are always invoked, and if they use echo in their bodies, their output distracts from errors/warnings in other targets.

Reproducible: Always

Steps to Reproduce:
1. make -s
2. make -s

Actual Results:  
the second make prints some trivial messages

Expected Results:  
the second make is silent

when make is invoked with -s, $(MAKE_FLAGS) var contains 's', which can be detected with $(filter).

Patch follows.
Attached patch proposed fixSplinter Review
This patch silences most repeating build messages, when -s or --silent make flag is present.
Attachment #320907 - Flags: review?(benjamin)
Attachment #320907 - Flags: review?(benjamin) → review+
Comment on attachment 320907 [details] [diff] [review]
proposed fix

This patch doesn't affect build products, only build system output in --silent mode. It is .999999 safe to landed.
Attachment #320907 - Flags: approval1.9?
Blocks: 439050
Keywords: checkin-needed
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → ynvich
http://hg.mozilla.org/index.cgi/mozilla-central/rev/d7cf7f42cdc1
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a1
Keywords: checkin-needed
This patch seems broken to me. My makefile knowledge is weak but surely $(filter s,$(MAKE_FLAGS)) will give s if MAKE_FLAGS contains s, so at the moment -s turns /on/ all the output you've changed. Additionally at least on my OSX system MAKE_FLAGS never seems to get set anyway.

I'd like this to be backed out or fixed please.
(In reply to comment #4)
> This patch seems broken to me. My makefile knowledge is weak but surely
> $(filter s,$(MAKE_FLAGS)) will give s if MAKE_FLAGS contains s, so at the
> moment -s turns /on/ all the output you've changed.

Right. But the check is ifNeq, so the patch works correctly in this respect.

> Additionally at least on my
> OSX system MAKE_FLAGS never seems to get set anyway.

$(MAKE_FLAGS) is GNU make extension, but GNU make is supposed to be a requirement. Not sure what goes wrong, and, unfortunately, don't have Mac hardware to find out. The patch works seamlessly for me on Linux (Fedora 7, Debian/unstable), and Windows XP SP2.
(In reply to comment #5)
> (In reply to comment #4)
> > This patch seems broken to me. My makefile knowledge is weak but surely
> > $(filter s,$(MAKE_FLAGS)) will give s if MAKE_FLAGS contains s, so at the
> > moment -s turns /on/ all the output you've changed.
> 
> Right. But the check is ifNeq, so the patch works correctly in this respect.

Yeah it says "if s is not equal to '' (i.e. the -s flag is present) then define ECHO as echo, otherwise (no -s flag) define ECHO as true".

Perhaps I don't understand what MAKE_FLAGS is actually meant to contain.
Thanks to Dave for bringing up this issue.

The correct name of the make file is $(MAKEFLAGS), not $(MAKE_FLAGS). This variable contains 's' whenever make is invoked with '-s' or '--silent'. Due to a double mistake in the previous version, certain output was always blocked (not just when make runs in silent mode).
Attachment #329525 - Flags: review?(benjamin)
Attachment #329525 - Flags: review?(benjamin) → review+
Dave,

does the attachment 329525 [details] [diff] [review] address your concerns?

And if so could you commit it (i don't have commit access)?
Status: RESOLVED → REOPENED
Keywords: checkin-needed
Resolution: FIXED → ---
Looks good, landed in changeset 3605ad884d0f
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.