Bug 177279 – [Win][JSCOnly] Make jsconly build testapi and dlls and copy dlls when running tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Stephan Szabo]

Non-jsc-only Windows builds of the JSC shell build a jscLib.dll and testapi* differently from other builds.
Do the same for jsc-only windows builds and copy the dll along with the jsc executable when copying in run-jsc-stress-tests on Windows.

[Stephan Szabo]

Also, there are some definitions we set up in OptionsWin that we probably should also be setting in OptionsJSCOnly when we're on Windows.

[Stephan Szabo]

Created attachment 321487 [details]
Build jsc/testapi/testmasm on windows jsc-only similar to win platform

[Stephan Szabo]

Comment on attachment 321487 [details]
Build jsc/testapi/testmasm on windows jsc-only similar to win platform

So, it seems like we do need more of the Windows configuration than initially thought in order for the whole test side to be happier. I'm not sure if it's time to move some of those options elsewhere or if this is still okay.

Added Yusuke Suzuki to cc list because he made a comment on the subject in https://bugs.webkit.org/show_bug.cgi?id=172833 and seemed like it would be good to get opinion.

[Yusuke Suzuki]

Comment on attachment 321487 [details]
Build jsc/testapi/testmasm on windows jsc-only similar to win platform

View in context: https://bugs.webkit.org/attachment.cgi?id=321487&action=review

Making JSC tests work in Windows (in JSCOnly port) is great. I have several questions.

> Source/JavaScriptCore/shell/PlatformJSCOnly.cmake:3
> +    add_definitions(-DWIN_CAIRO)

Is this necessary? JSCOnly port is not relying on Cairo.

> Source/JavaScriptCore/shell/PlatformJSCOnly.cmake:4
> +endif ()

Why can't we do the same to the other non-Windows port (and removing PlatformWin.cmake)?
Is there any specific issues for Windows port?

[Stephan Szabo]

> > Source/JavaScriptCore/shell/PlatformJSCOnly.cmake:3
> > +    add_definitions(-DWIN_CAIRO)
> Is this necessary? JSCOnly port is not relying on Cairo.

Right now shell/DLLLauncherMain.cpp checks for WIN_CAIRO in places to separate it from applewin, for example 
https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/shell/DLLLauncherMain.cpp#L125
where the library path added differs. And we're currently really only handling the wincairo paths in jsc-only, but that might change.

That file's related to the tiny launcher app mentioned in the below.

> Why can't we do the same to the other non-Windows port (and removing  PlatformWin.cmake)?
> Is there any specific issues for Windows port?

I'm not sure of the history, but the shell, testapi, etc on Windows (non-jsc-only) build a separate <something>Lib.dll and then a small launcher to run the code from the dll. The description from when the launcher seemed to be added seemed like it was related to getting the apple support libraries for the testapi and similar apps.

[Stephan Szabo]

And I guess even for wincairo-ish builds it prevents you from needing to add the support library directory to your PATH if you have WEBKIT_LIBRARIES defined properly.

[Yusuke Suzuki]

(In reply to Stephan Szabo from comment #5)
> Right now shell/DLLLauncherMain.cpp checks for WIN_CAIRO in places to
> separate it from applewin, for example 
> https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/shell/
> DLLLauncherMain.cpp#L125
> where the library path added differs. And we're currently really only
> handling the wincairo paths in jsc-only, but that might change.
> 
> That file's related to the tiny launcher app mentioned in the below.
>
> I'm not sure of the history, but the shell, testapi, etc on Windows
> (non-jsc-only) build a separate <something>Lib.dll and then a small launcher
> to run the code from the dll. The description from when the launcher seemed
> to be added seemed like it was related to getting the apple support
> libraries for the testapi and similar apps.

OK, if there is no reason for this, I believe Windows difference should be removed. But it's ok for now.
Could you add FIXME to clean up these things if there is no reason to make Win build results separated from others?
Adding the FIXME comment to PlatformJSCOnly.cmake is preferable.

# FIXME: https://bugs.webkit.org/url-to-newly-filed-bug
# description...

[Stephan Szabo]

Created attachment 324684 [details]
Build jsc/testapi/testmasm on windows jsc-only similar to win platform - added fixme

[WebKit Commit Bot]

Comment on attachment 324684 [details]
Build jsc/testapi/testmasm on windows jsc-only similar to win platform - added fixme

Clearing flags on attachment: 324684

Committed r223904: <https://trac.webkit.org/changeset/223904>

[Radar WebKit Bug Importer]

<rdar://problem/35568802>