madler / zlib

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

zlibstatic - purpose, make optional? #575

Open aleksandaratanasov opened 3 years ago

aleksandaratanasov commented 3 years ago

I am assuming that zlibstatic is the static version of zlib. I am confused as to how building zlibstatic is controlled. From what I am seeing

add_library(zlib SHARED ${ZLIB_SRCS} ${ZLIB_ASMS} ${ZLIB_DLL_SRCS} ${ZLIB_PUBLIC_HDRS} ${ZLIB_PRIVATE_HDRS})
add_library(zlibstatic STATIC ${ZLIB_SRCS} ${ZLIB_ASMS} ${ZLIB_PUBLIC_HDRS} ${ZLIB_PRIVATE_HDRS})

is outside any control block (compiler- or platform-specific, e.g. CYGWIN, MSVC, MINGW etc., installation routine etc.) so it means that it is always built regardless of whether BUILD_SHARED_LIBS is set or not.

If this is indeed the case I suggest making the process optional depending on the BUILD_SHARED_LIBS (if set, static will not be built, otherwise it will) or some other variable in case the case when both versions of the library are required. Example:

if(BUILD_SHARED_LIBS)
    add_library(zlib SHARED ${ZLIB_SRCS} ${ZLIB_ASMS} ${ZLIB_DLL_SRCS} ${ZLIB_PUBLIC_HDRS} ${ZLIB_PRIVATE_HDRS})
else()
    add_library(zlibstatic STATIC ${ZLIB_SRCS} ${ZLIB_ASMS} ${ZLIB_PUBLIC_HDRS} ${ZLIB_PRIVATE_HDRS})
endif()

Further tweaks are necessary since the code above produces binary such as zlibd1, which should not be possible (adding release AND debug suffix to the binary's name). It is just a demonstration of what I am describing.

The reason why I am suggesting this is simple: a project should not build specific binaries unless told so. In my case I don't want to build the static version. While zlib is small, building unwanted binaries adds to the complexity of a build process that depends on zlib including build time, sources of error and last but not least automated deployment (e.g. copying all *.lib files from the binary directory by a simple file(GLOB ...) call and automatically including them into another project; currently not possible without extra work since both zlib1.lib/zlibd.lib and zlibstatic/zlibstaticd.lib (Windows platforms) are being captured).

timosachsenberg commented 2 years ago

We have a similar issue. Is there any chance to get this fixed in the main zlib repo?

nmoinvaz commented 2 years ago

You can use add_subdirectory(zlib EXCLUDE_FROM_ALL) and it won't build all the projects.

timosachsenberg commented 2 years ago

Hi @nmoinvaz not sure if this helps here. The context of our issue is that we build the zlib dependency as part of a contrib repository and then use FindZLIB in our main project to include it into the build. If zlib builds both static and dynamic always the dynamic version gets picked up by FindZLIB. As @aleksandaratanasov explained, it would be good to choose what static vs. dynamic gets build as this would also resolve the problem with FindZLIB.

Vollstrecker commented 2 years ago

Depending this on BUILD_SHARED_LIBS is the worst you can do. The complete concept of this var is just to set a default if the dev didn't. This would mean for every packager to always run two complete configure/build/install cylces. The right approach here would be to make both of them optional to let you turn one off.

And yes, you can use EXCLUDE_FROM_ALL and then let your FindZLIB point to the target here, so this one would be built.

I'm thinking about doing this and merging almost all other PRs into one bigger one, so everyone could just point to this PR until it get's merged, But as always, too many other projects.