Bug 135662 – cmake is coming along nicely on mac

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Alex Christensen]

WebCore builds most of the way.  There are some objc quirks still, but this is a lot of progress.

[Alex Christensen]

Created attachment 236120 [details]
Patch

[Alex Christensen]

Created attachment 236121 [details]
Patch

[Laszlo Gombos]

Comment on attachment 236121 [details]
Patch

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

> ChangeLog:13
> +        * Source/cmake/WebKitFeatures.cmake:

Sadly we should also list all the new flags to cmakeconfig.h.cmake for consistency.

[Alex Christensen]

Created attachment 236127 [details]
Patch

[Alex Christensen]

(In reply to comment #3)
> Sadly we should also list all the new flags to cmakeconfig.h.cmake for consistency.
Thanks.  That explains a few quirks I was running into.  Those two files are not as consistent as you might think, but I made my same changes to both.

[Csaba Osztrogonác]

Comment on attachment 236127 [details]
Patch

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

Just out of curiosity: Is there a plan to migrate the Apple Mac port to cmake?

> Source/WebCore/CMakeLists.txt:3595
> -    target_include_directories(ANGLESupport PRIVATE "${THIRDPARTY_DIR}/ANGLE/include")
> +    include_directories(ANGLESupport PRIVATE "${THIRDPARTY_DIR}/ANGLE/include")

There was a reason why target_include_directories is used and 
not include_directories. Please check was 171475 for details.
And that's why minimum cmake version was bumped to 2.8.11

[Alex Christensen]

(In reply to comment #6)
> (From update of attachment 236127 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=236127&action=review
> 
> Just out of curiosity: Is there a plan to migrate the Apple Mac port to cmake?
I'm working on a proof-of-concept right now.  I'm definitely not saying it will be immediately accepted by everyone at Apple.
> 
> > Source/WebCore/CMakeLists.txt:3595
> > -    target_include_directories(ANGLESupport PRIVATE "${THIRDPARTY_DIR}/ANGLE/include")
> > +    include_directories(ANGLESupport PRIVATE "${THIRDPARTY_DIR}/ANGLE/include")
> 
> There was a reason why target_include_directories is used and 
> not include_directories. Please check was 171475 for details.
> And that's why minimum cmake version was bumped to 2.8.11
I'll remove this change and we'll worry about this in a later patch.

[Alex Christensen]

Created attachment 236131 [details]
Patch

[Alex Christensen]

Created attachment 236202 [details]
Patch

[Alex Christensen]

Created attachment 236207 [details]
Patch

[Alex Christensen]

Created attachment 236208 [details]
Patch

[Mark Rowe (bdash)]

Comment on attachment 236208 [details]
Patch

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

> Source/WebCore/config.h:64
> +// Using CMake with Unix makefiles does not use precompiled headers.

Presumably the problem here is the lack of prefix headers. Whether they're precompiled or not doesn't seem relevant.

[Alex Christensen]

(In reply to comment #12)
> (From update of attachment 236208 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=236208&action=review
> 
> > Source/WebCore/config.h:64
> > +// Using CMake with Unix makefiles does not use precompiled headers.
> 
> Presumably the problem here is the lack of prefix headers. Whether they're precompiled or not doesn't seem relevant.
Yes, that's exactly it.

I can't figure out why GTK and EFL don't like this patch.  I don't think the GTK failure is my fault, and I think the VTT stuff on EFL shouldn't have been changed.

[Laszlo Gombos]

Comment on attachment 236208 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        Moved ImageSource.cpp and image-decoders to platform CMake files because they are not used on mac.

Hmm, but they used everywhere else but the mac. Can we somehow make them conditional instead (either with a CMake variable or in the cpp files with a PLATFORM()/OS() check ) ?

[Alex Christensen]

(In reply to comment #14)
> (From update of attachment 236208 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=236208&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        Moved ImageSource.cpp and image-decoders to platform CMake files because they are not used on mac.
> 
> Hmm, but they used everywhere else but the mac. Can we somehow make them conditional instead (either with a CMake variable or in the cpp files with a PLATFORM()/OS() check ) ?

They're not used in AppleWin either.  Just WinCairo, EFL, and GTK.  I think they should go in those platform files.

[Alex Christensen]

I don't think I trust the EWS bots with this patch.

[Laszlo Gombos]

(In reply to comment #16)
> I don't think I trust the EWS bots with this patch.

Seems the patch does apply on the win bot either - https://webkit-queues.appspot.com/results/5588216753160192

Perhaps because http://trac.webkit.org/changeset/172259 just landed ?

[Alex Christensen]

Created attachment 236229 [details]
Patch

[Alex Christensen]

Created attachment 236282 [details]
Patch

[Alex Christensen]

I'm pretty sure the EFL failure is not the fault of this patch.

[Laszlo Gombos]

Comment on attachment 236282 [details]
Patch

LGTM

[Alex Christensen]

http://trac.webkit.org/changeset/172346