Bug 197732 - Review Request: optipng - a PNG optimizer and converter
Review Request: optipng - a PNG optimizer and converter
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Package Reviews List
Depends On:
  Show dependency treegraph
Reported: 2006-07-05 16:38 EDT by Till Maas
Modified: 2012-09-17 17:10 EDT (History)
1 user (show)

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

Attachments (Terms of Use)

  None (edit)
Description Till Maas 2006-07-05 16:38:06 EDT
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
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 00:37:54 EDT
Correct your SRPM URL. I think it MUST be
Or upload that SRPM.
Comment 2 Parag AN(पराग) 2006-07-06 01:16:04 EDT
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
      - 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
      - MUST: This package  have a %clean section, which contains rm -rf
      - 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 06:42:25 EDT
Spec URL: http://www-users.kawo2.rwth-aachen.de/~tmaas/fedora/optipng.spec

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 02:36:38 EDT
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}). 

Comment 6 Chris Weyl 2006-07-21 02:38:04 EDT
(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 08:56:26 EDT
Spec URL: http://www-users.kawo2.rwth-aachen.de/~tmaas/fedora/optipng.spec

* 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 15:23:10 EDT
FE-NEEDSPONSOR blocker already set, no need to be in Summary too.
Comment 9 Kevin Fenzi 2006-09-02 01:20:14 EDT
Removing FE-NEEDSPONSOR as submitter was already sponsored in: 
Comment 10 Kevin Fenzi 2006-09-13 00:04:21 EDT
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.


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


1. version 0.5.4 is out now.

2. You have 2 commented out lines:

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 00:06:34 EDT
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 13:51:18 EDT
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 14:32:17 EDT
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

Sorry for the delay.
Comment 14 Kevin Fenzi 2006-10-01 00:43:13 EDT
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 00:14:56 EDT
Is there any hold up on importing and building this? 
Anything I can do to assist? 
Comment 16 Till Maas 2006-10-10 17:02:04 EDT
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 12:14:24 EDT
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 16:57:17 EDT
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 15:08:38 EDT
Package Change Request
Package Name: optipng
New Branches: EL-5
Comment 20 Jason Tibbitts 2007-09-24 21:40:36 EDT
CVS done.

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