Closed Bug 375714 Opened 13 years ago Closed 13 years ago

implement tinderbox log parser

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rhelmer, Assigned: rhelmer)

References

Details

Attachments

(1 file, 3 obsolete files)

Need to be able to parse tinderbox logs, here are the primary reasons:

* better error detection in the Build/Repack->Verify() step
* figure out where Tinderbox uploaded the files to, to automate the push-to-candidates dir step
* determine the build ID, needed for config file generation (bug 371325)
Status: NEW → ASSIGNED
Assignee: build → rhelmer
Status: ASSIGNED → NEW
No longer blocks: end2end-bld
This is in Step.pm because that's where the other log checking method CheckLog() is. This is untested, just want to get feedback on the general idea before I hook this up.
Attachment #260364 - Flags: review?(preed)
Comment on attachment 260364 [details] [diff] [review]
base class method to get build ID from tinderbox log

Couple of things:

$log is going to be pretty big (megabytes), so might want to be careful with stuffing the entire thing into memory.

Also, you could save time parsing by stopping parsing once you find a build id.

I'm not so sure this shoudl go into Bootstrap::Step.

There are other parsing routines we'll need, so maybe a MozBuild::TinderLogParse module is more appropriate.

Can you include a sample (in the comments) of the string you're trying to capture, too?
Attachment #260364 - Flags: review?(preed) → review-
Attached patch tinderbox log parser (obsolete) — Splinter Review
Yeah good call on just making this it's own class. Attached is  a proposed TinderLogParser.pm, usage would look something like:

    my $logParser = TinderLogParser->new(
        log => $buildLog,
    );
    $buildID = $logParser->GetBuildID();

Presumably we'll want to add methods like GetPushDir() (to get the directory tbox pushed to) and an error scanner as well.

This is untested, just trying to get the design right before we go through a lengthy testing round.
Attachment #260364 - Attachment is obsolete: true
Attachment #260493 - Flags: review?(preed)
Comment on attachment 260493 [details] [diff] [review]
tinderbox log parser

>##
># TinderLogParse - A tinderbox log parser
>##
>
>sub new {

I think you need a package statement somewhere here.

>    my $proto = shift;
>    my $class = ref($proto) || $proto;
>    my %args = @_;
>
>    my $logFile = $args{'logFile'};
>    if (! defined($logFile)) {
>        die("logFile is a required argument");

I'd put an "ASSERT: TinderLogParse::new: logFile is..."


>    if (not defined($log)) {
>        die("No log file specified");
>    }

In normal usage, would this ever be reached?

>        die("BuildID not found in log file $log");

Should this always die?

>sub GetLogFile() {

I'd call this "GetLogfileName.

Also, no prototype.

Other than that, this is pretty close; would this go in MozUtil::TinderLogParse.pm?
Attachment #260493 - Flags: review?(preed) → review+
Good stuff, couple responses:

(In reply to comment #4)
> (From update of attachment 260493 [details] [diff] [review])
> In normal usage, would this ever be reached?
> 
> >        die("BuildID not found in log file $log");
> Should this always die?


The caller can handle it however they like, so I think so.

 
> Other than that, this is pretty close; would this go in
> MozUtil::TinderLogParse.pm?


Right, sorry about the missing package name :O
Status: NEW → ASSIGNED
Attached patch address preed's comments (obsolete) — Splinter Review
I know you +'d the last one, just wanted to make sure this is what you mean about the "no proto". 

I'd like to go through the other new() methods and do this too, as a separate patch (since it's extraneous)
Attachment #260493 - Attachment is obsolete: true
Attachment #260546 - Flags: review?
Attachment #260546 - Flags: review? → review?(preed)
Going to land this one as per comments above and conversation w/ preed.
Attachment #260546 - Attachment is obsolete: true
Attachment #260546 - Flags: review?(preed)
Landed:

RCS file: /cvsroot/mozilla/tools/release/MozBuild/TinderLogParse.pm,v
done
Checking in MozBuild/TinderLogParse.pm;
/cvsroot/mozilla/tools/release/MozBuild/TinderLogParse.pm,v  <--  TinderLogParse.pm
initial revision: 1.1
done
I'm sure we'll want to add more methods later, but this I think this is enough for one bug.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.