madler / zlib

A massively spiffy yet delicately unobtrusive compression library.
http://zlib.net/
Other
5.55k stars 2.43k forks source link

[Bug] ZLib 1.3.1 does not build on Windows #919

Closed levicki closed 6 months ago

levicki commented 6 months ago

Problem:

zlibvc.def(4): fatal error LNK1118: syntax error in 'VERSION' statement

From zlibvc.def:

LIBRARY
; zlib data compression and ZIP file I/O library

VERSION     1.3.1 <- this isn't valid version

If you read Microsoft documentation, this is the version which consist only of major and minor version and it gets written into PE file header (which means it could affect DLL loading by the OS) — it is not written into DLL resource table and it's not the same one users see when they check the details tab of the file in Windows Explorer.

This version should not be specified in module definition file unless you know what you are doing and have a very specific reason to do it. The fix is to remove the VERSION statement from zlibvc.def.

madler commented 6 months ago

https://github.com/madler/zlib/commit/b14484997a50c01b8d78f9db32516423573fc083

levicki commented 6 months ago

b144849

Thanks for a quick fix Mark, but I still don't see the value in setting VERSION to 1.3 — what you are doing is basically setting these fields:

image

They are completely invisible to end users and library users alike, and even if library users wanted to check them programmatically they'd have to deal with PE header structures only to get a truncated version.

If I am not mistaken, there's already zlibVersion() API as well as a way to extract a full file version from the PE resource table using Windows API. I must admit I am curious now as to why that was done.

madler commented 6 months ago

I left the VERSION's in there for now since they've always been there. (I have no idea why.) However I may be deleting the entire vstudio directory in favor of users using cmake to generate the build files.

levicki commented 6 months ago

@madler

I left the VERSION's in there for now since they've always been there. (I have no idea why.)

I guess it doesn't hurt to remain, but I hope that someone somewhere doesn't depend on it being there by now.

However I may be deleting the entire vstudio directory in favor of users using cmake to generate the build files.

Speaking of that, I tried using CMake to build it (by generating for VS 2022 x64) and it kinda works.

There's a difference in output file naming though: vstudio CMake
Debug DLL zlibwapi.dll zlibd.dll
Debug import LIB zlibwapi.lib zlibd.lib
Debug static LIB zlibstat.lib zlibstaticd.lib
Release DLL zlibwapi.dll zlib.dll
Release import LIB zlibwapi.lib zlib.lib
Release static LIB zlibstat.lib zlibstatic.lib

Also, there's a considerable size difference in produced files (both .lib and .dll) so you might want to investigate that.

When we are on the subject of naming, allow me to express my vote for having the same names for files produced by Debug and Release configurations.

Having same .lib and .dll names means I only have to set each of those options once when setting up a new project so I hope you stick with same output names like you did before.