json-schema-validator icon indicating copy to clipboard operation
json-schema-validator copied to clipboard

Cmake clean up

Open vrince opened this issue 2 years ago • 11 comments

This is a draft of cmake simplification following the discussion from #185

note: It's maybe too big of a move as I see a lot of effort have been put in a couple of years ago to make json-schema-validator works with find-package and this MR goes completely in the other direction.

The goal of this MR is to make CMake process dead simple and make usage of this library really easy to help adoption. Bottom line the following cmake is all it needs to use both json-schema-validator and appropriate version of nlohmann_json for people starting from scratch :

cmake_minimum_required(VERSION 3.16)
include(FetchContent)

project(stand-alone VERSION 1.0.0 LANGUAGES CXX)

set(JSON_VALIDATOR_BUILD_EXAMPLES OFF)
set(JSON_VALIDATOR_BUILD_TESTS OFF)

FetchContent_Declare(json_schema_validator 
GIT_REPOSITORY https://github.com/vrince/json-schema-validator
GIT_TAG 379cf8c2b4ad49fe3a21e926ffdf543b9e9da8df)
FetchContent_MakeAvailable(json_schema_validator)

add_executable(stand-alone stand-alone.cpp)
target_link_libraries(stand-alone nlohmann_json_schema_validator)

This is just an example once merged the GIT_TAG will make more sense...

This MR also contains:

  • [x] automatic fetch nlohmann_json is not found
  • [x] no more JSON_VALIDATOR_BUILD_INSTALL
  • [x] add CI with and without nlohmann_json presintalled
  • [x] rename examples json-schema-*
  • [x] remove HUNTER
  • [x] remove old travis thing
  • [x] add a stand-alone user can copy past to quick start
  • [x] no more cmakeConfig to export
  • [x] bump conversion to 2
  • [ ] ~~make it a c++11 project and drop boost regex ?~~

vrince avatar Dec 10 '21 16:12 vrince

@pboettch here is a draft of FetchContent move ... still some stuff to clean on the test side since there is a test to check if find_package(...) works and I remove everything export-related (feeling a little bad about that .... sorry).

vrince avatar Dec 10 '21 16:12 vrince

Contemplating to set JSON_VALIDATOR_BUILD_EXAMPLES and JSON_VALIDATOR_BUILD_TESTS off by default document then for internal developers.

vrince avatar Dec 10 '21 16:12 vrince

Beautiful so far.

pboettch avatar Dec 10 '21 17:12 pboettch

This will supersede #162 and #185, right?

pboettch avatar Dec 10 '21 17:12 pboettch

@pboettch being at it ... making it a c++11 project and dropping the regex boost gcc 4.9 thing?

vrince avatar Dec 10 '21 17:12 vrince

@pboettch being at it ... making it a c++11 project and dropping the regex boost gcc 4.9 thing?

Making in C++11 OK, but not dropping the boost-regex-thing ftm. Or in a separate PR. I have one project with gcc-4.9 I'd need to check before.

pboettch avatar Dec 10 '21 17:12 pboettch

It might be worth keeping Boost regex support since std::regex has pretty poor performance: https://stackoverflow.com/questions/70583395/why-is-stdregex-notoriously-much-slower-than-other-regular-expression-librarie

I'd suggest reworking the CMake setup logic to allow users to provide Boost as an existing target or by providing the config file. Using NOT TARGET Boost::regex before using find_package will probably suffice. This also implies using Boost::regex instead of Boost_* variables.

Making the use of Boost regex possible on any platform would also be good.

SamVanheer avatar May 10 '22 15:05 SamVanheer

Guys this is unfinished business my goal was to simplify integration into other projects using CMake Fetch content (with it does in its current state) but then it crashed on me for some reason when I use it. So I used another lib to do json schema validation. Not sure I'll devote the time needed to finalize this MR.

vrince avatar May 13 '22 13:05 vrince

Yeah, sorry for not having continued until now. Time is going by so fast.

What crash did you observe?

pboettch avatar May 13 '22 13:05 pboettch

All PRs have been closed due to me changing the main-branch to main. Please reissue your pull-request to that branch. Sorry and thanks.

pboettch avatar Jun 06 '22 21:06 pboettch

This pull request is quite promising. @pboettch do you need any help to finalize the integration?

nine avatar Sep 02 '22 12:09 nine

Consider #262 for a more complete implementation

LecrisUT avatar May 09 '23 20:05 LecrisUT