Bug 149963

Summary: Teach webkitperl how to figure out IOS versions from the SDK
Product: WebKit Reporter: BJ Burg <bburg>
Component: Tools / TestsAssignee: BJ Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: dbates, ddkilzer, lforschler
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Proposed Fix dbates: review+, dbates: commit-queue-

Description BJ Burg 2015-10-09 11:55:58 PDT
.
Comment 1 BJ Burg 2015-10-09 12:02:17 PDT
Created attachment 262784 [details]
Proposed Fix
Comment 2 Daniel Bates 2015-10-09 12:09:27 PDT
Comment on attachment 262784 [details]
Proposed Fix

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

> Tools/Scripts/copy-webkitlibraries-to-product-directory:235
> +            my $majorSDKVersion = IOSVersion()->{"major"};

We tend to follow the Webkit Code Style Guidelines for Perl code. So, this should be named iosVersion().

> Tools/Scripts/webkitdirs.pm:118
> +my $IOSVersion;

Similarly this should be name iosVersion.

> Tools/Scripts/webkitdirs.pm:1294
> +        $IOSVersion = -1;

Ditto.

> Tools/Scripts/webkitdirs.pm:1301
> +    $IOSVersion = {

Ditto.

> Tools/Scripts/webkitdirs.pm:1306
> +    my $version = xcodeSDKVersion();
> +    my @splitVersion = split(/\./, $version);
> +    @splitVersion >= 2 or die "Invalid version $version";
> +    $IOSVersion = {
> +            "major" => $splitVersion[0],
> +            "minor" => $splitVersion[1],
> +            "subminor" => (defined($splitVersion[2]) ? $splitVersion[2] : 0),
> +    };
> +}

This duplicates code in function osXVersion. Would it make sense to extract the common code into a shared function, say splitVersionString(), and call it from both this function and determineOSXVersion()?

> Tools/Scripts/webkitdirs.pm:1308
> +sub IOSVersion()

This should be named iosVersion.

> Tools/Scripts/webkitdirs.pm:1311
> +    return $IOSVersion;

This should be named iosVersion.
Comment 3 Daniel Bates 2015-10-09 12:10:36 PDT
(In reply to comment #2)
> [...]
> This duplicates code in function osXVersion. 

I meant to write:

This duplicates code in function determineOSXVersion().
Comment 4 Daniel Bates 2015-10-09 12:15:28 PDT
Comment on attachment 262784 [details]
Proposed Fix

Also, we should add iosVersion() to the list of exported functions @EXPORT.
Comment 5 BJ Burg 2015-10-09 12:23:17 PDT
(In reply to comment #2)
> Comment on attachment 262784 [details]

> > Tools/Scripts/webkitdirs.pm:1306
> > +    my $version = xcodeSDKVersion();
> > +    my @splitVersion = split(/\./, $version);
> > +    @splitVersion >= 2 or die "Invalid version $version";
> > +    $IOSVersion = {
> > +            "major" => $splitVersion[0],
> > +            "minor" => $splitVersion[1],
> > +            "subminor" => (defined($splitVersion[2]) ? $splitVersion[2] : 0),
> > +    };
> > +}
> 
> This duplicates code in function osXVersion. Would it make sense to extract
> the common code into a shared function, say splitVersionString(), and call
> it from both this function and determineOSXVersion()?

I could, though I'm not sure that subminors are even used by iOS SDKs.
Comment 6 BJ Burg 2015-10-09 15:01:47 PDT
Committed r190821: <http://trac.webkit.org/changeset/190821>