Bug 74687 - [Qt] Incremental build problem revealed by r102981
Summary: [Qt] Incremental build problem revealed by r102981
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Ádám Kallai
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks: 76528
  Show dependency treegraph
 
Reported: 2011-12-15 22:47 PST by Csaba Osztrogonác
Modified: 2012-01-24 05:28 PST (History)
6 users (show)

See Also:


Attachments
proposed patch (1.41 KB, patch)
2012-01-23 06:31 PST, Ádám Kallai
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2011-12-15 22:47:55 PST
This change http://trac.webkit.org/changeset/102981/trunk/Source/WebCore/mathml/mathattrs.in
caused a build fail on the Qt bot, because the necessarry generated file wasn't
regenerated. It must be a bug in the Qt build system. I'll check it.

Thanks for you kicked the bot with touching a file. ;)
Comment 1 Ádám Kallai 2012-01-23 06:31:41 PST
Created attachment 123550 [details]
proposed patch

It is necessary to set mathattrs.in dependency for generating MathMLNames.cpp file. I added depend this file /Source/WebCore/DerivedSources.pri.
Comment 2 Tor Arne Vestbø 2012-01-23 06:52:18 PST
Comment on attachment 123550 [details]
proposed patch

While the fix is correct it unfortunately doesn't solve the general problem, for exxample is $$PWD/mathml/mathtags.in still left out of depends. And there's a bunch of other generators that have the same problem.

The better approach I think would be to add code in default_post.prf that walks every argument of commands and checks if it's a file that exists, and if so, adds it to depends. That should ideally fix it for all of these cases.

Feel free to have a stab at it, or assign the bug to me and I'll have a look. Thanks!
Comment 3 Csaba Osztrogonác 2012-01-24 04:05:51 PST
(In reply to comment #2)
> (From update of attachment 123550 [details])
> While the fix is correct it unfortunately doesn't solve the general problem, for exxample is $$PWD/mathml/mathtags.in still left out of depends. And there's a bunch of other generators that have the same problem.

The correct is absolutely correct for this bug. And $$PWD/mathml/mathtags.in isn't missing from the dependency list, because MATHML_NAMES is $$PWD/mathml/mathtags.in which is the input of this generator.

> The better approach I think would be to add code in default_post.prf that walks every argument of commands and checks if it's a file that exists, and if so, adds it to depends. That should ideally fix it for all of these cases.

> Feel free to have a stab at it, or assign the bug to me and I'll have a look. Thanks!

The idea is good, but I'm not sure if it works always. Let's try it.

But I don't think if we shouldn't land this trivial, necssarry and correct fix because maybe we can do a general fix sometimes in the future. I filed a new bug report for this future development: https://bugs.webkit.org/show_bug.cgi?id=76902
Comment 4 Csaba Osztrogonác 2012-01-24 04:07:12 PST
(In reply to comment #3)

> The correct is absolutely correct for this bug. And $$PWD/mathml/mathtags.in isn't missing from the dependency list, because MATHML_NAMES is $$PWD/mathml/mathtags.in which is the input of this generator.

I should have reread my comment. :) s/The correct/The fix/g
Comment 5 Csaba Osztrogonác 2012-01-24 04:08:57 PST
Comment on attachment 123550 [details]
proposed patch

r=me to decrease the number of possible incremental build bugs in the future.
Comment 6 Csaba Osztrogonác 2012-01-24 05:28:19 PST
Comment on attachment 123550 [details]
proposed patch

Clearing flags on attachment: 123550

Committed r105731: <http://trac.webkit.org/changeset/105731>
Comment 7 Csaba Osztrogonác 2012-01-24 05:28:28 PST
All reviewed patches have been landed.  Closing bug.