Fedora Merge Review: netpbm http://cvs.fedora.redhat.com/viewcvs/devel/netpbm/ Initial Owner: jnovy
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