conda-forge / spdlog-feedstock

A conda-smithy repository for spdlog.
BSD 3-Clause "New" or "Revised" License
3 stars 19 forks source link

Shared library build + external fmt? #23

Closed bluescarni closed 3 years ago

bluescarni commented 4 years ago

Hello,

I'd like to open a PR aiming at building spdlog as a shared library and using the fmt library provided by conda-forge, instead of the bundled one.

Would this be ok, or are there specific reasons why the package is being built as a static library with the bundled fmt instead?

rongou commented 4 years ago

Those are just default options in spdlog's CMakeLists.txt. Wouldn't these changes affect existing users?

bluescarni commented 4 years ago

I don't know if using an external fmt library would change the API for the end-users. I'll try to investigate.

Regarding static vs dynamic library, dynamic libraries are just better for software distributions like conda. As it is now, packages using spdlog in the conda ecosystem will all have their own copy of the spdlog static library, which increases the package size and which requires a recompilation of each package whenever a new version of spdlog is released. Besides, the static library approach suffers from the problem that loading two DLLs which are both using spdlog internally results in a duplication of the spdlog global data structures (which are present in each DLLs), and likely unpredictable runtime failures/crashes. The problem is sketched out here, although the description is a bit outdated (it refers to the header-only version of spdlog, which does not exist any more):

https://github.com/gabime/spdlog/wiki/How-to-use-spdlog-in-DLLs

If spdlog is compiled as a shared library, the problem disappears because only one copy of the spdlog global state exists per process.

rongou commented 4 years ago

I am using the header-only version, so this doesn't really affect me. :)

With the external fmt library, perhaps all we need to do is to add a requirement for the fmt package in the recipe.

Feel free to send a PR.

druvus commented 4 years ago

Feel free to send a PR and at the same time add yourself as a maintainer. I was also only using header-only user but I am not using it so often at the moment so it would be great to have more active users as maintainers of this feedstock.