Bug 58801 – [WinCairo] "build-webkit --wincairo" thinks you need WebKitSupportLibraries.zip

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Brent Fulgham]

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".

[Brent Fulgham]

Created attachment 90492 [details]
Patch

[Brent Fulgham]

Created attachment 90493 [details]
Patch

[Martin Robinson]

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?

[Brent Fulgham]

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.

[Brent Fulgham]

Created attachment 90668 [details]
Revised per mrobinson's comments

[Martin Robinson]

Comment on attachment 90668 [details]
Revised per mrobinson's comments

Great!

[Brent Fulgham]

Landed in http://trac.webkit.org/changeset/84601