Bug 50174

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:

Keywords:

Version:

528+ (Nightly build)

Hardware:

OS:

All

Bug Depends on:

53400

Bug Blocks:

50251

Attachments:

Description	Flags
Adding config.h to every WebKit2 cpp file.	levin: review-
Adding config.h to every WebKit2 cpp file.	none
Adding config.h to every WebKit2 cpp file.	laszlo.gombos: review+, laszlo.gombos: commit-queue-
rebaselined patch for cq to commit	commit-queue: commit-queue-
try again, this time patch generated from svn	commit-queue: commit-queue-
rebaseline WebKit2Common.vsprops	none

Laszlo Gombos

Reported 2010-11-29 13:06:50 PST

WebKit(1) does not need "prefix header" support from the build system (see config.h), but WebKit2 has this dependency. This bug is to track building WebKit2 without "prefix header" support from the compiler. The topic has been discussed in the webkit-dev mailing list - https://lists.webkit.org/pipermail/webkit-dev/2010-July/013427.html. RVCT 2.2 with the current Symbian (and likely other embedded) build system does not seems to support "prefix header". Builds with icecc would also take advantage of this work.

Attachments
Adding config.h to every WebKit2 cpp file. (148.25 KB, patch) 2010-12-23 10:58 PST, Greg	levin: review-	Details Formatted Diff Diff
Adding config.h to every WebKit2 cpp file. (182.41 KB, patch) 2011-01-21 12:03 PST, Greg	no flags	Details Formatted Diff Diff
Adding config.h to every WebKit2 cpp file. (182.35 KB, patch) 2011-01-21 14:40 PST, Greg	laszlo.gombos: review+ laszlo.gombos: commit-queue-	Details Formatted Diff Diff
rebaselined patch for cq to commit (175.91 KB, patch) 2011-01-27 19:18 PST, Laszlo Gombos	commit-queue: commit-queue-	Details Formatted Diff Diff
try again, this time patch generated from svn (181.49 KB, patch) 2011-01-27 19:43 PST, Laszlo Gombos	commit-queue: commit-queue-	Details Formatted Diff Diff
rebaseline WebKit2Common.vsprops (181.49 KB, patch) 2011-01-27 21:05 PST, Laszlo Gombos	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Balazs Kelemen

Comment 1 2010-11-29 15:37:22 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?

Andras Becsi

Comment 2 2010-11-29 16:08:28 PST

(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.

Balazs Kelemen

Comment 3 2010-11-30 02:05:24 PST

Gabor(s), do you know how to simulate -include with rvct?

Laszlo Gombos

Comment 4 2010-11-30 04:51:51 PST

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.

Siddharth Mathur

Comment 5 2010-12-13 09:46:42 PST

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

Greg

Comment 6 2010-12-23 10:58:36 PST

Created attachment 77351 [details] Adding config.h to every WebKit2 cpp file.

Early Warning System Bot

Comment 7 2010-12-23 11:11:51 PST

Attachment 77351 [details] did not build on qt: Build output: http://queues.webkit.org/results/7257124

Build Bot

Comment 8 2010-12-23 13:28:44 PST

Attachment 77351 [details] did not build on win: Build output: http://queues.webkit.org/results/7235150

David Levin

Comment 9 2011-01-03 09:13:31 PST

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.

Greg

Comment 10 2011-01-03 09:25:03 PST

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.

Greg

Comment 11 2011-01-21 12:03:00 PST

Created attachment 79772 [details] Adding config.h to every WebKit2 cpp file.

Greg

Comment 12 2011-01-21 14:40:28 PST

Created attachment 79797 [details] Adding config.h to every WebKit2 cpp file.

WebKit Review Bot

Comment 13 2011-01-21 14:42:30 PST

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.

Balazs Kelemen

Comment 14 2011-01-23 06:01:06 PST

Looks absolutely good to me but we should convince the Apple folks about the need of this change.

Laszlo Gombos

Comment 15 2011-01-26 06:18:03 PST

This looks good to me as well. I plan to r+ it and land it on Thursday, unless someone object.

Laszlo Gombos

Comment 16 2011-01-27 19:07:09 PST

Comment on attachment 79797 [details] Adding config.h to every WebKit2 cpp file. r=me. cq- as the patch needs to be re-baselined.

Laszlo Gombos

Comment 17 2011-01-27 19:18:35 PST

Created attachment 80402 [details] rebaselined patch for cq to commit

WebKit Commit Bot

Comment 18 2011-01-27 19:19:57 PST

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

Laszlo Gombos

Comment 19 2011-01-27 19:43:49 PST

Created attachment 80404 [details] try again, this time patch generated from svn

WebKit Commit Bot

Comment 20 2011-01-27 20:26:45 PST

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

Laszlo Gombos

Comment 21 2011-01-27 21:05:20 PST

Created attachment 80415 [details] rebaseline WebKit2Common.vsprops

WebKit Commit Bot

Comment 22 2011-01-27 21:08:26 PST

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

WebKit Review Bot

Comment 23 2011-01-27 22:36:19 PST

http://trac.webkit.org/changeset/76916 might have broken SnowLeopard Intel Release (Tests)

Laszlo Gombos

Comment 24 2011-01-28 00:08:07 PST

Comment on attachment 80415 [details] rebaseline WebKit2Common.vsprops Landed manually as http://trac.webkit.org/changeset/76916.

Note You need to log in before you can comment on or make changes to this bug.