Fixes for compiling with a newer (system) jsoncpp #4429
Conversation
This is needed to avoid having to specify the minetest src directory as a system include when fixing the json includes.
They used "", so that the compiler searches the project's directory first. The result was that when compiling with a system jsoncpp, the project's own version of json.h was still included, instead of the system version. The includes now use <>, so a system location, or one specified with '-Ilocation' is searched only.
| @@ -1250,8 +1250,13 @@ static bool push_json_value_helper(lua_State *L, const Json::Value &value, | |||
| lua_newtable(L); | |||
| for (Json::Value::const_iterator it = value.begin(); | |||
| it != value.end(); ++it) { | |||
| #ifndef JSONCPP_STRING | |||
There was a problem hiding this comment.
Can you put these entirely to the right?
When compiling with a newer version of jsoncpp (and ENABLE_SYSTEM_JSONCPP=true), jsoncpp emits a warning about a deprecated function that minetest uses.
|
👍 |
est31
added
@ Build
|
👍 |
|
Using Arch with gcc 5, I was unable to compile for android, many instances of unable to find json/json.h. I manually backed out the subdirectory changes and was then able to compile successfully. |
|
@zing269 This sounds like you compiled with Please try |
|
@Rogier-5 I tried your suggestion but I'm still getting the No such file or directory error. I found a couple of entries in /build/android/jni/Android.mk that referenced /jni/src/json and modified those to /jni/src/jsoncpp/json but that didn't help either. |
I think that for LOCAL_SRC_FILES += jni/src/jsoncpp/json/jsoncpp.cpp(which is how you probably changed it already) and for [...]
jni/src/jsoncpp \
[...]which you probably changed to Does that help ? |
|
Changing the LOCAL_C_INCLUDES to jni/src/jsoncpp did the trick. Thanks. |
|
Mhhh these things need to be done on master as well... |
|
@est31 naturally @zing269 Are you planning to create a pull request ? If not: I pushed a fix, but I haven't been able to compile-test it yet. Would you mind verifying that it works for you ? It is at https://github.com/Rogier-5/minetest/tree/json-android-fix. |
|
@Rogier-5 I confirm the android build works with your patch. |
|
@Rogier-5 Please go ahead and create the pull request. I'm still figuring out this whole git thing. |
|
In the mean time, I have managed to configure the android environment as well, and I can confirm too that compilation fails without the patch, and succeeds with patch. @zing269: the following shell commands should work: (if that's of any use) |
|
@Rogier-5 thanks, that is quite helpful. |
See issue #4306.
The primary problem turns out to be that minetest uses
#include "json/json.h", where it should use#include <json/json.h>.Another problem which surfaces then, is that jsoncpp contains a bug, which makes it fail to compile in non-c++11 mode. The following change is neede to
/usr/include/jsoncpp/json/config.h, to make it compile.Change:
To:
Jsoncpp's config.h contains other similar code (similar to
#if defined(_MSC_VER) && _MSC_VER <= 1600 // MSVC <= 2010), and I haven't assessed whether that should be changed as well. With the jsoncpp patch above, minetest compiles, and the server starts and seems to work fine. I couldn't test the client.