Bug 180521 - [WinCairo][Ninja] Incremental build failure of WTF
Summary: [WinCairo][Ninja] Incremental build failure of WTF
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on: 180578
Blocks:
  Show dependency treegraph
 
Reported: 2017-12-06 21:32 PST by Fujii Hironori
Modified: 2017-12-17 18:09 PST (History)
9 users (show)

See Also:


Attachments
Patch (1.53 KB, patch)
2017-12-07 03:08 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (2.56 KB, patch)
2017-12-07 18:47 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (2.78 MB, application/zip)
2017-12-07 20:26 PST, EWS Watchlist
no flags Details
Patch (1.55 KB, patch)
2017-12-08 06:00 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2017-12-06 21:32:53 PST
Some incremental build failure have been observed on WinCairo BuildBots and EWS.

For example,
https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/9024

I can reproduce the same incremental build failure with the following steps.

1) Git checkout trunk@225616 (bcbc974a3982284d32449531810406d4a1ccdd8e)
2) Clean Ninja build WinCairo Release
3) Git checkout trunk@225617 (54cbaddd85dba9d8e5cf32e82731524444df142e)

Then, same incremental build faiure happens.
Comment 1 Fujii Hironori 2017-12-06 22:18:19 PST
There is Assertions.cpp in the error message.

> Assertions.cpp.obj : error LNK2001: unresolved external symbol "private: void __cdecl WTF::LockBase::lockSlow(void)" (?lockSlow@LockBase@WTF@@AEAAXXZ)

This means that Assertions.cpp was not recompiled by https://trac.webkit.org/changeset/225617/

This is the excerpt result of ninja -t deps

> Source/WTF/wtf/CMakeFiles/WTF.dir/Assertions.cpp.obj: #deps 117, deps mtime 534326939 (VALID)
>     ../../Source/WTF/config.h
>     ../../WebKitLibraries/win/include/Availability.h
>     ../../WebKitLibraries/win/include/AvailabilityInternal.h
(...)
>     DerivedSources/ForwardingHeaders/wtf/HashFunctions.h
>     DerivedSources/ForwardingHeaders/wtf/HashTraits.h
>     DerivedSources/ForwardingHeaders/wtf/KeyValuePair.h
>     DerivedSources/ForwardingHeaders/wtf/Lock.h
(...)

Assertions.cpp depends on DerivedSources/ForwardingHeaders/wtf/Lock.h.
WTF shouldn't include its forwarding header.
Comment 2 Fujii Hironori 2017-12-06 22:57:14 PST
WTF forwarding headers are generated by using WTF_PRE_BUILD_COMMAND in Source/WTF/wtf/PlatformWin.cmake.

But, BYPRODUCTS of add_custom_target is not specified.
<https://cmake.org/cmake/help/v3.10/command/add_custom_target.html>

So, Ninja doesn't know the build rule of DerivedSources/ForwardingHeaders/wtf/Lock.h.
Comment 3 Fujii Hironori 2017-12-06 23:10:50 PST
This is one of problems caused by copied forwarding header (Bug 179814).

There seems two options of solution.

1. WTF shouldn't include its forwarding headers.
2. Generate WTF forwarding headers before building WTF.
Comment 4 Fujii Hironori 2017-12-06 23:15:54 PST
(In reply to Fujii Hironori from comment #3)
> 1. WTF shouldn't include its forwarding headers.

1-a. Reorder include paths to push back forwarding headers directory.
1-b. Use #include "Lock.h" in WTF. (I guess it's tough because WTF forwarding headers should be flattened.)

> 2. Generate WTF forwarding headers before building WTF.

Bug 180063 Comment 16 sounds a nice approach.
Comment 5 Fujii Hironori 2017-12-07 03:08:59 PST
Created attachment 328685 [details]
Patch
Comment 6 Fujii Hironori 2017-12-07 03:12:29 PST
There is another solution. Windows ports unnecessarily have DerivedSources/ForwardingHeaders as a include path. This patch just removes it.
Comment 7 Fujii Hironori 2017-12-07 05:47:37 PST
Comment on attachment 328685 [details]
Patch

> C:\cygwin\home\buildbot\WebKit\Tools\ImageDiff\cg\PlatformImageCG.cpp(40): fatal error C1083: Cannot open include file: 'wtf/MathExtras.h': No such file or directory

Oops. AppleWin EWS fails.
Comment 8 Fujii Hironori 2017-12-07 18:47:48 PST
Created attachment 328773 [details]
Patch
Comment 9 EWS Watchlist 2017-12-07 20:26:26 PST
Comment on attachment 328773 [details]
Patch

Attachment 328773 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5538408

New failing tests:
fast/text/vertical-rl-rtl-linebreak.html
Comment 10 EWS Watchlist 2017-12-07 20:26:27 PST
Created attachment 328787 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 11 Fujii Hironori 2017-12-07 20:51:31 PST
I filed Bug 180570 for fast/text/vertical-rl-rtl-linebreak.html crash.
Comment 12 Konstantin Tokarev 2017-12-08 01:48:23 PST
Comment on attachment 328773 [details]
Patch

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

> Tools/ImageDiff/cg/PlatformImageCG.cpp:-40
> -#include <wtf/MathExtras.h>

This is probably not important in this particular patch, but please try to submit indepenedent changes as separate patches. It simplifies work for maintainers of non-trunk branches
Comment 13 Fujii Hironori 2017-12-08 02:08:40 PST
Thank you for the review. I'm goint to do it in Bug 180578.
Comment 14 Fujii Hironori 2017-12-08 06:00:25 PST
Created attachment 328808 [details]
Patch
Comment 15 WebKit Commit Bot 2017-12-08 08:31:51 PST
Comment on attachment 328808 [details]
Patch

Clearing flags on attachment: 328808

Committed r225677: <https://trac.webkit.org/changeset/225677>
Comment 16 WebKit Commit Bot 2017-12-08 08:31:52 PST
All reviewed patches have been landed.  Closing bug.
Comment 17 Radar WebKit Bug Importer 2017-12-08 08:32:51 PST
<rdar://problem/35935013>
Comment 18 Fujii Hironori 2017-12-17 18:09:13 PST
Still happens. I'll fix it in Bug 180921.