-
-
Notifications
You must be signed in to change notification settings - Fork 55.7k
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
Use zlib target instead of ZLIB_INCLUDE_DIRS and ZLIB_LIBRARIES #25569
base: 4.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1403,8 +1403,4 @@ if(ENABLE_SOLUTION_FOLDERS) | |
set_target_properties(${ZLIB_INSTALL_LIBRARIES} PROPERTIES FOLDER "3rdparty") | ||
endif() | ||
|
||
if(NOT BUILD_SHARED_LIBS) | ||
ocv_install_target(${ZLIB_INSTALL_LIBRARIES} EXPORT OpenCVModules ARCHIVE DESTINATION ${OPENCV_3P_LIB_INSTALL_PATH} COMPONENT dev) | ||
endif() | ||
Comment on lines
-1406
to
-1408
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It breaks distribution with self-built zlib-ng. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is moved to cmake/OpenCVFindLibsGrfmt.cmake which means to install target when building or finding zlib. At first, I didn't touch these lines. However, there is a CI check fails without these lines. I don't remember which check now, sorry. |
||
|
||
ocv_install_3rdparty_licenses(${ZLIB_INSTALL_LIBRARIES} LICENSE.md) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,8 +41,6 @@ endif() | |
configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/zconf.h.cmakein | ||
${CMAKE_CURRENT_BINARY_DIR}/zconf.h @ONLY) | ||
|
||
ocv_include_directories("${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}") | ||
|
||
set(ZLIB_PUBLIC_HDRS | ||
${CMAKE_CURRENT_BINARY_DIR}/zconf.h | ||
zlib.h | ||
|
@@ -77,6 +75,9 @@ set(ZLIB_SRCS | |
) | ||
|
||
add_library(${ZLIB_LIBRARY} STATIC ${OPENCV_3RDPARTY_EXCLUDE_FROM_ALL} ${ZLIB_SRCS} ${ZLIB_PUBLIC_HDRS} ${ZLIB_PRIVATE_HDRS}) | ||
target_include_directories(${ZLIB_LIBRARY} PUBLIC | ||
"$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR};${CMAKE_CURRENT_SOURCE_DIR}>" | ||
"$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>") | ||
set_target_properties(${ZLIB_LIBRARY} PROPERTIES DEFINE_SYMBOL ZLIB_DLL) | ||
|
||
ocv_warnings_disable(CMAKE_C_FLAGS -Wshorten-64-to-32 -Wattributes -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations -Wshift-negative-value | ||
|
@@ -87,9 +88,9 @@ ocv_warnings_disable(CMAKE_C_FLAGS -Wshorten-64-to-32 -Wattributes -Wstrict-prot | |
) | ||
|
||
set_target_properties(${ZLIB_LIBRARY} PROPERTIES | ||
OUTPUT_NAME ${ZLIB_LIBRARY} | ||
OUTPUT_NAME "${ZLIB_LIBRARY}" | ||
DEBUG_POSTFIX "${OPENCV_DEBUG_POSTFIX}" | ||
COMPILE_PDB_NAME ${ZLIB_LIBRARY} | ||
COMPILE_PDB_NAME "${ZLIB_LIBRARY}" | ||
COMPILE_PDB_NAME_DEBUG "${ZLIB_LIBRARY}${OPENCV_DEBUG_POSTFIX}" | ||
ARCHIVE_OUTPUT_DIRECTORY ${3P_LIBRARY_OUTPUT_PATH} | ||
) | ||
|
@@ -98,8 +99,4 @@ if(ENABLE_SOLUTION_FOLDERS) | |
set_target_properties(${ZLIB_LIBRARY} PROPERTIES FOLDER "3rdparty") | ||
endif() | ||
|
||
if(NOT BUILD_SHARED_LIBS) | ||
ocv_install_target(${ZLIB_LIBRARY} EXPORT OpenCVModules ARCHIVE DESTINATION ${OPENCV_3P_LIB_INSTALL_PATH} COMPONENT dev) | ||
endif() | ||
Comment on lines
-101
to
-103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The patch breaks build with own zlib instance, e.g. static build, cross-compile, etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same reason as above. |
||
|
||
ocv_install_3rdparty_licenses(zlib LICENSE) | ||
asmorkalov marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to be restored. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This patch does install a license. It just uses variable |
||
ocv_install_3rdparty_licenses(${ZLIB_LIBRARY} LICENSE) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
find_dependency(ZLIB REQUIRED) |
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.
_LIBRARY
is not used in documentation. Only_LIBRARIES
is there:https://cmake.org/cmake/help/book/mastering-cmake/chapter/Finding%20Packages.html
The same is related to FindZLIB: https://cmake.org/cmake/help/latest/module/FindZLIB.html
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.
Yeah. Previously OpenCV used this variable. So, I used it. Essentially, we just need a name for interface library if we link system zlib and target library if we build zlib manually.