Bug 476829

Are you aware that rpmbuild since F-10 can create a noarch subpackage while the main package is "arch"? It was announced as one of the new features in rpm 4.6.

I am, although another reason for having a separate source package is so that bug fixes in the database engine can be distributed separately from the entire database which is quite large.

Yes, that sounds completely reasonable.

Few notes before full review:
- you can drop setting the CFLAGS for the "make" commands in %build, there is no C/C++ code built, if I see it right
- are the INSTALLSITELIB and INSTALLSITEARCH really required for the "make" commands in %install? They looks like some pythonic paths.
- what is the upstream versioning scheme? Does the version in the included archive mean that it is a snapshot made 20081124 after the 3.0 release? Then you should apply "post-release package guideline" - https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Post-Release_packages (version=3.0, release=1.20081124)
- according to the README in foomatic-db, there should be files licensed under MIT too, please recheck and update the License tag to "GPLv2+ and MIT"
- new snapshot was released today :-)

formal review is here, see the notes below:

source files match upstream:

BAD	package meets naming and versioning guidelines.
BAD	specfile is properly named, is cleanly written and uses macros consistently.
OK	dist tag is present.
BAD?	license field matches the actual license.
OK*	license is open source-compatible. License text not included upstream.
BAD	latest version is being packaged.
OK	BuildRequires are proper.
N/A	compiler flags are appropriate.
BAD	%clean is present.
OK	package builds in mock (Rawhide/x86_64).
N/A	debuginfo package looks complete.
OK	rpmlint is silent.
OK	final provides and requires look sane.
N/A	%check is present and all tests pass.
OK	no shared libraries are added to the regular linker search paths.
OK	owns the directories it creates.
OK	doesn't own any directories it shouldn't.
OK	no duplicates in %files.
OK	file permissions are appropriate.
OK	no scriptlets present.
OK	allowed content
OK	documentation is small, so no -docs subpackage is necessary.
OK	%docs are not necessary for the proper functioning of the package.
OK	no headers.
OK	no pkgconfig files.
OK	no libtool .la droppings.
OK	not a GUI app.

- see comment #3 for more detailed description of the issues
- versioning is wrong for a post-release snapshop
- AFAICT unused variable are passed in the make calls
- recheck the licenses for the files
- include the license(s) as %doc
- new snapshot is available
- using only "rm -rf %{buildroot}" is sufficient for the %clean section
- is the database useful for other purpose then for cups?

(In reply to comment #3)
> - you can drop setting the CFLAGS for the "make" commands in %build, there is
> no C/C++ code built, if I see it right

Yep.

> - are the INSTALLSITELIB and INSTALLSITEARCH really required for the "make"
> commands in %install? They looks like some pythonic paths.

No, removed.

> - what is the upstream versioning scheme? Does the version in the included
> archive mean that it is a snapshot made 20081124 after the 3.0 release? Then
> you should apply "post-release package guideline" -
> https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Post-Release_packages
> (version=3.0, release=1.20081124)

It's daily snapshots and very infrequent version releases.  I've switch to the post-release scheme now.

> - according to the README in foomatic-db, there should be files licensed under
> MIT too, please recheck and update the License tag to "GPLv2+ and MIT"

Fixed (new sub-package carries MIT license).

> - using only "rm -rf %{buildroot}" is sufficient for the %clean section

Fixed.

> - is the database useful for other purpose then for cups?  

No.

I've updated the package to match the current devel foomatic package (newer snapshot, oki4linux obsoleted, etc), and also split out a 'ppds' sub-package for bug #461234.

Spec URL: http://twaugh.fedorapeople.org/foomatic-db/foomatic-db.spec
SRPM URL:
http://twaugh.fedorapeople.org/foomatic-db/foomatic-db-4.0-1.20090702.fc11.src.rpm

Spec URL: http://twaugh.fedorapeople.org/foomatic-db/foomatic-db.spec
SRPM URL:
http://twaugh.fedorapeople.org/foomatic-db/foomatic-db-4.0-2.20090702.fc11.src.rpm

A few more changes:

* Mon Aug  3 2009 Tim Waugh <twaugh> 4.0-2.20090702
- Move foomatic-db-ppds symlink to ppds sub-package.
- Use sed instead of perl in raster PPDs (bug #512739).
- Removed code to convert old-style printer IDs (there are none).
- Ship README file.

Would be really useful to have this done for Fedora 12, as it is causing perl to be required on the Live CD.

I am back in work from vacations, so the review continues.

I have still found few issues:
- you should have the license as "GPLv2+ and MIT" for the ppd subpackage because at least PPDs for Brother, OKI and SHARP are licensed under GPLv2+
- add the COPYING file from foomatic-db-20090702 as %doc for the main package
- the question is who should own the /usr/share/foomatic/db/sources directory, because when only the "ppds" subpackage is installed then this dir (and all "upper dirs") will be unowned. If the "ppds" subpackage can be installed independently then creating "foomatic-db-filesystem" subpackage (containing the empty /usr/share/foomatic/db structure) looks like an option to me.

The rest looks good.

Spec URL: http://twaugh.fedorapeople.org/foomatic-db/foomatic-db.spec
SRPM URL:
http://twaugh.fedorapeople.org/foomatic-db/foomatic-db-4.0-3.20090702.fc11.src.rpm

* Mon Aug 17 2009 Tim Waugh <twaugh> 4.0-3.20090702
- License for ppds sub-package should include GPLv2+.
- Ship COPYING file in main package.
- Added filesystem sub-package for directory ownership.

All issues are fixed, this package is APPROVED.

Thank you!

New Package CVS Request
=======================
Package Name: foomatic-db
Short Description: Database of printers and printer drivers
Owners: twaugh
Branches: devel
InitialCC:

cvs done.