zlib: fix lib name in CMakeLists directly instead of fragile renaming

[SpaceIm]

closes #10770

I don't know whether it was a good idea in the first place to rename zlibstatic on Windows, and remove d postfix for debug build type, but now it's done directly in CMakeLists with simple criteria (zlib for Windows, z for other platforms).

---

• I've read the guidelines for contributing.
• I've followed the PEP8 style guides for Python code in the recipes.
• I've used the latest Conan client version.
• I've tried at least one configuration locally with the conan-center hook activated.

[ghost]

I detected other pull requests that are modifying zlib/all recipe:

• Fix zlib packaging on latest conan not renaming binary appropriately #8789

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

[SpaceIm]

Well, it may generate a different dll name for MinGW (it should be zlib1.dll).
I'll push a slightly better patch to match zlib names for MinGW:
https://github.com/madler/zlib/blob/v1.2.12/win32/Makefile.gcc#L28-L30
https://packages.msys2.org/package/mingw-w64-x86_64-zlib

No change for msvc, but like MinGW, build files are not consistent upstream (less discrepancy after our patch, only import lib differs):
https://github.com/madler/zlib/blob/v1.2.12/win32/Makefile.msc#L16-L18

see madler/zlib#652

[SpaceIm]

Do you think we should honor import lib name from https://github.com/madler/zlib/blob/v1.2.12/win32/Makefile.msc#L16-L18 for Visual Studio (zdll.lib), or it's too dangerous for downstream recipes with fragile build files?

[madebr]

Linux systems would just provide both by providing a symbolic link.

FindZLIB.cmake looks for all names, but higher priority to z: https://github.com/Kitware/CMake/blob/e00e67cb924495aab72bfc60dec0451a6566d8a2/Modules/FindZLIB.cmake#L80

I've not tested building packages with a shared zlib dependency, but can assume most autotools libraries will happily use z.lib instead of zdll.lib.
I think the risk is minimum.

If some dependency breaks, we can fix that dependency only by e.g. a patch or copying z.lib to zdll.lib.

[SpaceIm]

Linux systems would just provide both by providing a symbolic link.

It's always libz.a and libz.so on *Nix systems.

FindZLIB.cmake looks for all names, but higher priority to z: https://github.com/Kitware/CMake/blob/e00e67cb924495aab72bfc60dec0451a6566d8a2/Modules/FindZLIB.cmake#L80

I'm not afraid by FindZLIB, it tests all flavors, even weird ones.

I've not tested building packages with a shared zlib dependency, but can assume most autotools libraries will happily use z.lib instead of zdll.lib.

It would be zdll.lib instead of zlib.lib (for cl like compilers only). I've written a summary of all flavors for Windows in upstream repo: madler/zlib#652

[madebr]

It would be zdll.lib instead of zlib.lib (for cl like compilers only).

I think we have 2 major possible issues:

• autotools: I guess most autotools projects target mostly linux and mingw, which always have libz.a, libz.so or libz.dll.a (https://packages.msys2.org/package/mingw-w64-x86_64-zlib)
  Most autotools project do some level of autodection, which mostly only includes z
  I just checked autoconf-archive, and it only looks for z: https://git.savannah.nongnu.org/cgit/autoconf-archive.git/tree/m4/ax_check_zlib.m4#n112
• Visual studio projects: those don't do any auto-detection, so it depends on the maintainer whether he wants to use a shared or static zlib...

IOW, there will always be risks.
We should accept that there simply does not exist a correct library name.
To make our lives easier, perhaps add a user environment variable which dependencie can use to create a correctly named library. (perhaps not needed because it is always in libs)
An alternative could be to just provide alll: zdll.lib, z.lib and zlib.dll.
But that's perhaps wasteful.
Otoh, if all are the same compression should be able to minimize the overhead :P

[SSE4]

@madebr what about non-autotools projects? e.g. ffmpeg: https://github.com/FFmpeg/FFmpeg/blob/master/configure#L4585

[SpaceIm]

For ffmpeg, I guess something like https://github.com/FFmpeg/FFmpeg/blob/710dce131fdb6c1ebec1f26a7a4173d82d828330/configure#L4585 should be patched dynamically like we do already for several handwritten Makefiles hardcoding a specific flavor of their dependencies.

[madebr]

@madebr what about non-autotools projects? e.g. ffmpeg: https://github.com/FFmpeg/FFmpeg/blob/master/configure#L4585

Same as for vcxproj: pray we don't need to do deep surgery.

[conan-center-bot]

All green in build 5 (9e152ff157af266bc54b7f98ceefebbc4a97c6cc):

• zlib/1.2.12@:
  All packages built successfully! (All logs)

• zlib/1.2.11@:
  All packages built successfully! (All logs)

[Chnossos]

Removing the d from zlib1.dll in Debug mode broke all compiled binaries like lupdate.exe from the qt package.

[SpaceIm]

You have to recompile msvc debug binaries which have a dependency to zlib shared.

[AndreyMlashkin]

There is a conan flag --build outdated --build cascade for this

[p-groarke]

@SpaceIm The debug postfix shouldn't have been removed. It shouldn't be up to package maintainers to choose the output of the build. zlib uses a postfix, and thus, the conan package should too.

[SpaceIm]

zlib doesn't have a consistent naming convention for windows, and doesn't add a postfix in 2 build files among the 3 available for Visual Studio. For simplicity and to be able to fix original issue related to this PR, we follow Makefile.msvc convention of upstream project instead of conventions in its CMakeLists. See madler/zlib#652

Nobody should assume a specific zlib file name on Windows, it can be any of these names depending on which build file has been used: zlib.lib, zdll.lib, zlibstat.lib, zlibwapi.lib, zlibstatic.lib, zlibstaticd.lib, zlibd.lib

[p-groarke]

@SpaceIm
From https://www.zlib.net/fossils/zlib-1.2.13.tar.gz CMakeList :

(sorry for image, they don't have a github apparently).

In the context of conan consuming conan producing conan, I think it doesn't really matter. But I'm guessing we aren't alone in a "conan adoption" state where conan must live side-by-side our (horrible) legacy systems. It must integrate and "play ball" with other systems for the time being, until conan conquers the world ;)

We can agree to disagree, but I strongly believe this was a mistake. Though maybe I don't understand the true motivation behind the change, it's breaking a lot of our builds.

[SpaceIm]

As I said, there is no convention in zlib repo, so you can't assume that a package manager would have used upstream CMakeList to build zlib. Upstream Makefile.msvc or MSBuild files produce different file names.

Anyway, this PR didn't remove d postfix, it was already removed before as a post process during packaging.