Bug 162493

Summary: Fix Mac CMake build after r206261
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cgarcia, commit-queue, dbates, jeremyhu, lforschler, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Jonathan Bedard 2016-09-23 09:32:03 PDT
Build fix for Mac CMake.
Comment 1 Jonathan Bedard 2016-09-23 09:36:01 PDT
Created attachment 289685 [details]
Patch
Comment 2 Jonathan Bedard 2016-09-23 09:37:34 PDT
Still in the process of testing this patch locally.  Unless others find this fix urgent and indicate so, I will not commit it until local testing confirms it's working.
Comment 3 Jonathan Bedard 2016-09-23 11:39:50 PDT
Locally confirmed that this patch allows Mac CMake to build.
Comment 4 Daniel Bates 2016-09-23 13:17:02 PDT
Comment on attachment 289685 [details]
Patch

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

> Source/WebKit2/CMakeLists.txt:664
> +if (NOT APPLE)

Is this the preferred way to add files to non-Apple ports? I thought we prefer to keep the purpose of this file for platform-independent files and put platform-specific files in the appropriate Platform*.cmake files.

> Source/WebKit2/CMakeLists.txt:666
> +    set(WebKit2_SOURCES
> +        ${WebKit2_SOURCES}

Notice that list(APPEND ...) can be used to append elements to the end of a list [1]. I suggest that we make use of this function to append WebTextChecker.cpp and WebTextCheckerClient.cpp to the end of the list.

[1] <https://cmake.org/cmake/help/v3.0/command/list.html>
Comment 5 Alex Christensen 2016-09-23 13:19:51 PDT
Comment on attachment 289685 [details]
Patch

Yes, these files should just be moved to PlatformGTK.cmake and PlatformEFL.cmake.
Comment 6 Jonathan Bedard 2016-09-23 13:48:38 PDT
Created attachment 289705 [details]
Patch
Comment 7 WebKit Commit Bot 2016-09-23 14:22:36 PDT
Comment on attachment 289705 [details]
Patch

Clearing flags on attachment: 289705

Committed r206331: <http://trac.webkit.org/changeset/206331>
Comment 8 WebKit Commit Bot 2016-09-23 14:22:40 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Jeremy Huddleston Sequoia 2016-10-12 11:35:29 PDT
So that addresses the problem on !APPLE platforms, but the issue still remains on darwin because of the incorrect preprocessor checks.

I filed a ticket to track that followup in:

https://bugs.webkit.org/show_bug.cgi?id=163346