Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drop '#ifdef HAVE_ZLIB' from mz_compat.h? #309

Closed
praiskup opened this issue Sep 5, 2018 · 7 comments
Closed

drop '#ifdef HAVE_ZLIB' from mz_compat.h? #309

praiskup opened this issue Sep 5, 2018 · 7 comments
Labels
compilation Issues related to compiling source code fixed Issue or bug has been fixed

Comments

@praiskup
Copy link

praiskup commented Sep 5, 2018

Commit 41331dd added:

+#ifdef HAVE_ZLIB
 #include "zlib.h"
+#endif

Which complicates compat work; simply -> those who want to replace zlib's header by <mz_compat.h> also have to define HAVE_ZLIB.

@nmoinvaz
Copy link
Member

nmoinvaz commented Sep 6, 2018

I don’t understand. It is not necessary to compile with zlib. You can compile with only lzma hence the HAVE_ZLIB indicates that you want to use zlib.

@praiskup
Copy link
Author

praiskup commented Sep 6, 2018

well, maybe I am missing something.. I wanted to say that I thought that mz_compat.h is supposed to be 1:1 replacement for the headers in zlib/contrib/minizip. But trying to port two packages, I've faced several incompatibilities .. (being forced to define HAVE_ZLIB explicitly is one of them, there are some missing constants packages seem to use, etc.)

I'm not sure, porting to this minizip implementation is not trivial, is it supposed to be? Maybe we could make the move easier.

So far I reported psi-im/psi/issues/388 and assimp/assimp/issues/2121.

@nmoinvaz
Copy link
Member

I see. What are the other constants? If you use CMAKE to compile the project, it automatically adds everything you need. Are you just dropping in the files? I don't know if there is a way to just make it work for those other constants.

@nmoinvaz
Copy link
Member

In 2.5.2 release, #include "zlib.h" was removed from mz_compat.h. Maybe this is a case of using an older version of the library?

@praiskup
Copy link
Author

We tried to build against 2.5.0.

What are the other constants?

One project required about a dozen of them. And even after back-porting those, I wasn't able to build (some methods missing, etc.).

I'll give it another run as soon as I have some time and update this ticket; but some migration docs/guidance for existing projects would be awesome.

@Coeur
Copy link
Contributor

Coeur commented Sep 11, 2018

We tried to build against 2.5.0.

Could you give a try against 2.5.2 as recommended by Nathan? The history of this file shows there is a continuous effort to solve compatibility issues: https://github.com/nmoinvaz/minizip/commits/master/mz_compat.h

@praiskup praiskup changed the title dorp '#ifdef HAVE_ZLIB' from mz_compat.h? drop '#ifdef HAVE_ZLIB' from mz_compat.h? Sep 11, 2018
@nmoinvaz
Copy link
Member

I have added some upgrade info also to:
https://github.com/nmoinvaz/minizip/wiki
Hopefully that helps.

@nmoinvaz nmoinvaz added 2.0 compilation Issues related to compiling source code fixed Issue or bug has been fixed labels Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compilation Issues related to compiling source code fixed Issue or bug has been fixed
Projects
None yet
Development

No branches or pull requests

3 participants