Bug 112576 - [Qt] An incremental build bug appeared after r145712.
Summary: [Qt] An incremental build bug appeared after r145712.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-18 08:45 PDT by Ádám Kallai
Modified: 2013-07-08 05:57 PDT (History)
4 users (show)

See Also:


Attachments
proposed patch (1.82 KB, patch)
2013-03-29 08:05 PDT, Ádám Kallai
jturcotte: review+
Details | Formatted Diff | Diff
For example (10.00 KB, application/x-tar)
2013-04-02 07:41 PDT, Ádám Kallai
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ádám Kallai 2013-03-18 08:45:13 PDT
All inspector tests started to fail after r145712. We did clean build the on Qt bots and all inspector tests ran as expected again. So when the bots build incrementally then important files are not generated again. I will try to fix it.
Comment 1 Ádám Kallai 2013-03-18 08:59:32 PDT
It was necessary to do clean build after http://trac.webkit.org/changeset/146048. It caused less inspector tests fail than previous revision, but it seems that the problem is the same.
Comment 2 Csaba Osztrogonác 2013-03-20 08:16:42 PDT
The bug is caused by a missing dependency. 
Here is the related part of the Makefile:

.rcc/release-shared/qrc_InspectorBackendCommands.cpp: generated/InspectorBackendCommands.qrc
        /usr/local/Trolltech/Qt5/Qt-5.0.1/bin/rcc -name InspectorBackendCommands generated/InspectorBackendCommands.qrc -o .rcc/release-shared/qrc_InspectorBackendCommands.cpp


.rcc/release-shared/qrc_InspectorBackendCommands.cpp must depend on InspectorBackendCommands.js too. I'm not sure where is the bug, but
I suspect somewhere in qmake or rcc. 

qmake generates dependency with "rcc -list <qrc-file>" command, but
it might be confused because of this relative path: generated/InspectorBackendCommands.qrc

Could you check it on a small example?
Comment 3 Ádám Kallai 2013-03-29 08:05:14 PDT
Created attachment 195744 [details]
proposed patch

It's a workaround. Ossy, I tried out to your example. I reproduced it successfully. I will able to report this bug. Only I should make a smaller example than I have.
Comment 4 Jocelyn Turcotte 2013-04-02 07:01:08 PDT
Comment on attachment 195744 [details]
proposed patch

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

> Source/WebCore/DerivedSources.pri:813
> +inspectorBackendCommands.depends = $$INSPECTOR_JSON

I'm not sure what this should fix, Inspector.json isn't a dependency of InspectorBackendCommands.qrc
Comment 5 Csaba Osztrogonác 2013-04-02 07:04:43 PDT
Comment on attachment 195744 [details]
proposed patch

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

>> Source/WebCore/DerivedSources.pri:813
>> +inspectorBackendCommands.depends = $$INSPECTOR_JSON
> 
> I'm not sure what this should fix, Inspector.json isn't a dependency of InspectorBackendCommands.qrc

I agree, it is incorrect. But adding InspectorBackendCommands.js as dependency
can be a good workaround, because touching InspectorBackendCommands.js must
force the regeneration of qrc_InspectorBackendCommands.cpp.
Comment 6 Ádám Kallai 2013-04-02 07:41:55 PDT
Created attachment 196137 [details]
For example

I uploaded a smaller example. I reproduced the problem successfully.

The first one was created when I called the qmake from the root. (  qmake foo.pro )
qrc_foo.cpp: build/foo.qrc \
                build/foo.txt
        /usr/local/Trolltech/Qt5/Qt-5.0.1/bin/rcc -name foo build/foo.qrc -o qrc_foo.cpp

The second one was created when I called the qmake from build/ directory. (  qmake ../foo.pro)
 
qrc_foo.cpp: foo.qrc
        /usr/local/Trolltech/Qt5/Qt-5.0.1/bin/rcc -name foo foo.qrc -o qrc_foo.cpp

You can see that build/foo.txt missing, but it should exist there. So when I change foo.txt content and I call make then it doesn't detect the file changes.
Comment 7 Jocelyn Turcotte 2013-04-02 11:01:23 PDT
I tracked this bug into qmake.
It uses a DEPENDPATH array of path to resolve relative file names which is generated from I'm not too sure what (pass "-d" to qmake to see it at the line "Dependency Directories:").

All-in-all it seems related to a mismatch between the current directory used by qmake and rcc, which is an issue when the qrc is situated in the shadowbuild. It probably have been like that for the last 15 years, so the workaround sounds fine to me if you can use the proper filename.
Comment 8 Jocelyn Turcotte 2013-07-02 07:29:42 PDT
Comment on attachment 195744 [details]
proposed patch

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

>>> Source/WebCore/DerivedSources.pri:813
>>> +inspectorBackendCommands.depends = $$INSPECTOR_JSON
>> 
>> I'm not sure what this should fix, Inspector.json isn't a dependency of InspectorBackendCommands.qrc
> 
> I agree, it is incorrect. But adding InspectorBackendCommands.js as dependency
> can be a good workaround, because touching InspectorBackendCommands.js must
> force the regeneration of qrc_InspectorBackendCommands.cpp.

I see, InspectorBackendCommands.js is generated from Inspector.json, and triggering a copy of the qrc in this case will work around the bug in qmake.

The patch looks fine to me after all, this is quite hackish though so it would be nice to have a comment explaining how INSPECTOR_JSON is connected to InspectorBackendCommands.qrc and why we do this.
r=me if you add such comment.
Comment 9 Ádám Kallai 2013-07-08 05:51:01 PDT
It was interesting for us. The InspectorBackendCommands.js is generated by CodeGeneratorInspector.py from Inspector.json. 
The problem was that when the inspector.json is changed and the qrc already exists then the qmake does not realize the changes.
Therefore the qrc_InspectorBackendCommands.cpp won't be regenerated.

So the copy of qrc should be triggered by inspector.json.

I will land it.
Comment 10 Ádám Kallai 2013-07-08 05:57:53 PDT
Committed r152448: <http://trac.webkit.org/changeset/152448>