GDCM: fix missing zlib include path when ITK_USE_SYSTEM_ZLIB#6563
GDCM: fix missing zlib include path when ITK_USE_SYSTEM_ZLIB#6563dzenanz wants to merge 1 commit into
Conversation
The original error message was:
------ Build started: Project: gdcmCommon, Configuration: Debug x64 ------
gdcmDeflateStream.cxx
M:\a\Slicer-deb\ITK-build\Modules\ThirdParty\ZLIB\src\itk_zlib.h(35,11): error C1083: Cannot open include file: 'zlib.h': No such file or directory
Root cause:
The include chain:
gdcmDeflateStream.cxx
→ zipstreamimpl.h → gdcm_zlib.h
→ [GDCM_USE_SYSTEM_ZLIB defined] → itk_zlib.h
→ [ITK_USE_SYSTEM_ZLIB defined] → #include <zlib.h> ← C1083
When the bundled zlib is used, GDCM receives headers through
ITK::ITKZLIBModule's INTERFACE_INCLUDE_DIRECTORIES, so the empty string
remains correct. When the system zlib is used, ZLIB_INCLUDE_DIR (a CMake
cache variable populated by find_package(ZLIB)) is forwarded as
ZLIB_INCLUDE_DIRS so GDCM's include_directories() call picks it up.
github-actions
Bot
added
type:Infrastructure
|
@greptileai review |
|
| Filename | Overview |
|---|---|
| Modules/ThirdParty/GDCM/src/CMakeLists.txt | Updates the private GDCM zlib include-directory handoff for system-zlib builds. |
Reviews (2): Last reviewed commit: "COMP: GDCM: fix missing zlib include pat..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Here are the lines that pass the ZLIB config to ITK: The current expectation would be that when find_package ZLIB with ZLIB_ROOT alone set, it would create the ZLIB::ZLIB imported target with the properties of the include and library paths. This should make the other variable not needed. Clearly something is not working correctly there, when the ZLIB::ZLIB import target to ITK needs to be inspected to determine what is going on the the FindZLIB in this situation. |
| set(ZLIB_INCLUDE_DIRS "") | ||
| if(ITK_USE_SYSTEM_ZLIB) | ||
| # ZLIB_INCLUDE_DIR is populated by find_package(ZLIB) in ITK's ZLIB module. | ||
| # Pass it explicitly so GDCM's root CMakeLists can find the real system <zlib.h> |
There was a problem hiding this comment.
These statements are verbose and misleading.
There was a problem hiding this comment.
Is the suggested course of actions to reduce/remove comments, or to abandon the PR? I was trying to build Slicer from this commit: dzenanz/Slicer@d177327
There was a problem hiding this comment.
It looks like that branch is based on a couple months ago version of ITK. There have been several fixes for using system libraries in just the past couple of weeks that should be included. So this issue may already be fixed in ITK main?
There was a problem hiding this comment.
Main from 2 days ago is merged there. Nothing related to GDCM got merged since.
There was a problem hiding this comment.
I tried to reproduce the error. I checkout dzenanz/Slicer@d177327 and was successfully able to built the ITK target on a clean build. So I wasn't able to reproduce to look at this further.
There was a problem hiding this comment.
I was able to build your slicer branch with your ITK 7ed87c5 (before this change) and was not able to reproduce the issue either. This is on a linux system while above it is reported as a windows system.
I wonder if this reported issue is related to the multi-build (release/debug) VS system.
There was a problem hiding this comment.
Let me try building that on Linux.
hjmjohnson
left a comment
hjmjohnson
left a comment
There was a problem hiding this comment.
I assume this was a transient build error, and that a clean build could not replicate the problem as Brad indicated.
3 participants
The original error message was:
While trying to build within Slicer. While this does fix it for Slicer, I am not sure whether this is the best fix.
PR Checklist