Bug 138193

Summary: bisect-builds should filter out nightlies that predate the introduction of an OS X operating system
Product: WebKit Reporter: Matthew Hanson <matthew_hanson>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ddkilzer, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch ddkilzer: review+

Description Matthew Hanson 2014-10-29 15:53:19 PDT
bisect-builds should only attempt to bisect the subset of nightlies that ships a framework for the version of OS X that bisect-builds is running on.

For Yosemite, this corresponds to nightlies created at or after r174650.
For Mavericks, this corresponds to nightlies created at or after r157846.
For Mountain Lion, this corresponds to nightlies created at or after r122421.
Comment 1 Matthew Hanson 2014-10-29 16:03:54 PDT
Created attachment 240636 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2014-10-29 16:05:25 PDT
<rdar://problem/18818279>
Comment 3 David Kilzer (:ddkilzer) 2014-10-29 16:53:15 PDT
Comment on attachment 240636 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=240636&action=review

r=me with the comments resolved.

> Tools/ChangeLog:10
> +        It also does a bit of refactoring, including distinguishing between versions and version strings and
> +        saving versions to local variables instead of using eval in each conditional.

+1 for the performance optimization.  :)

> Tools/ChangeLog:18
> +        Restrict the set of nifhtlies to r122421 and above when running Mountain Lion.

Typo:  nifhtlies => nightlies

> Tools/Scripts/bisect-builds:349
> +    }
> +
> +    if ($osxVersion ge v10.5) {

Should this turn into an elsif?  There's no need to re-run the code in this block for 10.8, 10.9 or 10.10.

    } elsif ($osxVersion ge v10.5) {

If we wanted to be thorough, we could also add a FIXME comment for v10.6 and v10.7, but it probably doesn't matter at this point.

> Tools/Scripts/bisect-builds:364
>          if ($safariVersion eq "4 Public Beta") {

This line needs to change to:

        if ($safariVersionString eq "4 Public Beta") {

> Tools/Scripts/bisect-builds:375
>              die "Requires Safari 2.0 or newer";

You could change the die statement below this patch context to be:

        die "Requires Mac OS X 10.4 (Tiger) or later";
Comment 4 Matthew Hanson 2014-10-29 17:13:11 PDT
Committed r175357: <http://trac.webkit.org/changeset/175357>
Comment 5 Matthew Hanson 2014-10-30 13:57:53 PDT
Committed r175377: <http://trac.webkit.org/changeset/175377>