Bug 106443 – [CMake][EFL] Build ThirdParty/leveldb when IndexedDB is enabled

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Jussi Kukkonen (jku)]

We'll need to copypasta at least part of leveldb (see bug 103220), but there are also build fixes needed.

I'll attach a patch, but let's not commit it until there's a decision on 103220.

[Jussi Kukkonen (jku)]

Created attachment 181901 [details]
Patch

[Jussi Kukkonen (jku)]

Comment on attachment 181901 [details]
Patch

Martin ended up adding leveldb to ThirdParty/. I'll redo this patch

[Jussi Kukkonen (jku)]

Created attachment 183422 [details]
Patch

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 183422 [details]
Patch

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

> Source/WebCore/CMakeLists.txt:2666
> +        "${THIRDPARTY_DIR}/leveldb/"

Minor nitpick: the trailing slash is probably unneeded.

> Source/WebCore/CMakeLists.txt:3066
> +    set(LEVELDB_LIBRARY_NAME LEVELDB)

While technically OK, I think lowercasing the library name makes it more consistent (ie. having it called libleveldb.a instead of libLEVELDB.a).

> Source/WebCore/CMakeLists.txt:3069
> +    set_target_properties(${LEVELDB_LIBRARY_NAME} PROPERTIES FOLDER "WebCore")

This is probably unneeded; these days we put all generated libraries in the same directory by setting the `CMAKE_LIBRARY_OUTPUT_DIRECTORY' variable in the top-level CMakeLists.txt; the fact that we still have calls like these hanging around in other files is a relic from the past.

[Jussi Kukkonen (jku)]

Created attachment 183428 [details]
ImImplement changes suggested by mr Kubo

[Jussi Kukkonen (jku)]

Comment on attachment 183428 [details]
ImImplement changes suggested by mr Kubo

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

> Source/WebCore/CMakeLists.txt:3070
> +    set_target_properties(${LEVELDB_LIBRARY_NAME} PROPERTIES COMPILE_FLAGS "-fno-builtin-memcmp -fPIC")

The '-fPIC' I'm not totally sure about... It is needed to build with efl wk2 but that's probably because of SHARED_CORE, right? Does this need to be conditional in some way?

[Laszlo Gombos]

(In reply to comment #6)
> (From update of attachment 183428 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=183428&action=review
> 
> > Source/WebCore/CMakeLists.txt:3070
> > +    set_target_properties(${LEVELDB_LIBRARY_NAME} PROPERTIES COMPILE_FLAGS "-fno-builtin-memcmp -fPIC")
> 
> The '-fPIC' I'm not totally sure about... It is needed to build with efl wk2 but that's probably because of SHARED_CORE, right? Does this need to be conditional in some way?

Can we use the WEBKIT_SET_EXTRA_COMPILER_FLAGS macro instead to set fPIC (and whatever else is set in the macro) just like we do for the ANGLE library ?

[Jussi Kukkonen (jku)]

Created attachment 184462 [details]
Use WEBKIT_SET_EXTRA_COMPILER_FLAGS

[Jussi Kukkonen (jku)]

Latest patch uses WEBKIT_SET_EXTRA_COMPILER_FLAGS as Laszlo suggested. This automatically sets -fPIC when needed (among other things). I've dropped no-builtin-memcmp for now, let's add that if the speed really is a problem.

[Laszlo Gombos]

Comment on attachment 184462 [details]
Use WEBKIT_SET_EXTRA_COMPILER_FLAGS

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

> Source/WebCore/CMakeLists.txt:3075
> +    WEBKIT_SET_EXTRA_COMPILER_FLAGS(${LEVELDB_LIBRARY_NAME})

We should disable warnings for LevelDB as those should be addressed upstream for LevelDB and not in the WebKit tree. Can we use
WEBKIT_SET_EXTRA_COMPILER_FLAGS(${LEVELDB_LIBRARY_NAME} IGNORECXX_WARNINGS)
instead ?

[Jussi Kukkonen (jku)]

Created attachment 184516 [details]
Add IGNORECXX_WARNINGS as Laszlo suggests

[Laszlo Gombos]

Comment on attachment 184516 [details]
Add IGNORECXX_WARNINGS as Laszlo suggests

r=me.

[Gyuyoung Kim]

Comment on attachment 184516 [details]
Add IGNORECXX_WARNINGS as Laszlo suggests

Looks fine as well.

[WebKit Review Bot]

Comment on attachment 184516 [details]
Add IGNORECXX_WARNINGS as Laszlo suggests

Clearing flags on attachment: 184516

Committed r140804: <http://trac.webkit.org/changeset/140804>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.