[Webkit-unassigned] [Bug 37945] CMake buildsystem

[bugzilla-daemon at webkit.org]

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





--- Comment #38 from Patrick R. Gansterer <paroga at paroga.com>  2010-05-14 12:46:44 PST ---
> +CMAKE_MINIMUM_REQUIRED(VERSION 2.8)
Do we realy need 2.8?


> +FIND_PACKAGE(ICU)
> +FIND_PACKAGE(Threads)
If we move them into the plaform files, they have to be removed in the root CMakeLists.txt


> +INSTALL(TARGETS ${WebCore_LIBRARY_NAME} DESTINATION lib)
I would remove the INSTALL calls in the moment.


> +ADD_TARGET_PROPERTIES(${WebKit_LIBRARY_NAME} LINK_FLAGS ${WebKit_LINK_FLAGS})
Do you realy need the linker flags here? I don't like the idea of per target linker flags. If EFL needs special linker flags they should be declared in the OptionsEfl.cmake with CMAKE_SHARED_LINKER_FLAGS/CMAKE_EXE_LINKER_FLAGS.


> +IF (NOT ICU_FOUND)
> +    MESSAGE(FATAL_ERROR "ICU is required for EFL port")
> +ENDIF (NOT ICU_FOUND)
Should become FIND_PACKAGE(ICU REQUIRED) here.


> +SET(JSC_EXECUTABLE_NAME jsc)
> +SET(WTF_LIBRARY_NAME wtf)
> +SET(JavaScriptCore_LIBRARY_NAME javascriptcore)
> +SET(WebCore_LIBRARY_NAME webcore)
> +SET(WebKit_LIBRARY_NAME ewebkit)
> +
> +SET(WTF_LIBRARY_TYPE STATIC)
> +SET(JavaScriptCore_LIBRARY_TYPE STATIC)
> +SET(WebCore_LIBRARY_TYPE STATIC)
> +SET(WebKit_LIBRARY_TYPE SHARED)
I think we should declare them in the root CMakeLists.txt and overrided them only in the port files.


Can you please remove the unnecessary FindXXX files. Many of them will be installed by CMake already. If we want do depend on CMake 2.8 we can remove even more.

What about coding style?
I would prefere space only between IF, ENDIF, FOREACH and ENDFOREACH. The END should have empty braces. All other keyworks should have no space between (SET, LIST, ...). Additionaly we should use a 4 space intention like the other WebKit code does.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.