Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add full CMake support #215

Merged
merged 6 commits into from
Apr 6, 2023
Merged

Conversation

skaravos
Copy link
Contributor

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:

  1. add_subdirectory() # works well with git submodules
  2. FetchContent() # works well with external git repos
  3. find_package() # works well to share a common install

Example

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

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.

Changes:
- Added a cmake file for the project that can be used to import loguru
  using all of the following strategies
  1. add_subdirectory()  # works well with git submodules
  2. FetchContent()      # works well with external git repos
  3. find_package()      # works well to share a common install

Plans:
- Thoroughly test the current file with each of the three methods
- Update the README to contain cmake instructions
- Add CPack support
- Add CTest support
Notes:
- Still need to test this on linux
- Still need to test the actual behavior of
  `"CPACK_INCLUDE_TOPLEVEL_DIRECTORY"=0`
Changes:
- Only try to install .pdb for shared libraries in debug mode
@bylowerik
Copy link
Collaborator

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.

@0EVSG
Copy link

0EVSG commented Jul 5, 2022

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:

  • CMake build and install works, and the config is picked up in my own projects. Nice!
  • We have to choose some default compile time settings for the packaged library, e.g. LOGURU_STACKTRACES.
  • loguru.cpp is not installed. Consumers need it if they want different compile time settings.
  • In that case, it would make sense to export a loguru::loguru-header-only target, similar to fmt::fmt-header-only. Basically just the include paths, without linker flags.

Hope this helps.

@oturpe
Copy link

oturpe commented Aug 23, 2022

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.

@eli-schwartz
Copy link

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).

@oturpe
Copy link

oturpe commented Aug 27, 2022

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.

@eli-schwartz
Copy link

Maybe @kcgen knows the answer to that.

@kcgen
Copy link
Contributor

kcgen commented Aug 28, 2022

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.

Changes:
- The package description variables used by CPack now get their values
  from top-level cache variables

Notes:
- This should make it easy to pass descriptions into a pkgconfig file...

Plans:
- Add support for a configured pkgconfig file
Changes:
- Added a `loguru.pc.in` template file
- Added cmake support for configuring a basic loguru.pc pkgconfig file
- Added cache variable `LOGURU_INSTALL_PKGCONFIGDIR` that can set the
  install location of the generated .pc file
- Change linking logic for the `dl` library
  (now loguru target only links to `dl` if `LOGURU_STACKTRACES=1`)

Notes:
- I'm not familiar with pkgconfig, but the .pc file generated by the
  [meson wrapdb subproject][1] marks the dependencies as Lib.private
  so I'm just copying what they do!

Refs:
[1]: <https://github.com/mesonbuild/wrapdb/blob/42afffac0610cc9b0c4cdd333ec7a21f12ecc4dd/subprojects/packagefiles/emilk-loguru/meson.build>
@skaravos
Copy link
Contributor Author

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

@eli-schwartz
Copy link

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

@WildRackoon
Copy link

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.

@skaravos
Copy link
Contributor Author

@emilk Any chance you could take a look at this for review?

@bylowerik bylowerik merged commit 4adaa18 into emilk:master Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants