Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUILD_SHARED_LIBS #207

Closed
mdenna-nviso opened this issue Oct 24, 2017 · 7 comments
Closed

BUILD_SHARED_LIBS #207

mdenna-nviso opened this issue Oct 24, 2017 · 7 comments

Comments

@mdenna-nviso
Copy link

Hi Dave,

the CMakeList.txt for cJSON sets the BUILD_SHARED_LIBS global flag to ON.
This can cause surprising results when cJSON is built inside a bigger project
where this variable was not set.

Would it be possible to modify the CMakeList to avoid this?

Best regards
Maurizio

@FSMaxB
Copy link
Collaborator

FSMaxB commented Oct 24, 2017

Currently I am maintaining cJSON, haven't heard from Dave in a while.

Anyway, I don't quite understand what you mean. My understanding is that you somehow use add_subdirectory in your project to directly include cJSON. This means that you have potential namespace conflicts between cJSON's variables/options and your own.

To be honest, I still don't fully understand how the scope and lifetime of different variables and options in CMake work.

Avoiding global namespace issues would have required the CMake options to have the name of the library in them, which not all of them do. But I can't just rename them, without breaking compatibility, maybe you have an idea how this can be done.

If you use BUILD_SHARED_LIBS in your own project as well, it shouldn't be a problem as long as you don't want different settings in your parent project and cJSON. If that is the case however, I'm not sure how to proceed.

Maybe there is something you can do with CMake's import/export feature (cJSON can export it's targets via CMake). Or with CMake's external project functionality.

@mdenna-nviso
Copy link
Author

Hi Max,
thanks for your quick reply.
The fact is that BUILD_SHARED_LIBS is a global flag, so enabling it in cJSON's cmake has effect on the entire project. At the moment I've been able to avoid the issue by setting BUILD_SHARED_LIBS to OFF before I do add_subdirectory for cJSON, but this will not be possible in case I need a different setting in my parent project and cJSON. A possibility could be to add a specific flag for cJSON, something like CJSON_BUILD_SHARED_LIB, that if enabled will build only cJSON libs as shared. This can be done by specifying the shared property in each add_library command using the SHARED option when needed.

Thanks
Maurizio

@FSMaxB
Copy link
Collaborator

FSMaxB commented Oct 26, 2017

Ok, I think I understand the problem now.

I am not sure though if it is possible to introduce a new cJSON-only option while still honoring the BUILD_SHARED_LIB setting in cJSON. I will have to investigate that. Please give me some time.

One possibility would be to still set BUILD_SHARED_LIBS by default but make it overridable with CJSON_BUILD_SHARED_LIBS, so BUILD_SHARED_LIBS can be turned off while still building cJSON as a shared library.

If you have more ideas please tell me.

@mdenna-nviso
Copy link
Author

What you suggest would be perfect I think, it keeps the current behaviour by default while giving the user the possibility to override it when needed.
Actually what I would do is to avoid setting BUILD_SHARED_LIBS at all, just use it as the default value for CJSON_BUILD_SHARED_LIBS (if BUILD_SHARED_LIBS is not defined just set the default value of CJSON_BUILD_SHARED_LIBS to ON).
I don't need the feature right now, but I think it is something that could improve the usability of cJSON library.

Best regards
Maurizio

@FSMaxB
Copy link
Collaborator

FSMaxB commented Oct 26, 2017

Yes, I haven't really considered the use of cJSONs CMakeLists.txt in the context of another CMake project so far. That would definitely be an improvement, thank you for the feedback.

@FSMaxB
Copy link
Collaborator

FSMaxB commented Nov 28, 2017

This has been implemented on the develop branch. Althoug a little differently. There now is a CJSON_OVERRIDE_BUILD_SHARED_LIBS option. If that is enabled, BUILD_SHARED_LIBS can be overriden with CJSON_BUILD_SHARED_LIBS.

@FSMaxB FSMaxB closed this as completed Nov 28, 2017
@mdenna-nviso
Copy link
Author

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants