Skip to content

Conversation

@remicollet
Copy link
Member

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.

@nikic
Copy link
Member

nikic commented Mar 18, 2016

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.

@remicollet
Copy link
Member Author

What hacks were necessary prior to PHP 5.6?

Before libzip 0.11 we were using lot of private symbols from the library.
Lot of work have be done in both projects to allow the clean state.

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)

@weltling
Copy link
Contributor

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.

@pierrejoye
Copy link
Contributor

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)

@Tyrael
Copy link
Contributor

Tyrael commented Apr 21, 2016

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/

@jpauli
Copy link
Member

jpauli commented Apr 27, 2016

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...

@pierrejoye
Copy link
Contributor

It is not much work per se.

The work is mainly due to slow releases as well as some portable issues or
designs issues.

We will gain nothing to unbundle it but uncertainties about the
compatibility of the upstream/system lib.
On Apr 27, 2016 2:23 PM, "jpauli" notifications@github.com wrote:

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...


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#1828 (comment)

@remicollet
Copy link
Member Author

remicollet commented May 28, 2016

It is not much work per se.

@pierrejoye so, please take care of updating bundled libzip top 1.1.3 (just released, Windows only change)

@pierrejoye
Copy link
Contributor

Not sure it is necessary as 1.1.3 fixes the builds only. 1.1.2 in 1.13.2
builds just fine for us.

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
wrote:

It is not much work per se.

So, please take care of updating bundled libzip top 1.1.3 (just released,
Windows only change)


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1828 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AARPKB20SK7CyRarow5wGOJ7bGpPyLEkks5qGGYwgaJpZM4HzrWc
.

Pierre

@pierrejoye | http://www.libgd.org

@smalyshev
Copy link
Contributor

So, should this be closed?

@krakjoe
Copy link
Member

krakjoe commented Jan 3, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants