Bug 175705 – [PAL] Headers should be grouped into a single directory

• Status: NEW
• Component: Platform

[Don Olmstead]

PAL currently requires includes to use subdirectories. This is against the conventions of WebKit. All headers should be placed in a single directory for consumption by downstream projects.

Outside of PAL includes should be <pal/xxx.h> not <pal/yyy/xxx.h>
Inside of PAL includes should be "xxx.h" (If possible a style check should validate this)

[Don Olmstead]

Created attachment 333437 [details]
WIP Patch

Expecting Xcode to fail. The CMake ports SHOULD be fine.

[EWS Watchlist]

Attachment 333437 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm:45:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:55:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:57:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:27:  Found WebCore config.h after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDevice.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:61:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.cpp:41:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/AVFoundationMIMETypeCache.mm:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.mm:53:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/ios/PlaybackSessionInterfaceAVKit.mm:44:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDeviceManager.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/Modules/plugins/QuickTimePluginReplacement.mm:59:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/AVAssetTrackUtilities.mm:36:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebKit/Platform/ios/AccessibilityIOS.mm:27:  Found WebCore config.h after a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/PAL/pal/PlatformMac.cmake:83:  There should be exactly one empty line instead of 0 between "spi/mac/TUCallSPI.h" and "spi/win/CoreTextSPIWin.h".  [list/emptyline] [5]
ERROR: Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:47:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVMediaCaptureSource.mm:44:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:36:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.cpp:39:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/RealtimeOutgoingVideoSourceCocoa.cpp:37:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/RealtimeIncomingAudioSourceCocoa.cpp:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:66:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/audio/mac/AudioSampleDataSource.mm:43:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mac/VideoFullscreenInterfaceMac.mm:43:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mac/PlaybackSessionInterfaceMac.mm:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:57:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/ios/WebAVPlayerController.mm:41:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:155:  *SoftLink.h header should be included after all other headers.  [build/include_order] [4]
Total errors found: 28 in 378 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Don Olmstead]

Myles could we just use a makefile in XCode to copy the headers into a single directory?

[Myles C. Maxfield]

(In reply to Don Olmstead from comment #3)
> Myles could we just use a makefile in XCode to copy the headers into a
> single directory?

I don't think a makefile is necessary. We can probably just do something like https://stackoverflow.com/questions/24613009/rsync-make-flat-copy

[Elliott Williams]

I was just CC'd on this, as I recently started at Apple and am taking a fresh look at our build system integrations. *dusts off bug*

> Myles could we just use a makefile in XCode to copy the headers into a single directory?

In modern Xcodes, the "right thing" to do might be to repackage PAL as a static framework, which would replace the rsync script we currently use and allow us to use Xcode's built-in header copy logic, which aligns with WebKit's conventions and copies everything to a top-level library directory.

Doing that in tandem with an updated version of this patch ought to solve this.

[Myles C. Maxfield]

> In modern Xcodes, the "right thing" to do might be to repackage PAL as a static framework

Cool!

[Elliott Williams]

(In reply to Elliott Williams from comment #5)
> In modern Xcodes, the "right thing" to do might be to repackage PAL as a
> static framework

It turns out that Apple forbids static frameworks inside of SDKs, and PAL needs to be included in SDK under some circumstances :(

That said, I think it would be possible to use an Xcode-standard "Copy Files" phase to copy everything into $(DSTROOT)/usr/local/include/pal.

If not, we could do what WTF's Xcode project does and script this with rsync (or maybe, we should change what WTF does, too…)