Bug 197732 - Review Request: optipng - a PNG optimizer and converter
Summary: Review Request: optipng - a PNG optimizer and converter
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Keywords:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2006-07-05 20:38 UTC by Till Maas
Modified: 2012-09-17 21:10 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-10-11 20:57:17 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
opensource: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description Till Maas 2006-07-05 20:38:06 UTC
Spec URL: http://www-users.kawo2.rwth-aachen.de/~tmaas/fedora/optipng.spec
SRPM URL: http://www-users.kawo2.rwth-aachen.de/~tmaas/fedora/repo/optipng-0.5.3-1.src.rpm
Description: 
OptiPNG is a PNG optimizer that recompresses image files to a smaller size,
without losing any information. This program also converts external formats
(BMP, GIF, PNM; TIFF support is coming up) to optimized PNG, and performs PNG
integrity checks and corrections.

OptiPNG uses it's own versions of libpng and zlib to work properly. The systems zlib could be used but then the efficiency of OptiPNG is less than with its own version. With libpng this is not possible at this moment and patching upstream libpng will not happen soon, because OptiPNG accesses internal functions and structures of libpng.

I need a sponsor. I have submitted another package for review: fatsort.

Comment 1 Parag AN(पराग) 2006-07-06 04:37:54 UTC
Correct your SRPM URL. I think it MUST be
http://www-users.kawo2.rwth-aachen.de/~tmaas/fedora/repo/optipng-0.5.2-1.src.rpm
Or upload that SRPM.

Comment 2 Parag AN(पराग) 2006-07-06 05:16:04 UTC
Ok i search your website and found that SRPM.

== Not an official review as I'm not yet sponsored ==
* Mock build for development i386 is sucessfull
 
MUST Items:
     - MUST: rpmlint shows error as
      rpmlint -i optipng-0.5.2-1.src.rpm
      W: optipng invalid-license zlib/libpng
      The license you specified is invalid. The valid licenses are:

      -GPL                                    -LGPL
      -Artistic                               -BSD
      -MIT                                    -QPL
      -MPL                                    -IBM Public License
      -Apache License                         -PHP License
      -Public Domain                          -Modified CNRI Open Source License 
      -zlib License                           -CVW License
      -Ricoh Source Code Public License       -Python license
      -Vovida Software License                -Sun Internet Standards Source License
      -Intel Open Source License              -Jabber Open Source License

      if the license is close to an existing one, you can use '<license> style'.

==>You Need provide package with Open Source Licenses Mentioned above. Though
you have provided zlib/libpng and zlib is present, you can ignore this warning.

     - MUST: dist tag is present
     - MUST: The package is named according to the Package Naming Guidelines.
     - MUST: The spec file name matching the base package optipng, in the
format optipng.spec
      - MUST: This package meets the Packaging Guidelines.
      - MUST: The package is licensed with an open-source compatible license
zlib/libpng.
      - MUST: License file License.txt is included in package.
      - MUST: The sources used to build the package matches the upstream source,
as provided in the spec URL. md5sum is correct (8cc507e596c95ee44621f7adc8ce0534).
      - MUST: This package owns all directories that it creates. 
      - MUST: This package did not contain any duplicate files in the %files
listing.
      - MUST: This package  have a %clean section, which contains rm -rf
$RPM_BUILD_ROOT.
      - MUST: This package used macros.
      - MUST: Document files are included like README.txt.
      - MUST: Package did NOT contained any .la libtool archives.
      * Source URL is present and working.
      * BuildRoot is correct BuildRoot:       
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
      * BuildRequires is correct
   


Comment 3 Till Maas 2006-07-06 10:42:25 UTC
Spec URL: http://www-users.kawo2.rwth-aachen.de/~tmaas/fedora/optipng.spec
SRPM URL: 
http://www-users.kawo2.rwth-aachen.de/~tmaas/fedora/repo/optipng-0.5.2-1.src.rpm

Sorry, SRPM contained a type, you reviewed the correct SRPM.

http://www.opensource.org/licenses/zlib-license.php calls the license 
zlib/libpng, libpng in Core uses OSI approved as license text. But I can 
change the license tag to zlib next time I boot my development machine.

Thx for the review

Comment 5 Chris Weyl 2006-07-21 06:36:38 UTC
One note in your spec file that you'll get flagged on: you're mixing
macro/variable styles (e.g. using $RPM_BUILD_ROOT and %{optimize}). 

See
http://www.fedoraproject.org/wiki/Packaging/Guidelines#head-f3d77b27a5d29dfc1f5600ef3fc836f2e317badf

Comment 6 Chris Weyl 2006-07-21 06:38:04 UTC
(In reply to comment #5)
> One note in your spec file that you'll get flagged on: you're mixing
> macro/variable styles (e.g. using $RPM_BUILD_ROOT and %{optimize}). 

...make that %{optflags}.


Comment 7 Till Maas 2006-07-28 12:56:26 UTC
Spec URL: http://www-users.kawo2.rwth-aachen.de/~tmaas/fedora/optipng.spec
SRPM URL: 
http://www-users.kawo2.rwth-aachen.de/~tmaas/fedora/repo/optipng-0.5.3-1.fc5.src.rpm

* Sa Jul 29 2006 Till Maas <opensource[AT]till.name> - 0.5.3-1
- version bump
- Changed license tag back to zlib/libpng (#198616 rpmlint)
- use $RPM_OPT_FLAGS instead of %{optflags}


Comment 8 Rex Dieter 2006-08-17 19:23:10 UTC
FE-NEEDSPONSOR blocker already set, no need to be in Summary too.

Comment 9 Kevin Fenzi 2006-09-02 05:20:14 UTC
Removing FE-NEEDSPONSOR as submitter was already sponsored in: 
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=197442

Comment 10 Kevin Fenzi 2006-09-13 04:04:21 UTC
OK - Package name
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
OK - License (zlib/libpng)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
a6f3234a47464ecc1f455b52f0769492  optipng-0.5.3.tar.gz
a6f3234a47464ecc1f455b52f0769492  optipng-0.5.3.tar.gz.1
OK - Package compiles and builds on at least one arch.
OK - BuildRequires correct
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package doesn't own any directories other packages own.
See below - No rpmlint output.

SHOULD Items:

OK - Should include License or ask upstream to include it.
 - Should build in mock.

Issues:

1. version 0.5.4 is out now.

2. You have 2 commented out lines:
#BuildRequires:
#Requires:   

Might just remove those?

3. rpmlint says:

W: optipng invalid-license zlib/libpng
W: optipng invalid-license zlib/libpng
W: optipng-debuginfo invalid-license zlib/libpng

This can be ignored (see bug 198616).

E: optipng-debuginfo empty-debuginfo-package

Not sure whats causing that. You are building with -g
It would be good to fix however.


Comment 11 Kevin Fenzi 2006-09-13 04:06:34 UTC
Oh, forgot to mention... in the 0.5.4. release they mention that they 
"Added support for builds based on the system-supplied libpng."
So, perhaps you can switch to using the system libpng?

Comment 12 Kevin Fenzi 2006-09-29 17:51:18 UTC
Ping Till. 

Do you still want to submit this package? 
Any idea when you might address the items from comments #10 and #11?

Comment 13 Till Maas 2006-09-29 18:32:17 UTC
I tend to forget to finally submit my comments here, everything is done already
except that I did not send my comment here.

SPEC: http://www-users.kawo2.rwth-aachen.de/~tmaas/fedora/optipng.spec
SRPM:
http://www-users.kawo2.rwth-aachen.de/~tmaas/fedora/repo/optipng-0.5.4-1.src.rpm

Sorry for the delay.

Comment 14 Kevin Fenzi 2006-10-01 04:43:13 UTC
No problem. This version addresses all the blockers I saw... 
so this package is APPROVED. 

Don't forget to close this bug NEXTRELEASE once it's been imported and built. 

Comment 15 Kevin Fenzi 2006-10-08 04:14:56 UTC
Is there any hold up on importing and building this? 
Anything I can do to assist? 

Comment 16 Till Maas 2006-10-10 21:02:04 UTC
Thank you for your offer, it was only a lack of time. But now that I wanted to
import it, the import script said something about the time beeing wrong and that
it might have run incompletely. Actually it did not import the .tar.gz. I hope I
will look into this tomorrow and finish the import.

Comment 17 Kevin Fenzi 2006-10-11 16:14:24 UTC
Yeah, it looks like it didn't upload the source to the lookaside cache, or 
update sources and .cvsignore. 

You might create a new src.rpm with a bumped release and re-run cvs-import.sh 
on it to get everything uploaded. Let me know if I can assist any...

Comment 18 Till Maas 2006-10-11 20:57:17 UTC
built for devel (Job 19525)

A make new-sources FILE=... helped, it wrote that the tarball was already there
but updated sources and .cvsignore. Thank you for the review.

Comment 19 Till Maas 2007-09-24 19:08:38 UTC
Package Change Request
======================
Package Name: optipng
New Branches: EL-5

Comment 20 Jason Tibbitts 2007-09-25 01:40:36 UTC
CVS done.


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