Bug 108425 - [Qt] Adjust the way of determining whether build-webkit needs full incremental build
Summary: [Qt] Adjust the way of determining whether build-webkit needs full incrementa...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 525.x (Safari 3.2)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-30 21:49 PST by Seulgi Kim
Modified: 2014-02-03 03:24 PST (History)
8 users (show)

See Also:


Attachments
Patch (1.70 KB, patch)
2013-01-30 21:53 PST, Seulgi Kim
no flags Details | Formatted Diff | Diff
Patch (1.97 KB, patch)
2013-04-24 17:57 PDT, Seulgi Kim
vestbo: review-
vestbo: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Seulgi Kim 2013-01-30 21:49:06 PST
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.
Comment 1 Seulgi Kim 2013-01-30 21:53:27 PST
Created attachment 185675 [details]
Patch
Comment 2 Jocelyn Turcotte 2013-03-13 10:14:18 PDT
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".
Comment 3 Seulgi Kim 2013-04-24 17:57:07 PDT
Created attachment 199569 [details]
Patch
Comment 4 Tor Arne Vestbø 2013-04-24 23:57:51 PDT
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?
Comment 5 Jocelyn Turcotte 2013-04-25 02:08:48 PDT
(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.
Comment 6 Jocelyn Turcotte 2013-04-25 02:10:56 PDT
(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.
Comment 7 Tor Arne Vestbø 2013-04-25 07:32:45 PDT
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.
Comment 8 Jocelyn Turcotte 2014-02-03 03:24:32 PST
=== 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.