Spec URL: http://people.fedoraproject.org/~jmoskovc/libmirage.spec SRPM URL: http://people.fedoraproject.org/~jmoskovc/libmirage-1.0.0-2.fc10.src.rpm Description: CD-ROM image access library, and part of the userspace-cdemu suite, a free, GPL CD/DVD-ROM device emulator for linux. It is written in C and based on GLib. The aim of libMirage is to provide uniform access to the data stored in different image formats, by creating a representation of disc stored in image file, which is based on GObjects. rpmlint: libmirage.i686: E: zero-length /usr/share/doc/libmirage-1.0.0/NEWS libmirage.i686: E: zero-length /usr/share/doc/libmirage-1.0.0/ChangeLog These files are really empty, but will be probably used in future.
MUST Items: xx - rpmlint is unclean on RPM + [rishi@ginger x86_64]$ rpmlint libmirage-1.0.0-2.fc8.x86_64.rpm libmirage.x86_64: E: zero-length /usr/share/doc/libmirage-1.0.0/ChangeLog libmirage.x86_64: E: zero-length /usr/share/doc/libmirage-1.0.0/NEWS [rishi@ginger x86_64]$ OK - follows Naming Guidelines OK - spec file is named as %{name}.spec xx - package does not meet Packaging Guidelines + http://cdemu.sourceforge.net/pkg_libmirage.php looks to be a more appropriate choice for URL. + Did you try to get the patch accepted upstream? It might affect SPARC too. + The versioned dependencies on pkgconfig, flex and glib2-devel are not needed. According to https://fedoraproject.org/wiki/Packaging/Guidelines#Requires: "if the lowest possible requirement is so old that nobody has a version older than that installed on any target distribution release, there's no need to include the version in the dependency at all. ... As a rule of thumb, if the version is not required, don't add it just for fun." + No need to delete %{_libdir}/libmirage/*.a in %install since --disable-static was passed to %configure. + Remove the empty ChangeLog and NEWS from %doc. OK - license meets Licensing Guidelines xx - License field meets actual license + Should be GPLv2+ instead of LGPLv2+. See http://fedoraproject.org/wiki/Licensing#SoftwareLicenses OK - upstream license file included in %doc OK - spec file uses American English OK - spec file is legible xx - sources match upstream sources + The gzipped tarball is no longer available at the given Source location. You could use the bzipped tarball instead. However it looks like the bzipped tarball has problems with parallel builds. OK - package builds successfully OK - ExcludeArch not needed xx - missing build dependencies + In order to build the documentation 'BuildRequires: gtk-doc' is needed. OK - no locales OK - %post and %postun invoke ldconfig OK - package is not relocatable xx - missing dependency on package that creates directory + The -devel subpackage should have 'Requires: gtk-doc' since it puts files in a sub-directory within /usr/share/gtk-doc. OK - no duplicates in %file OK - file permissions set properly OK - %clean present OK - macros used consistently + While %{name} is used in Source, libmirage is used in the rest of the cases. You can consider using %{name} throughout the Spec file. OK - contains code and permissable content OK - -doc is not needed OK - contents of %doc does not affect the runtime OK - header files in -devel OK - no static libraries OK - -devel has *.pc file and requires pkgconfig OK - library files without suffix in -devel OK - -devel requires base package OK - no libtool archives OK - %{name}.desktop file not needed OK - does not own files or directories owned by other packages OK - buildroot correctly prepped OK - all file names valid UTF-8 SHOULD Items: OK - upstream provides license text xx - no translations for description and summary OK - package builds in mock successfully OK - package builds on all supported architectures OK - package functions as expected OK - scriptlets are sane OK - subpackages other than -devel are not needed OK - pkgconfig files in -devel OK - no file dependencies
Ping?
Hi, sorry for the delay,I was on vacation. The following issues you pointed out should be fixed: + http://cdemu.sourceforge.net/pkg_libmirage.php looks to be a more -fixed + Did you try to get the patch accepted upstream? It might affect SPARC too. - accepted (should be applied in the latest released version) + The versioned dependencies on pkgconfig, flex and glib2-devel are not - fixed + No need to delete %{_libdir}/libmirage/*.a in %install since -fixed + Remove the empty ChangeLog and NEWS from %doc. - fixed + The gzipped tarball is no longer available at the given Source location. - a new version has been released during this package review, should I update to it? + In order to build the documentation 'BuildRequires: gtk-doc' is needed. - fixed + The -devel subpackage should have 'Requires: gtk-doc' since it puts files - fixed + Should be GPLv2+ instead of LGPLv2+ - fixed
(In reply to comment #3) Please provide URLs to your new Spec/SRPM pair, bump their Release field and update the %changelog, when you make such a series of changes. That helps in tracking the changes and progress made during the course of the review. > + The versioned dependencies on pkgconfig, flex and glib2-devel are not > - fixed The -devel subpackage still has 'Requires: pkgconfig >= 1:0.14'. It should be 'Requires: pkgconfig'. > + The gzipped tarball is no longer available at the given Source location. > - a new version has been released during this package review, should I update to it? Even then the bzipped tarball of 1.0.0 is still available, while the gzipped counterpart is not. That is enough reason to fix the Source0. Whether you want to update to a new upstream release is a different matter and is your decision (I think you should). Also, the 'BuildRequires: pkgconfig' is not needed and should be removed, because those build dependencies which provide *.pc files should themselves bring it in.
Hi, thanks for the tips, I've updated to the latest version and made followong changes to spec: - fixed Source url - removed pkgconfig from BuildRequires - added shared-mime-info to Requires - dropped ppc patch - add the missing changelog Updated files: Spec: http://people.fedoraproject.org/~jmoskovc/libmirage.spec SRPM: http://people.fedoraproject.org/~jmoskovc/libmirage-1.0.0-2.fc10.src.rpm
sorry, wrong srpm url ^ Updated files: Spec: http://people.fedoraproject.org/~jmoskovc/libmirage.spec SRPM: http://people.fedoraproject.org/~jmoskovc/libmirage-1.1.0-1.fc10.src.rpm
(In reply to comment #6) > Updated files: > Spec: http://people.fedoraproject.org/~jmoskovc/libmirage.spec > SRPM: http://people.fedoraproject.org/~jmoskovc/libmirage-1.1.0-1.fc10.src.rpm Fails to build: http://koji.fedoraproject.org/koji/taskinfo?taskID=768031 Looks like the new version has a dependency on zlib. Please add 'BuildRequires: zlib-devel'.
(In reply to comment #6) + Since you are installing new mimeinfo, the MIME database needs to be updated. Moreover 'Requires: shared-mime-info' is not needed. See https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#mimeinfo for details. + You could also consider using: make install INSTALL="%{__install} -p" DESTDIR=%{buildroot} ... to preserve timestamps.
Hi, I did a following changes to spec file: - removed shared-mime-info dependency - add script to %post and %postun to update mimeinfo db if s-m-info is installed - added zlib-devel to BuildRequires (now it builds in koji) - added INSTALL="%{__install} -p" to preserve timestamps Spec: http://people.fedoraproject.org/~jmoskovc/libmirage.spec SRPM: http://people.fedoraproject.org/~jmoskovc/libmirage-1.1.0-2.fc10.src.rpm
The libmirage.spec that you posted and the one in the SRPM are not the same. The difference is trivial, but still: [rishi@ginger SRPMS]$ diff -uNp libmirage.spec ../SPECS/libmirage.spec --- libmirage.spec 2008-08-11 14:33:55.000000000 +0530 +++ ../SPECS/libmirage.spec 2008-08-11 14:18:21.000000000 +0530 @@ -75,9 +75,8 @@ update-mime-database %{_datadir}/mime &> %changelog * Mon Aug 11 2008 Jiri Moskovcak <jmoskovc> - 1.1.0-2 - more spec file fixes: - - added INSTALL="%{__install} -p" to preserve timestamps + - added INSTALL="%{__install} -p" to preserve timestamp - removed shared-mime-info from Requires since it's not needed - - added zlib-devel do BuildRequires
Rpmlint errors: [rishi@ginger x86_64]$ rpmlint -i libmirage-1.1.0-2.fc8.x86_64.rpm libmirage.x86_64: E: postin-without-ldconfig /usr/lib64/libmirage.so.1.0.0 This package contains a library and its %post scriptlet doesn't call ldconfig. libmirage.x86_64: E: postun-without-ldconfig /usr/lib64/libmirage.so.1.0.0 This package contains a library and its %postun doesn't call ldconfig. libmirage.x86_64: E: non-empty-%post /sbin/ldconfig libmirage.x86_64: E: non-empty-%postun /sbin/ldconfig [rishi@ginger x86_64]$ Since your %post and %postun stanzas contain more than one command, you need to change them to: %post /sbin/ldconfig update-mime-database %{_datadir}/mime &> /dev/null || : %postun /sbin/ldconfig update-mime-database %{_datadir}/mime &> /dev/null || :
Fixed post and postun, it builds in koji and rpmlint is quiet. Spec: http://people.fedoraproject.org/~jmoskovc/libmirage.spec SRPM: http://people.fedoraproject.org/~jmoskovc/libmirage-1.1.0-3.fc10.src.rpm
Rpmlint warnings: [rishi@ginger SPECS]$ rpmlint libmirage libmirage.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmirage.so.1.0.0 /usr/lib64/libsndfile.so.1 libmirage.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmirage.so.1.0.0 /lib64/libdl.so.2 [rishi@ginger SPECS]$ As shown in https://fedoraproject.org/wiki/PackageMaintainers/Common_Rpmlint_Issues#unused-direct-shlib-dependency you can modify your %build stanza as follows: %build %configure --enable-gtk-doc --disable-static # Omit unused direct shared library dependencies. sed --in-place --expression 's! -shared ! -Wl,--as-needed\0!g' libtool make %{?_smp_mflags} Everything else looks fine. +---------------------------------+ | This package is APPROVED by me. | +---------------------------------+
New Package CVS Request ======================= Package Name: libmirage Short Description: library for reading various CD/DVD image files Owners: jmoskovc Branches: F-9 F-10
cvs done.