grzegorzmazur / yacas

Computer calculations made easy
http://www.yacas.org
GNU Lesser General Public License v2.1
120 stars 23 forks source link

wrong path for json/json.h #340

Closed doronbehar closed 2 years ago

doronbehar commented 2 years ago

Here on NixOS, we need to apply the following patch for the json/json.h include to work:

diff --git i/cyacas/yacas-kernel/CMakeLists.txt w/cyacas/yacas-kernel/CMakeLists.txt
index fe1600aa..dcc329f8 100644
--- i/cyacas/yacas-kernel/CMakeLists.txt
+++ w/cyacas/yacas-kernel/CMakeLists.txt
@@ -22,8 +22,9 @@ find_library (ZEROMQ_LIBRARY NAMES zmq)
 find_path (ZMQPP_INCLUDE_DIR zmqpp.hpp)
 find_library (ZMQPP_LIBRARY NAMES zmqpp)

-find_path (JSONCPP_INCLUDE_DIR json.h)
-find_library (JSONCPP_LIBRARY NAMES jsoncpp)
+# https://github.com/open-source-parsers/jsoncpp/wiki/Building#another-approach-for-cmake
+find_package(jsoncpp REQUIRED)
+get_target_property(JSON_INC_PATH jsoncpp_lib INTERFACE_INCLUDE_DIRECTORIES)

 find_package (OpenSSL)
 find_package (Boost REQUIRED date_time filesystem)
@@ -31,6 +32,6 @@ find_package (Boost REQUIRED date_time filesystem)
 include_directories (include)

 add_executable (yacas-kernel src/main.cpp src/yacas_kernel.cpp src/yacas_engine.cpp src/hmac_sha256.cpp src/base64.cpp)
-target_link_libraries (yacas-kernel libyacas ${ZMQPP_LIBRARY} ${ZEROMQ_LIBRARY} ${JSONCPP_LIBRARY} ${OPENSSL_CRYPTO_LIBRARY} ${Boost_LIBRARIES} pthread ${CMAKE_DL_LIBS})
+target_link_libraries (yacas-kernel libyacas ${ZMQPP_LIBRARY} ${ZEROMQ_LIBRARY} jsoncpp_lib ${OPENSSL_CRYPTO_LIBRARY} ${Boost_LIBRARIES} pthread ${CMAKE_DL_LIBS})

 install (TARGETS yacas-kernel DESTINATION ${CMAKE_INSTALL_BINDIR})
diff --git i/cyacas/yacas-kernel/include/yacas_kernel.hpp w/cyacas/yacas-kernel/include/yacas_kernel.hpp
index 91d36ac0..d12f905c 100644
--- i/cyacas/yacas-kernel/include/yacas_kernel.hpp
+++ w/cyacas/yacas-kernel/include/yacas_kernel.hpp
@@ -29,7 +29,7 @@
 #include "yacas_engine.hpp"

 #include <boost/uuid/random_generator.hpp>
-#include <jsoncpp/json/json.h>
+#include <json/json.h>
 #include <zmqpp/zmqpp.hpp>

 #include <map>
diff --git i/cyacas/yacas-kernel/src/main.cpp w/cyacas/yacas-kernel/src/main.cpp
index c31f17f2..832e9128 100644
--- i/cyacas/yacas-kernel/src/main.cpp
+++ w/cyacas/yacas-kernel/src/main.cpp
@@ -24,7 +24,7 @@

 #include "yacas_kernel.hpp"

-#include <jsoncpp/json/json.h>
+#include <json/json.h>

 #include <boost/dll/runtime_symbol_info.hpp>

diff --git i/cyacas/yacas-kernel/src/yacas_engine.cpp w/cyacas/yacas-kernel/src/yacas_engine.cpp
index 6ed60ca3..18e9b3fd 100644
--- i/cyacas/yacas-kernel/src/yacas_engine.cpp
+++ w/cyacas/yacas-kernel/src/yacas_engine.cpp
@@ -22,7 +22,7 @@
  * Created on November 7, 2015, 12:52 PM
  */

-#include <jsoncpp/json/writer.h>
+#include <json/writer.h>

 #include "yacas_engine.hpp"

It applies only on v1.9.1, and not on master, but the change for the master branch should be similar. I wonder whether this patch can be suggested in a PR here, because it might break builds on other distros. But I don't know why the build of jsoncpp in NixOS' is different that it requires such a change from yacas. Here's how jsoncpp is built on NixOS. And here's how the build tree of it looks like:

``` jsoncpp/ └── lib ├── libjsoncpp.so -> libjsoncpp.so.24 ├── libjsoncpp.so.1.9.4 └── libjsoncpp.so.24 -> libjsoncpp.so.1.9.4 ├── include │   └── json │   ├── allocator.h │   ├── assertions.h │   ├── config.h │   ├── forwards.h │   ├── json_features.h │   ├── json.h │   ├── reader.h │   ├── value.h │   ├── version.h │   └── writer.h ├── lib │   ├── cmake │   │   └── jsoncpp │   │   ├── jsoncppConfig.cmake │   │   ├── jsoncppConfig-release.cmake │   │   └── jsoncppConfigVersion.cmake │   └── pkgconfig │   └── jsoncpp.pc └── nix-support └── propagated-build-inputs 8 directories, 18 files ```
grzegorzmazur commented 2 years ago

Hi,

Seems to work at least on ubuntu impish. And frankly speaking the build procedure for the kernel was neither thoughtfully designed nor widely tested, I'll be happy to get a PR with your solution.

Thanks, Grzesiek

doronbehar commented 2 years ago

Indeed the list of files of the package seems different on NixOS and on Ubuntu. I think that using cmake should work for both environments. Will open a PR.