-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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 CMake fetchcontent documentation and tests #2074
Conversation
Hi Niels, I was very interested seeing what kind of CI you had. Nice! Let's see how it goes. Please do not accept this pull request yet. I just realized by cloning this repository that its download size is huge: 170MiB. CMake FetchContent use shallow clone, so this will be only 113MiB. I think this might be a problem for some users. What do you think about proposing the users to use some alternatives repository URL(s). There are some nice fork tracking only the required files likes: |
Thanks! I am already working on splitting tests and code (see https://github.com/nlohmann/json/tree/tests_external), but this will take some more time. Until then, I think we should live with a shallow checkout. It's not perfect, but it works and this is about documenting possibilities for integration - not forcing anyone to actually use this way. |
The branch "tests_external" looks promising indeed! git clone --depth 1 --branch tests_external https://github.com/nlohmann/json => 4.61 MiB (download size) git clone --depth 1 --branch masterl https://github.com/nlohmann/json => 113 MiB (download size) git clone --branch masterl https://github.com/nlohmann/json => 170 MiB (download size) |
c6b8e71
to
9ab2193
Compare
Github issue: nlohmann#2073 nlohmann::json documents 2 way of depending on it using CMake 1) Copy-paste the project/source into your own project. 2) Install nlohman::json and then use find_package. (1) pollutes your git repository, (2) requires everyone to install the dependencies themselves. Since 2018, CMake provide some kind of 'package manager' features using [FetchContent](https://cmake.org/cmake/help/v3.17/module/FetchContent.html) It gives the following: ~~~cmake include(FetchContent) FetchContent_Declare(json GIT_REPOSITORY https://github.com/nlohmann/json GIT_TAG v3.7.3) FetchContent_GetProperties(json) if(NOT json_POPULATED) FetchContent_Populate(json) add_subdirectory( ${json_SOURCE_DIR} ${json_BINARY_DIR} EXCLUDE_FROM_ALL) endif() ~~~ Then declares the dependency in the target using it: ~~~cmake target_link_library(my_project PRIVATE nlohmann_json::nlohmann_json ~~~ This patch updates the documentation and provides tests.
Co-Authored-By: Niels Lohmann <niels.lohmann@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Thanks! I may remove the link to your repo once we have slimmed this repository a bit, see #96. |
🔖 Release itemThis issue/PR will be part of the next release of the library. This template helps preparing the release notes. Type
Description
|
Yes sure. This make sense! |
About pull request: nlohmann#2074 An error has been introduced by accepting the suggestions: 4be4a03 One was about removing ~~~, but it was meant to be replaced by ``` in reality. This caused the README.md to be slightly broken.
Github issue:
#2073
nlohmann::json documents 2 way of depending on it using CMake
(1) pollutes your git repository, (2) requires everyone to install the
dependencies themselves.
Since 2018, CMake provide some kind of 'package manager' features using
FetchContent
It gives the following:
Then declares the dependency in the target using it:
target_link_library(my_project PRIVATE nlohmann_json::nlohmann_json
This patch updates the documentation and provides tests.