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.

Sign up for GitHub

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

Jump to bottom

FetchContent Improvements #373

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

FetchContent Improvements #373

wants to merge 7 commits into from

Commits on Sep 29, 2023

  1. Raise minimum required CMake version to 3.25 

    This allows us to use FIND_PACKAGE_ARGS for FetchContent with
the SYSTEM argument.
    @milianw
    milianw committed Sep 29, 2023
    Configuration menu
    Copy the full SHA
    6bc3f41 View commit details
    Browse the repository at this point in the history

  2. Use SYSTEM for FetchContent_Declare statements 

    Warnings in thirdparty dependencies are not very interesting for
the development of an application. By setting the SYSTEM arg we
will include the dependency header via `-isystem` instead.
    @milianw
    milianw committed Sep 29, 2023
    Configuration menu
    Copy the full SHA
    db8c33d View commit details
    Browse the repository at this point in the history

  3. Use FIND_PACKAGE_ARGS for tomlplusplus 

    Straightforward adaption to leverage `find_package` on those systems
that have a tomlplusplus that's new enough for our purposes.
    @milianw
    milianw committed Sep 29, 2023
    Configuration menu
    Copy the full SHA
    58763e1 View commit details
    Browse the repository at this point in the history

  4. Leverage FIND_PACKAGE_ARGS for glog 

    This is also relatively straight forward, except for the fact
that we don't get the glog unittest targets when the dependency
is found. Thus the changeset is slightly larger than one might
expect otherwise.
    @milianw
    milianw committed Sep 29, 2023
    Configuration menu
    Copy the full SHA
    7297265 View commit details
    Browse the repository at this point in the history

  5. Use FetchContent + FIND_PACKAGE_ARGS for msgpack v5.0.0 or higher 

    The version bump was done so that I can test it with the package
I have available on my system. Previously, the embedded msgpack
code didn't compile there. Now I simply can use the system provided
msgpack here which is much better.
    @milianw
    milianw committed Sep 29, 2023
    Configuration menu
    Copy the full SHA
    acdb28a View commit details
    Browse the repository at this point in the history

  6. Import FindRocksDB.cmake from caffe2 

    This code is copied from [1] which is Apache-2.0 licensed and
owned by Meta, so I hope this is fine licensing wise.

[1]: https://github.com/facebookarchive/caffe2/blob/157ff81a12cf48eb4b0a2ad854c4ae205ad8aa07/cmake/Modules/FindRocksDB.cmake
    @milianw
    milianw committed Sep 29, 2023
    Configuration menu
    Copy the full SHA
    c671e90 View commit details
    Browse the repository at this point in the history

  7. Make it possible to link against a system provided RocksDB 

    Due to the custom build system integration for RocksDB we cannot
simply use FIND_PACKAGE_ARGS. Instead, we first do a `find_package`
and only if that fails do we fallback to the old rocksdb integration.

To better facilitate this, I took the liberty to also modernize
the CMake code a bit. Consumers of the lib just need to link
against the new `rocksdb::librocksdb` alias target to get the
required library and include directory.
    @milianw
    milianw committed Sep 29, 2023
    Configuration menu
    Copy the full SHA
    5a6ef38 View commit details
    Browse the repository at this point in the history