Summary: | Get rid of prefix header dependency for WebKit2 build system | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Laszlo Gombos <laszlo.gombos> | ||||||||||||||
Component: | WebKit2 | Assignee: | Greg <greg.coletta> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | abarth, abecsi, buildbot, commit-queue, eric, greg.coletta, kbalazs, kenneth, loki, rgabor, sam, s.mathur, webkit-ews, webkit.review.bot | ||||||||||||||
Priority: | P2 | Keywords: | Qt | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | PC | ||||||||||||||||
OS: | All | ||||||||||||||||
Bug Depends on: | 53400 | ||||||||||||||||
Bug Blocks: | 50251 | ||||||||||||||||
Attachments: |
|
Description
Laszlo Gombos
2010-11-29 13:06:50 PST
I agree that a config.h style solution would be more straightforward. The last feedback I got from the Apple guys was that they do not want another config.h for WebKit2. However, distcc and icecc are happy with the prefix header at least with gcc (only precompiled header is a problem). Does RVCT 2.2 really luck the -include option or something equivalent? (In reply to comment #1) > Does RVCT 2.2 really lack the -include option or something equivalent? RVCT 4.0 has a -include option but drops a warning: Warning: C3910W: Old syntax, please use '-I'. So I guess previous versions also supported -include, but it might behave like -I, which prepends a directory to the include path. As far as I know RVCT has a --pch option since 2.0, and at least 4.0 has a --use_pch option, which should behave similar to gcc's -include. Gabor(s), do you know how to simulate -include with rvct? RVCT (including 2.2) does have a compiler option for prefix headers - it is called "--preinclude". There is more on this here: http://www.keil.com/support/man/docs/armccref/armccref_CHDIIAEI.htm However there is a bug in the 2.2 version of RVCT that it only takes the last instance of the preinclude header into account (which typically points to RVCT2_2.h). I have not found a way to convince RVCT to take our own preinclude header with the existing Qt/Symbian toolchain. As noted RVCT is not the only environment where this is an issue. If you happen to compile for Symbian OS, then a provisional solution with RVCT 2.2 is to add the following line at the end of your $$EPOCROOT/epoc32/include/rvct/rvct.h #ifdef BUILDING_WEBKIT #include "..\..\..\webkit\WebKit2\WebKit2Prefix.h" #endif Created attachment 77351 [details]
Adding config.h to every WebKit2 cpp file.
Attachment 77351 [details] did not build on qt: Build output: http://queues.webkit.org/results/7257124 Attachment 77351 [details] did not build on win: Build output: http://queues.webkit.org/results/7235150 Comment on attachment 77351 [details] Adding config.h to every WebKit2 cpp file. View in context: https://bugs.webkit.org/attachment.cgi?id=77351&action=review r- because this breaks some builds. Also, does this change have consensus with the folks working on WebKit2? This seems like it should be resolved on webkit-dev (rather than a bug/patch). > Tools/Scripts/webkitpy/style/checker.py:161 > + # Certain directories have other idiosyncracies. Nit: "other" would no longer apply here. Thanks for the review David, I'm looking into the build failures. The topic has been raised on webkit-dev mailing list - https://lists.webkit.org/pipermail/webkit-dev/2010-November/015162.html . No notable objections has been received so far. Created attachment 79772 [details]
Adding config.h to every WebKit2 cpp file.
Created attachment 79797 [details]
Adding config.h to every WebKit2 cpp file.
Attachment 79797 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebKit2/config.h:117: "WebCore/config.h" already included at Source/WebKit2/config.h:28 [build/include] [4]
Total errors found: 1 in 361 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Looks absolutely good to me but we should convince the Apple folks about the need of this change. This looks good to me as well. I plan to r+ it and land it on Thursday, unless someone object. Comment on attachment 79797 [details]
Adding config.h to every WebKit2 cpp file.
r=me. cq- as the patch needs to be re-baselined.
Created attachment 80402 [details]
rebaselined patch for cq to commit
Comment on attachment 80402 [details] rebaselined patch for cq to commit Rejecting attachment 80402 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'ap..." exit_code: 2 Last 500 characters of output: h |+++ Source/WebKit2/config.h -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored patching file Source/WebKit2/win/WebKit2Common.vsprops Hunk #1 FAILED at 6. 1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit2/win/WebKit2Common.vsprops.rej patching file Tools/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Tools/Scripts/webkitpy/style/checker.py Failed to run "[u'/Projects/CommitQueue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/7542395 Created attachment 80404 [details]
try again, this time patch generated from svn
Comment on attachment 80404 [details] try again, this time patch generated from svn Rejecting attachment 80404 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'ap..." exit_code: 2 Last 500 characters of output: cess/qt/WebProcessQt.cpp patching file Source/WebKit2/WebProcess/win/WebProcessMainWin.cpp patching file Source/WebKit2/WebProcess/win/WebProcessWin.cpp patching file Source/WebKit2/win/WebKit2Common.vsprops Hunk #1 FAILED at 6. 1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit2/win/WebKit2Common.vsprops.rej patching file Tools/ChangeLog patching file Tools/Scripts/webkitpy/style/checker.py Failed to run "[u'/Projects/CommitQueue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/7525396 Created attachment 80415 [details]
rebaseline WebKit2Common.vsprops
Comment on attachment 80415 [details] rebaseline WebKit2Common.vsprops Rejecting attachment 80415 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sf', 'ap..." exit_code: 2 Last 500 characters of output: cess/qt/WebProcessQt.cpp patching file Source/WebKit2/WebProcess/win/WebProcessMainWin.cpp patching file Source/WebKit2/WebProcess/win/WebProcessWin.cpp patching file Source/WebKit2/win/WebKit2Common.vsprops Hunk #1 FAILED at 6. 1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit2/win/WebKit2Common.vsprops.rej patching file Tools/ChangeLog patching file Tools/Scripts/webkitpy/style/checker.py Failed to run "[u'/Projects/CommitQueue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/7546380 http://trac.webkit.org/changeset/76916 might have broken SnowLeopard Intel Release (Tests) Comment on attachment 80415 [details] rebaseline WebKit2Common.vsprops Landed manually as http://trac.webkit.org/changeset/76916. |