Bug 58801 - [WinCairo] "build-webkit --wincairo" thinks you need WebKitSupportLibraries.zip
Summary: [WinCairo] "build-webkit --wincairo" thinks you need WebKitSupportLibraries.zip
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-18 11:20 PDT by Brent Fulgham
Modified: 2011-04-21 22:51 PDT (History)
0 users

See Also:


Attachments
Patch (2.18 KB, patch)
2011-04-20 21:56 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (2.91 KB, patch)
2011-04-20 22:19 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Revised per mrobinson's comments (2.99 KB, patch)
2011-04-21 21:50 PDT, Brent Fulgham
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2011-04-18 11:20:55 PDT
If you run "build-webkit --wincairo", you should be building a release that does not use any of the proprietary Apple libraries.  Unfortunately, the "build-webkit" script thinks that you need this zip file and will not continue without it.

This issue modifies the "build-webkit" script to ignore the missing zip file if you are building with "--wincairo".
Comment 1 Brent Fulgham 2011-04-20 21:56:56 PDT
Created attachment 90492 [details]
Patch
Comment 2 Brent Fulgham 2011-04-20 22:19:31 PDT
Created attachment 90493 [details]
Patch
Comment 3 Martin Robinson 2011-04-21 09:31:01 PDT
Comment on attachment 90493 [details]
Patch

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

> Tools/Scripts/build-webkit:-514
>  } elsif (isAppleWinWebKit()) {
>      # Copy WebKitSupportLibrary to the correct location in WebKitLibraries so it can be found.
>      # Will fail if WebKitSupportLibrary.zip is not in source root.
> -    (system("perl Tools/Scripts/update-webkit-support-libs") == 0) or die;

It's odd that isAppleWinWebKit returns true for WinCairo. It probably shouldn't.

> Tools/Scripts/update-webkit:101
>  } elsif (isAppleWinWebKit()) {

Ditto. :)

> Tools/Scripts/webkitdirs.pm:212
> +        $configuration .= "_Cairo_CFLite" if (isWinCairo() && isCygwin());

Perhaps you should move this out of the block. You never want to build the Release or Debug configurations if you've specified the WinCairo flag. Why is isCygwin important here?
Comment 4 Brent Fulgham 2011-04-21 09:38:35 PDT
Comment on attachment 90493 [details]
Patch

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

>> Tools/Scripts/build-webkit:-514
>> -    (system("perl Tools/Scripts/update-webkit-support-libs") == 0) or die;
> 
> It's odd that isAppleWinWebKit returns true for WinCairo. It probably shouldn't.

Yes, that's probably true.  WinCairo piggybacks off of the isAppleWinWebKit for most of its functionality.  Changing all of these places would expand the scope of this change considerably.

>> Tools/Scripts/update-webkit:101
>>  } elsif (isAppleWinWebKit()) {
> 
> Ditto. :)

Ditto! :-)

>> Tools/Scripts/webkitdirs.pm:212
>> +        $configuration .= "_Cairo_CFLite" if (isWinCairo() && isCygwin());
> 
> Perhaps you should move this out of the block. You never want to build the Release or Debug configurations if you've specified the WinCairo flag. Why is isCygwin important here?

That's a good point.  I was only thinking about the default case, which is where I ran into the problem.  I'll change that.
Comment 5 Brent Fulgham 2011-04-21 21:50:12 PDT
Created attachment 90668 [details]
Revised per mrobinson's comments
Comment 6 Martin Robinson 2011-04-21 22:34:41 PDT
Comment on attachment 90668 [details]
Revised per mrobinson's comments

Great!
Comment 7 Brent Fulgham 2011-04-21 22:51:26 PDT
Landed in http://trac.webkit.org/changeset/84601