-
-
Notifications
You must be signed in to change notification settings - Fork 430
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
fix_zstd_libraries_cmake #554
Conversation
set(ZSTD_VERSION ${ZSTD_VERSION_STRING}) | ||
if(ZSTD_FOUND) | ||
set(ZSTD_VERSION ${ZSTD_VERSION_STRING}) | ||
if(TARGET libzstd_static) |
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.
Perhaps this should be:
if(NOT DEFINED BUILD_SHARED_LIBS OR NOT ${BUILD_SHARED_LIBS})
Like on line 379.
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.
because there is not always the case that we would get shared libs, for e.g. if vcpkg is used to link zstd in Linux, then build would fail, because vcpkg don't support shared in linux
set(ZSTD_VERSION ${ZSTD_VERSION_STRING}) | ||
if(ZSTD_FOUND) | ||
set(ZSTD_VERSION ${ZSTD_VERSION_STRING}) | ||
if(TARGET libzstd_static) |
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.
Why not do:
set(ZSTD_LIBRARIES libzstd_static)
Then we can use:
list(APPEND MINIZIP_LIB ${ZSTD_LIBRARIES})
down below in if(ZSTD_FOUND AND NOT MZ_FORCE_FETCH_LIBS)
like before?
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.
i can simplify this a bit, i just added this because I thought it would be pointless to add
set(PC_PRIVATE_LIBS "${PC_PRIVATE_LIBS} -lzstd")
list(APPEND MINIZIP_DEP_PKG ZSTD)
if we are linking statically, but after further thinking, it would fail if minizip is itself is static
2b0c121
to
11d2dd1
Compare
Thanks! |
Fixes zstd linking with find_package