Bug 122787 - JavaScriptCore tests should run on Windows
Summary: JavaScriptCore tests should run on Windows
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 122472
  Show dependency treegraph
 
Reported: 2013-10-14 16:11 PDT by Alex Christensen
Modified: 2013-10-17 15:26 PDT (History)
4 users (show)

See Also:


Attachments
patch (34.70 KB, patch)
2013-10-14 16:18 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (34.38 KB, patch)
2013-10-14 16:26 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (34.66 KB, patch)
2013-10-14 20:52 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (33.05 KB, patch)
2013-10-14 20:56 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (33.84 KB, patch)
2013-10-14 21:38 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (30.27 KB, patch)
2013-10-17 12:51 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2013-10-14 16:11:34 PDT
JavaScriptCore tests haven't run on Windows, and the Win64 JavaScriptCore is broken.  To help us fix this, I'm making the tests work again.  The 64-bit tests still don't work, but this is a step in the right direction.

I was getting "application unable to start correctly (0xc0000022)" errors when WTF and JavaScriptCore were built separately, and the JavaScriptCore.submit.sln wasn't building any of the LLint projects, so I fixed that.

I also copy the necessary dlls to the right places to run JavaScriptCore tests.  It's a bit unconventional to copy into Source, but actual.html is written there already.  Reviewers might not like that.  This solution works and it doesn't mess with the mozilla jsDriver.pl.  Otherwise we'd need to do strange things with the working directory on windows and importing webkitdirs.pm.
Comment 1 Alex Christensen 2013-10-14 16:18:37 PDT
Created attachment 214205 [details]
patch
Comment 2 Alex Christensen 2013-10-14 16:26:43 PDT
Created attachment 214206 [details]
Patch
Comment 3 Brent Fulgham 2013-10-14 16:29:57 PDT
(In reply to comment #0)
> JavaScriptCore tests haven't run on Windows, and the Win64 JavaScriptCore is broken.  To help us fix this, I'm making the tests work again.  The 64-bit tests still don't work, but this is a step in the right direction.

Wonderful!

> I was getting "application unable to start correctly (0xc0000022)" errors when WTF and JavaScriptCore were built separately, and the JavaScriptCore.submit.sln wasn't building any of the LLint projects, so I fixed that.

Great!

> I also copy the necessary dlls to the right places to run JavaScriptCore tests.  It's a bit unconventional to copy into Source, but actual.html is written there already.  Reviewers might not like that.  This solution works and it doesn't mess with the mozilla jsDriver.pl.  Otherwise we'd need to do strange things with the working directory on windows and importing webkitdirs.pm.

No!

This is exactly why we have the DLLLauncher.cpp stuff that looks for the AAS install.  We have to make sure that the necessary runtime libraries supporting WebKit are available.

Your other option would be to modify the PATH environment variable to include the project file output so that the programs will run.

Copying files into the tree is a non-starter!
Comment 4 Alex Christensen 2013-10-14 20:52:02 PDT
Created attachment 214227 [details]
Patch
Comment 5 Alex Christensen 2013-10-14 20:56:01 PDT
Created attachment 214228 [details]
Patch
Comment 6 Alex Christensen 2013-10-14 21:38:34 PDT
Created attachment 214231 [details]
Patch
Comment 7 Alex Christensen 2013-10-14 21:41:26 PDT
This fifth patch fixes JavaScriptCore tests, and it makes Win64 JavaScriptCore tests run and fail (which is what I wanted to expose).  I haven't tested this patch with the AppleWin port, so let me know if I broke anything.
Comment 8 Brent Fulgham 2013-10-16 10:52:23 PDT
Comment on attachment 214231 [details]
Patch

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

The overall idea here is great, but your change would cause catastrophic breaking of our internal build process so I must reject this patch.

> Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.submit.sln:44
> +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "WTF", "..\..\WTF\WTF.vcxproj\WTF.vcxproj", "{8EF73779-BED3-45BB-816D-9FF58399AFA5}"

You can't do this. WTF.submit.sln and JavaScriptCore.submit.sln are isolated from each-other for a reason.

What you COULD do, is make sure there is a JavaScriptCore.sln that looks exactly like this file, and have build-jsc use that file (instead of JavaScriptCore.submit.sln).

> Source/WTF/WTF.vcxproj/WTF.submit.sln:-2
> -Microsoft Visual Studio Solution File, Format Version 11.00

Why are you removing this file?  It's critical.
Comment 9 Alex Christensen 2013-10-16 11:17:13 PDT
(In reply to comment #8)
> What you COULD do, is make sure there is a JavaScriptCore.sln that looks exactly like this file, and have build-jsc use that file (instead of JavaScriptCore.submit.sln).
Sounds good.  I'll submit a patch that does this tomorrow, but you should probably look into adding the LLint projects to JavaScriptCore.submit.sln.
> 
> > Source/WTF/WTF.vcxproj/WTF.submit.sln:-2
> > -Microsoft Visual Studio Solution File, Format Version 11.00
> 
> Why are you removing this file?  It's critical.
I'm not using it.  I removed a reference to it from build-jsc and I thought nobody else was using it.  I have no way of testing the Apple internal build.  If there are Apple internal references to it I'll leave it in.
Comment 10 Brent Fulgham 2013-10-16 11:35:47 PDT
The *.submit.sln files are part of Apple's internal build (submissions) process and cannot be modified the way you propose in this bug. They need to be isolated from each-other the way they currently are.

It looks like this whole thing is fallout from the move to VS 2010.  We somehow lost the original "JavaScriptCore.sln" file, which is what build-jsc should be using.

So, to do this properly we need to do the following:

1. Leave JavaScriptCore.submit.sln and WTF.submit.sln alone.
2. Copy your JavaScriptCore.submit.sln to "JavaScriptCore.sln" and check it in.
3. Modify build-jsc to use "JavaScriptCore.sln" instead of "JavaScriptCore.submit.sln"  This was wrongly done previously (probably by me or rfong).

At that point, all of this should be perfect.
Comment 11 Alex Christensen 2013-10-17 12:51:46 PDT
Created attachment 214489 [details]
Patch
Comment 12 Roger Fong 2013-10-17 14:52:53 PDT
Based off of Brent's comments, the new patch looks good, unofficial r+ from me.
Comment 13 WebKit Commit Bot 2013-10-17 15:26:56 PDT
Comment on attachment 214489 [details]
Patch

Clearing flags on attachment: 214489

Committed r157607: <http://trac.webkit.org/changeset/157607>
Comment 14 WebKit Commit Bot 2013-10-17 15:26:58 PDT
All reviewed patches have been landed.  Closing bug.