-
Couldn't load subscription status.
- Fork 8k
Drop bundled libzip #1828
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 bundled libzip #1828
Conversation
|
What hacks were necessary prior to PHP 5.6? If we're really just bundling a raw libzip without patches, +1 on dropping it. The Travis config needs to be adjusted to install libzip. |
Before libzip 0.11 we were using lot of private symbols from the library. I mostly waiting feeback from @weltling about Windows part. But each update of the bundled library imply lot of work, not related to PHP (usually done by Anatol and me) |
|
Finally I came to this and checked the libzip builds. The default distribution doesn't provide any static builds, but the code is for some reason commented out in the cmake scripts. Tests revealed issues in 32-bit builds with PHP. But when linked with external libzip, not all PHP tests pass on 32-bit. The bundled libzip however has no issues. I haven't dig deeper into that, but for me this were the reason to not to unbundle libzip. Otherwise we land with the same story - to patch cmake build scripts and eventually some places in libzip. Thanks. |
|
Having it bundled provides an out of the box, stable zip management extensions in the core. Even if there were a better contribution and release cycles in upstream libzip, I do not think we should drop it. But that's far from being the case as of now. I cannot close PRs here (this whole PR process thing is broken for us. We should either move or stop accepting them as it is now... but that's another story) |
no, that is intentional, otherwise people would just merge pull requests here by mistake (which wouldn't get into git.php.net and our git mirroring would bork), for closing github PRs you should be using https://qa.php.net/pulls/ |
|
The problem of that is that code diverges a lot, and that makes us a lot of additionnal work, as we are starting maintining a diverging branch from upstream. Remi keeps on fighting at every release with those bundled libs, like libzip or libgd, and the job beeing more and more complex as time goes... |
|
It is not much work per se. The work is mainly due to slow releases as well as some portable issues or We will gain nothing to unbundle it but uncertainties about the
|
@pierrejoye so, please take care of updating bundled libzip top 1.1.3 (just released, Windows only change) |
|
Not sure it is necessary as 1.1.3 fixes the builds only. 1.1.2 in 1.13.2 We may just do it with the next release with actual code changes. On Sat, May 28, 2016 at 10:58 PM, Remi Collet notifications@github.com
Pierre |
|
So, should this be closed? |
|
Since Pierre wanted to close this months ago, I'm closing this PR. On a side note, I find the arguments that we should bundle something because it provides "out of the box" support, extremely weak indeed. That we are wasting time (regardless of amount of time) maintaining libraries that are maintained to a higher standard, with more contributors (and so knowledge of the library) does not make sense today. We do not have the required active contributors to undertake this kind of work. This discussion, about bundling libraries, needs to be had fairly in a general context, there needs to be overwhelming reasons for us to do this, or continue to do it; that we have always done it for X is not at all a good reason. |
Since PHP 5.6 we can use system libzip without any hack.
As for other library, I think it doesn't make sense to keep this one bundled, and thus will higly reduce maintaince of this extension.
Notice: if needed the pecl extension history will allow people to find version matching their system libzip version.