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.

Sign up for GitHub

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

Jump to bottom

Backwards compatibility issue with minizip-ng #747

Open
brad0 opened this issue Dec 9, 2023 · 7 comments · May be fixed by #815
Open

Backwards compatibility issue with minizip-ng #747

brad0 opened this issue Dec 9, 2023 · 7 comments · May be fixed by #815
Labels
compatibility Backwards compatibility help wanted Need outside help

Comments

@brad0
Copy link
Contributor

brad0 commented Dec 9, 2023 •
edited
Loading

I have run into a project that has an embedded copy of minizip 1.x. It tries to use an external copy if found but fails to build with minizip 4.x.

It uses the zip.h / unzip.h headers which in minizip 4.x I see are empty and just use mz_compat.h.

The build trips up trying to find the constants Z_BEST_COMPRESSION and Z_DEFAULT_STRATEGY.

The minizip 1.x zip.h / unzip.h headers include the zlib.h header, the minizip 4.x headers do not.

I would expect that the minizip 4.x headers would provide the constants if they're part of the minizip API.
@nmoinvaz
Copy link
Member

Feel free to submit a PR.

@nmoinvaz nmoinvaz added help wanted Need outside help compatibility Backwards compatibility labels Dec 15, 2023
@brad0
Copy link
Contributor Author

brad0 commented Dec 15, 2023

Feel free to submit a PR.

Ok, I wanted to see what you would say before trying to come up with something.

@nmoinvaz
Copy link
Member

Can add it here: https://github.com/zlib-ng/minizip-ng/blob/master/mz_compat_shim.h.in

brad0 added a commit to brad0/minizip-ng that referenced this issue Dec 16, 2023
@brad0
Expose zlib constants for backwards compat zlib-ng#747
f48863d 
Building a project that had an internal copy of minizip 1.x, but
prefering an external library, with minizip 4.x showed that the
compat layer was not exposing some constants from zlib that
minizip 1.x headers did.
brad0 added a commit to brad0/minizip-ng that referenced this issue Dec 16, 2023
@brad0
Expose zlib constants for backwards compat zlib-ng#747
7f76835 
Building a project that had an internal copy of minizip 1.x, but
preferring an external library, with minizip 4.x showed that the
compat layer was not exposing some constants from zlib that
minizip 1.x headers did.
brad0 added a commit to brad0/minizip-ng that referenced this issue Dec 16, 2023
@brad0
Expose zlib constants for backwards compat zlib-ng#747
f1f89a3 
Building a project that had an internal copy of minizip 1.x, but
preferring an external library, with minizip 4.x showed that the
compat layer was not exposing some constants from zlib that
minizip 1.x headers did.
brad0 added a commit to brad0/minizip-ng that referenced this issue Dec 18, 2023
@brad0
Expose zlib constants for backwards compat zlib-ng#747
25ff004 
Building a project that had an internal copy of minizip 1.x, but
preferring an external library, with minizip 4.x showed that the
compat layer was not exposing some constants from zlib that
minizip 1.x headers did.
brad0 added a commit to brad0/minizip-ng that referenced this issue Dec 22, 2023
@brad0
Expose zlib constants for backwards compat zlib-ng#747
0f8cf66 
Building a project that had an internal copy of minizip 1.x, but
preferring an external library, with minizip 4.x showed that the
compat layer was not exposing some constants from zlib that
minizip 1.x headers did.
@Coeur
Copy link
Contributor

Coeur commented Oct 30, 2024 •
edited
Loading

Z_BEST_COMPRESSION and Z_DEFAULT_STRATEGY are not part of minizip.
They are part of zlib.h:
https://github.com/madler/zlib/blob/d476828316d05d54c6fd6a068b121b30c147b5cd/zlib.h#L192
https://github.com/madler/zlib/blob/d476828316d05d54c6fd6a068b121b30c147b5cd/zlib.h#L200

And they are also present in zlib-ng:
https://github.com/zlib-ng/zlib-ng/blob/94aacd8bd69b7bfafce14fbe7639274e11d92d51/zlib.h.in#L203
https://github.com/zlib-ng/zlib-ng/blob/94aacd8bd69b7bfafce14fbe7639274e11d92d51/zlib.h.in#L211

So I don't think minizip should redefine Z_BEST_COMPRESSION and Z_DEFAULT_STRATEGY.

All you're supposed to do, @brad0, is:

#include <zlib.h>

@Coeur
Copy link
Contributor

Coeur commented Oct 30, 2024

Ah, I just found your pull request at #750 which does just that (adding #include <zlib.h>).
Yes, that's all we need to do.

@nmoinvaz
Copy link
Member

I don't think that PR was ever finished.

@brad0
Copy link
Contributor Author

brad0 commented Oct 30, 2024

@nmoinvaz I meant to circle back to this but forgot about it.

@Coeur Coeur linked a pull request Nov 1, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compatibility Backwards compatibility help wanted Need outside help
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Expose zlib constants for backwards compat Coeur/minizip
3 participants
@Coeur @brad0 @nmoinvaz