454055 - Allow messages to be shown and highlighted in brief log

Status: VERIFIED FIXED

(Webtools Graveyard :: Tinderbox, enhancement)

Description

[Serge Gautherie (:sgautherie)]

This would easy noticing/fixing broken machines... (at least)

Example:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1220791048.1220791111.3649.gz
WINNT 5.2 mozilla-central leak test build on 2008/09/07 05:37:28
[
nsinstall: cannot copy .\nspr4.dll to e:\builds\moz2_slave\mozilla-central-win32-debug\build\obj-firefox\dist\bin\nspr4.dll: The process cannot access the file because it is being used by another process.
make[5]: *** [export] Error 3
]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1220791048.1220791111.3649.gz&fulltext=1
[
======== BuildStep started ========
'echo Building on: moz2-win32-slave05'
]

Comment 1

[timeless]

it could, but unless there's some reason that the build clients can't use the existing tinderbox parser to get the info you need displayed, i'd rather we not change the parser.

Comment 2

[[e-mail] (:bhearsum)]

Is there a way to tell Tinderbox "hey! put this in the brief log!"? The reason we get this on Firefox3.0 is because the hostname is part of the column name. Obviously, we need a different solution here.

Comment 3

[timeless]

http://mxr.mozilla.org/webtools/source/tinderbox/showlog.cgi#101 is the code in place

Comment 4

[Aki Sasaki (not active)]

Attachment: add BriefMessage: functionality to showlog.cgi

The brief log only contains errors currently.

Solution 1: We can make the hostname line look like an error using the existing error parsing.   For example, we could make it look like a cvs conflict, e.g. "C HOSTNAME=foo.mozilla.org".  The drawback here is that it will appear to be an error in the full log as well, and it's certainly possible that people will get confused when trying to actually find a cvs conflict in the log.

Solution 2: this is a 3-line showlog.cgi patch that will put any line beginning with "BriefMessage:" in the Build Error Summary in the brief log view only, not linked to any line in the Build Error Log.  The line will show up in the full log as-is, but only in the body of the message.

(I can change the behavior if that's desired).

I've set up tinderbox1 on my laptop and have verified it works here.  If we put this patch in place on the tinderbox server, then we need to alter the hostname line to start with "BriefMessage:" (sans quotes) to show in the brief tinderbox view.

Comment 5

[Reed Loden [:reed]]

Comment on attachment 346764 [details] [diff] [review]
add BriefMessage: functionality to showlog.cgi

Let's see if cls has any thoughts on if this is the best approach. I'll grab this back if cls can't get to it soon.

Comment 6

[Aki Sasaki (not active)]

We could potentially color code as well; e.g. the BriefMessage: lines are inside a <font color="green" style="font-color: #115511"></font> or something.

Comment 7

[cls]

Comment on attachment 346764 [details] [diff] [review]
add BriefMessage: functionality to showlog.cgi

I'm lukewarm to the entire idea.  I don't think the Brief Log page needs to be cluttered with unnecessary info.  I'm going to imagine that if you're going to debug something, that you'll need to look at the full log anyway to get a clue about what's going on. 

In any case, I'd prefer if you would use TinderboxBriefMessage: or TinderboxLogMessage as the token instead of BriefMessage: .  

If you're going to start special casing the BriefMessages, then I'd argue that you should treat it just like another log error, which would mean putting the check down in output_summary_line().

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #7)
> I don't think the Brief Log page needs to be cluttered with unnecessary info.

I disagree: the full log is there to "debug" things, yes; but the brief log should be enough to check that the failure is already known...

Comment 9

[Aki Sasaki (not active)]

Attachment: TinderboxSummaryMessage: patch; shows in green in both brief and full summaries

Renamed to TinderboxSummaryMessage:, moved to output_summary_line(), colored green, and placed in both the Brief and Full summary sections.

This is for information that may otherwise be buried in the full log, but would be helpful to know either while looking at the brief log, or looking at the full log before downloading the entire thing (or scrolling down and looking for it).

Previously we would have embedded this information in the tinderbox client name, but that also has the side effect of creating a new column.  Since our tinderbox pages are already overly large, having alternate methods of showing this information without having to parse through the entire full log would be helpful.

Comment 10

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

cls: any update on this patch review?

Comment 11

[cls]

Committed on tbox1_20080527_cls_branch

Checking in showlog.cgi;
/cvsroot/mozilla/webtools/tinderbox/showlog.cgi,v  <--  showlog.cgi
new revision: 1.29.2.4; previous revision: 1.29.2.3

Comment 12

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

(In reply to comment #11)
> Committed on tbox1_20080527_cls_branch
> 
> Checking in showlog.cgi;
> /cvsroot/mozilla/webtools/tinderbox/showlog.cgi,v  <--  showlog.cgi
> new revision: 1.29.2.4; previous revision: 1.29.2.3

Cool, thanks cls. 

hmm... dont know this area very well... now what? Do we have to do something to get this used in production? Or are we all done, and can close this bug?

Comment 13

[Reed Loden [:reed]]

(In reply to comment #12)
> hmm... dont know this area very well... now what? Do we have to do something to
> get this used in production? Or are we all done, and can close this bug?

The patch has yet to be committed to trunk, afaik. cls only committed it to his private branch.

Comment 14

[cls]

(In reply to comment #13)
> The patch has yet to be committed to trunk, afaik. cls only committed it to his
> private branch.

Where "private" has been redefined to mean "stable & unaffected by the crash landing of bug 398050 ".

Comment 15

[:Gavin Sharp [email: [e-mail]]]

mozilla/webtools/tinderbox/showlog.cgi 	1.33

Comment 16

[Aki Sasaki (not active)]

Works on tinderbox-stage.
http://tinderbox-stage.mozilla.org/showlog.cgi?log=MozillaTest/1257275043.1257275506.25217.gz

To fix this bug fully, we just have to prepend a "TinderboxSummaryMessage: " to the output from our hostname step.