Bug 170267 - Support tvOS and watchOS in webkitdirs.pm
Summary: Support tvOS and watchOS in webkitdirs.pm
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-29 19:25 PDT by Aakash Jain
Modified: 2017-04-05 12:04 PDT (History)
6 users (show)

See Also:


Attachments
Proposed patch (4.37 KB, patch)
2017-03-29 19:31 PDT, Aakash Jain
ap: review+
Details | Formatted Diff | Diff
Updated patch (4.45 KB, patch)
2017-03-30 14:36 PDT, Aakash Jain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2017-03-29 19:25:24 PDT
webkitdirs.pm should support watchOS and tvOS.
Comment 1 Aakash Jain 2017-03-29 19:31:36 PDT
Created attachment 305823 [details]
Proposed patch
Comment 2 Alexey Proskuryakov 2017-03-29 20:35:13 PDT
Comment on attachment 305823 [details]
Proposed patch

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

> Tools/Scripts/webkitdirs.pm:469
> +    if (checkForArgumentAndRemoveFromARGV("--tv-device")) {

I wonder if it would be better to have "os" in the option name, for consistency with ios.

> Tools/Scripts/webkitdirs.pm:1353
> +    return  isIOSWebKit() || isTVOSWebKit() ||  isWATCHOSWebKit();

Double space here.
Comment 3 Daniel Bates 2017-03-29 20:44:54 PDT
Comment on attachment 305823 [details]
Proposed patch

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

> Tools/Scripts/webkitdirs.pm:105
> +    TVOS     => "tvOS",

TVOS => tvOS

> Tools/Scripts/webkitdirs.pm:106
> +    WATCHOS  => "watchOS",

WATCHOS => watchOS

> Tools/Scripts/webkitdirs.pm:471
> +        $xcodeSDK ||=  $hasInternalSDK ? 'appletvos.internal' : 'appletvos';

' => "

> Tools/Scripts/webkitdirs.pm:478
> +        $xcodeSDK ||=  $hasInternalSDK ? 'watchos.internal' : 'watchos';

Ditto.

> Tools/Scripts/webkitdirs.pm:504
> +    return "appletvos" if $xcodeSDK =~ /appletvos/i;

Please sort the added lines.

> Tools/Scripts/webkitdirs.pm:1343
> +    return portName() eq TVOS;

TVOS => tvOS

> Tools/Scripts/webkitdirs.pm:1346
> +sub isWATCHOSWebKit()

isWatchOSWebKit()

> Tools/Scripts/webkitdirs.pm:1348
> +    return portName() eq WATCHOS;

WATCHOS => watchOS
Comment 4 Aakash Jain 2017-03-30 14:36:54 PDT
Created attachment 305905 [details]
Updated patch

Incorporated review comments.
Comment 5 WebKit Commit Bot 2017-03-30 15:18:25 PDT
Comment on attachment 305905 [details]
Updated patch

Clearing flags on attachment: 305905

Committed r214631: <http://trac.webkit.org/changeset/214631>
Comment 6 WebKit Commit Bot 2017-03-30 15:18:26 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Aakash Jain 2017-04-05 12:04:35 PDT
Followup commit to rename isIOSLikeWebKit to isEmbeddedWebKit. https://trac.webkit.org/changeset/214957/webkit