Bug 15406

Summary: Generated files missing from WebCore's Xcode project file
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, sam, timothy
Priority: P2    
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
Bug Depends on:    
Bug Blocks: 24380, 24381    
Attachments:
Description Flags
Patch v1
ddkilzer: review-
Patch v2 darin: review+

Description David Kilzer (:ddkilzer) 2007-10-06 15:46:54 PDT
The following shell script demonstrates files missing from the Xcode project file for WebCore:

for F in `ls WebKitBuild/Debug/DerivedSources/WebCore`; do \
    if [ -z "`grep $F WebCore/WebCore.xcodeproj/project.pbxproj`" ]; then \
        echo $F; \
    fi; \
done

Missing files:

DOMCSSStyleSheetPrivate.h
DOMEventPrivate.h
DOMHTMLCollectionPrivate.h
DOMHTMLEmbedElementPrivate.h
DOMHTMLIFrameElementPrivate.h
DOMHTMLObjectElementPrivate.h
DOMHTMLSelectElementPrivate.h
DOMSVGPointInternal.h
DOMSVGRectInternal.h
DOMTextEventInternal.h
JSHTMLInputElementBaseTable.cpp
JSSVGAnimatedPoints.cpp
JSSVGAnimatedPoints.h

NOTE: This may be worthwhile to run on the feature branch as well.
Comment 1 David Kilzer (:ddkilzer) 2007-10-06 15:48:06 PDT
(In reply to comment #0)
> JSHTMLInputElementBaseTable.cpp

Note that this file is actually used as a header file since it's a #include in WebCore/bindings/js/JSHTMLInputElementBase.cpp.

Comment 2 David Kilzer (:ddkilzer) 2007-10-06 20:49:45 PDT
Created attachment 16570 [details]
Patch v1

Proposed fix.

I removed JSSVGAnimatedPoints.h from DerivedSources.make since the generated code was not being used anyway.

I did not rename JSHTMLInputElementBaseTable.cpp to JSHTMLInputElementBaseTable.h, even though this file name would make more sense since that's the way the file is used.
Comment 3 David Kilzer (:ddkilzer) 2007-10-07 17:47:43 PDT
(In reply to comment #2)
> I removed JSSVGAnimatedPoints.h from DerivedSources.make since the generated
> code was not being used anyway.

Added Eric to CC list in case he has any comments to add.

Comment 4 Eric Seidel (no email) 2007-10-07 18:02:46 PDT
(In reply to comment #2)
> I removed JSSVGAnimatedPoints.h from DerivedSources.make since the generated
> code was not being used anyway.

Hum..
SVGAnimatedPoints is an interface which is used by SVGPolyElement (not exposed in the bindings)
which is the base class for SVGPolyLineElment and SVGPolygonElement

I'm not sure if it needs to be compiled or not, given that it's only an interface.


Comment 5 Eric Seidel (no email) 2007-10-07 18:13:07 PDT
I wonder if it's possible to make this script a commit-hook for *just* the xcode file (w/o slowing down the rest of svn commits).
Comment 6 Sam Weinig 2007-10-07 19:45:01 PDT
You need to make sure to export/migrate the objective-c headers and make sure they migrated in WebKit
Comment 7 David Kilzer (:ddkilzer) 2007-10-07 21:40:44 PDT
(In reply to comment #6)
> You need to make sure to export/migrate the objective-c headers and make sure
> they migrated in WebKit

If they weren't migrated before, why would they need to be migrated now?  (I'm not trying to be obtuse, just curious as to why they would need to be migrated.)

Comment 8 David Kilzer (:ddkilzer) 2007-10-08 13:04:37 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > You need to make sure to export/migrate the objective-c headers and make sure
> > they migrated in WebKit
> 
> If they weren't migrated before, why would they need to be migrated now?

Oh, are they considered SPI and thus should be a part of the WebKit.framework?

Comment 9 Timothy Hatcher 2007-10-08 13:43:10 PDT
DOM*Private.h is SPI and should be migrated. The other files are just internal to WebCore and should not be migrated to WebKit.
Comment 10 David Kilzer (:ddkilzer) 2007-10-08 14:17:14 PDT
Comment on attachment 16570 [details]
Patch v1

Per Comment #9, I need to make sure these headers are copied over.

Not sure if that's done in the WebCore or WebKit project file (or in a script).  Need to investigate further.
Comment 11 Timothy Hatcher 2007-10-08 14:30:42 PDT
It is done by WebKit/MigrateHeaders.make. Just add the files to the list at the top, and make sure to migrate them to the WebKit private headers directory.
Comment 12 David Kilzer (:ddkilzer) 2007-10-29 23:42:59 PDT
Missing files for r27253:

DOMCSSStyleSheetPrivate.h
DOMEventPrivate.h
DOMHTMLCollectionPrivate.h
DOMHTMLEmbedElementPrivate.h
DOMHTMLIFrameElementPrivate.h
DOMHTMLObjectElementPrivate.h
DOMHTMLSelectElementPrivate.h
DOMSVGException.mm
DOMSVGExceptionInternal.h
DOMTextEventInternal.h
JSHTMLInputElementBaseTable.cpp
JSSVGAnimatedPoints.cpp
JSSVGAnimatedPoints.h

Comment 13 David Kilzer (:ddkilzer) 2007-10-30 01:09:55 PDT
(In reply to comment #11)
> It is done by WebKit/MigrateHeaders.make. Just add the files to the list at the
> top, and make sure to migrate them to the WebKit private headers directory.

Tim, is there a rule for copying header files in the MigrateHeaders.make file?  There seem to be a lot of missing files if every generated *Private.h and *Internal.h file is supposed to be migrated this way:

$ for F in `ls WebKitBuild/Debug/DerivedSources/WebCore/*Private.h`; do \
    if [ -z "`grep $F WebKit/MigrateHeaders.make`" ]; then \
        echo $F; \
    fi; done | wc -l
      33

$ for F in `ls WebKitBuild/Debug/DerivedSources/WebCore/*Internal.h`; do \
    if [ -z "`grep $F WebKit/MigrateHeaders.make`" ]; then \
        echo $F; \
    fi; done | wc -l
     236
Comment 14 David Kilzer (:ddkilzer) 2007-10-30 01:42:53 PDT
(In reply to comment #13)
> Tim, is there a rule for copying header files in the MigrateHeaders.make file? 
> There seem to be a lot of missing files if every generated *Private.h and
> *Internal.h file is supposed to be migrated this way:
> 
> $ for F in `ls WebKitBuild/Debug/DerivedSources/WebCore/*Private.h`; do \
>     if [ -z "`grep $F WebKit/MigrateHeaders.make`" ]; then \
>         echo $F; \
>     fi; done | wc -l
>       33
> 
> $ for F in `ls WebKitBuild/Debug/DerivedSources/WebCore/*Internal.h`; do \
>     if [ -z "`grep $F WebKit/MigrateHeaders.make`" ]; then \
>         echo $F; \
>     fi; done | wc -l
>      236

Oops!  Those weren't correct.  These are:

$ for F in `ls WebKitBuild/Debug/DerivedSources/WebCore/ | grep 'Private.h$'`; do \
    if [ -z "`grep $F WebKit/MigrateHeaders.make`" ]; then \
        echo $F; \
    fi; done | wc -l
       0

$ for F in `ls WebKitBuild/Debug/DerivedSources/WebCore/ | grep 'Internal.h$'`; do \
    if [ -z "`grep $F WebKit/MigrateHeaders.make`" ]; then \
        echo $F; \
    fi; done | wc -l
     110

Comment 15 Timothy Hatcher 2007-10-30 03:02:13 PDT
There is isn't a hard/fast rule. Only headers that are needed as public and private OS X headers need migrated. Internal headers should not be migrated.
Comment 16 David Kilzer (:ddkilzer) 2007-10-30 16:41:57 PDT
Created attachment 16951 [details]
Patch v2

Changes since Patch v1:

- Removed DOMSVGException.h in addition to JSSVGAnimatedPoints.h from WebCore/DerivedSources.make.
- Don't add DOMSVGPointInternal.h and DOMSVGRectInternal.h to the project file since those were already fixed.
- Added *Private.h files to the Copy Generated Files Build Phase in WebCore.
- Modified WebKit/MigrateHeaders.make to add the same *Private.h files that were added to the Copy Generated Headers Build Phase in WebCore.

Note that I did a clean build (wiped WebKitBuild dir) of r27257 and everything built fine.
Comment 17 Darin Adler 2007-10-30 16:42:49 PDT
Comment on attachment 16951 [details]
Patch v2

r=me
Comment 18 David Kilzer (:ddkilzer) 2007-10-30 18:20:07 PDT
Committed r27278