Bug 58801

Summary: [WinCairo] "build-webkit --wincairo" thinks you need WebKitSupportLibraries.zip
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: Tools / TestsAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Attachments:
Description Flags
Patch
none
Patch
none
Revised per mrobinson's comments mrobinson: review+

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