[JSCOnly] Restore Windows build.
Created attachment 343821 [details] Patch
Created attachment 343822 [details] Patch
Comment on attachment 343822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343822&action=review Overall LGTM > Source/cmake/OptionsJSCOnly.cmake:74 > + set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib64) This is now listed twice
(In reply to Don Olmstead from comment #3) > Comment on attachment 343822 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=343822&action=review > > Overall LGTM > > > Source/cmake/OptionsJSCOnly.cmake:74 > > + set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib64) > > This is now listed twice It's two different variables. Seems that both are needed (just as in OptionsWin).
Comment on attachment 343822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343822&action=review >>> Source/cmake/OptionsJSCOnly.cmake:74 >>> + set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib64) >> >> This is now listed twice > > It's two different variables. Seems that both are needed (just as in OptionsWin). Misread my fault there.
Comment on attachment 343822 [details] Patch Attachment 343822 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/8372472 New failing tests: storage/indexeddb/modern/blob-svg-image.html
Created attachment 343838 [details] Archive of layout-test-results from ews104 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
LayoutTest failures are unrelated: https://bugs.webkit.org/show_bug.cgi?id=187156
Comment on attachment 343822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343822&action=review > Source/cmake/OptionsJSCOnly.cmake:-21 > - # FIXME: Enable FTL on Windows. https://bugs.webkit.org/show_bug.cgi?id=145366 > - WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_FTL_JIT PRIVATE OFF) You don't expect anyone to build JSCOnly using CMake directly? I guess the build-jsc script suffices... but for GTK/WPE, build-webkit and build-jsc are just wrappers above the real (CMake) build system, which we expect to work independently of it, so I wouldn't consider it misleading to disable an unsupported option here. > Source/cmake/OptionsJSCOnly.cmake:76 > - set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) > + set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin64) Windows is interesting. > Tools/Scripts/webkitdirs.pm:1299 > return if defined($isWin64); > - $isWin64 = checkForArgumentAndRemoveFromARGV("--64-bit") || isWinCairo() && !shouldBuild32Bit(); > + $isWin64 = checkForArgumentAndRemoveFromARGV("--64-bit") || ((isWinCairo() || isJSCOnly) && !shouldBuild32Bit()); This will probably break for Apple Windows since you're missing the parentheses after isJSCOnly()
(In reply to Michael Catanzaro from comment #9) > Comment on attachment 343822 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=343822&action=review > > > Source/cmake/OptionsJSCOnly.cmake:-21 > > - # FIXME: Enable FTL on Windows. https://bugs.webkit.org/show_bug.cgi?id=145366 > > - WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_FTL_JIT PRIVATE OFF) > > You don't expect anyone to build JSCOnly using CMake directly? I guess the > build-jsc script suffices... but for GTK/WPE, build-webkit and build-jsc are > just wrappers above the real (CMake) build system, which we expect to work > independently of it, so I wouldn't consider it misleading to disable an > unsupported option here. That's a fair counterpoint! It just surprised me as I was ruling out variables on what was causing the build to fail, but given that there is a scenario in which it has an effect, I'm okay with leaving it. > > Source/cmake/OptionsJSCOnly.cmake:76 > > - set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) > > + set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin64) > > Windows is interesting. Yeah... :( It would be fair game to have JSCOnly operate differently, but I figure if it's kept consistent with the full WebKit build, then a JSC test bot wouldn't need to worry about which type of build bot the archive is coming from. > > Tools/Scripts/webkitdirs.pm:1299 > > return if defined($isWin64); > > - $isWin64 = checkForArgumentAndRemoveFromARGV("--64-bit") || isWinCairo() && !shouldBuild32Bit(); > > + $isWin64 = checkForArgumentAndRemoveFromARGV("--64-bit") || ((isWinCairo() || isJSCOnly) && !shouldBuild32Bit()); > > This will probably break for Apple Windows since you're missing the > parentheses after isJSCOnly() Ack! What a silly typo.
Created attachment 343890 [details] Patch
Created attachment 343892 [details] Patch for landing
Comment on attachment 343892 [details] Patch for landing Clearing flags on attachment: 343892 Committed r233363: <https://trac.webkit.org/changeset/233363>
All reviewed patches have been landed. Closing bug.
<rdar://problem/41642809>