DARMA-tasking / vt

DARMA/vt => Virtual Transport
Other
35 stars 8 forks source link

fmt library incompatibilities #2216

Closed nmm0 closed 4 months ago

nmm0 commented 9 months ago

Recently, I've been working on improving the logging capabilities of distBVH, and ran into a conflict using a logging library like spdlog and vt.

While there is no ODR violation problem (like there was before we namespaced everything in libfmt and CLI11), my issue is that because spdlog uses fmt internally, it tries to use the vt bundled version of fmt (or if I change the include order vice-versa). That is because we make vt an inline namespace inside of fmt for our bundled fmt functions. That was done in order to avoid ODR violations when linking against a library that also uses libfmt but with a different version.

The problem is partly that we use a very old version of fmt (7.x), and there are a lot of breakages between 7.x and 10.x, the latest fmt. 7.x constructs will not compile in 10.x and vice-versa. That means that spdlog using the fmt provided by vt won't compile, and including my newer fmt headers before including vt will cause breakages in vt.

I think this can be broken up more or less into two issues. One is the inherent incompatibility between different versions of fmt. The second is that we "pollute" the fmt namespace with our older version of fmt.

To address the first, I think the best thing would be to update to more recent versions of fmt like 10.x. The newer versions are also closer to what was proposed and accepted into the C++ standard, so it will be easier to move to just a pure standard library solution if we ever want to do that.

The second problem could be addressed by a cmake switch such as "vt_external_fmt" or something similar. This would disable our internal fmt and instead call cmake find_package. This is the approach that spdlog itself uses (it bundles fmt but this can be disabled using that flag). This would also solve any potential ODR violations from duplicate symbols as downstream projects should specify this if they also use fmt, so we wouldn't need to do any weird namespace modifications to fmt.

JacobDomagala commented 9 months ago

Related https://github.com/DARMA-tasking/vt/issues/1848