Rename cmake target files to avoid overwrite of PACKAGE_VERSION#1988
Merged
Dead2 merged 1 commit intozlib-ng:developfrom Oct 28, 2025
Merged
Rename cmake target files to avoid overwrite of PACKAGE_VERSION#1988Dead2 merged 1 commit intozlib-ng:developfrom
Dead2 merged 1 commit intozlib-ng:developfrom
Conversation
Contributor
WalkthroughChanged CMake install/export target names to produce separate "-targets.cmake" files and updated config template includes to reference those "-targets.cmake" files instead of the previous target scripts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CM as CMake (configure/install)
participant FS as Installed files
participant CT as Config templates
rect rgba(40,120,200,0.06)
CM->>FS: install(EXPORT ${EXPORT_NAME}-targets FILE "${EXPORT_NAME}-targets.cmake" ...)
Note right of FS: Produces\n- ${EXPORT_NAME}-config.cmake\n- ${EXPORT_NAME}-targets.cmake
end
rect rgba(100,180,100,0.06)
CT->>FS: include("${CMAKE_CURRENT_LIST_DIR}/${EXPORT_NAME}-targets.cmake")
Note right of CT: `zlib-config.cmake.in` and `zlib-ng-config.cmake.in` now include targets file
end
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (120)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1988 +/- ##
===========================================
+ Coverage 81.40% 83.14% +1.74%
===========================================
Files 161 161
Lines 13748 12960 -788
Branches 3149 3149
===========================================
- Hits 11191 10776 -415
+ Misses 1533 1153 -380
- Partials 1024 1031 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Collaborator
|
We are officially supporting building zlib-ng for Windows and thus also support case-insensitive file systems... This seems quite clean workaround to me, even though on updates, it does require manually deleting the old target files to work as intended. |
mtl1979
suggested changes
Oct 26, 2025
Collaborator
mtl1979
left a comment
There was a problem hiding this comment.
According to CMake documentation default of FILE parameter depends on EXPORT parameter of install() command. Documentation suggests customizing EXPORT parameter directly.
Contributor
Author
|
Ready for next review! :-) Sorry for noise, I missunderstood your suggestion in the first shot and didn't test offline carfully, Now it should be fine ;-) |
Dead2
approved these changes
Oct 27, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Originally there where these 5 CMake config files installed:
The file
zlib-config-version.cmakeis executed by CMake in a private scope, so the changes it make to variables likePACKAGE_VERSIONdon't leak into the scope wherefind_packageis called.The problem was, that the CMake generated
ZLIB.cmakeincludes all files that match the patternZLIB-*.cmake. That's fine on case sensitive filesystems, but on Windows this triggers the execution ofzlib-config-version.cmake.So any call of
find_package(ZLIB)overwritesPACKAGE_VERSION. In case of LLVM I get lld and clang in version 1.3.1 instead of 24.1.4. The worst thing is, that LLVM compiles fine. Only after using libclang with Qt6 I got the very strange error, that clang was found in version 1.3.1.However, renaming the target files with the common
-targetspostfix solves this problem.I think that is completly backwards compatible.