Closed skaravos closed 1 year ago
To me this looks like a great initiative. From what I can see it looks good. But I must say I am not that good with cmake and other build systems.
I usually type cmake ..
and hope it works. :)
Maybe @emilk should also take a look.
If I may chime in - I was interested in using loguru from CMake, thus found this PR and made a local FreeBSD package based on it. Some feedback:
LOGURU_STACKTRACES
.loguru.cpp
is not installed. Consumers need it if they want different compile time settings.loguru::loguru-header-only
target, similar to fmt::fmt-header-only
. Basically just the include paths, without linker flags.Hope this helps.
I am also interested in cmake build support for Loguru. I am involved in maintenance of dosbox-staging package in Fedora. Currently, DOSBox Staging bundles a modified copy of Loguru. I would like to import Loguru in Fedora as a shared library and switch Fedora's dosbox-staging to use that. This is the Fedora preference: link to libraries as shared objects.
I successfully built a shared object with the following command:
cmake -S. -Bbuild -DCMAKE_INSTALL_PREFIX=test-install -DLOGURU_WITH_STREAMS=TRUE -DBUILD_SHARED_LIBS=TRUE
cmake --build build --target install
One missing feature is pkgconfig definition. DOSBox Staging's build system is Meson, which uses pkgconfig to find shared libraries. Before discovering this pull request, I wrote a simple CMake script for Loguru, which contains the pkgconfig part. See 17f616ca in my fork for that part.
Would you accept the pkgconfig in this pull request, so that the shared object would be importable from Meson also?
EDIT: Note that my branch is just work in progress. Some solution for defining library path, version number and so on is still needed. For example, see how another DOSBox Staging dependency, mt32emu, does it.
BTW: the Meson WrapDB version of loguru automatically generates a pkg-config file using Meson's pkgconfig generator: https://github.com/mesonbuild/wrapdb/blob/master/subprojects/packagefiles/emilk-loguru/meson.build
The resulting pkg-config file has Libs.private: -pthread -ldl
, to go with the dependencies on those libraries (dl only if stacktraces are configured).
BTW: the Meson WrapDB version of loguru automatically generates a pkg-config file using Meson's pkgconfig generator: https://github.com/mesonbuild/wrapdb/blob/master/subprojects/packagefiles/emilk-loguru/meson.build
The resulting pkg-config file has
Libs.private: -pthread -ldl
, to go with the dependencies on those libraries (dl only if stacktraces are configured).
Thanks, I have to take a closer look at the Meson wrap for Loguru. I did notice it exists, but DOSBox Staging does not use it for some reason, despite using wraps for some other dependencies.
Maybe @kcgen knows the answer to that.
Maybe @kcgen knows the answer to that.
When using Loguru under DOSBox Staging, it raised many static analysis issues as well as not compiling under Haiku OS, which otherwise is able to compile all our other dependencies (using Meson).
Both of these issues were PR'd into this repo, https://github.com/emilk/loguru/pulls/kcgen, but have sat for roughly a year now, so in the meantime we've been using our own fork of Loguru as a stopgap until these are merged.
We also strive to have zero warnings (at meson level 3), and so those PRs also cleanup some of those too.
Hey guys, thanks for the feedback!
@oturpe, great idea for pkgconfig support and thanks for the reference implementation.
My latest commits to this PR branch add support for generation of a loguru.pc
file.
However, I've never actually used pkg-config and I have no way of confirming that my implementation is solid.
If someone could try out my newest changes and see if the generated loguru.pc
file works for them, that would be awesome.
BTW: the Meson WrapDB version of loguru automatically generates a pkg-config file using Meson's pkgconfig generator: https://github.com/mesonbuild/wrapdb/blob/master/subprojects/packagefiles/emilk-loguru/meson.build
The resulting pkg-config file has
Libs.private: -pthread -ldl
, to go with the dependencies on those libraries (dl only if stacktraces are configured).
Thank you, this was super helpful. I'd never used Meson before, but I learned enough tonight to get the project built/installed so I could see the generated pkg-config file... the experience was so painless it almost makes me want to abandon CMake altogether... almost
Ha, good to hear. Since Meson's description is:
Meson is an open source build system meant to be both extremely fast, and, even more importantly, as user friendly as possible.
The main design point of Meson is that every moment a developer spends writing or debugging build definitions is a second wasted. So is every second spent waiting for the build system to actually start compiling code.
... it's good to hear we're succeeding
It is funny how Kitware uses Loguru extensively in their projects and yet never bothered to get a CMakeLists here.
I think this is a very good start too, It would indeed be nice if this was merged.
@emilk Any chance you could take a look at this for review?
Adds comprehensive CMake support to the project (requires CMake version 3.10+)
All feedback is welcome, I'm happy to answer any questions you may have.
New Features
You can now import loguru into your CMake projects with any of the following three methods:
add_subdirectory()
# works well with git submodulesFetchContent()
# works well with external git reposfind_package()
# works well to share a common installExample
For a demonstration of exactly how to import loguru into your CMake project, see the included example CMakeLists.txt file
Note about existing Tests
I didn't convert any of the existing CMakeLists.txt files in the project that are used for the tests and examples because that would require bumping their minimum required versions from 2.8 all the way to 3.10.
With that said: version 2.8 is officially deprecated and is no longer supported from CMake 3.21 onward and the only major distribution that still comes with CMake version 2.8 is CentOS 7.
If you like what you see here and are okay with me upping the minimum versions of the tests (and examples), I can integrate all of it into the main CMake build.
Closes
173
Notes to other users
I will be maintaining this branch on my fork indefinitely regardless of the result of this PR, so feel free to clone from it as needed.