community: conan-spdlog doesn't define SPDLOG_FMT_EXTERNAL when using the cmake_paths generator
Upstream issue: https://github.com/gabime/spdlog/issues/904
I spent the last few days tracking down a segfault which turned out to be an ODR violation because I was using both spdlog and fmt without -DSPDLOG_FMT_EXTERNAL. If I was using the cmake generator, the conan-spdlog package would have set that for me. However, I was using cmake_paths (or rather, I was using conan_set_find_paths()). It would be nice if we could do something to make it harder to run into this bug.
For instance, we could patch the spdlogConfig.cmake to make the spdlog::spdlog package set SPDLOG_FMT_EXTERNAL if the fmt_external option is set.
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 1
- Comments: 24 (12 by maintainers)
Commits related to this issue
- - remote fmt_external option Signed-off-by: SSE4 <tomskside@gmail.com> — committed to bincrafters/conan-spdlog by SSE4 6 years ago
@solvingj unfortunately, I am not very familiar with spdlog project, as I’ve never used it before studying the doc (https://github.com/gabime/spdlog/wiki/8.-Tweaking), my understanding that it may use either its own copy of
fmtlibrary, or external one, andSPDLOG_FMT_EXTERNALcontrols exactly that usage. this clearly introduces different package ids, as CMake config files will be different, so we should do one of the following:header_onlyfrom the recipefmt_externaloption althogether and always require external fmt installationI personally vote for the second approach, as it make things more simple. probably, it makes recipe a bit less flexible (as it removes an option to use embedded fmt installation), but for me personally embedded fmt not very transparent (read - not clear which fmt version is in use), and I’d prefer to control fmt version via conan. if you need another version - you always can override it in
conanfile.txt, right?@Quincunx271 I’ve applied required changes, please give an another try
@Quincunx271 okay, I am closing the issue then
@Quincunx271 applied changes to 1.1.0 and 1.2.1 branches, please give a try
okay, there were no replies for 20 days, so I am removing
fmt_externaloption thenI’m leaning towards removing the
fmt_externaloption altogether and just requiring an external fmt. I don’t see any real downsides, since we are in the context of conan anyway