tgockel / json-voorhees

A killer modern C++ library for interacting with JSON.
http://tgockel.github.io/json-voorhees/
Apache License 2.0
128 stars 18 forks source link

Linking with CMake is done in a completely incorrect way #61

Closed DavidHammen closed 8 years ago

DavidHammen commented 8 years ago

Sorry to burst the bubble of getting your latest release out on Friday the 13th, but I am having significant problems building and using jsonv on OSX (with multiple compilers) and lesser problems on multiple linux distros (again with multiple compilers). The issue is the dependency on Boost, and in particular, Boost locale.

On a linux box, the boost libraries are typically named along the lines of libboost_locale.so. When I use jsonv in some application, all I need to do is to ensure I use -lboost_locale -lboost_filesystem -lboost_system when linking. The problem: I shouldn't have to do that. The jsonv library should make that dependency transparent. This is a small, easily solvable issue: Just add those -l<library_name> to my makefile.

On a mac, there's no reason to need Boost locale. The LLVM compilers have long supported the header <codecvt>. The problem results from a test in src/jsonv/char_convert.cpp and from unexpected behavior from LLVM-based compilers. The LLVM compilers (clang et al.) define __GNUC__ (why do they do this?), and they make it look like the compiler in question is an ancient gcc compiler (and why do that do that???) Based on the preprocessor names, it appears that the compilers as released by Apple are gcc 4.1.2; they look like gcc 4.2.1 in the case of compilers from llvm.org. The problem is compounded by the way tools such as fink, macports, and homebrew mangle library names.

The easiest solution (maybe not the best) is to add an else clause to the if(USE_BOOST_LOCALE) in CMakeLists.txt (git diff shown below):

@@ -72,6 +72,8 @@ option(USE_BOOST_LOCALE
 if (USE_BOOST_LOCALE)
     add_definitions("-DJSONV_CHAR_CONVERT_USE_BOOST_LOCALE=1")
     set(REQUIRED_BOOST_LIBRARIES "locale" ${REQUIRED_BOOST_LIBRARIES})
+else(USE_BOOST_LOCALE)
+    add_definitions("-DJSONV_CHAR_CONVERT_USE_BOOST_LOCALE=0")
 endif(USE_BOOST_LOCALE)
tgockel commented 8 years ago

Thanks for the report -- I agree that changing it in CMake is the most reasonable thing to do.

I really need to get better at addressing platforms that aren't Linux.

tgockel commented 8 years ago

Okay, I think this should all be working a hell of a lot better now.

DavidHammen commented 8 years ago

Much nicer! Thanks!

On Jan 22, 2016, at 2:41 PM, Travis Gockel notifications@github.com wrote:

61