-
Notifications
You must be signed in to change notification settings - Fork 9
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
#1845: bundled fmt library can conflict with fmt installed in the same prefix #1847
#1845: bundled fmt library can conflict with fmt installed in the same prefix #1847
Conversation
We don't have a test for this but I have verified that this works in a spack install environment with |
Pipelines resultsPR tests (gcc-5, ubuntu, mpich) Build for 164ba7c
PR tests (clang-3.9, ubuntu, mpich) Build for 164ba7c
PR tests (gcc-10, ubuntu, openmpi, no LB) Build for 164ba7c
PR tests (clang-5.0, ubuntu, mpich) Build for 164ba7c
PR tests (gcc-9, ubuntu, mpich, zoltan) Build for 164ba7c
PR tests (gcc-6, ubuntu, mpich) Build for 164ba7c
PR tests (gcc-7, ubuntu, mpich, trace runtime, LB) Build for 164ba7c
PR tests (clang-9, ubuntu, mpich) Build for 164ba7c
PR tests (nvidia cuda 10.1, ubuntu, mpich) Build for 164ba7c
PR tests (clang-13, alpine, mpich) Build for 164ba7c
PR tests (nvidia cuda 11.0, ubuntu, mpich) Build for 164ba7c
PR tests (clang-11, ubuntu, mpich) Build for 164ba7c
PR tests (intel icpx, ubuntu, mpich) Build for 164ba7c
PR tests (clang-13, ubuntu, mpich) Build for 164ba7c
PR tests (clang-12, ubuntu, mpich) Build for 164ba7c
PR tests (clang-14, ubuntu, mpich) Build for 164ba7c
PR tests (gcc-8, ubuntu, mpich, address sanitizer) Build for 164ba7c
PR tests (gcc-11, ubuntu, mpich) Build for 164ba7c
PR tests (clang-10, ubuntu, mpich) Build for 164ba7c
PR tests (gcc-12, ubuntu, mpich) Build for 164ba7c
PR tests (intel icpc, ubuntu, mpich) Build for 164ba7c
|
@@ -14,6 +14,7 @@ set(FMT_SOURCES src/format.cc) | |||
|
|||
add_library(fmt ${FMT_SOURCES} ${FMT_HEADERS}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we change the CMake name as well? If a downstream code ends up picking this up, it'll be a bit confusing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm yeah, there might be a problem with duplicate targets if they happen to also be including fmt as a subdirectory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I'd be more in favor of a more permanent solution which would be to optionally force vt to use external versions of these libraries. That would be ideal for the spack case. I'd like to get this merged now however because it is causing some breakages in some internal CI processes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good for me as is.
Possibly another issue can be opened for the "more permanent solution" as discussed?
Added #1848 |
This PR renames
libfmt{d}.a
tolibfmt-vt{d}.a
in order to prevent conflicts with fmt installed in the same root. Additionally, it renames the include directory fromfmt
tofmt-vt
.Closes #1845