localeconv() workaround doesn't help on Android

[DrMetallius]

The workaround for missing locale support introduced in 894e78b doesn't seem to be working. I compile with NDK r12b, with clang as the compiler, c++_shared as my library, and android-9 as the platform, and it tells me that it can't build it because of the "undefined reference to 'localeconv'". This comes from json_tool.h.

sizeof(struct lconv) is 56. So it clearly is larger than a simple stub for whatever reason, that's why everything fails. Honestly, I think that a preprocessor macro would be much simpler and more reliable than this template magic.

[cdunn2001]

Agreed. If you submit a macro-based fix, we'll merge it right away.

[cdunn2001]

See discussion on 52cfe5a

[Gachapen]

CMake can check if a type exists and if a struct has a member. This should be sufficient. I can create a PR and then @DrMetallius can test it.

See https://cmake.org/cmake/help/v3.0/module/CheckTypeSize.html and https://cmake.org/cmake/help/v3.0/module/CheckStructHasMember.html

[DrMetallius]

There is a problem with CMake in CI environments for Android developers. Besides, I'm pretty sure that in our case the structure wasn't empty since it had the size more than zero, but it didn't work regardless. Checking anything about the structure just doesn't seem reliable to me.

[Gachapen]

So you're approach to CMake on Android is quite different than ours. We use the system CMake with the toolchain file provided by the NDK. So in our docker build image CMake is installed as an apt package, while the NDK and SDK are downloaded. If you're interested, see https://gitlab.com/suttungdigital/nox-engine/blob/master/tools/docker/build-ubuntu-latest-all/Dockerfile.

From what I can see from your issue description, your problem was that localeconv was undefined. What if we, in addition to the aforementioned checks, also check if the localeconv symbol exists using https://cmake.org/cmake/help/v3.0/module/CheckSymbolExists.html?

If we check if lconv exists, then if localeconv is defined, and finally if lconv has the member decimal_point, we should cover everything used.

[DrMetallius]

This isn't my approach, it's the official one. We don't even use CMake, we use NDK-build. Theoretically I can transition to CMake, but it will be rather cumbersome to work with given the problems I said about (or if I go the manual way), not to mention that transitioning to CMake itself will require time.

[Gachapen]

Oh, sure, I'm not suggesting CMake as a requirement. With the change I'm suggesting, the code wouldn't change at all. You could still manually provide the NO_LOCALE_SUPPORT flag. But if CMake is being used, CMake could automatically detect lconv support with the suggested methods and set the NO_LOCALE_SUPPORT flag if appropriate.

[DrMetallius]

That sounds like a good solution, although I doubt I can help you test it given the lack of CMake setup on my side.

[Gachapen]

Good. Of course, that makes sense. I'll go ahead and create a PR in acouple of days.

[cdunn2001]

Feel free to re-open if anyone finds a problem.Thanks, @Gachapen and @DrMetallius.