Bug 478581 - Package Review: pnglite - A lightweight C library for loading PNG images
Package Review: pnglite - A lightweight C library for loading PNG images
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Robert Scheck
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 462181
  Show dependency treegraph
 
Reported: 2009-01-01 07:28 EST by Lubomir Rintel
Modified: 2009-01-04 16:57 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-01-04 16:57:00 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
redhat: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Lubomir Rintel 2009-01-01 07:28:30 EST
SPEC: http://v3.sk/~lkundrak/SPECS/pnglite.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/pnglite-0.1.17-1.el5.src.rpm

Description:

pnglite is a C library for loading PNG images. It was created as a
substitute for libpng in situations when libpng is more than enough. It
currently requires zlib for inflate and crc checking and it can read the
most common types of PNG images. The library has a small and simple to use
interface.

Notes:

RPMlint complains about no documentation files, but there are no documentation files in the distribution. See the comment in %files.

There's a typo in Summary line, I'd fix that on import or next package spin:
A lighteeight C library for loading PNG images
Comment 1 Robert Scheck 2009-01-01 13:41:17 EST
[16:37:47] < lkundrak> cassmodiah: I'm wondering if you can review pnglite?
[16:37:57] < lkundrak> cassmodiah: should be fairly simple -- just one .c file :)
[16:38:46] < cassmodiah> i'm not sure with this
[16:38:58] < cassmodiah> wait, i know which is a better guy for that
[16:39:00] < cassmodiah> rsc ping
[16:39:14] < lkundrak> :)
[16:44:20] < rsc> cassmodiah: pong
[16:46:02] < cassmodiah> rsc do you have time for a short look at a review request?
[16:47:05] < cassmodiah> https://bugzilla.redhat.com/show_bug.cgi?id=478581
[16:47:06] < buggbot> Bug 478581: medium, low, ---, nobody@fedoraproject.org, NEW, Package Review: pnglite - A lightweight C library for loading PNG images
[16:49:44] < rsc> lkundrak: I'm not absolutely sure, whether the ABI soname stuff is clever. Was this practised somewhere else was well?
[16:51:17] < lkundrak> rsc: I'm not really sure. I know I did that, but don't remember if it was in Fedora package. But if upstream is not willing to maintain soname (which I did not ask though), is there anything else we can do?
[16:53:16] < lkundrak> rsc: you know -- pnglite is just a single pnglite.c file, upstream doesn't even maintian a build script where soname could be enforced
[16:54:52] < rsc> lkundrak: understood. Why is %{abi_minor} == %{release}? If you've to rebuild for e.g. a new GCC; soname would be minorly bumped...
[16:55:31] < lkundrak> rsc: abi_minor was not meant to be the part of soname. If it is, then it is a mistake
[16:56:04] < lkundrak> rsc: -Wl,--soname,libpnglite.so.%{abi_major}
[16:56:10] < lkundrak> rsc: there's only abi_major
[16:56:12] < rsc> libpnglite.so.%{abi_major}.%{abi_minor}
[16:56:24] < lkundrak> rsc: right, that's not a soname. only a filename.
[16:56:35] < rsc> ah right. Sorry.
[16:57:43] < rsc> but anyway: If you bump %{version}, %{release} should be reset. Then you would have to increase %{abi_major} to ensure, that libpnglite.so.%{abi_major}.%{abi_minor} is newer, not older from the filename.
[16:57:53] < rsc> is that expected behaviour?
[17:00:18] < lkundrak> rsc: yes. that's why there's a huge comment in the beginning
[17:00:54] < lkundrak> rsc: but in case a new major version is released without abi change, then we would unnecessarily change soname
[17:01:05] < lkundrak> rsc: I don't think that's likely though
[17:11:03] < lkundrak> rsc: on a second thought -- I don't know what is the last digit in the library file name for. Probably it won't be a problem if it was reset. Probably we could decouple if from release.
[17:17:59] < cassmodiah> lkundrak one of my teeworlds testers has no sound
[17:19:26] < lkundrak> cassmodiah: well, I do not think it is related to any change we did
[17:20:34] < rsc> lkundrak: I'll try the formal review in a few minutes
Comment 2 Robert Scheck 2009-01-01 13:49:10 EST
Lubomir, can you please add the license to %doc of the base package? I know,
it is in the header file of -devel as well, but that's not perfect IMHO. So
just the license for %doc, not the whole header file. Maybe the license just
from the header file as Source2 simply.

Package seems sane to me, source matches, debuginfo is okay, description and
overall readability of the spec file, file locations/ownings fit etc.

Please add license to %doc and correct your typo in Summary. As this is very
minor: APPROVED.
Comment 3 Lubomir Rintel 2009-01-01 14:22:04 EST
(In reply to comment #2)
> Lubomir, can you please add the license to %doc of the base package? I know,
> it is in the header file of -devel as well, but that's not perfect IMHO. So
> just the license for %doc, not the whole header file. Maybe the license just
> from the header file as Source2 simply.

That can't be done, see [1]. And given the size of the package and license being included in the header I don't think it would make sense to make upstream distribute one more file with a copy of the license.

[1] https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text

New Package CVS Request
=======================
Package Name: pnglite
Short Description: A lightweight C library for loading PNG images
Owners: lkundrak, cassmodiah
Branches: EL-5 F-9 F-10
Comment 4 Kevin Fenzi 2009-01-04 15:47:35 EST
cvs done.
Comment 5 Lubomir Rintel 2009-01-04 16:57:00 EST
Imported and built. Thanks!

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