Prefer documented zlib library names #2354

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants
@richardthe3rd
Contributor

richardthe3rd commented Mar 2, 2018

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".

Prefer documented 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".

@bagder bagder added the build label Mar 3, 2018

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Mar 3, 2018

Member

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!

Member

bagder commented Mar 3, 2018

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!

@rodwiddowson

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"

@richardthe3rd

This comment has been minimized.

Show comment
Hide comment
@richardthe3rd

richardthe3rd Mar 3, 2018

Contributor

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.

Contributor

richardthe3rd commented Mar 3, 2018

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.

@MarcelRaad

This comment has been minimized.

Show comment
Hide comment
@MarcelRaad

MarcelRaad Mar 3, 2018

Member

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.

Member

MarcelRaad commented Mar 3, 2018

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.

@richardthe3rd

This comment has been minimized.

Show comment
Hide comment
@richardthe3rd

richardthe3rd Mar 3, 2018

Contributor

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?

Contributor

richardthe3rd commented Mar 3, 2018

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?

@richardthe3rd

This comment has been minimized.

Show comment
Hide comment
@richardthe3rd

richardthe3rd Mar 3, 2018

Contributor

(sorry, too easy to hit close)

Contributor

richardthe3rd commented Mar 3, 2018

(sorry, too easy to hit close)

@MarcelRaad

This comment has been minimized.

Show comment
Hide comment
@MarcelRaad

MarcelRaad Mar 3, 2018

Member

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.

Member

MarcelRaad commented Mar 3, 2018

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.

Add zlbwapi/zlibstat as options
zlib built from vstudio projects creates zlibwapi.lib/dll for the dynamic library and zlibstat.lib for the static library.
@MarcelRaad

👍

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Mar 4, 2018

Member

Thanks!

Member

bagder commented Mar 4, 2018

Thanks!

@bagder bagder closed this in cc1d4c5 Mar 4, 2018

@curl curl locked as resolved and limited conversation to collaborators Jun 2, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.