Fixes jsoncpp compilation when it is also installed system-wide #3933
Conversation
|
@chfast I am not sure what I am doing, but this fixes it locally for me when I install Without these changes, I get the same error as shown in https://github.com/ethereum/homebrew-ethereum/issues/164 |
libdevcore/CMakeLists.txt
Outdated
| target_link_libraries(devcore PRIVATE jsoncpp ${Boost_FILESYSTEM_LIBRARIES} ${Boost_REGEX_LIBRARIES} ${Boost_SYSTEM_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}) | ||
| target_link_libraries(devcore PRIVATE ${JSONCPP_LIBRARY} ${Boost_FILESYSTEM_LIBRARIES} ${Boost_REGEX_LIBRARIES} ${Boost_SYSTEM_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}) | ||
| target_include_directories(devcore PUBLIC "${CMAKE_SOURCE_DIR}") | ||
| target_include_directories(devcore PUBLIC "${JSONCPP_INCLUDE_DIR}") |
There was a problem hiding this comment.
Can you try keeping this line only?
There was a problem hiding this comment.
I'm not how this works, but basically libdevcore/JSON.h and json/json.h are included in many different libraries (libevmasm, libsolidity, libsolc, test), so I'd think the include directory should be made valid for all of them, e.g. should be put into the main CMake file and not into libdevcore.
The same applies to jsoncpp library then.
How to accomplish this?
There was a problem hiding this comment.
Add inlcude_directories(${JSONCPP_INCLUDE_DIR}) just after include(ProjectJsoncpp).
There was a problem hiding this comment.
Will that cover both the header and the lib?
There was a problem hiding this comment.
No. Only iclude path.
You still have to use target_link_libraries(). But should be enough to specify that in devcore, because for static libs this dependency in transitive.
There was a problem hiding this comment.
I cannot find a include(ProjectJsoncpp) line? There is only a include(jsoncpp) line in the main cmake file.
libdevcore/CMakeLists.txt
Outdated
|
|
||
| add_library(devcore ${sources} ${headers}) | ||
| target_link_libraries(devcore PRIVATE jsoncpp ${Boost_FILESYSTEM_LIBRARIES} ${Boost_REGEX_LIBRARIES} ${Boost_SYSTEM_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}) | ||
| target_link_libraries(devcore PRIVATE ${JSONCPP_LIBRARY} ${Boost_FILESYSTEM_LIBRARIES} ${Boost_REGEX_LIBRARIES} ${Boost_SYSTEM_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}) |
There was a problem hiding this comment.
@chfast are you sure jsoncpp here will be correct and we don't need to use ${JSONCPP_LIBRARY} here?
There was a problem hiding this comment.
It's better to use a CMake target representing a library (defined in https://github.com/ethereum/solidity/blob/develop/cmake/jsoncpp.cmake#L47) than the path to the library directly (old school CMake).
The jsoncpp defines also include paths, however we have to workaround it with additional include_directories() because we want to make sure jsoncpp's include path is before /usr/include.
So in other words. both work but using jsoncpp is better style.
|
This works now locally (fixes the problem when jsoncpp is installed system wide by homebrew) and also fixes #3467. |
Fixes https://github.com/ethereum/homebrew-ethereum/issues/163 and https://github.com/ethereum/homebrew-ethereum/issues/164