billyquith / ponder

C++ reflection library with Lua binding, and JSON and XML serialisation.
http://billyquith.github.io/ponder/
Other
640 stars 93 forks source link

Ponder format lib clashes if another format lib is used #59

Closed Weeena closed 6 years ago

Weeena commented 7 years ago

Ponder now uses the popular formatting library by Victor Zverovich (files include\ponder\detail\format.hpp and src\format.cpp).

Since this formatting library is rather popular, it may be the case that some project or some other library also makes use of it. It this is the case, the two libraries clash. These are the reasons:

Additional note (= bug?): In Ponder's formatting library files, there is code that is controlled by a macro FMT_HEADER_ONLY. Alas, FMT_HEADER_ONLY must not be defined. If it is defined, there is a compile error since there is no file format.cc that could be included.

In our case, the other library that also makes use of the formatting library by Victor Zverovich is spdlog, a library for logging. This library is somewhat aware of the fact that there may be another version of the formatting library already used. To make use of this other library, it is possible to set a macro SPDLOG_FMT_EXTERNAL for spdlog. In this case, as the comment for this macro states, "spdlog will try to include <fmt/format.h> so set your -I flag accordingly". Alas, this possiblity will not work with Ponder's formatting library since 1) the header file has extension .hpp rather than .h and 2) the files are modified such that the formatting is in namespace ponder/detail.

How could it be solved? I don't know what the best way would be. One way would be to use the formatting library without any modifications (as spdlog does). The other way (which I would personally rate as unfavorable) would be to modify it such that all clashes are eliminated.

billyquith commented 7 years ago

This is interesting user feedback. I did expect to have this conversation one day as this clash is inevitable when a popular library is integrated. It already exists in the ponder namespace, but this is only a partial attempt at "absorption". My feeling was to modify it so that it becomes internal, although I appreciate that will lead to substantial modifications. It is used in some templates so cannot be completely hidden in the implementation files.

The thinking here is that at some point in the future there may also exist a version of format that, or a library that it is dependent on a version of format, that is not compatible with this one.

Additional note (= bug?): In Ponder's formatting library files, there is code that is controlled by a macro FMT_HEADER_ONLY. Alas, FMT_HEADER_ONLY must not be defined. If it is defined, there is a compile error since there is no file format.cc that could be included.

I renamed format.cc to format.cpp for consistency and FMT_HEADER_ONLY isn't used.

spdlog's dual modes are of interest: format internal and external.

Another option is to remove fmt and treat it as a third party library. It would just be installed like other libraries. I didn't do this because I wanted Ponder to be stand-alone. When I revisit XML and JSON serialisation I also hope to do the same. These libraries will be easier to hide as the Ponder API shouldn't require them.

Options:

Dual mode is quite appealing since I'd like the default to be format included. Since it can be included header only this simplifies things, although this does have the problem of conflicting versions. That said, I'm not sure there is a massive risk here and I like to update libraries anyway.

Ponder also has header only include libs now, like the Lua scripting extension. To link that you add a define, rather than link a library. This simplifies the build config.

billyquith commented 7 years ago

I think I'll just remove fmt and use iostream. I was anticipating using it for serialisation but I think individual serialisation solutions will take care of this.

billyquith commented 6 years ago

Format library removed in dev branch.