chronoxor / CppServer

Ultra fast and low latency asynchronous socket server & client C++ library with support TCP, SSL, UDP, HTTP, HTTPS, WebSocket protocols and 10K connections problem solution
MIT License
1.43k stars 284 forks source link

fmt header dependncies in cppcommon which bleed into cppserver #74

Open eniv opened 2 years ago

eniv commented 2 years ago

Hi, I ran into this when I included cppserver in a custom C++ project, and was specifically including server/asio/tcp_client.h. This file is including several files from CppCommon some of them (for example: /cppserver/modules/CppCommon/include/system/stack_trace.inl and /cppserver/modules/CppCommon/include/system/source_location.inl) depend on the fmt library but do not specifically include the fmt header file they depend on. This creates an issue for compilation unless the user will include the fmt header before including the high level file from cppserver So for server/asio/tcp_client.h, this is required:

cppserver/modules/CppCommon/modules/fmt/include/fmt/ostream.h"
server/asio/tcp_client.h

I think it would be better if these files explicitly include the fmt header file they depend on.

chronoxor commented 2 years ago

Fixed in CppCommon. Please update and check on your side.

eniv commented 2 years ago

Great. I updated and can confirm that this fixed the issue. Thank you very much!

eniv commented 2 years ago

Hi Ivan, I'm opening this again because I realized that the following code section (in CppCommon include/system/stack_trace_inl and include/system source_location.inl):

#if defined(FMT_VERSION)
template <> struct fmt::formatter<CppCommon::StackTrace> : ostream_formatter {};
#endif

Can only work for FMT_VERSION newer than 9 and not on any fmt. Ubuntu 22.04 LTS is packaged with fmt 8.1 by default and those header files fail to compile.

eniv commented 2 years ago

For reference, I ran into this only because FMT updated their master branch which broke a header only library I'm using (spdlog) that includes FMT as well. I didn't realize it, but spdlog was using the libFMT headers from CppCommon because they were both included in the same source file. image

Ideally, there is more encapsulation. Maybe it's possible to remove the FMT headers from CppServer's (and CppCommon) public headers?

The .gitlinks file often points to the head commit of master branches of 3rd party dependencies. This means that things will break every time a dependency developer made a breaking change on the master branch. Perhaps it is better to point to specific commits?