-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Prefer documented zlib library names #2354
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
Conversation
Check for existence of import and static libraries with documented names and use them if they do. Fallback to previous names. According to https://github.com/madler/zlib/blob/master/win32/README-WIN32.txt on Windows, the names of the import library is "zdll.lib" and static library is "zlib.lib".
I'd appreciate a 👍 or otherwise positive comments from a windows dev or two first, but to me (as a non-windows dev) it looks fine! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can make not comment on the static side but it seems OK to me if the names are OK. I note that in the static leg it looks for zlib.lib, but the default lib is zlib_a.lib
I have done a build using my usual build process and it shows no regressions. But that build, while using MakeFileBuiild.vc is nonstandard and so shouldn't be relied upon.
So in gerrit-speak "looks OJK to me, but some wlse must confirm"
To be fair, I haven't checked the static branch either, but the names are consistent with my reading of https://github.com/madler/zlib/blob/master/win32/README-WIN32.txt - I don't see any mention of zlib_a.lib there. For builds that are using zlib_a.lib it should continue to work same as before. |
How did you build zlib to get these output file names? When using the Visual Studio 14 project files from versoon 1.2.11, they are zlibwapi.lib/dll for the dynamic library and zlibstat.lib for the static library. |
I used nmake on https://github.com/madler/zlib/blob/master/win32/Makefile.msc I had seen reference to zlibwapi as well - a bit more reading suggests there are 2 "official" builds - according to https://github.com/madler/zlib/blob/master/contrib/vstudio/readme.txt "There is also an official DLL build of zlib, named zlib1.dll" What about adding another exists check on the library name? |
(sorry, too easy to hit close) |
Ah, I had just gotten these names as well when using the makefile build in the win32 folder. Yes, I think checking for all three would be best. |
zlib built from vstudio projects creates zlibwapi.lib/dll for the dynamic library and zlibstat.lib for the static library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Thanks! |
As promised on mailing list "Building on Windows: zlib library names in winbuild/MakeBuild.vc" this PR means can build on Windows with zlib without needing to modify zlib library names.
Check for existence of import and static libraries with documented names and use them if they do. Fallback to previous names.
According to https://github.com/madler/zlib/blob/master/win32/README-WIN32.txt on Windows, the names of the import library is "zdll.lib" and static library is "zlib.lib".