Bug 204477 – Set 64-bit as default architecture on Windows

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Per Arne Vollan]

This will make 64-bit building and testing the default.

[Per Arne Vollan]

Created attachment 384100 [details]
Patch

[Brent Fulgham]

Comment on attachment 384100 [details]
Patch

Yes, please!

[Per Arne Vollan]

Created attachment 384107 [details]
Patch

[Per Arne Vollan]

Created attachment 384175 [details]
Patch

[Jonathan Bedard]

While the code looks fine, I'm a not super excited about landing a Windows change that's failing the Windows build in EWS.

[Per Arne Vollan]

Created attachment 384197 [details]
Patch

[Per Arne Vollan]

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

[Ross Kirsling]

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?)

[Ross Kirsling]

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

[Per Arne Vollan]

(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!

[Per Arne Vollan]

Created attachment 384212 [details]
Patch

[Ross Kirsling]

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?

[Per Arne Vollan]

Created attachment 384217 [details]
Patch

[Per Arne Vollan]

(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!

[Per Arne Vollan]

Will land this once the EWS bots are green.

[WebKit Commit Bot]

Comment on attachment 384217 [details]
Patch

Clearing flags on attachment: 384217

Committed r252827: <https://trac.webkit.org/changeset/252827>

[Radar WebKit Bug Importer]

<rdar://problem/57560495>