Bug 460260

This is a F-10 blocker, since gtkmm needs to follow the gtk2 2.14 track.

Just a few minor issues. Ownership of dirs created with the %dir macro and the
Requires for the devel package. 

+ rpmlint output

rpmlint pangomm-2.13.7-1.fc10.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
+ license matches the actual package license

+ %doc includes license file
+ spec file written in American English
+ spec file is legible
+ upstream sources match sources in the srpm
  148578fbfa7286de2141f3291d64d1e3 pangomm-2.13.7.tar.bz2
+ package successfully builds on at least one architecture
  tested on x86_64 using mock
+ BuildRequires list all build dependencies
+ %find_lang instead of %{_datadir}/locale/*
+ binary RPM with shared library files must call ldconfig in %post and %postun
+ does not use Prefix: /usr
- package owns all directories it creates
  doesn't own the following dirs that it creates:
  %{_datadir}/gtk-doc/html/pangomm-1.4
  %{_includedir}/pangomm-1.4
  %{_includedir}/pangomm-1.4/pangomm
n/a no duplicate files in %files
+ %defattr line
+ %clean contains rm -rf $RPM_BUILD_ROOT
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package
+ header files should be in -devel
n/a static libraries should be in -static
- packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
+ libfoo.so must go in -devel
+ devel must require the fully versioned base
+ packages should not contain libtool .la files
n/a packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ %install must start with rm -rf %{buildroot} etc.
+ filenames must be valid UTF-8

Optional:

n/a if there is no license file, packager should query upstream
n/a translations of description and summary for non-English languages, if
available
+ reviewer should build the package in mock
n/a the package should build into binary RPMs on all supported architectures
+ review should test the package functions as described (basic testing using
kvm)
+ scriptlets should be sane
+ pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or
/usr/sbin

I think also the devel package needs to have a "Requires: gtk-doc" as well as
the pkgconfig requires as it uses gtk-doc

Well,

(In reply to comment #2)
> - package owns all directories it creates
>   doesn't own the following dirs that it creates:
>   %{_datadir}/gtk-doc/html/pangomm-1.4
>   %{_includedir}/pangomm-1.4
>   %{_includedir}/pangomm-1.4/pangomm

  - -devel subpackage actually owns these directories.
    When written as
---------------------------------------------------------------------------
%files
%{_includedir}/pangomm-%{apiver}
---------------------------------------------------------------------------
    This contains the directory %{_includedir}/pangomm-%{apiver} and all directories/files/etc
    under this directory.

Some notes:
  - "BuildRequires: pango-devel" is listed twice.
  - The %description of -devel subpackage is not right.
---------------------------------------------------------------------------
This package contains the static libraries and header files needed for
                          ^^^^^^
---------------------------------------------------------------------------
  - Would you consider to use
---------------------------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
---------------------------------------------------------------------------
    to keep timestamps on as many files as possible? This method usually works for
    recent autotool based Makefiles.
  - Would you explain why you want to remove files under %_libdir/pangomm-%apiver ?
  - %{_datadir}/gtk-doc/html/ is already marked as docDirs (see Changelog in "rpm" package:
    from 2007-06-29)

> - package owns all directories it creates

Hmm, looks good to me over here. I think Mamoru-san addressed this in his comment.

# rpm -qf /usr/share/gtk-doc/html/pangomm-1.4
pangomm-devel-2.13.7-1.fc10.i386
# rpm -qf /usr/include/pangomm-1.4
pangomm-devel-2.13.7-1.fc10.i386

> - packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'

Fixed.

> I think also the devel package needs to have a "Requires: gtk-doc"
> as well as the pkgconfig requires as it uses gtk-doc

Fixed.

> "BuildRequires: pango-devel" is listed twice.

Oops. Fixed.

> The %description of -devel subpackage is not right.

Yes, odd, I can't remember where that's coming from. Fixed.

> Would you consider to use make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"

Sure. Fixed.

> %{_datadir}/gtk-doc/html/ is already marked as docDirs

Fixed.


> Would you explain why you want to remove files under
> %_libdir/pangomm-%apiver ?

In all gtkmm project tarballs, %{libdir}/%{name} always contains files
that are related to the code generation scripts use by the C++ wrapper
(the actual C++ interface code is generated dynamically by m4 scripts
when the tarball is created, i.e before build time). So these files
are not particularly useful even to a gtkmm-based project developer. I
removed them mainly to reduce polution of the libdir directory. I've
asked upstream before about this, the answer iirc was "in case someone
is interested"...



Spec URL: http://www.poolshark.org/src/pangomm.spec
SRPM URL: http://www.poolshark.org/src/pangomm-2.13.7-2.fc10.src.rpm

Spec file seems to be broken and hence I'm seeing buld failure in mock.

pm-ad%files
%defattr(-, root, root, -)
%doc AUTHORS ChangeLog COPYING NEWS README
%{_libdir}/*.so.*

From mock build.log

Processing files: pangomm-debuginfo-2.13.7-2.fc10
Checking for unpackaged file(s): /usr/lib/rpm/check-files /builddir/build/BUILDROOT/pangomm-2.13.7-2.fc10.x86_64
RPM build errors:
error: Installed (but unpackaged) file(s) found:
   /usr/lib64/libpangomm-1.4.so.1
   /usr/lib64/libpangomm-1.4.so.1.0.30
    Installed (but unpackaged) file(s) found:
   /usr/lib64/libpangomm-1.4.so.1
   /usr/lib64/libpangomm-1.4.so.1.0.30
Child returncode was: 1
EXCEPTION: Command failed. See logs for output.
 # ['bash', '--login', '-c', 'rpmbuild -bb --target x86_64 --nodeps builddir/build/SPECS/pangomm.spec']
Traceback (most recent call last):
  File "/usr/lib/python2.5/site-packages/mock/trace_decorator.py", line 70, in trace
    result = func(*args, **kw)
  File "/usr/lib/python2.5/site-packages/mock/util.py", line 316, in do
    raise mock.exception.Error, ("Command failed. See logs for output.\n # %s" % (command,), child.returncode)
Error: Command failed. See logs for output.
 # ['bash', '--login', '-c', 'rpmbuild -bb --target x86_64 --nodeps builddir/build/SPECS/pangomm.spec']
LEAVE do --> EXCEPTION RAISED

Oops, bad case of emacs-itis. Try again, I've fixed the uploads.

Now I leave this review request to how Peter-san judges.

(In reply to comment #7)
> Now I leave this review request to how Peter-san judges.

Mamoru are you happy with the explanation for your "Would you explain why you want to remove files under %_libdir/pangomm-%apiver ?" question?

Peter

(In reply to comment #8)
> (In reply to comment #7)
> Mamoru are you happy with the explanation for your "Would you explain why you
> want to remove files under %_libdir/pangomm-%apiver ?" question?
> 
> Peter

Yes.

Looks good to me now. Approved.

+ rpmlint output

rpmlint pangomm-2.13.7-2.fc10.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
+ license matches the actual package license

+ %doc includes license file
+ spec file written in American English
+ spec file is legible
+ upstream sources match sources in the srpm
  148578fbfa7286de2141f3291d64d1e3 pangomm-2.13.7.tar.bz2
+ package successfully builds on at least one architecture
  tested on x86_64 using mock
+ BuildRequires list all build dependencies
+ %find_lang instead of %{_datadir}/locale/*
+ binary RPM with shared library files must call ldconfig in %post and %postun
+ does not use Prefix: /usr
+ package owns all directories it creates
n/a no duplicate files in %files
+ %defattr line
+ %clean contains rm -rf $RPM_BUILD_ROOT
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package
+ header files should be in -devel
n/a static libraries should be in -static
+ packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
+ libfoo.so must go in -devel
+ devel must require the fully versioned base
+ packages should not contain libtool .la files
n/a packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ %install must start with rm -rf %{buildroot} etc.
+ filenames must be valid UTF-8

Optional:

n/a if there is no license file, packager should query upstream
n/a translations of description and summary for non-English languages, if
available
+ reviewer should build the package in mock
n/a the package should build into binary RPMs on all supported architectures
+ review should test the package functions as described (basic testing using kvm)
+ scriptlets should be sane
+ pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or
/usr/sbin

+ I think also the devel package needs to have a "Requires: gtk-doc" as well as the pkgconfig requires as it uses gtk-doc

Peter, Mamoru-san, many thanks for the review.


New Package CVS Request
=======================
Package Name: pangomm
Short Description: C++ wrapper for pango
Owners: denis
Branches:
InitialCC:

cvs done.