Repository navigation
Windows: DLL export mz functions in mz_zip.h #475
Description
On Linux and macOS, I can link dynamically to minizip and still access these symbols because they are exported by default. On Windows, they are not exported. Only the compatibility layer is exported. I'd like to move to start using the mz functions on all three platforms. Can we decorate the mz functions with MZ_EXPORT?
Reacted by zed and Benjamin Buch
Activity
nmoinvaz commentedon May 27, 2020
It should be possible to call cmake with
-DCMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=ON.jimpark commentedon May 27, 2020
Yes, it's just not consistent with the other header files that do have the export macro. If that's the solution, I'd suggest adding it directly to the CMakeLists.txt. It has no effect in linux or macOS but makes the shared library work on Windows. (One thing to note is that this CMake option does not export variables -- though I don't think we need to worry about it in minizip.)
zedxxx commentedon May 28, 2020
+1 for
MZ_EXPORTCurrently dll compiled with mingw exports all symbols by default and there is a lot of "garbage" (functions from zlib etc). Adding export directive should fix this problem and we will have exported only
mz_functions.Here is a current
dumpbinoutput:zedxxx commentedon May 28, 2020
And one more thing:
MZ_EXPORTshould be used with all exported functions from allmz_xxx.hheaders, not only frommz_zip.h.nmoinvaz commentedon May 29, 2020
Those extra functions are bzip, lzma, and zlib. I can easily see a scenario where somebody might want to use the zlib that was compiled with minizip. Lzma and bzip2 not so much.
zedxxx commentedon May 29, 2020
What about to make it optional? If
EXPORT_ALL_SYMBOLSnot defined then defineMZ_EXPORTand export onlymz_functions.dr-dba commentedon Jul 11, 2020
I am also struggling with getting minizip.DLLfor windows (not exe), no luck meanwhile
Tried with CMake and VS2017
SpaceIm commentedon Apr 13, 2021
Shouldn't
_WINDOWSmacro be replaced by_WIN32here:minizip-ng/mz.h
Lines 150 to 154 in 1593060
nmoinvaz commentedon Apr 13, 2021
@SpaceIm Can you please explain why you think it should be changed?
SpaceIm commentedon Apr 13, 2021
AFAIK,
_WINDOWSdefinition doesn't exist (or it's not standard on Windows compilers),_WIN32should be used to detect windows.This simple macro should be sufficient:
__declspec(dllimport)can be omitted at consume time for minizip-ng, there are no global symbols.Moreover,
MZ_EXPORTshould be put in front of all functions declarations in public headers (all headers actually, since they are all installed). Currently it's missing everywhere, except inmz_compat.h.Finally,
C_VISIBILITY_PRESET hiddenshould be added to properties ofminiziptarget.I've a PR more or less ready if you want.
Symbol visibility issue was detected while packaging minizip-ng in public repository of conan: conan-io/conan-center-index#5166 (comment). For the moment, the workaround is to set
CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON. The neater solution is to hide all symbols, and exports symbols of the API (not only with Visual Studio).nmoinvaz commentedon Apr 13, 2021
It appears
_WINDOWSwas added in Visual Studio .NET from what I read.We would end up exposing almost all of the functions with
MZ_EXPORTso it doesn't make much difference fromCMAKE_WINDOWS_EXPORT_ALL_SYMBOLS.SpaceIm commentedon Apr 13, 2021
_WINDOWSis not listed in https://docs.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-160, it's the first time I see this macro.symbols of dependencies would be hidden.
uilianries commentedon Apr 13, 2021
True, but it's not defined for all Windows compilers. _WIN32 is well supported and commonly used. I strongly suggest replacing _WINDOWS by _WIN32.
nmoinvaz commentedon Apr 13, 2021
@uilianries if you want to do a PR for changing that #define I will accept it.
uilianries commentedon Apr 13, 2021
@nmoinvaz It would be great, because it allow us using -DMZ_EXPORTS on Windows. CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS works but is ugly, it exports everything.
nmoinvaz commentedon Apr 13, 2021
What i mean is only changing _WINDOWS to _WIN32. I still think there is a valid use case for exporting the third party libraries.
zlib-ng#475 Replace _WINDOWS by _WIN32
#475 Replace _WINDOWS by _WIN32
neheb commentedon Nov 21, 2022
@jimpark my solution for meson is to use a custom def file: https://github.com/mesonbuild/wrapdb/blob/master/subprojects/packagefiles/minizip-ng/mz.def
IIRC I took all the functions found in docs only. Probably not enough.
edit: funny. vcpkg lists minizip-ng as static-only.
neheb commentedon Nov 22, 2022
one issue I noticed is these functions:
these functions are only selectively added to the dll depending on whether the specific mz_strm_x.c files are compiled in, which makes having a def file difficult.
I recently updated that def file. Can anyone confirm or deny that those are public facing APIs?
\zlib-ng#475 Replace _WINDOWS by _WIN32
modbw commentedon Jun 17, 2024
+1 on adding MZ_EXPORT to every public function.
CMAKE_WINDOWS_EXPORT_ALL_SYMBOLSdosn't work for me when cross compiling minizip-ng from linux to windows usingcmake version 3.28.2.