So that forwarding headers script is only called when source code changes, instead of for every build.
Created attachment 239206 [details] Patch
Comment on attachment 239206 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239206&action=review > Source/WebKit2/PlatformGTK.cmake:854 > + > +file(GLOB_RECURSE WebKit2_HEADERS > + *.h > +) This is run during the configuration phase, so doesn't this break if you add a new header without re-running cmake?
(In reply to comment #2) > (From update of attachment 239206 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=239206&action=review > > > Source/WebKit2/PlatformGTK.cmake:854 > > + > > +file(GLOB_RECURSE WebKit2_HEADERS > > + *.h > > +) > > This is run during the configuration phase, so doesn't this break if you add a new header without re-running cmake? Yes, that's only another problem of not adding headers to the makefiles, it happens with other things like rules using wilcard characters, if you add a new file, you need to force a configure for that to work. This is still much better than running the forwarding headers script 3 times every build. The situation of adding a new header after the configure phase that requires a new forwarding header is not common at all.
Committed r174422: <http://trac.webkit.org/changeset/174422>
(In reply to comment #4) > Committed r174422: <http://trac.webkit.org/changeset/174422> After this change, the GTK EWS stucked with this strange error: ninja: error: '../../Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.h', needed by 'WebKit2-forwarding-headers.stamp', missing and no known rule to make it
(In reply to comment #5) > (In reply to comment #4) > > Committed r174422: <http://trac.webkit.org/changeset/174422> > > After this change, the GTK EWS stucked with this strange error: > > ninja: error: '../../Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.h', needed by 'WebKit2-forwarding-headers.stamp', missing and no known rule to make it Yes, this is indeed one of the cases when this fails because of the wildcard characters, a header file was removed, but non of our makefiles changed (it was an efl header), so a new cmake configure was required. Already forced a clean build on the bots. If this case happens more often that what I expected, I'll roll out the patch.
One more problem: https://trac.webkit.org/changeset/177764 ninja: error: '../../Source/WebKit2/Shared/API/Cocoa/WKRenderingProgressEventsInternal.h', needed by 'WebKit2-forwarding-headers.stamp', missing and no known rule to make it I triggered clean build on the buildbots, but the EWS still needs a kick.
One more error due to this bug after https://trac.webkit.org/changeset/181777: ninja: error: '../../Source/WebKit2/UIProcess/mac/WebMediaPlaybackTargetPickerProxyMac.h', needed by 'WebKit2-forwarding-headers.stamp', missing and no known rule to make it It isn't good to have incremental build issue after all header deleting/renaming in Webkit2 :-/
(In reply to comment #8) > One more error due to this bug after > https://trac.webkit.org/changeset/181777: > ninja: error: > '../../Source/WebKit2/UIProcess/mac/WebMediaPlaybackTargetPickerProxyMac.h', > needed by 'WebKit2-forwarding-headers.stamp', missing and no known rule to > make it > > It isn't good to have incremental build issue after > all header deleting/renaming in Webkit2 :-/ This hasn't happened that often, IMO, so for me it's still worth it.
I kicked the bots with https://trac.webkit.org/changeset/181786 , but it isn't good to have to land this kind of fixes or poking the EWS maintainers to kick the bots. We really need a proper fix. Why was it problem that forwaring header generator ran all the time? It is quite quick. I understand that all seconds count during working on a task locally. But can't we run this generator unconditionally only on the bots somehow?
(In reply to comment #10) > I kicked the bots with https://trac.webkit.org/changeset/181786 , > but it isn't good to have to land this kind of fixes or poking > the EWS maintainers to kick the bots. We really need a proper fix. > > Why was it problem that forwaring header generator ran all the time? > It is quite quick. I understand that all seconds count during working > on a task locally. But can't we run this generator unconditionally > only on the bots somehow? - empty build on GTK takes 3-4 seconds (fw header generator doesn't run at all) - empty build on EFL takes 6-7 seconds (fw header generator always run 10 times) It is trivial to refactor fw header generator to run only once for each project: WebKit2, WTR, TestWebKitAPI. It reduced the EFL build time to 5 seconds for me, I'm going to upload the patch soon to bug142907.
One more incrementail build issue due to this change: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Build%29/builds/56936
and again: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Build%29/builds/57167 Do you really want to have this issue again and again on the bots? I can't understand don't we run this generator unconditonally? It's only 3-4 seconds per build and it guarantess we won't have this ugly issue ever. But it's your choice, of course.
I kicked the GTK bots with this commit - http://trac.webkit.org/changeset/182326. But I'm against to do this kind of changes, and done last time.
TBH I'm with Ossy on this one. The time I lose doing an initial build after a pull, discovering that it failed, figuring out it's an incremental build issue, and then running a clean build is not really worth the benefit. Bug #142907 looks like a better approach.
Ok, let's optimize and reduce the amount of times the script is run instead.
(In reply to comment #16) > Ok, let's optimize and reduce the amount of times the script is run instead. bug142907 reduces the amount from 4 to 3 on GTK and from 10 to 3 on EFL.
(In reply to comment #17) > (In reply to comment #16) > > Ok, let's optimize and reduce the amount of times the script is run instead. > > bug142907 reduces the amount from 4 to 3 on GTK and from 10 to 3 on EFL. With this fix the empty EFL build takes only 5 seconds. Additionally I measured the runtime of the forwarding header generator script itself: (these three execution can be run in parallel) - 0.12 seconds for WTR - 0.17 seconds for TestWebKitAPI - 1.05 seconds for WebKit2
And again after https://trac.webkit.org/changeset/182873: ninja: error: '../../Source/WebKit2/UIProcess/API/Cocoa/_WKWebsiteDataStoreInternal.h', needed by 'WebKit2-forwarding-headers.stamp', missing and no known rule to make it (on the other hand we really don't need forwarding header for cocoa headers ...)
I filed two bug reports: - make forwarding header generator run unconditionally: bug143819 - make forwarding header generate less unnecessary headers: bug143820 (In reply to comment #19) > And again after https://trac.webkit.org/changeset/182873: > ninja: error: > '../../Source/WebKit2/UIProcess/API/Cocoa/_WKWebsiteDataStoreInternal.h', > needed by 'WebKit2-forwarding-headers.stamp', missing and no known rule to > make it > > (on the other hand we really don't need forwarding header for cocoa headers > ...) The forwarding header generator didn't generate forwarding header for Source/WebKit2/UIProcess/API/Cocoa/_WKWebsiteDataStoreInternal.h, but the *.h rule added false dependency for this header.