search

adamfowleruk / groundupdb

Creating a database from the ground up in C++ for fun!
Apache License 2.0
109 stars 25 forks source link

Fixes #9. CMake working out-of-the-box on Debian Buster (stable) #10

Closed phlash closed 3 years ago

phlash commented 3 years ago

A number of environmental and tooling-related fixes to get GroundupDB building out-of-the-box on Debian Buster (stable):

1/ Reference to Google highwayhash as a git sub-module 2/ CMakeLists.txt updates to use above sub-module, rather than assume a peer repo 3/ CMakeLists.txt updates to fix missing libstdc++fs on GCC8 (default on Buster): https://en.cppreference.com/w/cpp/filesystem 4/ Minor include fixes (also probably due to earlier GCC)

Currently fails first two tests around file system checks, not sure why... all actual database I/O tests pass.

adamfowleruk commented 3 years ago

I found a couple of issues with this PR. Firstly, on initial clone you need to issue the following and not just git submodule update:-

git submodule update --init --recursive

Also on Mac running your modified cmake with -lstdc++fs results in:-

...
[ 88%] Building CXX object groundupdb-tests/CMakeFiles/groundupdb-tests.dir/query-tests.cpp.o
[ 94%] Linking CXX executable groundupdb-cli
ld: library not found for -lstdc++fs
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [groundupdb-cli/groundupdb-cli] Error 1
make[1]: *** [groundupdb-cli/CMakeFiles/groundupdb-cli.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
[100%] Linking CXX executable groundupdb-tests
ld: library not found for -lstdc++fs
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [groundupdb-tests/groundupdb-tests] Error 1
make[1]: *** [groundupdb-tests/CMakeFiles/groundupdb-tests.dir/all] Error 2
make: *** [all] Error 2

I think this is because the std c++ library on mac has the C++17 fs library built in. Did you find you had to include this even though the C++17 standard was specified in the cmake file config?

Thanks for the PR though. Great work. Please have a quick look at the above and advise. I can happily test it but I don't want to break your build for Debian by fixing mine on a mac!

phlash commented 3 years ago

Gah - we are victim of older GCC vs newer Clang, guess I'll have to learn how to get CMake to adapt to the tooling it finds...

Thanks for the obvious git submodule options - dunno how I missed those!

phlash commented 3 years ago

OK, from here: https://github.com/OpenRCT2/OpenRCT2/pull/10522 it looks like all non-Apple targets need this library, so that's now a dependency of the core groundupdb unless building for Apple. This dependency gets automatically added when building binaries. Please check it works for you!

adamfowleruk commented 3 years ago

I'm going to take another look at this tomorrow. Apologies for the delay. FYI I've also got CMake working on Windows with Visual Studio Code and the standard MSVCC compiler. I'll test on a Linux box tomorrow. Thanks for the PR!

phlash commented 3 years ago

Just merged your latest updates and fixed a couple of minor header differences with GCC/clang

adamfowleruk commented 3 years ago

Awesome thanks. I'm just downloading a Linux ISO so I can test this today.