Closed
Bug 841786
Opened 12 years ago
Closed 12 years ago
Fix TBPL bug suggestion for bug 840165 (Shutdown hang on Windows with topframe in summary)
Categories
(Tree Management Graveyard :: TBPL, defect)
Tree Management Graveyard
TBPL
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: emorley, Assigned: emorley)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
RyanVM
:
review+
|
Details | Diff | Splinter Review |
At the moment TBPL is unable to star bug 840165, even with the "use the top crash frame" fallback.
The extra adding I added in bug 793159 allowed me to work out what was going on - we just never match using:
https://hg.mozilla.org/webtools/tbpl/file/389170d95421/php/inc/AnnotatedSummaryGenerator.php#l100
{
100 if (empty($bugs) && preg_match('/application crashed \[@ (.+)\]$/', $line, $matches))
}
The reason being that on Windows, $line ends with "\r" from the \r\n, which GzipUtils::getLines hasn't stripped out:
https://hg.mozilla.org/webtools/tbpl/file/389170d95421/php/inc/GzipUtils.php#l73
{
72 // we want to have lines that are terminated by \n.
73 $lines = preg_split("/([^\n]*\n)/", $content, -1,
74 PREG_SPLIT_NO_EMPTY | PREG_SPLIT_DELIM_CAPTURE);
75 return $lines;
}
I have no idea why we choose to keep the line endings in the array of lines; and a play about with the regexp above was breaking more things than it fixed - so the easiest solution for now looks to just be to trim($line) in AnnotatedSummaryGenerator.php and call it a day.
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #714434 -
Flags: review?(ryanvm)
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to Ed Morley [:edmorley UTC+0] from comment #0)
> The extra adding I added in bug 793159
err 'logging'.
And just to clarify, we were trying to match against:
application crashed \[@ (.+)\]$
...when the string on Windows was actually:
application crashed \[@ (.+)\]\r$
Updated•12 years ago
|
Attachment #714434 -
Flags: review?(ryanvm) → review+
Assignee | ||
Comment 3•12 years ago
|
||
Landed then backed out for breaking multiple line annotated summaries. Have a new version locally, will test first.
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #714434 -
Attachment is obsolete: true
Attachment #717218 -
Flags: review?(ryanvm)
Comment 5•12 years ago
|
||
Comment on attachment 717218 [details] [diff] [review]
Patch v2
Makes sense.
Attachment #717218 -
Flags: review?(ryanvm) → review+
Assignee | ||
Comment 6•12 years ago
|
||
Assignee | ||
Comment 7•12 years ago
|
||
In production :-)
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 8•12 years ago
|
||
:-D
Updated•10 years ago
|
Product: Webtools → Tree Management
Updated•10 years ago
|
Product: Tree Management → Tree Management Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•