search

XQF / xqf

XQF game server browser
http://xqf.github.io
GNU General Public License v2.0
37 stars 12 forks source link

rely on external minizip dependency instead of the builtin zip #141

Closed illwieckz closed 9 years ago

illwieckz commented 9 years ago

@skybon has on his side a change I think is very good: removing the very old src/zip/ code to rely on an external build dependency like minizip, it's included with the port to cmake he is doing.

I think it's very good to reduce the codebase size and to rely on a maintained project like minizip.

Let’s use what others are doing better than us. :+1:

vorot93 commented 9 years ago

minizip is not shipped by Ubuntu 12.04 LTS "Precise Pangolin". That's the version used by Travis CI build farm. I'm pretty sure that any attempts to pull 14.04 packages are doomed to fail.

Therefore we face two options here:

  1. Keep unzip source in-tree and enable it when user host lacks required headers.
  2. Explore other CI solutions.

So, we should either rename this issue to "Allow third party unzip.h and ioctl.h" or create another ticket for replacing Travis CI.

lnussel commented 9 years ago

was there a reason for choosing minizip over libzip? minizip is not in openSUSE wheres ziplib is available even in very old distros.

http://www.nih.at/libzip/

vorot93 commented 9 years ago

@lnussel libzip is an entirely different library and does not provide unzip.h and ioapi.h.

lnussel commented 9 years ago

slightly different api but xqf only needs a handfull calls

illwieckz commented 9 years ago

@skybon, do you have any solution for that? XQF must be buildable by OpenSUSE.

vorot93 commented 9 years ago

@illwieckz I am not an openSUSE packager. If openSUSE does not ship a library that everyone else have then it's distro-wide problem.

illwieckz commented 9 years ago

Yes, but we can't introduce regressions for OpenSUSE users. So even if you are not an openSUSE packager, you must allow any openSUSE packager to build XQF.

lnussel commented 9 years ago

Artem Vorotnikov wrote:

@illwieckz https://github.com/illwieckz I am not an openSUSE packager.

That's not the point :-) I just wonder whether it's worth going through the hassle of introducing a new minizip package if xqf can be changed to use just libzip which should be available everywhere.

illwieckz commented 9 years ago

@lnussel: using a more common package like ziplib could be good, but perhaps OpenSUSE is already shipping minizip inside another generic package? For example in the past, minizip were shipped by Debian in the libkml-dev package.

illwieckz commented 9 years ago

Also, it seems that sometime zlib is shipping minizip in some distros.

illwieckz commented 9 years ago

@lnussel: in fact @skybon choose to rely on minizip because the code that were previously shipped within xqf's source code were minizip. It's just now an external dependency.

It's just an effort to not ship ourselves dependency that are provided by the distro. But if some distro does not ship minizip, it's problematic. It seems that many packages rely on minizip, so if a distro does not ship it, this is only because every package ship it itself as a workaround…

illwieckz commented 9 years ago

Just for the story, it seems that about ~130 Debian packages are relying on unzip.sh, but the minizip package were only introduced in Debian jessie, so it seems that all these ~130 packages were shipping this code themselves before. It's a shame.

OpenSUSE has probably the same problem with more than 100 packages shipping the same source code, a common minizip package for OpenSUSE could be a very good idea for everyone.

lnussel commented 9 years ago

it's about 30 but it doesn't change the fact that I can't just if xqf's package during lunch break

lnussel commented 9 years ago

https://bugzilla.suse.com/show_bug.cgi?id=935864

lnussel commented 9 years ago

ok, so we will get minizip from libz in Tumbleweed. Thanks for the pointer! Nevertheless this package will not be available on older distributions, so I'll still have to build it somehow.

illwieckz commented 9 years ago

Nice !

jmallach commented 9 years ago

Hi Ludwig,

On Wed, Jun 24, 2015 at 04:37:07AM -0700, Ludwig Nussel wrote:

ok, so we will get minizip from libz in Tumbleweed. Thanks for the pointer! Nevertheless this package will not be available on older distributions, so I'll still have to build it somehow.

So a fix would be to check for minizip at configure time, and ifdef a system-wide minizip if found, or use an embedded copy if not.

I'd know how to do this with autotools, but I guess I don't with cmake ;)

I have no time to look at it right now. I just had a baby.

Jordi

Jordi Mallach Pérez -- Debian developer http://www.debian.org/ jordi@sindominio.net jordi@debian.org http://www.sindominio.net/ GnuPG public key information available at http://oskuro.net/