Bug 575485 (libharu)

Hey Alex. I'd be happy to review this for you. 

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name. 
OK - Spec has consistant macro usage. 
OK - Meets Packaging Guidelines. 
OK - License
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
0623b8fb08ae1b28af08b2cdbd66b662  libharu-2.1.0.tar.gz
0623b8fb08ae1b28af08b2cdbd66b662  libharu-2.1.0.tar.gz.orig

OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good. 
OK - Package has a correct %clean section. 
OK - Package has correct buildroot
OK - Package is code or permissible content. 
OK - Packages %doc files don't affect runtime. 
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Headers/static libs in -devel subpackage. 
OK - Spec has needed ldconfig in post and postun
See below - .so files in -devel subpackage.
OK - -devel package Requires: %{name} = %{version}-%{release}
OK - .la files are removed. 

OK - Package compiles and builds on at least one arch. 
OK - Package has no duplicate files in %files. 
OK - Package doesn't own any directories other packages own. 
OK - Package owns all the directories it creates. 
OK - Package obey's FHS standard (except for 2 exceptions)
See below - No rpmlint output. 
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock. 
OK - Should build on all supported archs
OK - Should have dist tag
OK - Should package latest version
OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin

Issues: 

1. The .so files are not quite right here. 

the .so should be in the devel subpackage. 
The versioning is odd also, as they have: 

libhpdf-2.1.0.so instead of libhpdf.so.2.1.0 ?

I guess it might be ok to have *.so in the devel package and *-2.1.0.so in the main pkg?
Do any of the demo programs work if you do this?

2. rpmlint says: 

libharu.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/libharu-2.1.0/README
libharu.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/libharu-2.1.0/CHANGES
libharu-devel.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/libharu-devel-2.1.0/README
libharu-devel.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/libharu-devel-2.1.0/CHANGES

This can be ignored, but you should probibly not include those both in both files. 
The base file should be fine, as thats required by the devel package. 

libharu-debuginfo.x86_64: E: empty-debuginfo-package

This seems like it might be fallout from the weird library naming. ;( 

3. Please don't include the INSTALL file. There is no need to tell people how
to build the file if they are installing the package. 

4. Is it worth including the demos in the devel package as doc files?

(In reply to comment #1)
> Hey Alex. I'd be happy to review this for you. 
> 
> OK - Package meets naming and packaging guidelines
> OK - Spec file matches base package name. 
> [...cut...]
> OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or
> /usr/sbin
> 
> Issues: 
> 
> 1. The .so files are not quite right here. 
> 
> the .so should be in the devel subpackage. 
> The versioning is odd also, as they have: 
> 
> libhpdf-2.1.0.so instead of libhpdf.so.2.1.0 ?
> 
> I guess it might be ok to have *.so in the devel package and *-2.1.0.so in the
> main pkg?

 - you are right: the .so versioning is odd; anyway I put .so in devel and -2.1.0 in the main one

> Do any of the demo programs work if you do this?

 - yes, they do (after the modification mentioned above)

> 2. rpmlint says: 
> 
> libharu.x86_64: W: wrong-file-end-of-line-encoding
> /usr/share/doc/libharu-2.1.0/README
> [...cut...]
> libharu-devel.x86_64: W: wrong-file-end-of-line-encoding
> /usr/share/doc/libharu-devel-2.1.0/CHANGES
> 
> This can be ignored, but you should probibly not include those both in both
> files. 
> The base file should be fine, as thats required by the devel package. 

 - removed CHANGES and README from devel pkg

> libharu-debuginfo.x86_64: E: empty-debuginfo-package
> 
> This seems like it might be fallout from the weird library naming. ;( 

 - resolved enabling --enable-debug in configure

> 3. Please don't include the INSTALL file. There is no need to tell people how
> to build the file if they are installing the package. 
 
 - INSTALL file removed

> 4. Is it worth including the demos in the devel package as doc files?
 - yes, it is :-) Included underd doc/ in devel pkg

Release 2 available here:

Spec URL:
http://digilander.libero.it/nyrk/Fedora/libharu/2.1.0-2/libharu.spec
SRPM URL:
http://digilander.libero.it/nyrk/Fedora/libharu/2.1.0-2/libharu-2.1.0-2.fc12.src.rpm

 - Build on koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=2069230


rpmlint says:

libharu.i686: W: wrong-file-end-of-line-encoding /usr/share/doc/libharu-2.1.0/CHANGES
libharu.i686: W: wrong-file-end-of-line-encoding /usr/share/doc/libharu-2.1.0/README

 - As you already said, this can be ignored but, if you think it is worth, can be fixed using dos2unix: let me know.

libharu-devel.i686: W: file-not-utf8 /usr/share/doc/libharu-devel-2.1.0/demo/text_annotation.c
libharu-devel.i686: W: file-not-utf8 /usr/share/doc/libharu-devel-2.1.0/demo/outline_demo.c

 - This should be ok: these are 2 examples to show how libharu deals with non-UTF8 encoded text (so I think it is definitely better do not fix it with iconv ;-))

4 packages and 1 specfiles checked; 0 errors, 4 warnings.

all issues from comment #1 seem fixed now. 

I don't see any further blockers, so this package is APPROVED. 
I will go ahead and sponsor you now. 

Continue the process from: 

https://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_CVS_and_Set_Owner

and if you have any questions at all, contact me on irc, email or in this review. ;) 
Welcome to packaging!

New Package CVS Request
=======================
Package Name: libharu
Short Description: C library for generating PDF files
Owners: nyrk71
Branches: F-12 F-13

(In reply to comment #4)
> all issues from comment #1 seem fixed now. 
> 
> I don't see any further blockers, so this package is APPROVED. 
> I will go ahead and sponsor you now. 

Thank you very much for reviewing and sponsoring

> Continue the process from: 
> 
> https://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_CVS_and_Set_Owner

Done!
 
> and if you have any questions at all, contact me on irc, email or in this
> review. ;) 
> Welcome to packaging!

Thank you again: I'm very glad to give my little contribution to the growth of Fedora

CVS Done

libharu-2.1.0-2.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/libharu-2.1.0-2.fc12

libharu-2.1.0-2.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/libharu-2.1.0-2.fc13

libharu-2.1.0-2.fc12 has been pushed to the Fedora 12 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update libharu'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/libharu-2.1.0-2.fc12

libharu-2.1.0-2.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

libharu-2.1.0-2.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.