[webkit-reviews] review granted: [Bug 96245] [Qt] Fix generation of forward headers for generated files in WebCore on Windows : [Attachment 163063] Patch

[bugzilla-daemon at webkit.org]

Tor Arne Vestbø <vestbo at webkit.org> has granted Simon Hausmann
<hausmann at webkit.org>'s request for review:
Bug 96245: [Qt] Fix generation of forward headers for generated files in
WebCore on Windows
https://bugs.webkit.org/show_bug.cgi?id=96245

Attachment 163063: Patch
https://bugs.webkit.org/attachment.cgi?id=163063&action=review

------- Additional Comments from Tor Arne Vestbø <vestbo at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=163063&action=review


>>> Source/WebKit2/DerivedSources.pri:148
>>> +	 win32: eval($${header_target}.commands = ($${QMAKE_MKDIR}
$$toSystemPath($$dest_dir) 2>nul || echo>nul))
>> 
>> It looks like we use $${QMAKE_CHK_DIR_EXISTS} || $${QMAKE_MKDIR} in the rest
of Qt's makefiles, perhaps stick with that?
> 
> The problem with that approach is that it doesn't work well with parallel
rules that try to create the same directory ;(
> 
> IOW there's a race condition between the check if the directory exists and
making it. If you have two jobs trying to do the same thing, then
> both jobs determine simultaneously that the directory doesn't exist, both try
to create the directory at the same time and only once succeeds. The
> second one will see mkdir aborting because the directory already exists.
> 
> I tried removing the mkdir from this rule altogether and instead depend on
the other generator that creates the directory also, but unfortunately when
> depending on such a symbol target nmake will always call it and thus always
re-generate the file.

Ouch! You're right, that will not be pretty. Wonder why Qt gets away with that
though.