[Bug 188542] Review Request: hylafax

Thu Apr 13 00:43:45 UTC 2006

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: hylafax

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=188542

------- Additional Comments From fedora.wickert at arcor.de  2006-04-12 20:43 EST -------
I was thinking about being that "somebody eelse", but that CAPI stuff is so
horribly broken sometimes I don't dare maintaining it. ;)

REVIEW:

> $ md5sum hylafax-4.2.5.5-1.src.rpm
> 373044fd59ff14554ccda17b2fcca028  hylafax-4.2.5.5-1.src.rpm

Good - MUST Items

- package and specfile naming according to guidelines
- package meets guidelines
- license ok
- license field in spec matches actual license
- license included in source and correctly installed in %doc
- spec written in American English 
- spec is legible
- BuildRequires ok, no duplicates and none of the listed exceptions
- no locales to worry about
- ldconfig correctly called for shared libs in %post and %postun
- relocatable
- package own all directories it creates
- packages doesn't own files or directories already owned by other packages
- %files section ok, no duplicates
- permissions ok, correct %defattr
- clean section present and ok
- macro usage consistent
- code, not content
- no large docs
- %doc section ok, docs don't affect runtime
- no headers, static libs or pkgconfigs to worry about
- no libtool archives

Good - SHOULD Items
- package builds im mock (Core 5 i386)
- package seems to work as usual, nevertheless I can't really test I here right
now in absence of a modem
- scriptlets match examples from wiki
- package uses disttag

Needswork - MUST Items

- rpmlint errors and warnings:
> $ rpmlint hylafax-4.2.5.5-1.fc5.src.rpm | sort
> E: hylafax no-%clean-section
> 
This is "[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT"
confusing rpmlint. I suggest you use the regular "rm -rf $RPM_BUILD_ROOT" (don't
worry about the buildhosts ;)) or ignore this message, the %clean section in
your spec is valid.

> $ rpmlint hylafax-4.2.5.5-1.fc5.i386.rpm | sort
> E: hylafax executable-marked-as-config-file /etc/cron.daily/hylafax
> E: hylafax executable-marked-as-config-file /etc/cron.hourly/hylafax
> 
safe to ignore, but IMO the cronjobs shouldn't be "noreplace" since they 
don't store any configuration.

> E: hylafax executable-marked-as-config-file /etc/rc.d/init.d/hylafax
> 
the initscript should not be a config file. It shouldn't be "noreplace" 
ether, since it won't be replaced on updates then.

> E: hylafax explicit-lib-dependency libtiff
> 
remove libtiff from Requires, rpm will find that dependency itself

> E: hylafax invalid-soname /usr/lib/libfaxserver.so.4.2.5.5 libfaxserver.so
> E: hylafax invalid-soname /usr/lib/libfaxutil.so.4.2.5.5 libfaxutil.so
> 
safe to ignore in our case

> E: hylafax non-executable-script /var/spool/hylafax/bin/notify.awk 0444
> 
is this on purpose?

> E: hylafax non-readable /var/spool/hylafax/etc/hosts.hfaxd 0600
> 
due to ownership of uucp, safe to ignore

> E: hylafax non-standard-dir-perm /var/spool/hylafax/archive 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/docq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/doneq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/pollq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/sendq 0700
> E: hylafax non-standard-dir-perm /var/spool/hylafax/tmp 0700
> 
these are ok, but there are other dirs o worry about. Take a look 
at /var/spool/hylafax or /var/spool/hylafax/bin: These should be 
owned by root. IMO all dirs should be owned by root as long as they
don't need to be writable by uucp or this doesn't affect the runtime of
the program.

> E: hylafax script-without-shellbang /usr/sbin/faxsetup.linux
> E: hylafax script-without-shellbang /var/spool/hylafax/bin/dictionary
> 
These files should have start with something like "#! /bin/bash". Fix this 
upstream ;-)

> W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxserver.so
> W: hylafax devel-file-in-non-devel-package /usr/lib/libfaxutil.so
> 
Usually these files should go into a seperate hylafax-devel package. From 
the review guidlines: "MUST: If a package contains library files with a 
suffix (e.g. libfoo.so.1.1), then library files that end in .so (without 
suffix) must go in a -devel package."
But I doubt there's much sense rolling a package with only two symlinks inside.

> W: hylafax non-conffile-in-etc /etc/hylafax/faxcover_example_sgi.ps
> 
Safe to ignore.

> W: hylafax no-version-in-last-changelog
> 
append "- <version>-<release>" to every changelog entry, e.g.
* Tue Apr 11 2006 Lee Howard <faxguy  at howardsilvan dot com> - 4.2.5.5-1

> W: hylafax service-default-enabled /etc/rc.d/init.d/hylafax
> 
Please change "# chkconfig: 345 95 5" to "# chkconfig: - 95 5" in
hylafax.rh.init. see
http://fedoraproject.org/wiki/ScriptletSnippets#head-55b46ef483e6a08c24a8fc3b0b7e2ef7bfb84efd

- License: You can change the license back to "BSD-Style" or "BSD like" or
something like that, I have seen other packages even in Core with that too. To
me the COPYRIGHT looks basically BSD, but I leave it up to you. Just make sure
that the COPYRIGHT is correctly included in %doc.

- Source does not match upstream:

the one included in your rpm
> c4de725b0a2721df02880bf77809d3bd  hylafax-4.2.5.5.tar.gz
taken from the URL in Source0
> 6d9886532cbf2c21675ecb802b5ef115  ../downloads/hylafax-4.2.5.5.tar.gz
> 
The source always must match with Source0 from the spec.

- package does compile on current Core 5, I'm attaching a log. Nevertheless it
builds in Core 5 mock.

NEEDSWORK

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.