Bug 180521

Summary: [WinCairo][Ninja] Incremental build failure of WTF
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: New BugsAssignee: Fujii Hironori <Hironori.Fujii>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, annulen, bfulgham, commit-queue, don.olmstead, ews-watchlist, pvollan, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 180578    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
none
Patch none

Fujii Hironori
Reported 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.
Attachments
Patch (1.53 KB, patch)
2017-12-07 03:08 PST, Fujii Hironori
no flags
Patch (2.56 KB, patch)
2017-12-07 18:47 PST, Fujii Hironori
no flags
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
Patch (1.55 KB, patch)
2017-12-08 06:00 PST, Fujii Hironori
no flags
Fujii Hironori
Comment 1 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.
Fujii Hironori
Comment 2 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.
Fujii Hironori
Comment 3 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.
Fujii Hironori
Comment 4 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.
Fujii Hironori
Comment 5 2017-12-07 03:08:59 PST
Fujii Hironori
Comment 6 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.
Fujii Hironori
Comment 7 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.
Fujii Hironori
Comment 8 2017-12-07 18:47:48 PST
EWS Watchlist
Comment 9 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
EWS Watchlist
Comment 10 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
Fujii Hironori
Comment 11 2017-12-07 20:51:31 PST
I filed Bug 180570 for fast/text/vertical-rl-rtl-linebreak.html crash.
Konstantin Tokarev
Comment 12 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
Fujii Hironori
Comment 13 2017-12-08 02:08:40 PST
Thank you for the review. I'm goint to do it in Bug 180578.
Fujii Hironori
Comment 14 2017-12-08 06:00:25 PST
WebKit Commit Bot
Comment 15 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>
WebKit Commit Bot
Comment 16 2017-12-08 08:31:52 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 17 2017-12-08 08:32:51 PST
Fujii Hironori
Comment 18 2017-12-17 18:09:13 PST
Still happens. I'll fix it in Bug 180921.
Note You need to log in before you can comment on or make changes to this bug.