-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Provide a clear option to depend on zlib-ng #9279
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
base: main
Are you sure you want to change the base?
Conversation
Are we the upstream and what is the predicament? 😄 |
I think!? right?
Today: i want to take advantage of zlib-ng with Pillow, but:
I want to have both:
|
|
now if only i understood the windows failures ^_^ |
So, just spelling this out, you're saying that if both zlib and zlib-ng are installed, then Pillow may pick zlib over zlib-ng? Probably just based on the order of the paths that it searches? Have you considered adjusting the search paths? https://pillow.readthedocs.io/en/stable/installation/building-from-source.html#installing
|
If you build out zlib and zlib-ng so that they can be co-installed, you have to disable zlib-ng's "compatibility" mode. Once you've disabled compatibility mode, we are "forced" to use
Yeah this is generally what we do. we set enough flags that it uses the isolated environment. |
| if feature.want("zlib"): | ||
| _dbg("Looking for zlib") | ||
| if _find_include_file(self, "zlib.h"): | ||
| if _find_include_file(self, "zlib-ng.h"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This introduces an automagic dependency on zlib-ng when both zlib-ng and zlib are installed. As such, Gentoo (as well as anyone who doesn't build in an entirely isolated environment) would end up having to patch this out in order to use plain zlib. I'm not opposed to having zlib-ng as an option, but then it should be an option, one that can be explicitly disabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for bringing this up. it is true. it is "priority for zlib-ng".
I'm happy to add an explicit "switch" between the two.
I'll add that to the todo list if the general strategy is agreeable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still working on the general strategy being agreeable. That is usually determined by interest and the number of folks looking for a fix for this issue. At present, we depend on zlib and zlib-ng satisfies that dependency via compat mode. How many folks are looking for co-existence ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That’s a good question.
I’m. Happy to let this PR sit
This is more of a nit than anything, but the problem arose in conda-forge where we still haven't gone all-in on zlib-ng.
As such, we ship BOTH zlib, and zlib-ng, which are designed to co-exist.
I thus wrote a patch for pillow to be able to directly target zlib-ng.
I figured I would upstream it and maybe this can help other distributions with this predicament.
xref:
zlib? conda-forge/zlib-ng-feedstock#10I'm happy to address any issues you all have with this but the idea is that:
if zlib-ng is found, then choose that, and just use the
zlib-ngheaders directly instead ofzlib.h.TODO:
zlib.horzlib-ng.h-- @mgorny