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

Is it possible to replace madler zlib completely with zlib-ng-compat? #1572

Closed
ljavorsk opened this issue Sep 12, 2023 · 9 comments
Closed
Labels

Comments

@ljavorsk
Copy link

Hi,

We're in the middle of the discussion about replacing the "classic" zlib with the zlib-ng (built with the ZLIB_COMPAT=ON flag) in Fedora.

There are a lot of concerns and so I wanted to reach out to you, as you could help us answer some of them. There were a lot of comments already in the fedora-devel thread.

However, do you think that the compat version of zlib-ng could really fully replace the madler/zlib package? Are there any worries from your side against doing it?

Please let me know anything you think is relevant regarding this topic.

@mtl1979
Copy link
Collaborator

mtl1979 commented Sep 13, 2023

The obvious is that some existing bad code needs to be fixed before zlib can be replaced with zlib-ng... The long-term goal is to update everything to work with compatibility turned off.

There is small but significant chance that switching to zlib-ng can reveal corner cases where it doesn't produce valid output.

@Dead2
Copy link
Member

Dead2 commented Sep 14, 2023

@mtl1979 What bad code are you talking about exactly?
Of course it would be best if applications changed to use the zlib-ng native api, both for performance and individual testing.

@ljavorsk
For replacing zlib system-wide, that is also possible of course. But to avoid most problems, it should trigger a rebuild of all applications depending on zlib. As @mtl1979 says there is a possibility that applications use zlib in ways we have so far not tested, such as depending on zlib internals that are not promised to be that way for example (nginx is one of these, but has a workaround now).

There is AFAIK currently only one known bug related to using our zlib-compat mode, and it does produce invalid output. The culprit is pigz, and that relies on zlib internals and might need a patch in pigz to support zlib-ng #1565

These problems should be few however.
One thing we have thought about, but not yet implemented is a compile-time wrapper that exposes a zlib compatible header file, but links to native zlib-ng. That way packages could more easily be compiled to use zlib-ng without being changed, and without changing the system-wide zlib library as well, making it easier to compile problematic packages with stock zlib.

@Dead2
Copy link
Member

Dead2 commented Sep 14, 2023

Just to be clear, if something breaks, then that would be a bug. But it might be a bug in zlib-ng or it might be a bug in how an application (mis)uses the zlib api.

@mtl1979
Copy link
Collaborator

mtl1979 commented Sep 14, 2023

@mtl1979 What bad code are you talking about exactly? Of course it would be best if applications changed to use the zlib-ng native api, both for performance and individual testing.

My main concern is if an application overrides part of zlib functions with its own to overcome limitations of zlib... This might break for example align restrictions of zlib-ng. The other issue is passing too small buffer to zlib-ng functions, which can cause buffer overflow. I know some projects have unit tests, but those are run only during building, not when just running the resulting binary.

@ljavorsk
Copy link
Author

Of course, we will have to rebuild all of the packages that depend on zlib, so hopefully some tests will stop us if it breaks the package.

Thanks for the insights guys, feel free to raise any concerns, it will help us a lot.

@nmoinvaz
Copy link
Member

nmoinvaz commented Sep 14, 2023

You may consider compiling with -D WITH_NEW_STRATEGIES=OFF, since deflate_quick will be a departure from what standard zlib users are expecting as far as compression ratio goes at level 1, even though the purpose of the lower compression levels is to favor speed. And I think in your case, you would prioritize compatibility.

@Neustradamus
Copy link

To follow

@dralley
Copy link

dralley commented May 19, 2024

Since this indeed did happen, I think it can probably be closed now!

@ljavorsk
Copy link
Author

Yes, we indeed transitioned to zlib-ng in Fedora 40 [1].

I almost forgot about this thread. Closing it as resolved.

[1] https://fedoraproject.org/wiki/Changes/ZlibNGTransition

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants