Bug 226191

This should be fun.

First off, here's the rpmlint output; most of it seems pretty easy.

W: netpbm summary-ended-with-dot A library for handling different graphics file formats.
W: netpbm-devel summary-ended-with-dot Development tools for programs which will use the netpbm libraries.
W: netpbm-progs summary-ended-with-dot Tools for manipulating graphics files in netpbm supported formats.
   Trivial to fix.

W: netpbm invalid-license freeware
W: netpbm-debuginfo invalid-license freeware
W: netpbm-devel invalid-license freeware
W: netpbm-progs invalid-license freeware
   It's tough to summarise the multitude of netpbm licenses; I'm not sure if
  "freeware" is the proper thing to use or not.  I'll see what spot has to
  say.
  Newsflash: spot says no, but something like:
  License: Assorted licenses, see %{_docdir}/%{name}-%{version}/copyright_summary 
  would be OK, assuming that that file gets added to the package.

E: netpbm obsolete-not-provided libgr
E: netpbm-devel obsolete-not-provided libgr-devel
E: netpbm-progs obsolete-not-provided libgr-progs
   I believe these should just go away.  No Fedora release has ever had
   libgr as far as I can tell.

E: netpbm-progs only-non-binary-in-usr-lib
   This is due to the four palm*.map files.  I wonder why these aren't under
   /usr/share since they shouldn't be arch-dependent.

W: netpbm-progs file-not-utf8 /usr/share/man/man1/pgmminkowski.1.gz
   A quick run through iconf should fix this up.

E: netpbm configure-without-libdir-spec
   This isn't a standard configure script, so this is OK

E: netpbm hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/lib*
   This is actually documented in the spec, and is OK.

So, the first fun question is: where do those tarballs come from?  Upstream
doesn't seem to produce any tarballs, so I guess the ones in the package are
made from svn export.  You should provide information on generating those
tarballs; see
http://fedoraproject.org/wiki/Packaging/SourceURL#RevisionControl

I'm guessing something like:
  svn export https://netpbm.svn.sourceforge.net/svnroot/netpbm/release_number/10.35.0 netpbm-10.35
was used, except that the above produces something a bit different from what's
in netpbm-10.35.l1.tar.bz2.  And I've no idea where netpbmdoc-10.35.l1.tar.bz2
comes from.  The changelog indicates that these have bits removed due to
license issues, which is fine but it should be made clear, perhaps with a
simple script that does the work.

There seems to be some good license information in doc/copyright_summary that
really should be in the pacakge.

This doesn't seem to be the latest version (that seems to be either 10.35.23
or 10.37.03 depending on which branch you want to follow).  But I'm sure
rebasing right now isn't a great idea, although it would be nice to carry
fewer than 17 patches.

There's no need to test RPM_BUILD_ROOT against "/" in %clean and %install.

I tried a parallel make but it dies pretty quickly; this should be commented.

This package includes a static library, which it shouldn't without a good
reason.  (And if it really needs to, it needs to be in a -static subpackage.)

I'll attach a patch which fixes up the trivial rpmlint warnings, fixes
License:, nukes the obsoletes, runs iconv on the errant manpage, fixes
buildroot cleaning and drops the static library.  This leaves the
"only-non-binary-in-usr-lib" complaint as the only remaining one to worry
about.  I'm afraid I can't help with the upstream source bits.

Review:
X Can't check source against upstream.
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
? license field matches the actual license.
* license is open source-compatible.
X license text included in package.
O latest version is not being packaged.
* BuildRequires are proper (don't need to list perl explicitly)
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
X rpmlint is silent.
* final provides and requires are sane:
  netpbm-10.35-11.fc7.x86_64.rpm
   libnetpbm.so.10()(64bit)
   netpbm = 10.35-11.fc7
  =
   /sbin/ldconfig
   libnetpbm.so.10()(64bit)

  netpbm-devel-10.35-11.fc7.x86_64.rpm
   netpbm-devel = 10.35-11.fc7
  =
   libnetpbm.so.10()(64bit)
   netpbm = 10.35-11.fc7

  netpbm-progs-10.35-11.fc7.x86_64.rpm
   netpbm-progs = 10.35-11.fc7
  =
   /bin/sh
   /usr/bin/perl
   libX11.so.6()(64bit)
   libjpeg.so.62()(64bit)
   libnetpbm.so.10()(64bit)
   libpng12.so.0()(64bit)
   libpng12.so.0(PNG12_0)(64bit)
   libtiff.so.3()(64bit)
   libz.so.1()(64bit)
   netpbm = 10.35-11.fc7
   perl >= 1:5.0
   perl(Cwd)
   perl(English)
   perl(Errno)
   perl(Fcntl)
   perl(File::Basename)
   perl(File::Spec)
   perl(File::Temp)
   perl(Getopt::Long)
   perl(strict)

* %check is not present; no test suite upstream.
* shared libraries present; ldconfig called properly.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets are OK (ldconfig)
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers are in the -devel subpackage.
* no pkgconfig files.
X a static library is presesnt.
* no libtool .la droppings.

Created attachment 148895 [details]
Suggested specfile patch fixing several issues.

(In reply to comment #2)
> First off, here's the rpmlint output; most of it seems pretty easy.
> 
> W: netpbm summary-ended-with-dot A library for handling different graphics
file formats.
> W: netpbm-devel summary-ended-with-dot Development tools for programs which
will use the netpbm libraries.
> W: netpbm-progs summary-ended-with-dot Tools for manipulating graphics files
in netpbm supported formats.
>    Trivial to fix.

Fixed.
 
> W: netpbm invalid-license freeware
> W: netpbm-debuginfo invalid-license freeware
> W: netpbm-devel invalid-license freeware
> W: netpbm-progs invalid-license freeware
>    It's tough to summarise the multitude of netpbm licenses; I'm not sure if
>   "freeware" is the proper thing to use or not.  I'll see what spot has to
>   say.
>   Newsflash: spot says no, but something like:
>   License: Assorted licenses, see %{_docdir}/%{name}-%{version}/copyright_summary 
>   would be OK, assuming that that file gets added to the package.

I added link to COPYRIGHT.PATENT which is there.

> E: netpbm obsolete-not-provided libgr
> E: netpbm-devel obsolete-not-provided libgr-devel
> E: netpbm-progs obsolete-not-provided libgr-progs
>    I believe these should just go away.  No Fedora release has ever had
>    libgr as far as I can tell.

Removed.

> E: netpbm-progs only-non-binary-in-usr-lib
>    This is due to the four palm*.map files.  I wonder why these aren't under
>    /usr/share since they shouldn't be arch-dependent.

These files doesn't seem to be required to let the palmtopnm function properly,
so I removed them.

> W: netpbm-progs file-not-utf8 /usr/share/man/man1/pgmminkowski.1.gz
>    A quick run through iconf should fix this up.

iconv is not needed here as the only bad character causing this is 0xa0 present
in the literature citation. I removed it as it's not needed.

> So, the first fun question is: where do those tarballs come from?  Upstream
> doesn't seem to produce any tarballs,



 so I guess the ones in the package are
> made from svn export.  You should provide information on generating those
> tarballs; see
> http://fedoraproject.org/wiki/Packaging/SourceURL#RevisionControl
> 
> I'm guessing something like:
>   svn export
https://netpbm.svn.sourceforge.net/svnroot/netpbm/release_number/10.35.0
netpbm-10.35
> was used, except that the above produces something a bit different from what's
> in netpbm-10.35.l1.tar.bz2.  And I've no idea where netpbmdoc-10.35.l1.tar.bz2
> comes from.  The changelog indicates that these have bits removed due to
> license issues, which is fine but it should be made clear, perhaps with a
> simple script that does the work.
> 
> There seems to be some good license information in doc/copyright_summary that
> really should be in the pacakge.

Added.

> This doesn't seem to be the latest version (that seems to be either 10.35.23
> or 10.37.03 depending on which branch you want to follow).  But I'm sure
> rebasing right now isn't a great idea, although it would be nice to carry
> fewer than 17 patches.

The 10.35 code tarball is the last published netpbm tarball before the upstream
decided to change a release policy. The tarball comes with hpcd and jpeg2000
support removed, so it's not the original one. The netpbmdoc tarball is
generated by my scripts from the html pages by makeman. This is the suggested
way how to get netpbm man pages by upstream.

> There's no need to test RPM_BUILD_ROOT against "/" in %clean and %install.
> 
> I tried a parallel make but it dies pretty quickly; this should be commented.
> 
> This package includes a static library, which it shouldn't without a good
> reason.  (And if it really needs to, it needs to be in a -static subpackage.)

Yes, let's get rid of it.

> I'll attach a patch which fixes up the trivial rpmlint warnings, fixes
> License:, nukes the obsoletes, runs iconv on the errant manpage, fixes
> buildroot cleaning and drops the static library.  This leaves the
> "only-non-binary-in-usr-lib" complaint as the only remaining one to worry
> about.  I'm afraid I can't help with the upstream source bits.

Thanks, applied most of your fixes.

> Review:
> X Can't check source against upstream.
> * package meets naming and versioning guidelines.
> * specfile is properly named, is cleanly written and uses macros consistently.
> * dist tag is present.
> * build root is correct.
> ? license field matches the actual license.
> * license is open source-compatible.
> X license text included in package.
> O latest version is not being packaged.
> * BuildRequires are proper (don't need to list perl explicitly)
> * compiler flags are appropriate.
> * %clean is present.
> * package builds in mock (development, x86_64).
> * package installs properly
> * debuginfo package looks complete.
> X rpmlint is silent.
> * final provides and requires are sane:
>   netpbm-10.35-11.fc7.x86_64.rpm
>    libnetpbm.so.10()(64bit)
>    netpbm = 10.35-11.fc7
>   =
>    /sbin/ldconfig
>    libnetpbm.so.10()(64bit)
> 
>   netpbm-devel-10.35-11.fc7.x86_64.rpm
>    netpbm-devel = 10.35-11.fc7
>   =
>    libnetpbm.so.10()(64bit)
>    netpbm = 10.35-11.fc7
> 
>   netpbm-progs-10.35-11.fc7.x86_64.rpm
>    netpbm-progs = 10.35-11.fc7
>   =
>    /bin/sh
>    /usr/bin/perl
>    libX11.so.6()(64bit)
>    libjpeg.so.62()(64bit)
>    libnetpbm.so.10()(64bit)
>    libpng12.so.0()(64bit)
>    libpng12.so.0(PNG12_0)(64bit)
>    libtiff.so.3()(64bit)
>    libz.so.1()(64bit)
>    netpbm = 10.35-11.fc7
>    perl >= 1:5.0
>    perl(Cwd)
>    perl(English)
>    perl(Errno)
>    perl(Fcntl)
>    perl(File::Basename)
>    perl(File::Spec)
>    perl(File::Temp)
>    perl(Getopt::Long)
>    perl(strict)
> 
> * %check is not present; no test suite upstream.
> * shared libraries present; ldconfig called properly.
> * owns the directories it creates.
> * doesn't own any directories it shouldn't.
> * no duplicates in %files.
> * file permissions are appropriate.
> * scriptlets are OK (ldconfig)
> * code, not content.
> * documentation is small, so no -docs subpackage is necessary.
> * %docs are not necessary for the proper functioning of the package.
> * headers are in the -devel subpackage.
> * no pkgconfig files.
> X a static library is presesnt.
> * no libtool .la droppings.
> 

After a long hiatus, I've found sime time to get back to this.

The rpmlind issues are down to:

W: netpbm invalid-license Assorted licenses, see
/usr/share/doc/netpbm-10.35/copyright_summary
E: netpbm configure-without-libdir-spec
E: netpbm hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/lib*

All of which are OK.

> The 10.35 code tarball is the last published netpbm tarball before the upstream
> decided to change a release policy. The tarball comes with hpcd and jpeg2000
> support removed, so it's not the original one. The netpbmdoc tarball is
> generated by my scripts from the html pages by makeman. This is the suggested
> way how to get netpbm man pages by upstream.

My point is that regareless of how the tarballs are generated, the method
needs to be documented in the specfile and any scripts you use need to be
included.

Really, that and a little comment indicating that parallel make doesn't work
are the only flaws I see with this package right now (unless I'm forgetting
something).

Mass reassigning all merge reviews to their component. For more details, see this FESCO ticket:

  https://fedorahosted.org/fesco/ticket/1269

If you don't know what merge reviews are about, please see:

  https://fedoraproject.org/wiki/Merge_Reviews

How to handle this bug is left to the discretion of the package maintainer.

rpmlint executed on rawhide gives these results:

$ rpmlint netpbm.spec 
netpbm.spec:10: W: macro-in-comment %{version}
netpbm.spec:11: W: macro-in-comment %{version}
netpbm.spec:12: W: macro-in-comment %{version}
netpbm.spec:14: W: macro-in-comment %{version}
netpbm.spec:122: W: configure-without-libdir-spec
netpbm.spec:188: E: hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/lib*
netpbm.spec: W: invalid-url Source0: netpbm-10.66.02.tar.xz
0 packages and 1 specfiles checked; 1 errors, 6 warnings.
$ 

First 4 warning are steps how to generate Source0.
I have added to dist-git a script which does it automatically.
https://lists.fedoraproject.org/pipermail/scm-commits/Week-of-Mon-20150302/1528975.html

Script is called netpbm2tar.sh.

More Packaging:Guidelines review: https://lists.fedoraproject.org/pipermail/scm-commits/Week-of-Mon-20150302/1529001.html