Bug 175705

Summary: [PAL] Headers should be grouped into a single directory
Product: WebKit Reporter: Don Olmstead <don.olmstead>
Component: PlatformAssignee: Don Olmstead <don.olmstead>
Status: NEW ---    
Severity: Normal CC: beidson, don.olmstead, emw, ews-watchlist, mcatanzaro, mmaxfield, yoshiaki.jitsukawa
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP Patch none

Description Don Olmstead 2017-08-17 20:12:45 PDT
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)
Comment 1 Don Olmstead 2018-02-08 18:40:26 PST
Created attachment 333437 [details]
WIP Patch

Expecting Xcode to fail. The CMake ports SHOULD be fine.
Comment 2 EWS Watchlist 2018-02-08 18:43:04 PST
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.
Comment 3 Don Olmstead 2018-11-12 10:55:30 PST
Myles could we just use a makefile in XCode to copy the headers into a single directory?
Comment 4 Myles C. Maxfield 2018-11-12 16:35:02 PST
(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
Comment 5 Elliott Williams 2021-12-20 17:06:52 PST
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.
Comment 6 Myles C. Maxfield 2021-12-20 18:54:25 PST
> In modern Xcodes, the "right thing" to do might be to repackage PAL as a static framework

Cool!
Comment 7 Elliott Williams 2022-01-11 12:29:41 PST
(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…)