search

open-source-parsers / jsoncpp

A C++ library for interacting with JSON.
Other
8.07k stars 2.63k forks source link

Avoid setting global cmake variables on all systems #1453

Closed mknaranja closed 4 days ago

mknaranja commented 1 year ago

In https://github.com/open-source-parsers/jsoncpp/issues/1163#issuecomment-750770862, the author already warned that the setting of global cmake variables would cause troblems when using jsoncpp as a dependency with CMake.

As the problem in https://github.com/open-source-parsers/jsoncpp/issues/1163 was only related to Windows I first would propose to only do changes for windows and MSVC.

Furthermore, I made the global variables local by removing the CACHE keyword: https://riptutorial.com/cmake/example/11821/variables-and-the-global-variables-cache

I built jsoncpp on Windows with Cmake to see that it still works. There is another problem reported in https://github.com/open-source-parsers/jsoncpp/issues/1451 but that is not related as it was there on Windows before.

mknaranja commented 1 year ago

any comments here?

nkh-lab commented 1 year ago

For me, more understandable and more universal approach with if(CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR) described in issues/1451, suggested by @atrelinski

nkh-lab commented 1 year ago

So, this is PR based on @atrelinski proposal.

atrelinski commented 1 year ago

IMO PR https://github.com/open-source-parsers/jsoncpp/pull/1459 address this problem better and I am for merging PR https://github.com/open-source-parsers/jsoncpp/pull/1459 instead of this one