Bug 202384 – Resurrect Mac CMake build

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Resurrect Mac CMake build

[Alex Christensen]

Created attachment 379864 [details]
Patch

[Don Olmstead]

Comment on attachment 379864 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=379864&action=review

> Source/JavaScriptCore/PlatformMac.cmake:26
> +    API/JSCallbackFunction.h
>      API/JSContext.h
> +    API/JSContextPrivate.h
>      API/JSExport.h
>      API/JSManagedValue.h
>      API/JSStringRefCF.h
>      API/JSValue.h
> +    API/JSValuePrivate.h

This shouldn't be in public framework headers. This should just be stuff included by JavaScriptCore.h.

Is there a reason you added these? They should all be in the private headers definition below.

> Source/WebKitLegacy/PlatformMac.cmake:10
>      "${WEBKITLEGACY_DIR}/mac"
> -    "${WEBKITLEGACY_DIR}/mac/Carbon"
> -    "${WEBKITLEGACY_DIR}/mac/DefaultDelegates"
> -    "${WEBKITLEGACY_DIR}/mac/DOM"
> -    "${WEBKITLEGACY_DIR}/mac/History"
> -    "${WEBKITLEGACY_DIR}/mac/icu"
> -    "${WEBKITLEGACY_DIR}/mac/Misc"
> -    "${WEBKITLEGACY_DIR}/mac/Panels"
> -    "${WEBKITLEGACY_DIR}/mac/Plugins"
> -    "${WEBKITLEGACY_DIR}/mac/Plugins/Hosted"
> -    "${WEBKITLEGACY_DIR}/mac/Storage"
> -    "${WEBKITLEGACY_DIR}/mac/WebCoreSupport"
> -    "${WEBKITLEGACY_DIR}/mac/WebInspector"
> -    "${WEBKITLEGACY_DIR}/mac/WebView"
> +    "${DERIVED_SOURCES_DIR}/ForwardingHeaders"
> +    "${DERIVED_SOURCES_DIR}/ForwardingHeaders/WebCore"
> +    "${DERIVED_SOURCES_DIR}/ForwardingHeaders/WebKitLegacy"

This isn't right this should only have additional mac related directories in here which you've deleted.

The root CMakeLists.txt has "${WebCore_PRIVATE_FRAMEWORK_HEADERS_DIR}". If something is in WebCore and its being included through "Foo.h" rather than <WebCore/Foo.h> that's wrong. The forwarding headers for WebKitLegacy aren't meant to be included either they're just copies for the next project. I think you copying here is why you got this built but you removed the includes for the individual directories above.

[Alex Christensen]

(In reply to Don Olmstead from comment #2)
> Comment on attachment 379864 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=379864&action=review
> 
> > Source/JavaScriptCore/PlatformMac.cmake:26
> > +    API/JSCallbackFunction.h
> >      API/JSContext.h
> > +    API/JSContextPrivate.h
> >      API/JSExport.h
> >      API/JSManagedValue.h
> >      API/JSStringRefCF.h
> >      API/JSValue.h
> > +    API/JSValuePrivate.h
> 
> This shouldn't be in public framework headers. This should just be stuff
> included by JavaScriptCore.h.
This is necessary to get it to build for now.  We can iron out the details once engineers are actually using it.
> 
> Is there a reason you added these? They should all be in the private headers
> definition below.
> 
> > Source/WebKitLegacy/PlatformMac.cmake:10
> >      "${WEBKITLEGACY_DIR}/mac"
> > -    "${WEBKITLEGACY_DIR}/mac/Carbon"
> > -    "${WEBKITLEGACY_DIR}/mac/DefaultDelegates"
> > -    "${WEBKITLEGACY_DIR}/mac/DOM"
> > -    "${WEBKITLEGACY_DIR}/mac/History"
> > -    "${WEBKITLEGACY_DIR}/mac/icu"
> > -    "${WEBKITLEGACY_DIR}/mac/Misc"
> > -    "${WEBKITLEGACY_DIR}/mac/Panels"
> > -    "${WEBKITLEGACY_DIR}/mac/Plugins"
> > -    "${WEBKITLEGACY_DIR}/mac/Plugins/Hosted"
> > -    "${WEBKITLEGACY_DIR}/mac/Storage"
> > -    "${WEBKITLEGACY_DIR}/mac/WebCoreSupport"
> > -    "${WEBKITLEGACY_DIR}/mac/WebInspector"
> > -    "${WEBKITLEGACY_DIR}/mac/WebView"
> > +    "${DERIVED_SOURCES_DIR}/ForwardingHeaders"
> > +    "${DERIVED_SOURCES_DIR}/ForwardingHeaders/WebCore"
> > +    "${DERIVED_SOURCES_DIR}/ForwardingHeaders/WebKitLegacy"
> 
> This isn't right this should only have additional mac related directories in
> here which you've deleted.
> 
> The root CMakeLists.txt has "${WebCore_PRIVATE_FRAMEWORK_HEADERS_DIR}". If
> something is in WebCore and its being included through "Foo.h" rather than
> <WebCore/Foo.h> that's wrong. The forwarding headers for WebKitLegacy aren't
> meant to be included either they're just copies for the next project. I
> think you copying here is why you got this built but you removed the
> includes for the individual directories above.

This is unfortunately the cleanest way to get WebKitLegacy to build.  It has lots of ObjC headers that are included with #import <WebKitLegacy/Header.h> and #import "Header.h" and it was determined that this was cleaner than making them consistent, which we do not intend to do.

[Alex Christensen]

I confirmed that moving the headers to the list of private headers makes it fail to build jsc.  We'll fix the publicness of headers later.
http://trac.webkit.org/r250550

[Radar WebKit Bug Importer]

<rdar://problem/55868125>

[Konstantin Tokarev]

Will we have buildbot for Mac/cmake? Otherwise it will bitrot quickly

[Alex Christensen]

fixing...