Bug 108425 – [Qt] Adjust the way of determining whether build-webkit needs full incremental build

• Component: Tools / Tests
• Status: RESOLVED INVALID

[Seulgi Kim]

Now, webkitdirs.pm caches SVN revision only when it is built successfully.
So if a build fails after getting a new revision, another full incremental build is needed after fixing the error.

[Seulgi Kim]

Created attachment 185675 [details]
Patch

[Jocelyn Turcotte]

Comment on attachment 185675 [details]
Patch

No need to duplicate this code, plus the file should be saved in every case, including the two "elsif" above.
You could just move this code out of "if ($result eq 0)" and run it right after we ran "system $command".

[Seulgi Kim]

Created attachment 199569 [details]
Patch

[Tor Arne Vestbø]

Comment on attachment 199569 [details]
Patch

The whole point is to ensure that we have a successful build before adding to BUILTREVISIONS. Why do you think the "second full incremental build is unnecessary"? What if 'make qmake_all' fails to generate a project file, or generating sources fails? Will we still end up running qmake for the whole project, even with -j1?

[Jocelyn Turcotte]

(In reply to comment #4)
> (From update of attachment 199569 [details])
> The whole point is to ensure that we have a successful build before adding to BUILTREVISIONS. Why do you think the "second full incremental build is unnecessary"? What if 'make qmake_all' fails to generate a project file, or generating sources fails? Will we still end up running qmake for the whole project, even with -j1?

The bug is real and annoying:
- Build successfully svn rev 12345
- git pull --rebase (to svn rev 12346)
- Merge conflicts, make some mistakes that would make the build fail
- build-webkit -> some .pro file change detected -> full build -> fails
- Fix mistakes
- build-webkit -> same .pro file change detected again -> full build again!!
And this goes on until you're able to fix the build with full builds between all tries.

This is actually caused by the side effect where maybeNeedsCleanBuild is set to 1. Since the build fails, we do a clean build every time.
So it's not the "second incremental build" that is unnecessary, it's the second full rebuild because the first one failed and maybeNeedsCleanBuild was set.

I wasn't following the difference quite well the first time, so I think you're right and the fix is wrong. But we still have to make sure that maybeNeedsCleanBuild isn't set the second time, or maybe just set it if on the buildbot.

[Jocelyn Turcotte]

(In reply to comment #5)
> - Merge conflicts, make some mistakes that would make the build fail
Plus it also happens every time the build is already broken in SVN for the platform you're building and while you're trying to fix it.

[Tor Arne Vestbø]

The terminology in the script is a bit off, but we have three levels:

 - clean
 - 'safe' incremental (make qmake_all && make)
 - 'regular' incremental (rely on make's dependency tracking)

You're right that we shouldn't do repeated clean builds. This patch was trying to skip the 'safe' incremental step before actually producing a working build, which is going to break builds.

I think one way to fix the repeated clean builds is to check for buildbot in the "elsif ($maybeNeedsCleanBuild)" block, and only wipe and re-make if we're on a build bot. Otherwise we can instruct the user to run make wipeclean manually.

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.