Windows: DLL export mz functions in mz_zip.h #475
Labels
distribution
Release distribution
Comments
|
It should be possible to call cmake with |
|
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.) |
|
+1 for Currently 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 Here is a current |
|
And one more thing: |
|
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. |
|
What about to make it optional? If |
|
I am also struggling with getting minizip.DLLfor windows (not exe), no luck meanwhile |
|
Shouldn't Lines 150 to 154 in 1593060 |
|
@SpaceIm Can you please explain why you think it should be changed? |
|
AFAIK, This simple macro should be sufficient: #ifndef MZ_EXPORTS
#define MZ_EXPORT
#elif defined(_WIN32)
#define MZ_EXPORT __declspec(dllexport)
#else
#define MZ_EXPORT __attribute__ ((visibility ("default")))
#endif
Moreover, Finally, 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 |
|
It appears We would end up exposing almost all of the functions with |
|
symbols of dependencies would be hidden. |
True, but it's not defined for all Windows compilers. _WIN32 is well supported and commonly used. I strongly suggest replacing _WINDOWS by _WIN32. |
|
@uilianries if you want to do a PR for changing that #define I will accept it. |
|
@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. |
|
What i mean is only changing _WINDOWS to _WIN32. I still think there is a valid use case for exporting the third party libraries. |
uilianries
added a commit
to uilianries/minizip-ng
that referenced
this issue
Apr 13, 2021
Signed-off-by: Uilian Ries <uilianries@gmail.com>
nmoinvaz
pushed a commit
that referenced
this issue
Apr 13, 2021
Signed-off-by: Uilian Ries <uilianries@gmail.com>
|
@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. |
|
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? |
mitchellwong
pushed a commit
to GroundControl-Solutions/minizip
that referenced
this issue
Feb 17, 2023
Signed-off-by: Uilian Ries <uilianries@gmail.com>
|
+1 on adding MZ_EXPORT to every public function. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
mzfunctions on all three platforms. Can we decorate the mz functions with MZ_EXPORT?The text was updated successfully, but these errors were encountered: