Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fmt library incompatibilities #2216

Closed
nmm0 opened this issue Nov 14, 2023 · 1 comment · Fixed by #2221
Closed

fmt library incompatibilities #2216

nmm0 opened this issue Nov 14, 2023 · 1 comment · Fixed by #2221
Assignees

Comments

@nmm0
Copy link
Collaborator

nmm0 commented Nov 14, 2023

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
Copy link
Contributor

Related #1848

JacobDomagala added a commit that referenced this issue Dec 7, 2023
JacobDomagala added a commit that referenced this issue Dec 7, 2023
JacobDomagala added a commit that referenced this issue Dec 11, 2023
…ersions. Make 9.1.0 fmt version as required for external fmt
JacobDomagala added a commit that referenced this issue Dec 12, 2023
JacobDomagala added a commit that referenced this issue Dec 12, 2023
JacobDomagala added a commit that referenced this issue Dec 12, 2023
…ersions. Make 9.1.0 fmt version as required for external fmt
JacobDomagala added a commit that referenced this issue Apr 30, 2024
JacobDomagala added a commit that referenced this issue Apr 30, 2024
…ersions. Make 9.1.0 fmt version as required for external fmt
JacobDomagala added a commit that referenced this issue Apr 30, 2024
cwschilly pushed a commit that referenced this issue Sep 20, 2024
cwschilly pushed a commit that referenced this issue Sep 20, 2024
cwschilly pushed a commit that referenced this issue Sep 20, 2024
…ersions. Make 9.1.0 fmt version as required for external fmt
cwschilly pushed a commit that referenced this issue Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants