Bug 147083 – resurrect CMake build on Windows

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

[Alex Christensen]

Updated and added missing PlatformWin.cmake files.  Based loosely on Mark and Patrick's work in https://bugs.webkit.org/show_bug.cgi?id=72816

[Alex Christensen]

Created attachment 257055 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 257055 [details]
Patch

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

> Source/WebKit/PlatformWin.cmake:211
> +    win/WebCoreSupport/EmbeddedWidget.h

Question. should we include header files to Source file list ?

[Alex Christensen]

(In reply to comment #2)
> Question. should we include header files to Source file list ?
They're not needed to build correctly, but Visual Studio will only search in files included in the generated projects. If they're not included in other CMake files, then it doesn't make any sense to include them here. I don't care either way right now

[Gyuyoung Kim]

Comment on attachment 257055 [details]
Patch

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

LGTM on cmake side except for some nits. However this patch modifies some sources though, I think it is trivial changes. rs=me for win port.

> Source/WebKit/PlatformWin.cmake:245
> +list(APPEND WebKit_SOURCES_WebCoreSupport

I don't know why below files can't be added to 206 lines.

> Source/WebKit/PlatformWin.cmake:425
> +set(WebKit_LIBRARY_TYPE SHARED)

WebKit_LIBRARY_TYPE is defined in CMakeLists.txt. So it looks this is duplicated definition. 

http://trac.webkit.org/browser/trunk/CMakeLists.txt#L143

[Alex Christensen]

Addressed nits and committed http://trac.webkit.org/changeset/187022
Thanks for the review!

[Alex Christensen]

Build fix in http://trac.webkit.org/changeset/187037
Sorry about that :(

[Csaba Osztrogonác]

Comment on attachment 257055 [details]
Patch

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

> Source/WebCore/CMakeLists.txt:3516
> +list(APPEND WebCore_SOURCES ${DERIVED_SOURCES_WEBCORE_DIR}/EventTargetInterfaces.h)

Why do we add a header to the source list? Do we really need it for the Windows build?

[Alex Christensen]

(In reply to comment #7)
> Comment on attachment 257055 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=257055&action=review
> 
> > Source/WebCore/CMakeLists.txt:3516
> > +list(APPEND WebCore_SOURCES ${DERIVED_SOURCES_WEBCORE_DIR}/EventTargetInterfaces.h)
> 
> Why do we add a header to the source list? Do we really need it for the
> Windows build?
Yes, it wasn't generating it unless I added this line. Removing this line would probably only break clean builds. Is it a problem?