Bug 759747 - Review Request: sk1libs - Universal vector graphics translator
Review Request: sk1libs - Universal vector graphics translator
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michel Alexandre Salim
Fedora Extras Quality Assurance
Depends On:
Blocks: 986552
  Show dependency treegraph
Reported: 2011-12-03 10:28 EST by Pavel Alexeev
Modified: 2013-07-21 11:20 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2012-02-18 07:53:46 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
michel: fedora‑review?

Attachments (Terms of Use)
spec with some unbundling (1.66 KB, text/x-rpm-spec)
2012-02-05 15:06 EST, Michel Alexandre Salim
no flags Details
libpdf stub module (110 bytes, text/x-python)
2012-02-05 15:07 EST, Michel Alexandre Salim
no flags Details
patch for removing the now-missing libpdf dirs (793 bytes, patch)
2012-02-05 15:08 EST, Michel Alexandre Salim
no flags Details | Diff

  None (edit)
Description Pavel Alexeev 2011-12-03 10:28:27 EST
Spec URL: http://hubbitus.info/rpm/Fedora15/sk1libs/sk1libs.spec
SRPM URL: http://hubbitus.info/rpm/Fedora15/sk1libs/sk1libs-0.9.1-1.fc15.src.rpm
sk1libs is a universal vector graphics library for Python.

P.S. Spec file formatted by tabs with 5 space width ( http://fedoraproject.org/wiki/PavelAlexeev/tabsize ). Please, do not start review if it is a problem for you.
Comment 1 Volker Fröhlich 2011-12-08 20:42:31 EST
* Doesn't build on my machine:

"src/pycms/_pycms.c:21:18: fatal error: lcms.h: No such file or directory"

* The tarball seems to bundle a couple of other Python modules. They should be removed in the prep section.

* You can drop "-n sk1libs-%{version}", as it's the default.

* Defattr is no longer necessary

* Defining python_sitearch is not necessary for Fedora anymore. If you're going for EPEL 5, it is necessary, but only for EPEL: http://fedoraproject.org/wiki/Packaging:Python#Macros

Drop the clean section and the rm in the install section in that case. Otherwise define buildroot: http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag
Comment 2 Pavel Alexeev 2011-12-17 13:09:23 EST
Volker, thank you.

I have address all mentioned issues except bundled libs. For that upstream bug filed: http://code.google.com/p/sk1/issues/detail?id=30
Comment 3 Volker Fröhlich 2011-12-19 16:22:56 EST
Please always use a new release number when you publish your file. That makes it a lot easier to compare and thus review.
Comment 4 Volker Fröhlich 2011-12-19 16:24:28 EST
Ah, OK, I thought you had replaced the spec file.
Comment 5 Pavel Alexeev 2011-12-20 15:33:09 EST
Yes, I always try bump release. Now I just think it have not worth upload it because other changes is not so important as bundled libs.
Comment 6 Michel Alexandre Salim 2012-02-04 11:43:37 EST
Volker, are you doing this review?
Comment 7 Volker Fröhlich 2012-02-04 14:19:59 EST
No, Michel, please feel free!
Comment 8 Michel Alexandre Salim 2012-02-05 07:21:18 EST
Thanks. Taking the review.
Comment 9 Pavel Alexeev 2012-02-05 09:27:22 EST
Michael, thank you for willing revкумц that - it very appreciated, but nor there bundled libs stop issue. I have submitted it to upstream, but have no any response yet.
Comment 10 Pavel Alexeev 2012-02-05 09:29:12 EST
Michel, thank you for willing review that - it very appreciated, but now
there bundled libs stop issue. I have submitted it to upstream, but have no any
response yet.

P.S. Sorry for the previous buggy message.
Comment 11 Michel Alexandre Salim 2012-02-05 14:40:04 EST
I had to strip a bundled library from a Python package before, so let me see if this can be quickly fixed.

BTW, upstream's project site is horribly messy -- the 0.9.1 final download is not listed on the download page, but they already have a 0.9.2pre -- and the download link works! Really odd
Comment 12 Michel Alexandre Salim 2012-02-05 15:06:38 EST
Created attachment 559500 [details]
spec with some unbundling

Attached spec has libpdf replaced by a stub that loads the system reportlab module. I tried doing the same to imaging, but it's a bit tricky as that is used during the build process.
Comment 13 Michel Alexandre Salim 2012-02-05 15:07:52 EST
Created attachment 559501 [details]
libpdf stub module

the stub file for src/libpdf/__init__.py
Comment 14 Michel Alexandre Salim 2012-02-05 15:08:50 EST
Created attachment 559502 [details]
patch for removing the now-missing libpdf dirs

libpdf itself is still present, but all the subdirectories (that are in reportlab) are now missing. Stop setup.py from trying to install them
Comment 15 Michel Alexandre Salim 2012-02-05 15:09:50 EST
Pavel, if you could upstream those, perhaps they'd be able to either stop bundling, or at least tell us how to remove the remaining bundled libraries. Ping me when there's any progress :)
Comment 16 Pavel Alexeev 2012-02-09 11:56:40 EST
Thank you very much. I've add comment to bug and will try contact upstream by mail.
Comment 17 Pavel Alexeev 2012-02-18 07:53:46 EST
I have got answer from author by private mail. Key ideas from it:
1. pycms is wrapper and not bundled lib!
2. PIL replaced by reportlab is bad idea - it is patched. Patches suggested to upstream but not accepted long time. May be sk1libs will work with reportlab, but lose some functionality. They unsure thought.
3. Branch 1.x freeze happened. Now all work done in Uniconvertor 2.0 which should came on LGM2012 (start of may) and will not require external libs like sk1lib.

So, I make decision what it have no worth to continue work under it.
Thank you, Michel, for help.

Note You need to log in before you can comment on or make changes to this bug.