Bug 147325 – Progress towards building AppleWin with CMake

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

[Alex Christensen]

Almost there.  It would be nice to upstream this then polish of the last few quirks.

[Alex Christensen]

Created attachment 257568 [details]
Patch

[Martin Robinson]

Comment on attachment 257568 [details]
Patch

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

Looks pretty reasonable to me, but I have a question about the new platform-independent include path.

> Source/JavaScriptCore/CMakeLists.txt:36
>      "${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/inspector"
> +    "${CMAKE_BINARY_DIR}/Source/JavaScriptCore"

What files are generated here? It might be more appropriate to generate them in another directory or to move this include to the platform-specific CMake file. I ask because the GTK+ port uses Ninja and this include is not necessary for it.

> Source/WebCore/PlatformAppleWin.cmake:12
> +)
> +list(APPEND WebCore_SOURCES

Style nit. I think there is usually an empty line of whitespace between definitions.

[Alex Christensen]

(In reply to comment #2)
> Comment on attachment 257568 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=257568&action=review
> 
> Looks pretty reasonable to me, but I have a question about the new
> platform-independent include path.
> 
> > Source/JavaScriptCore/CMakeLists.txt:36
> >      "${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/inspector"
> > +    "${CMAKE_BINARY_DIR}/Source/JavaScriptCore"
> 
> What files are generated here? It might be more appropriate to generate them
> in another directory or to move this include to the platform-specific CMake
> file. I ask because the GTK+ port uses Ninja and this include is not
> necessary for it.
It was something that's #included in LowLevelInterpreterWin.asm.  I should probably just generate that into a different directory somehow. r=you without this change and the style nit?

[Alex Christensen]

Created attachment 257597 [details]
Patch

[Martin Robinson]

Comment on attachment 257597 [details]
Patch

Thanks!

[Alex Christensen]

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