CMake Support

[liuchang0812]

Hi, @cmumford

Does it make sense if we add cmake support to leveldb? if the answer is YES, I will try to do it.

There are some useful LLVM tools likes clang-tidy/woboq that need cmake support. We will get code format automatically, static check and online code browser if here is CMakeLists.txt.

any comments are appreciated. thx

[Netzeband]

FYI: https://github.com/google/leveldb/issues/475

[jasjuang]

CMake supported is much needed in order to integrate it into vcpkg

[cmumford]

Work on CMake support is underway.

[navidR]

@cmumford is it going to be supported on *nix platforms as well as Windows too?

Please support CMake on *nix as well as Windows.

[pwnall]

Windows support requires a separate effort, because we'll need replacements for the POSIX port and env. That being said, one of the benefits of CMake is the ability to support Windows down the line.

Sorry we have no plans to announce right now, but we are aware of the demand!

[andyleejordan]

@pwnall @cmumford I was watching the Windows Support thread, and just want to make myself available to this effort. I'm leading the port of Mesos to Windows, where we had to switch from Autotools to CMake as well, and in fact, leveldb is one of remaining requirements to bring the Mesos master component to Windows.

I've also previously patched ZooKeeper's C client library to support Windows with a new CMake build system, and for Mesos, wrote documentation covering how to maintain a good CMake build system (after I rewrote it).

Not knowing how familiar leveldb developers are with CMake, I very highly recommend reading this list of common anti-patterns and this modern guide to CMake.

Please feel free to bug me with any questions or work items you have, I'd be happy to help. I would especially like to help test the CMake build as its developed, as I'll be integrating it as a third-party dependency into my build system back at Mesos (using External_ProjectAdd).

[chfast]

Can you share the current status of this task? I can help or even do it from scratch.

[pwnall]

@chfast Thank you very much for the offer! Unfortunately, unless you're a Googler, it's highly unlikely that you can help.

I have a patch for the open-source repository here: https://github.com/pwnall/leveldb/tree/cmake

The work is blocked on integrating this changes into Google's internal source repository. Sorry, that's all I have to share right now.

[chfast]

Any estimations how long it can take?

[jasjuang]

Someone wrote a CMakeLists for leveldb on vcpkg https://github.com/Microsoft/vcpkg/blob/master/ports/leveldb/CMakeLists.txt, and it is fully working on Windows. Simply install leveldb with vcpkg install leveldb

[isaachier]

@pwnall thanks for the CMake support. I think that is good enough for me. On a related note, I wonder why you chose to redefine all of the if defined macros to if macros. CMake can do either (cmakedefine vs. cmakedefine01) and it would have less impact on the existing build system to keep as if defined.

[chfast]

The #if is better than #if defined for compile time options, because it handles a case like -DOPTION=0.

[isaachier]

@chfast good point but I think it is harder to merge with the project unless CMake and autotools can both use identical source files. As of now, @pwnall's CMake build is pretty invasive in the way it redefines macros etc.

[chfast]

I'm not saying it was worth to change it in a side branch.

[maxim-allen]

@jasjuang Thanks! But produced result - 02/08/2018 04:40 PM 25,541,932 libleveldb.lib - weighs like it has some extra stuff. It was downloading something bitcoin-related: "-- Downloading https://github.com/bitcoin-core/leveldb/archive/8b1cd3753b184341e837b30383832645135d3d73.tar.gz...". Looks like some bitcoin-oriented code that's just using leveldb. So package name leveldb for vcpkg is just misguiding, but not an actual leveldb that you would expect...

[isaachier]

@maxim-allen bitcoin maintains a windows compatible LevelDB. Google does not.

[jasjuang]

@maxim-allen You are right the leveldb on vcpkg is actually from the bitcoin fork. You might want to try modifying the portfile to install the original leveldb and see if it works.

[k15tfu]

BTW, why are some tests skipped when building as a shared library? Makefile-based builds used all of them.

https://github.com/google/leveldb/blob/b7d302326961fb809d92a95ce813e2d26fe2e16e/CMakeLists.txt#L347

[pwnall]

I think that the CMake version now builds with -fvisibility=hidden (or MSVC equivalent) whereas the previous version didn't. This makes it so that only LEVELDB_EXPORT symbols are visible in the shared library.

I want us to be testing with -fvisibility=hidden, because this config is used in Chrome for development (in the component build). We do have full testing coverage for the static library build, and the underlying C++ code is the same as for the shared library build. I think / hope we have enough tests running for the shared library to catch any issues, under the assumption that we also run the full test suite for the static library.

I hope this helps.