mozilla-central tinderbox logs don't show commands that are being executed

RESOLVED FIXED

Status

Release Engineering
General
P2
normal
RESOLVED FIXED
10 years ago
4 years ago

People

(Reporter: ted, Assigned: bhearsum)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
This kind of sucks. For one, it makes it really hard to determine some things, like if we built a PGO build. It also means I can't search the log for 'buildsymbols' to find the breakpad symbol store build.
(Assignee)

Updated

10 years ago
Priority: -- → P3
(Reporter)

Updated

10 years ago
Blocks: 433384
(Assignee)

Comment 1

10 years ago
Yeah, this sucks. I'll see what I can do with this.
Assignee: nobody → bhearsum
(Assignee)

Comment 2

10 years ago
Here's a run with some dumping enabled:
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTest/1212765839.1212766063.32611.gz&fulltext=1

Unless there's objections I'll clean-up and post this patch.
(Assignee)

Comment 3

10 years ago
Created attachment 324058 [details] [diff] [review]
make tinderboxmailnotifier print the same log headers that buildbot does

This makes the log output very very similar to that on the Waterfall. Mostly, this is adding a distinct header to each BuildStep, and s/log.getText()/log.getTextWithHeaders()/
Attachment #324058 - Flags: review?(ted.mielczarek)
(Reporter)

Comment 4

10 years ago
Comment on attachment 324058 [details] [diff] [review]
make tinderboxmailnotifier print the same log headers that buildbot does

+                if not re.compile(".*[^\s].*").match(shortText):

If you're going to use re.compile, then do it once and save the result in a var, otherwise just use re.match directly.

Looks like you're compressing just the log sections and not the entire log, so r- for that. Should be able to build the entire log and then compress once at the end.
Attachment #324058 - Flags: review?(ted.mielczarek) → review-
(Assignee)

Comment 5

10 years ago
Created attachment 324078 [details] [diff] [review]
[checked in] again
Attachment #324078 - Flags: review?(ted.mielczarek)
(Reporter)

Comment 6

10 years ago
Comment on attachment 324078 [details] [diff] [review]
[checked in] again

+                tinderboxLogs += "======== BuildStep started ========\n"
+                tinderboxLogs += shortText
+                tinderboxLogs += "=== Output ===\n"
+                for log in logs:
+                    tinderboxLogs += log.getTextWithHeaders()
+
+                tinderboxLogs += "=== Output ended ===\n"
+                tinderboxLogs += "======== BuildStep ended ========\n"

Personally I'd do this as either a triple-quoted string with interpolation for the two variable bits, or as one big ''.join(list of strings). That's my stylistic preference, anyway. I'll leave issues of style up to your choice here, though.
Attachment #324078 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
Priority: P3 → P2
Whiteboard: [scheduled for monday, june 9]
(Assignee)

Comment 7

10 years ago
Comment on attachment 324078 [details] [diff] [review]
[checked in] again

Checking in buildbot/status/tinderbox.py;
/cvsroot/mozilla/tools/buildbot/buildbot/status/tinderbox.py,v  <--  tinderbox.py
new revision: 1.10; previous revision: 1.9
done
Attachment #324078 - Attachment description: again → [checked in] again
(Assignee)

Comment 8

10 years ago
Needs a Buildbot master restart to pick this up, I'll do that when there is some idle time.
(Assignee)

Updated

10 years ago
Attachment #324058 - Attachment is obsolete: true
(Assignee)

Comment 9

10 years ago
Okay, done.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Comment 10

10 years ago
This caused bug 438002, backing it out until a suitable workaround is found.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 11

10 years ago
Created attachment 324301 [details] [diff] [review]
[checked in] ignore invalid TinderboxPrint statements

I'm pretty sure this is the best fix for this. We could require that TinderboxPrint's be done in a BuildStep subclass (and therefore not end up in the summary), but I think that's silly.

This should fix bug 438002.
Attachment #324301 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

10 years ago
Whiteboard: [scheduled for monday, june 9]
(Assignee)

Comment 12

10 years ago
Comment on attachment 324301 [details] [diff] [review]
[checked in] ignore invalid TinderboxPrint statements

Any chance you can get to this soon?
(Reporter)

Comment 13

10 years ago
Comment on attachment 324301 [details] [diff] [review]
[checked in] ignore invalid TinderboxPrint statements

+                if re.match(".+TinderboxPrint.*", shortText):

I don't think you really need a regex here, you're just looking for a literal string. Why not shortText.find("TinderboxPrint") != -1 ?

Otherwise looks good.
Attachment #324301 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 14

10 years ago
(In reply to comment #13)
> (From update of attachment 324301 [details] [diff] [review])
> +                if re.match(".+TinderboxPrint.*", shortText):
> 
> I don't think you really need a regex here, you're just looking for a literal
> string. Why not shortText.find("TinderboxPrint") != -1 ?
> 
> Otherwise looks good.
> 

Yeah, you're right.
(Assignee)

Comment 15

10 years ago
(In reply to comment #14)
> (In reply to comment #13)
> > (From update of attachment 324301 [details] [diff] [review] [details])
> > +                if re.match(".+TinderboxPrint.*", shortText):
> > 
> > I don't think you really need a regex here, you're just looking for a literal
> > string. Why not shortText.find("TinderboxPrint") != -1 ?
> > 
> > Otherwise looks good.
> > 
> 
> Yeah, you're right.
> 

Hm, on second thought, I'm pretty sure I need a regex here. I'm only looking for TinderboxPrints that *aren't* at the start of the line, which lets us ignore them in the BuildStep headers.

Does that make sense?
(Reporter)

Comment 16

10 years ago
You could use find and test != 0 and != -1, or just use the regex if you're happy with that. It's not a big deal.
(Assignee)

Updated

10 years ago
Whiteboard: [scheduled for june 18]
(Assignee)

Comment 17

10 years ago
Comment on attachment 324301 [details] [diff] [review]
[checked in] ignore invalid TinderboxPrint statements

Checking in tinderbox.py;
/cvsroot/mozilla/tools/buildbot/buildbot/status/tinderbox.py,v  <--  tinderbox.py
new revision: 1.12; previous revision: 1.11
done
Attachment #324301 - Attachment description: ignore invalid TinderboxPrint statements → [checked in] ignore invalid TinderboxPrint statements
(Assignee)

Comment 18

10 years ago
Working properly this time.
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
Whiteboard: [scheduled for june 18]
(Assignee)

Updated

10 years ago
Duplicate of this bug: 438002
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.