Support for dependency-free nlohmann::json

[daminetreg]

Hi @ruslo,

Hope you are well. 😄

I cmaked the nlohmann::json library here : nlohmann::json cmaked.

Then I made it available with hunter, therefore this pull-request. I would like to move my json fork in the hunter-packages organization if you accept, and I parallely open a PR by nlohmann::json library #123 to get the changes mainline.

Cheers,

[ruslo]

I would like to move my json fork in the hunter-packages organization

Created: https://github.com/hunter-packages/json

[ruslo]

+set (json_VERSION "1.0.0-rc1")

If you will set version in project it will be used by write_basic_package_version_file. See documentation.

[ruslo]

+  set(CMAKE_CXX_FLAGS "-std=c++11 -stdlib=libc++")

user may set CMAKE_CXX_FLAGS from command line, if you will use set they will be lost. Should be at least set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -stdlib=libc++"). Also this stuff is quite tricky. I'm pretty sure it will not work on Linux (default lib is libstdc++).

Also see wiki about defaults.

[ruslo]

+check_required_components(nlohmann_json) 

Thanks, missed this stuff from documentation. I'm personally using this project as a template. Updated: forexample/package-example@cf2ea1d

[ruslo]

if compile flags are usage requirement we can add them in *Config.cmake by INTERFACE_COMPILE_OPTIONS property. Or better by target_compile_options.

[daminetreg]

I tried with COMPILE_FLAGS and this was not a cmake whitelisted prop and couldn't find out the right one. Thanks for pointer INTERFACE_COMPILE_OPTIONS.

[ruslo]

STRING(REPLACE "/O2" "/Od" CMAKE_CXX_FLAGS_RELEASE ${CMAKE_CXX_FLAGS_RELEASE})

A little bit confusing. What is the point of this line? Though it came from original code...

[daminetreg]

user may set CMAKE_CXX_FLAGS from command line, if you will use set they will be lost. Should be at least set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -stdlib=libc++"). Also this stuff is quite tricky. I'm pretty sure it will not work on Linux (default lib is libstdc++).

Also see wiki about defaults.

Yes I didn't change it as it came from the original code. I find it strange but it might be some support of libstdc++ which is missing which is in libc++ that the original code use. I removed these switches, as for me both libstdc++ libc++ get the test passed to the same level of success.

Speaking for clang 3.5.1 rc2.

STRING(REPLACE "/O2" "/Od" CMAKE_CXX_FLAGS_RELEASE ${CMAKE_CXX_FLAGS_RELEASE})

A little bit confusing. What is the point of this line? Though it came from original code...

Looking at the history it look like the author disabled the MSVC optimization, it might be due to bugs that those incurred? I'm unsure, hopefully @nlohmann can tell us why. As you can see in this commit
nlohmann/json@c89ca71.

[daminetreg]

If you will set version in project it will be used by write_basic_package_version_file. See documentation.

Good to know, it however makes usage of -rc1 suffix impossible there. But that's not a problem as it's anyway not allowing greater-than version check. Thanks.

[daminetreg]

Thanks, missed this stuff from documentation. I'm personally using this project as a template. Updated: forexample/package-example@cf2ea1d

I'll take this one as basis in the future, nice organization name. 😄

[nlohmann]

I had no time to follow this thread yet. But just as info: the MSVC flags were set to compile my code at AppVeyor.

[ruslo]

But just as info: the MSVC flags were set to compile my code at AppVeyor.

@nlohmann I've got assertion, see nlohmann/json#125

[daminetreg]

Thanks @nlohmann for joining the discussion. 😄 And thanks @ruslo for debugging and merging. 👍

[ghost]

@daminetreg, don't build on Mac OS X.
OS X: El Capitan
cmake version 3.3.2

Error:
clang: error: no such file or directory: ' -std=c++11'

Fix bug:
https://github.com/hunter-packages/json/blob/hunter/cmake/modules/nlohmann-json-config.cmake.in#L15

set(NLOHMANN_JSON_DEFINITIONS "@CMAKE_CXX_FLAGS@")
to
set(NLOHMANN_JSON_DEFINITIONS @CMAKE_CXX_FLAGS@)

[ruslo]

set(NLOHMANN_JSON_DEFINITIONS "@CMAKE_CXX_FLAGS@")

Actually we don't need this stuff at all. Can be added automatically...

[ruslo]

Just for your information though library is header only we still need to download > 70 MB because of 'test' and 'benchmarks' directories:

> du -hs *
4.0K    CMakeLists.txt
4.0K    LICENSE.MIT
4.0K    Makefile
 16K    README.md
4.0K    appveyor.yml
 57M    benchmarks
4.0K    cmake
1.5M    doc
440K    src
 14M    test

[ruslo]

clang: error: no such file or directory: ' -std=c++11'

Fixed: https://github.com/ruslo/hunter/releases/tag/v0.12.32

[ruslo]

need to download > 70 MB

Archive is 16 MB, but anyway

[ghost]

@ruslo, ths!!

[nlohmann]

Hi there, it's been a while since PR nlohmann/json#123 has been opened. I am not sure what it is good for, but it breaks the AppVeyor build. Could you please check if all proposed changes are actually required?

[ruslo]

@nlohmann Compile flags in good vs. broken build are quite the same, I think problem is C++ code.

@daminetreg I think you need to rebase your pull request to fetch new updates from C++ sources.

[ruslo]

clang: error: no such file or directory: ' -std=c++11'

Fixed: https://github.com/ruslo/hunter/releases/tag/v0.12.32

@daminetreg It make sense to pull this fixes too (see branch hunter in https://github.com/hunter-packages/json)

[daminetreg]

Thanks @ruslo for the fixes.

I'll reopen another PR from the hunter-packages version.

@nlohmann I'll check for appveyor, but I think this was the exception handling model chosen that I removed : /EHsc, I think it's something we shouldn't had to the exported compile flags and only use it in the unit test compilation. I'll ensure appveyor build again.