Skip to content

Rename cmake target files to avoid overwrite of PACKAGE_VERSION#912

Closed
bebuch wants to merge 1 commit into
zlib-ng:developfrom
bebuch:fix-cmake-config-files
Closed

Rename cmake target files to avoid overwrite of PACKAGE_VERSION#912
bebuch wants to merge 1 commit into
zlib-ng:developfrom
bebuch:fix-cmake-config-files

Conversation

@bebuch
Copy link
Copy Markdown
Contributor

@bebuch bebuch commented Oct 29, 2025

Same patch as zlib-ng/zlib-ng#1988 for zlib-ng.

@nmoinvaz nmoinvaz requested a review from Copilot October 29, 2025 17:06
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR standardizes CMake export naming conventions by appending -targets suffix to the export name throughout the installation configuration. This ensures consistency with CMake best practices where export files are typically named <package>-targets.cmake rather than <package>.cmake.

Key changes:

  • Updated CMake export name from ${MINIZIP_TARGET} to ${MINIZIP_TARGET}-targets
  • Modified corresponding include statement to reference the correct export file name

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Coeur
Copy link
Copy Markdown
Collaborator

Coeur commented Oct 29, 2025

One minute before the zlib-ng PR you link to was merged, another zlib-ng PR was merged with renamings and concerns by @nmoinvaz : zlib-ng/zlib-ng#1970

So we might need to look at both zlib-ng changes to evaluate what's the next move for minizip-ng.

@bebuch
Copy link
Copy Markdown
Contributor Author

bebuch commented Oct 30, 2025

This looks like a completely distinct thing. My PRs only affect the target file names, whereas this other PR only affects the target names. Note that target file names are an implementation detail. The CMake config file API doesn't depend on them, so my change won't break anything.

@nmoinvaz
Copy link
Copy Markdown
Member

This will be commit as part of #982.

@nmoinvaz nmoinvaz closed this Apr 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants