Summary: | [CMake][EFL] Build ThirdParty/leveldb when IndexedDB is enabled | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jussi Kukkonen (jku) <jussi.kukkonen> | ||||||||||||
Component: | WebKit EFL | Assignee: | Jussi Kukkonen (jku) <jussi.kukkonen> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | gyuyoung.kim, jussi.kukkonen, laszlo.gombos, lucas.de.marchi, rakuco, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | 103220, 107247 | ||||||||||||||
Bug Blocks: | 107248 | ||||||||||||||
Attachments: |
|
Description
Jussi Kukkonen (jku)
2013-01-09 04:50:35 PST
Created attachment 181901 [details]
Patch
Comment on attachment 181901 [details]
Patch
Martin ended up adding leveldb to ThirdParty/. I'll redo this patch
Created attachment 183422 [details]
Patch
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. Created attachment 183428 [details]
ImImplement changes suggested by mr Kubo
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? (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 ? Created attachment 184462 [details]
Use WEBKIT_SET_EXTRA_COMPILER_FLAGS
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. 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 ? Created attachment 184516 [details]
Add IGNORECXX_WARNINGS as Laszlo suggests
Comment on attachment 184516 [details]
Add IGNORECXX_WARNINGS as Laszlo suggests
r=me.
Comment on attachment 184516 [details]
Add IGNORECXX_WARNINGS as Laszlo suggests
Looks fine as well.
Comment on attachment 184516 [details] Add IGNORECXX_WARNINGS as Laszlo suggests Clearing flags on attachment: 184516 Committed r140804: <http://trac.webkit.org/changeset/140804> All reviewed patches have been landed. Closing bug. |