Workaround/fixes for cJSON symbol collision

Provide 2 new confiration options "disable_json" and "external_cjson" external_cjson is intended ONLY for static libraries that get linked to an application that already has cjson compiled in, so that it may use that. The version of cJSON MUST match the version used in libRIST or at least be ABI compatible. Would fix #111 (closed)

approved this merge request

Just to make sure, will the disable_json option have any effect on programs that use librist like ffmpeg, or only on the provided executables in this repo?

It shouldn't, afaik ffmpeg doesn't depend on json output, though other programs might.

However after sleeping on it I'm inclined to work towards a differing solution, right now we hardcode cJSON as included in the library on Windows, because in the past I had some issues with pkg-config and or cmake not working properly from within the windows build. I'm gonna remove that, update the documentation to reflect the changed build requirements, and if you're open for it I'll submit a PR to your project that builds cJSON as a static library, assuming your project has a working pkg-config/cJSON via msys2.

The solution of building with cJSON via the external option feels to hacky.

@sammirata you had some qualms about the disable json option, I think it's still worthwhile to have it. I think we should discuss here

hardcode cJSON as included in the library on Windows, because in the past I had some issues with pkg-config and or cmake not working properly from within the windows build. I'm gonna remove that

I just now realized that msys2 provides a cjson mingw64/mingw-w64-x86_64-cjson 1.7.14-2 package, and it properly provides a pkg-config file

mingw-w64-x86_64-cjson /mingw64/lib/pkgconfig/libcjson.pc
mingw-w64-x86_64-cjson /mingw64/lib/pkgconfig/libcjson_utils.pc

and also provides a static and shared library.

I guess that means a patch to vulkan-loader will probably need to be submitted since that's where the conflicting cjson symbols are coming from https://github.com/KhronosGroup/Vulkan-Loader

Looking at their cmake files it's hardcoded to build as well :/ (currently without any logic to detect it being present on the system).

tbh I'm surprised linux distro's even carry it then, since most have a strict policy of not accepting packages with statically compiled in libraries that would be available from the distro itself.

unapproved this merge request

closed