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

REFACTOR: rewrite CMakeLists.txt for better inlcude and reuse #669

Merged

Conversation

dan-42
Copy link
Contributor

@dan-42 dan-42 commented Jul 29, 2017

The rewrite uses more cmake build-in functions and build-in automaticly generated
variables to allow better generic reuse.
This is based on the cmake example project package-example

  • cmake files are installed to
    <install_prefix>/lib/cmake/nlohmann_json/ for best support on
    most systems
  • include path is set to include for usage as #include <nlohmann/json.hpp>

Motivation

The motivation is the reintegration of nlohmann_json into the cmake package-manager hunter
and to avoid possible naming collissions with other packages( e.g. jsoncpp, etc)

compile install and run with unit tests

dan@kay ~/git/json/build (git)-[change_installed_cmake_include_path] % rm -rf ./* && rm -rf ../test_install/* && cmake ..  -DCMAKE_INSTALL_PREFIX=../test_install -DJSON_BuildTests=ON && make -j8  && make install
zsh: sure you want to delete all 8 files in /home/dan/git/json/build/. [yn]? y
zsh: sure you want to delete all 2 files in /home/dan/git/json/build/../test_install [yn]? y
-- The C compiler identification is GNU 7.1.1
-- The CXX compiler identification is GNU 7.1.1
[....]
-- Configuring done
-- Generating done
-- Build files have been written to: /home/dan/git/json/build
Scanning dependencies of target catch_main
[  2%] Building CXX object test/CMakeFiles/catch_main.dir/src/unit.cpp.o
[...]
[100%] Built target json_unit
make -j8  157.98s user 5.96s system 580% cpu 28.240 total
[  2%] Built target catch_main
[100%] Built target json_unit
Install the project...
-- Install configuration: ""
-- Installing: /home/dan/git/json/test_install/include/nlohmann
-- Installing: /home/dan/git/json/test_install/include/nlohmann/json.hpp
-- Installing: /home/dan/git/json/test_install/lib/cmake/nlohmann_json/nlohmann_jsonConfig.cmake
-- Installing: /home/dan/git/json/test_install/lib/cmake/nlohmann_json/nlohmann_jsonConfigVersion.cmake
-- Installing: /home/dan/git/json/test_install/lib/cmake/nlohmann_json/nlohmann_jsonTargets.cmake
dan@kay ~/git/json/build (git)-[change_installed_cmake_include_path] % ctest -j2
Test project /home/dan/git/json/build
    Start 1: json_unit_default
    Start 2: json_unit_all
1/2 Test #1: json_unit_default ................   Passed   12.12 sec
2/2 Test #2: json_unit_all ....................   Passed  223.00 sec

100% tests passed, 0 tests failed out of 2

Total Test time (real) = 223.01 sec
ctest -j2  234.59s user 0.52s system 105% cpu 3:43.01 total

Usage on an external project after installation

CMakeLists.txt

cmake_minimum_required (VERSION 3.0)
project(test_json)
find_package(nlohmann_json CONFIG REQUIRED PATHS /home/dan/git/json/test_install/)
add_executable(main main.cpp)
target_link_libraries(main nlohmann_json)

main.cpp

#include <iostream>
#include <nlohmann/json.hpp>
int main (int argc, char** argv) {
  nlohmann::json j2 = {
    {"pi", 3.141},    
  };
  std::cout << j2 << std::endl;
  return 0;
}

I'm looking forward on comments and thoughts about the changes.
Many thanks in advance

The rewrite uses more cmake build-in automatisms and build-in generates
variables to allow better generic reuse.
* cmake  files are installed to
``` <install_prefix>/lib/cmake/nlohmann_json/ ``` for best support on
most systems
* include path is set to ``` include ```  for usage as ``` #include
<nlohmann/json.hpp> ```
@mention-bot
Copy link

@dan-42, thanks for your PR! By analyzing the history of the files in this pull request, we identified @robertmrk, @nlohmann and @ChrisKitching to be potential reviewers.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f434942 on dan-42:change_installed_cmake_include_path into c819a2d on nlohmann:develop.

## PROJECT
## name and version
##
project(nlohmann_json VERSION 2.1.1)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity: why did you remove the LANGUAGES part?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because it is enabled by default, so no need for that. see https://cmake.org/cmake/help/v3.0/command/project.html

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking back! Just out of coincidence: would it help to explicitly enable C++, because we do not use C at all?

@nlohmann nlohmann added this to the Release 3.0.0 milestone Jul 30, 2017
@nlohmann nlohmann self-assigned this Jul 30, 2017
@nlohmann nlohmann merged commit 67fb517 into nlohmann:develop Jul 30, 2017
@nlohmann
Copy link
Owner

Thanks a lot!

@dan-42
Copy link
Contributor Author

dan-42 commented Jul 30, 2017

awesome thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants