Bug 150700

Summary: [Win] build-jsc and run-javascriptcore-tests do not work
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: Tools / TestsAssignee: Brent Fulgham <bfulgham>
Severity: Normal CC: achristensen, bfulgham, commit-queue, dbates, lforschler, mark.lam, msaboff, ossy, peavo
Priority: P2    
Version: WebKit Nightly Build   
Hardware: PC   
OS: All   
Bug Depends on: 150707    
Bug Blocks:    
Description Flags
Patch mark.lam: review+

Description Brent Fulgham 2015-10-29 16:41:49 PDT
The "run-javascript-core" script does not work following our switch to CMake on Windows, because it attempts to build using the increasingly-out-of-date "Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.sln" solution to build the various pieces of JSC.

The actual failure was in the "build-jsc" script, which does not use the new CMake build system for Windows.

This patch corrects this oversight, allowing JSC developers to build only the JavaScript-related files. (820 files instead of 3500 files!)
Comment 1 Brent Fulgham 2015-10-29 16:49:08 PDT
Created attachment 264365 [details]
Comment 2 Mark Lam 2015-10-29 16:52:00 PDT
Comment on attachment 264365 [details]

Comment 3 Alex Christensen 2015-10-29 16:55:04 PDT
lgtm, too
Comment 4 Brent Fulgham 2015-10-29 17:09:48 PDT
The windows EWS failure looks like a bot problem. I'm investigating.
Comment 5 Brent Fulgham 2015-10-29 17:20:55 PDT
Committed r191766: <http://trac.webkit.org/changeset/191766>
Comment 6 Michael Saboff 2015-10-29 18:16:32 PDT
After this change, I now can't build JavaScriptCore or WebKit.  I get:
$ build-webkit --release --64-bit
Use of uninitialized value $expectedVersion in scalar chomp at Tools/Scripts/update-webkit-support-libs line 113.
Falling back to existing version of WebKitSupportLibrary.
Error: /home/msaboff/src/OpenSource/WebKitBuild/Release is not a directory

The directory does exist.

$ ls /home/msaboff/src/OpenSource/WebKitBuild/Release
ALL_BUILD.vcxproj*          cmakeconfig.h*            INSTALL.vcxproj*          RUN_TESTS.vcxproj*          x64/
ALL_BUILD.vcxproj.filters*  CMakeFiles/               INSTALL.vcxproj.filters*  RUN_TESTS.vcxproj.filters*  ZERO_CHECK.vcxproj*
bin64/                      CPackConfig.cmake*        lib32/                    Source/                     ZERO_CHECK.vcxproj.filters*
buildfailed*                CPackSourceConfig.cmake*  lib64/                    Tools/
build-webkit-options.txt    CTestTestfile.cmake*      obj64/                    WebKit.sln*
cmake_install.cmake*        DerivedSources/           PACKAGE.vcxproj*          webkit_errors.log*
CMakeCache.txt*             include/                  PACKAGE.vcxproj.filters*  webkit_warnings.log*

If I roll back to r191765, I can build.
Comment 7 Brent Fulgham 2015-10-29 19:42:48 PDT
I'll fix ASAP!
Comment 8 Csaba Osztrogonác 2015-10-30 03:23:45 PDT
(In reply to comment #5)
> Committed r191766: <http://trac.webkit.org/changeset/191766>

Apple Windows and WinCairo build is still broken.
Comment 9 Brent Fulgham 2015-10-30 10:06:46 PDT
Committed r191793: <http://trac.webkit.org/changeset/191793>
Comment 10 Csaba Osztrogonác 2015-11-10 03:52:55 PST
(In reply to comment #9)
> Committed r191793: <http://trac.webkit.org/changeset/191793>

Originally you landed this change in r191766:

-    buildCMakeProjectOrExit(0, cmakeBasedPortName(), undef, "jsc $makeArgs", (cmakeBasedPortArguments(), $cmakeArgs));
+    my $testapi = (isAnyWindows() ? "testapi" : "");
+    buildCMakeProjectOrExit(0, cmakeBasedPortName(), undef, "jsc $testapi $makeArgs", (cmakeBasedPortArguments(), $cmakeArgs));

And this one in r191793:

-    # By default we build using all of the available CPUs
-    $makeArgs .= ($makeArgs ? " " : "") . "-j" . numberOfCPUs() if $makeArgs !~ /-j\s*\d+/;
+    my $buildTarget = "";
+    unless (isAnyWindows()) {
+        # By default we build using all of the available CPUs
+        $makeArgs .= ($makeArgs ? " " : "") . "-j" . numberOfCPUs() if $makeArgs !~ /-j\s*\d+/;
+        $buildTarget = "jsc $makeArgs";
+    } elsif (canUseNinja()) {
+        $buildTarget .= "jsc testapi";
+    }

Could you explain why -jN causes problem for Windows-ninja ?
Comment 11 Csaba Osztrogonác 2015-11-12 03:58:26 PST
Comment 12 Csaba Osztrogonác 2015-11-17 05:23:19 PST
(In reply to comment #11)
> Brent?

Comment 13 Csaba Osztrogonác 2015-11-18 02:36:26 PST
(In reply to comment #11)
> Brent?

Comment 14 Csaba Osztrogonác 2015-11-19 04:42:00 PST
(In reply to comment #11)
> Brent?

Comment 15 Csaba Osztrogonác 2015-11-24 10:09:37 PST
(In reply to comment #11)
> Brent?

Comment 16 Csaba Osztrogonác 2015-11-25 08:54:28 PST
(In reply to comment #11)
> Brent?

Comment 17 Csaba Osztrogonác 2015-11-26 04:42:25 PST
Brent, it's not fair that you landed an unreviewed 
hack 2 weeks before and you don't answer my question.

Please answer as soon as possible.
Comment 18 Csaba Osztrogonác 2015-12-01 01:09:30 PST
Comment 19 Alex Christensen 2015-12-01 11:02:02 PST
The unless isAnyWindows is not necessary.  It works without it.  The canUseNinja is necessary because our bots use CMake and Visual Studio without Ninja.  If we change it, though, the testb3 that is there now shouldn't be a target on Windows because b3 isn't enabled on Windows.