Bug 453569

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.