Skip to content
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

Add the Imported Target ZLIB::ZLIB for CMake. #1320

Open
hwhsu1231 opened this issue Jul 25, 2022 · 4 comments
Open

Add the Imported Target ZLIB::ZLIB for CMake. #1320

hwhsu1231 opened this issue Jul 25, 2022 · 4 comments

Comments

@hwhsu1231
Copy link

hwhsu1231 commented Jul 25, 2022

It is related to the pull request of the old repository: madler/zlib#337

There are two features I agree to support:

  • Add ZLIB::ZLIB target alias (to follow FindZLIB.cmake convention).
  • Add Export Targets, Config and ConfigVersion cmake file.
@hwhsu1231 hwhsu1231 changed the title Add ZLIB::ZLIB target alias (to follow FindZLIB.cmake convention). Add the Imported Target ZLIB::ZLIB for CMake. Jul 25, 2022
@mtl1979
Copy link
Collaborator

mtl1979 commented Jul 25, 2022

I have nothing against adding ZLIB::ZLIB alias when configured for compatibility mode, but when building for normal mode, we should use ZLIBNG::ZLIBNG alias instead...

Only issue I can think of is that when we are building both static and shared version, which one does the alias point to... Do we use STATIC variable like other projects do to decide?

@hwhsu1231
Copy link
Author

but when building for normal mode, we should use ZLIBNG::ZLIBNG alias instead

About that, I have some points to ask and discuss.

Usually, the IMPORTED library target name is related to the package name. For example, the IMPORTED target name of fmt library is fmt::fmt. Besides, the REPOSITORY name is often the same as the PROJECT name. However, I noticed that this library uses zlib in its project() command, but the repository name is zlib-ng.

Moreover, there are lots of downstream libraries using zlib as its dependency. For example, openssl, opencv, poco, qt...etc. However, the reason why this library zlib-ng is created is because the original one is no more updated.

Therefore, I have some questions to ask the AUTHOR of this library:

  • Will the project name be modified the same as the repository name?

  • Is it recommended that those downstream libraries replace zlib with zlib-ng in the future?

  • If so, do you think it is better to use different name of IMPORTED library target?

By the way, Conan Package Manager already provides zlib-ng in conan-center-index. And with the generator CMakeDeps, it provides the IMPORTED library target, zlib-ng::zlib-ng, for us to use.

@mtl1979
Copy link
Collaborator

mtl1979 commented Jul 28, 2022

By the way, Conan Package Manager already provides zlib-ng in conan-center-index. And with the generator CMakeDeps, it provides the IMPORTED library target, zlib-ng::zlib-ng, for us to use.

We can't rely on some third-party package manager to provide imported targets, as it is not guaranteed that everyone use same package manager. Exact spelling of the imported target is however open for discussion at this stage.

@bebuch
Copy link
Contributor

bebuch commented Jan 22, 2024

Are PR #1601 and Issue #1646 related to this Issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants