This will make 64-bit building and testing the default.
Created attachment 384100 [details] Patch
Comment on attachment 384100 [details] Patch Yes, please!
Created attachment 384107 [details] Patch
Created attachment 384175 [details] Patch
While the code looks fine, I'm a not super excited about landing a Windows change that's failing the Windows build in EWS.
Created attachment 384197 [details] Patch
(In reply to Jonathan Bedard from comment #5) > While the code looks fine, I'm a not super excited about landing a Windows > change that's failing the Windows build in EWS. The build failures are addressed in https://bugs.webkit.org/show_bug.cgi?id=204534.
Comment on attachment 384197 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384197&action=review > Tools/Scripts/webkitdirs.pm:1295 > - $isWin64 = checkForArgumentAndRemoveFromARGV("--64-bit") || ((isFTW() || isWinCairo() || isJSCOnly()) && !shouldBuild32Bit()); > + $isWin64 = checkForArgumentAndRemoveFromARGV("--64-bit") || ((isFTW() || isWinCairo() || isAppleWinWebKit() || isJSCOnly()) && !shouldBuild32Bit()); Should we do `isAnyWindows()` instead of `isFTW() || isWinCairo() || isAppleWinWebKit()`? (I guess we have the same trio in determineConfigurationProductDir, but it seems needless to give the complete list, right?)
Also we can remove this line, which seems to have been added unnecessarily: https://github.com/WebKit/webkit/blob/master/Tools/Scripts/webkitdirs.pm#L498
(In reply to Ross Kirsling from comment #8) > Comment on attachment 384197 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=384197&action=review > > > Tools/Scripts/webkitdirs.pm:1295 > > - $isWin64 = checkForArgumentAndRemoveFromARGV("--64-bit") || ((isFTW() || isWinCairo() || isJSCOnly()) && !shouldBuild32Bit()); > > + $isWin64 = checkForArgumentAndRemoveFromARGV("--64-bit") || ((isFTW() || isWinCairo() || isAppleWinWebKit() || isJSCOnly()) && !shouldBuild32Bit()); > > Should we do `isAnyWindows()` instead of `isFTW() || isWinCairo() || > isAppleWinWebKit()`? > That makes sense, I will update the patch. > (I guess we have the same trio in determineConfigurationProductDir, but it > seems needless to give the complete list, right?) Thanks for reviewing!
Created attachment 384212 [details] Patch
Comment on attachment 384212 [details] Patch r=me, but can we also remove the redundant `push(@args, '--64-bit') if (isFTW());` line I mentioned in comment 9?
Created attachment 384217 [details] Patch
(In reply to Ross Kirsling from comment #12) > Comment on attachment 384212 [details] > Patch > > r=me, but can we also remove the redundant `push(@args, '--64-bit') if > (isFTW());` line I mentioned in comment 9? Done. Thanks for reviewing!
Will land this once the EWS bots are green.
Comment on attachment 384217 [details] Patch Clearing flags on attachment: 384217 Committed r252827: <https://trac.webkit.org/changeset/252827>
<rdar://problem/57560495>