160659 2016-08-08 08:19:33 -0700 Do not check if WebKitLibraries folder exists on EFL and GTK 2016-08-09 00:27:01 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 gyuyoung.kim gyuyoung.kim commit-queue dbates lforschler ossy oldest_to_newest 1218491 0 gyuyoung.kim 2016-08-08 08:19:33 -0700 As far as I know WebKitLibraries is only needed by mac and win ports though, EFL and GTK ports have checked whether it exists. 1218492 1 285566 gyuyoung.kim 2016-08-08 08:21:26 -0700 Created attachment 285566 Patch 1218493 2 285566 ossy 2016-08-08 08:35:21 -0700 Comment on attachment 285566 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285566&action=review > Tools/Scripts/build-webkit:161 > -if (!isIOSWebKit() && !-d "WebKitLibraries") { > +if ((!isIOSWebKit() && !isEfl() && !isGtk()) && !-d "WebKitLibraries") { > die "Error: No WebKitLibraries directory found. Please do a fresh checkout.\n"; > } AFAIK WebKitLibraries is used by Apple pots: Mac, iOS and Win too. There are *IOS* files in WebKitLibraries directory, so I don't understand this !isIOSWebKit() check. It was added by https://trac.webkit.org/changeset/161685 , we should as Daniel if we still need it for iOS. I prefer whitelisting these platforms instead of blacklisting others. But we can be conservative until get answer from Apple, and leave iOS unchanged. - if (!isIOSWebKit() && !-d "WebKitLibraries") + if (!isIOSWebKit() && isAppleWebKit() && !-d "WebKitLibraries") 1218537 3 285566 dbates 2016-08-08 11:39:57 -0700 Comment on attachment 285566 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285566&action=review > Tools/Scripts/build-webkit:159 > +if ((!isIOSWebKit() && !isEfl() && !isGtk()) && !-d "WebKitLibraries") { We should remove the !isIOSWebKit() from this line. As Csaba pointed out in comment #2, we expect the directory WebKitLibraries to exist when building the Apple WebKit ports. For completeness, originally the iOS port did not make use of files in directory WebKitLibraries. So, we skipped the directory existence check for this directory when building for iOS. >> Tools/Scripts/build-webkit:161 >> } > > AFAIK WebKitLibraries is used by Apple pots: Mac, iOS and Win too. There are *IOS* files in > WebKitLibraries directory, so I don't understand this !isIOSWebKit() check. It was added > by https://trac.webkit.org/changeset/161685 , we should as Daniel if we still need it for iOS. > > I prefer whitelisting these platforms instead of blacklisting others. But we > can be conservative until get answer from Apple, and leave iOS unchanged. > > - if (!isIOSWebKit() && !-d "WebKitLibraries") > + if (!isIOSWebKit() && isAppleWebKit() && !-d "WebKitLibraries") From my understanding the WinCairo port also makes use of WebKitLibraries. 1218658 4 285621 gyuyoung.kim 2016-08-08 17:49:12 -0700 Created attachment 285621 Patch 1218659 5 gyuyoung.kim 2016-08-08 17:51:37 -0700 (In reply to comment #3) > Comment on attachment 285566 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=285566&action=review > > > Tools/Scripts/build-webkit:159 > > +if ((!isIOSWebKit() && !isEfl() && !isGtk()) && !-d "WebKitLibraries") { > > We should remove the !isIOSWebKit() from this line. As Csaba pointed out in > comment #2, we expect the directory WebKitLibraries to exist when building > the Apple WebKit ports. Daniel, I remove the !isIOSWebKit() in the line. > For completeness, originally the iOS port did not make use of files in > directory WebKitLibraries. So, we skipped the directory existence check for > this directory when building for iOS. > > >> Tools/Scripts/build-webkit:161 > >> } > > > > AFAIK WebKitLibraries is used by Apple pots: Mac, iOS and Win too. There are *IOS* files in > > WebKitLibraries directory, so I don't understand this !isIOSWebKit() check. It was added > > by https://trac.webkit.org/changeset/161685 , we should as Daniel if we still need it for iOS. > > > > I prefer whitelisting these platforms instead of blacklisting others. But we > > can be conservative until get answer from Apple, and leave iOS unchanged. > > > > - if (!isIOSWebKit() && !-d "WebKitLibraries") > > + if (!isIOSWebKit() && isAppleWebKit() && !-d "WebKitLibraries") > > From my understanding the WinCairo port also makes use of WebKitLibraries. ok, I modify the condition which checks if port is isAppleWebKit() or isWinCairo(). Is it fine ? if ((isAppleWebKit() || isWinCairo()) && !-d "WebKitLibraries") 1218700 6 285621 ossy 2016-08-08 23:40:17 -0700 Comment on attachment 285621 Patch LGTM 1218705 7 285621 commit-queue 2016-08-09 00:26:56 -0700 Comment on attachment 285621 Patch Clearing flags on attachment: 285621 Committed r204277: <http://trac.webkit.org/changeset/204277> 1218706 8 commit-queue 2016-08-09 00:27:01 -0700 All reviewed patches have been landed. Closing bug. 285566 2016-08-08 08:21:26 -0700 2016-08-08 17:48:50 -0700 Patch bug-160659-20160809001953.patch text/plain 1241 gyuyoung.kim U3VidmVyc2lvbiBSZXZpc2lvbjogMjA0MjUyCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggYmU5NWE0NjAzZTg5NzljOWIwN2ExODFkYjFjNmVjZTJm YmI2MGVmYS4uZDJjMDQ5MWIyMWJkZTZjMjhiZjE4NjM4NDVkNTI4NWNiNzc0NTNlMyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1 IEBACisyMDE2LTA4LTA4ICBHeXV5b3VuZyBLaW0gIDxneXV5b3VuZy5raW1Ad2Via2l0Lm9yZz4K KworICAgICAgICBEbyBub3QgY2hlY2sgaWYgV2ViS2l0TGlicmFyaWVzIGZvbGRlciBleGlzdHMg b24gRUZMIGFuZCBHVEsKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTE2MDY1OQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFRob3VnaCBXZWJLaXRMaWJyYXJpZXMgaXMgb25seSBuZWVkZWQgYnkgbWFjIGFuZCB3 aW4gcG9ydHMsCisgICAgICAgIEVGTCBhbmQgR1RLIHBvcnRzIGhhdmUgY2hlY2tlZCB3aGV0aGVy IGl0IGV4aXN0cy4KKworICAgICAgICAqIFNjcmlwdHMvYnVpbGQtd2Via2l0OgorCiAyMDE2LTA4 LTA3ICBEYW4gQmVybnN0ZWluICA8bWl0ekBhcHBsZS5jb20+CiAKICAgICAgICAgW0NvY29hXSBS ZXBseSBibG9jayBsZWFrcyBpZiB0aGUgcmVtb3RlIG9iamVjdCBkb2VzbuKAmXQgY2FsbCBpdApk aWZmIC0tZ2l0IGEvVG9vbHMvU2NyaXB0cy9idWlsZC13ZWJraXQgYi9Ub29scy9TY3JpcHRzL2J1 aWxkLXdlYmtpdAppbmRleCA2NTNkMzc0NTIwMzE4YzNhMjBlYWUwNjY1Yzc5MDhhYTc3MjE0Zjlk Li5hNjNlYWQ2NmQ1MThlMjdlMzAxMzUwOWM5NWZmNDc1NDcyZDQ4MWEyIDEwMDc1NQotLS0gYS9U b29scy9TY3JpcHRzL2J1aWxkLXdlYmtpdAorKysgYi9Ub29scy9TY3JpcHRzL2J1aWxkLXdlYmtp dApAQCAtMTU2LDcgKzE1Niw3IEBAIGZvciBteSAkZGlyIChAcHJvamVjdHMpIHsKICAgICB9CiB9 CiAKLWlmICghaXNJT1NXZWJLaXQoKSAmJiAhLWQgIldlYktpdExpYnJhcmllcyIpIHsKK2lmICgo IWlzSU9TV2ViS2l0KCkgJiYgIWlzRWZsKCkgJiYgIWlzR3RrKCkpICYmICEtZCAiV2ViS2l0TGli cmFyaWVzIikgewogICAgIGRpZSAiRXJyb3I6IE5vIFdlYktpdExpYnJhcmllcyBkaXJlY3Rvcnkg Zm91bmQuIFBsZWFzZSBkbyBhIGZyZXNoIGNoZWNrb3V0LlxuIjsKIH0KIAo= 285621 2016-08-08 17:49:12 -0700 2016-08-09 00:26:56 -0700 Patch bug-160659-20160809094723.patch text/plain 1234 gyuyoung.kim U3VidmVyc2lvbiBSZXZpc2lvbjogMjA0MjUyCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggYmU5NWE0NjAzZTg5NzljOWIwN2ExODFkYjFjNmVjZTJm YmI2MGVmYS4uZDJjMDQ5MWIyMWJkZTZjMjhiZjE4NjM4NDVkNTI4NWNiNzc0NTNlMyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1 IEBACisyMDE2LTA4LTA4ICBHeXV5b3VuZyBLaW0gIDxneXV5b3VuZy5raW1Ad2Via2l0Lm9yZz4K KworICAgICAgICBEbyBub3QgY2hlY2sgaWYgV2ViS2l0TGlicmFyaWVzIGZvbGRlciBleGlzdHMg b24gRUZMIGFuZCBHVEsKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTE2MDY1OQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFRob3VnaCBXZWJLaXRMaWJyYXJpZXMgaXMgb25seSBuZWVkZWQgYnkgbWFjIGFuZCB3 aW4gcG9ydHMsCisgICAgICAgIEVGTCBhbmQgR1RLIHBvcnRzIGhhdmUgY2hlY2tlZCB3aGV0aGVy IGl0IGV4aXN0cy4KKworICAgICAgICAqIFNjcmlwdHMvYnVpbGQtd2Via2l0OgorCiAyMDE2LTA4 LTA3ICBEYW4gQmVybnN0ZWluICA8bWl0ekBhcHBsZS5jb20+CiAKICAgICAgICAgW0NvY29hXSBS ZXBseSBibG9jayBsZWFrcyBpZiB0aGUgcmVtb3RlIG9iamVjdCBkb2VzbuKAmXQgY2FsbCBpdApk aWZmIC0tZ2l0IGEvVG9vbHMvU2NyaXB0cy9idWlsZC13ZWJraXQgYi9Ub29scy9TY3JpcHRzL2J1 aWxkLXdlYmtpdAppbmRleCA2NTNkMzc0NTIwMzE4YzNhMjBlYWUwNjY1Yzc5MDhhYTc3MjE0Zjlk Li5mMGVmM2Q0M2NiMjMzNmY3ODJmMzNiODRiMDU3MWM1MTY3ZDE4MTExIDEwMDc1NQotLS0gYS9U b29scy9TY3JpcHRzL2J1aWxkLXdlYmtpdAorKysgYi9Ub29scy9TY3JpcHRzL2J1aWxkLXdlYmtp dApAQCAtMTU2LDcgKzE1Niw3IEBAIGZvciBteSAkZGlyIChAcHJvamVjdHMpIHsKICAgICB9CiB9 CiAKLWlmICghaXNJT1NXZWJLaXQoKSAmJiAhLWQgIldlYktpdExpYnJhcmllcyIpIHsKK2lmICgo aXNBcHBsZVdlYktpdCgpIHx8IGlzV2luQ2Fpcm8oKSkgJiYgIS1kICJXZWJLaXRMaWJyYXJpZXMi KSB7CiAgICAgZGllICJFcnJvcjogTm8gV2ViS2l0TGlicmFyaWVzIGRpcmVjdG9yeSBmb3VuZC4g UGxlYXNlIGRvIGEgZnJlc2ggY2hlY2tvdXQuXG4iOwogfQogCg==