Bug 17851 - Should be able to build WebCore without prefix header injection
Summary: Should be able to build WebCore without prefix header injection
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Mark Mentovai
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-14 11:50 PDT by Mark Mentovai
Modified: 2010-06-10 18:48 PDT (History)
3 users (show)

See Also:


Attachments
A list of what needs to be done and how to do it (9.13 KB, text/plain)
2008-03-14 11:59 PDT, Mark Mentovai
no flags Details
Include headers and provide forward declarations as needed (38.59 KB, patch)
2008-03-14 12:08 PDT, Mark Mentovai
darin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Mentovai 2008-03-14 11:50:13 PDT
It should be possible to build WebCore without prefix header injection.  WebCorePrefix.h even states:

> * The project should be able to build without this header, although we rarely test that.

Currently, on the Mac, the WebCore build depends on prefix header injection.  Source files that depend on declarations being provided by the prefix header should be modified to include the declarations they depend on themselves.  This doesn't need to be the default build environment, but it should be available as an option.

To provide ongoing testing to ensure that the source is buildable without prefix header injection, it might be a good idea to provide a BuildBot that tests with an empty prefix header, or with GCC_PREFIX_HEADER set to empty.
Comment 1 Mark Mentovai 2008-03-14 11:59:16 PDT
Created attachment 19765 [details]
A list of what needs to be done and how to do it

These are the notes I took as I fixed this, in case anyone's curious about why things were done how they were.
Comment 2 Mark Mentovai 2008-03-14 12:08:26 PDT
Created attachment 19766 [details]
Include headers and provide forward declarations as needed

Attachment 19765 [details] contains a description of these changes.

If bug 17833 lands before this, the SharedTimerMac.cpp diff in this patch should be applied to SharedTimerMac.mm instead.
Comment 3 Darin Adler 2008-03-14 13:15:04 PDT
Comment on attachment 19766 [details]
Include headers and provide forward declarations as needed

This is not a good change. There's no practical way to keep all these things compiling without a prefix header, and no real benefit either.

I've been moving the code in the opposite direction, removing includes added for only theoretical reasons.

Is there any real world advantage to this?
Comment 4 Mark Mentovai 2008-03-14 13:17:02 PDT
Comment on attachment 19766 [details]
Include headers and provide forward declarations as needed

Really?  There's no reason not to support this.  Projects should be able to work with WebKit without depending on the in-WebKit build machinery.  It's a pretty big imposition to ask that this be maintained as a local patch in such cases.
Comment 5 Mark Rowe (bdash) 2008-03-14 18:19:39 PDT
In what manner does the presence of a prefix header prevent any other project from working with WebKit?
Comment 6 Mark Mentovai 2008-03-14 20:52:27 PDT
It's not a matter of other projects working with WebKit, it's a matter of integrating the WebKit source into other build systems.
Comment 7 Mark Mentovai 2008-04-02 09:46:00 PDT
I'm going to make one last plea to fix this bug in the way proposed in attachment 19766 [details].  Specifically, prefix header injection interferes with the way that our build handles dependency scanning.  It doesn't seem that any platform other than the Mac is being required to use prefix header injection; I don't understand the disparity.
Comment 8 David Kilzer (:ddkilzer) 2009-08-15 17:00:36 PDT
(In reply to comment #3)
> (From update of attachment 19766 [details])
> This is not a good change. There's no practical way to keep all these things
> compiling without a prefix header, and no real benefit either.

You could create a buildbot that simply compiles the project without the prefix header (or without the statement that does #import <Cocoa/Cocoa.h>).

> I've been moving the code in the opposite direction, removing includes added
> for only theoretical reasons.
> 
> Is there any real world advantage to this?

If this isn't the direction the project is moving in, then this comment in WebCorePrefix.h should be revised or removed:  <http://trac.webkit.org/browser/trunk/WebCore/WebCorePrefix.h#L21>

(In reply to comment #7)
> I'm going to make one last plea to fix this bug in the way proposed in
> attachment 19766 [details].  Specifically, prefix header injection interferes with the
> way that our build handles dependency scanning.  It doesn't seem that any
> platform other than the Mac is being required to use prefix header injection; I
> don't understand the disparity.

Apparently the Qt and Wx ports are using a prefix header based on code in WebCorePrefix.h.  Is this still an issue for Chromium builds?
Comment 9 David Kilzer (:ddkilzer) 2009-08-15 17:06:33 PDT
(In reply to comment #8)
> If this isn't the direction the project is moving in, then this comment in
> WebCorePrefix.h should be revised or removed: 
> <http://trac.webkit.org/browser/trunk/WebCore/WebCorePrefix.h#L21>

That comment was added in r12121:  <http://trac.webkit.org/changeset/12121>
Comment 10 Mark Mentovai 2009-08-17 13:02:17 PDT
I don't strictly require this for Chromium any longer.  The prefix header does not result in a performance advantage for us because we can't use precompiled headers in our build farms.  Our Mac WebKit build would still be slightly faster without prefix header injection.