Spec URL: https://jgu.fedorapeople.org/python-lz4.spec SRPM URL: https://jgu.fedorapeople.org/python-lz4-0.7.0-0.fc22.src.rpm Description: Python bindings for the lz4 compression library. Fedora Account System Username: jgu
Spec URL: https://jgu.fedorapeople.org/python-lz4.spec SRPM URL: https://jgu.fedorapeople.org/python-lz4-0.7.0-0.fc22.1.src.rpm * Sat Jun 27 2015 Jonathan Underwood <jonathan.underwood> - 0.7.0-0.1 - Fix permissions of shared objects to be 0755
Spec URL: https://jgu.fedorapeople.org/python-lz4.spec SRPM URL: https://jgu.fedorapeople.org/python-lz4-0.7.0-0.fc22.2.src.rpm * Sat Jun 27 2015 Jonathan Underwood <jonathan.underwood> - 0.7.0-0.2 - Include README.rst in python3 package as well
Unfortunately there's no LICENSE/COPYING file included. Issue filed upstream: https://github.com/steeve/python-lz4/issues/38
Looks like this package bundles the entire lz4 source code instead of using the system lz4 libs, that needs to be addressed, either by adapting/patching to use the system libs, or by getting a bundling exception from FPC. Also, I'm not sure what the "Requires: lz4" is there for? Release should start with > 0 and stuff after %{?dist} should be dropped too as this is not a pre-release package. Non-blocker: As long as there's no test suite, something like this in %check would be better than nothing: PYTHONPATH=$RPM_BUILD_ROOT%{python2_sitearch} %{__python2} -c "import lz4" %if %{with python3} PYTHONPATH=$RPM_BUILD_ROOT%{python3_sitearch} %{__python3} -c "import lz4" %endif Non-blocker: the %if %{with python3} ... %endif wrapping around %package -n python3-lz4 and %description -n python3-lz4 can be dropped if you like (it doesn't do any harm in non-python3 builds).
Yes, the bundling of lz4 is very unfortunate. The lz4 package doesn't ship any header files and isn't designed as a shared library, alas. The python-lz4 is also tied to a specific bundled version of lz4 as far as I can see. Not a great situation. I'll see if i can get a FPC exception.
FPC ticket: https://fedorahosted.org/fpc/ticket/548
Hm, I was wrong about the lz4 not shipping headers - there's a -devel sub-package. So, in pricniple it should be possible to build against a system shared lib. Will raise it with upstream.
Bundling issue raised with upstream python-lz4: https://github.com/steeve/python-lz4/issues/39
Spec URL: https://jgu.fedorapeople.org/python-lz4.spec SRPM URL: https://jgu.fedorapeople.org/python-lz4-0.7.0-1.fc22.src.rpm * Sat Jun 27 2015 Jonathan Underwood <jonathan.underwood> - 0.7.0-1 - Build against system lz4 libs - Rudimentary check to see if we can import the module
1) Requires: lz4 is still there, for no reason I can think of...? 2) Bundled code should be removed in %prep in order to make sure it doesn't get used (in this case at least some of it does get used, the private lz4.h ends up in -debuginfo). rm src/lz4*.[ch] early enough in %prep cures that. 3) Non-blocker: I think it would be cleaner to use libraries=["lz4"] instead of extra_link_args=["-llz4"]. 4) Cosmetic: the -O3 comment lines could be removed from %prep. Other than that, looks fine to me. Address at least 1) and 2) and you can set the review status to + yourself and proceed with importing, or let me know if you want further comments from me.
Spec URL: https://jgu.fedorapeople.org/python-lz4.spec SRPM URL: https://jgu.fedorapeople.org/python-lz4-0.7.0-2.fc22.src.rpm * Sat Jun 27 2015 Jonathan Underwood <jonathan.underwood> - 0.7.0-2 - Drop unneeded Requires for lz4 - Remove commented out cruft from spec - Regenerate setup.py patch to use libraries=["lz4"] - Remove bundled lz4 code in %%prep Thanks for the review Ville.
New Package SCM Request ======================= Package Name: python-lz4 Short Description: LZ4 Bindings for Python Upstream URL: https://github.com/steeve/python-lz4 Owners: jgu Branches: f21 f22 InitialCC:
FYI: as of xpra version 0.16 (due reasonably soon), we will require the VERSION number to be present (so we can turn off python-lz4 compression if a vulnerable version is found at either end, ie: that was the case with <0.7.0 for example), so we backported the patch that adds the version from upstream, see: http://xpra.org/trac/changeset/9745 (even better would be for upstream to merge the version bump and all the pending pull requests - thanks for re-doing mine properly BTW)
Thanks for the heads up Antoine. I see what you're doing is checking both the version of python-lz4 and the version of lz4, assuming that the version of lz4 being used is the one bundled with python-lz4. Unfortunately that's going to break for anyone (like us) using a system wide (not bundled) lz4 library. A better approach would be to add a function to the python module which calls the function in lz4 that returns the lz4 version number - I forget the name of the function, but I spotted it yesterday when browsing the source. Anyway, I'll knock up a patch to do that. I am planning to make some changes to setup.py too to allow the use of system lz4 or bundled lz4 depending on what's available at build time, and it'll adjust things accordingly. I hope upstream is receptive.
Hi Antoine, please see this pull request to see what I've implemented, thoughts welcome: https://github.com/steeve/python-lz4/pull/41 I really think in xpra you should be checking against the lz4 library version (as defined in lz4.h and now available from the new lz4version function) rather than the release version. The lz4 version properly indicates API breakage etc.
Side note: I just found this fork: https://github.com/darkdragn/lz4tools which seems to be a lot faster moving than the original python-lz4.
WARNING: fedora-review flag set by review submitter! Verify that review was approved by reviewer!
Gah - Ville, please can you set the review flag to +.
Git done (by process-git-requests).
python-lz4-0.7.0-3.fc21 has been submitted as an update for Fedora 21. https://admin.fedoraproject.org/updates/python-lz4-0.7.0-3.fc21
python-lz4-0.7.0-3.fc22 has been submitted as an update for Fedora 22. https://admin.fedoraproject.org/updates/python-lz4-0.7.0-3.fc22
Package python-lz4-0.7.0-3.fc21: * should fix your issue, * was pushed to the Fedora 21 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing python-lz4-0.7.0-3.fc21' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2015-11047/python-lz4-0.7.0-3.fc21 then log in and leave karma (feedback).
python-lz4-0.7.0-3.fc21 has been pushed to the Fedora 21 stable repository. If problems still persist, please make note of it in this bug report.
python-lz4-0.7.0-3.fc22 has been pushed to the Fedora 22 stable repository.