[Bug 188542] Review Request: hylafax

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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/show_bug.cgi?id=188542


tibbs@xxxxxxxxxxx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|                            |182235
              nThis|                            |




------- Additional Comments From tibbs@xxxxxxxxxxx  2008-05-10 18:02 EST -------
Note to FE-Legal folks, this package needs attention for two reasons and perhaps a third as well.  Please search for FE-Legal below.

Now, to the review.

Was the hylafax/hylafax+ issue ever resolved?  Do we need FE-Legal to be involved in that?

Can you explain why the tarball in the src.rpm does not match the tarball fetched from the Source0: URL?  There seem to be rather significant source differences.  This kind of thing is not permissible; the sources in the src.rpm must be identical to the upstream sources except in specific limited cases where we must remove something.  I note that the tarball in the src.rpm seems to be three days newer than the one upstream.

I don't believe we should package /etc/hylafax/faxcover_example_sgi.ps.  It contains the old SGI name and logo and I'm pretty certain we shouldn't be sticking it in /etc whenever someone installs this software.  I'm not even sure we have the legal right to distribute it, which is reason one for blocking FE-Legal.

It doesn't particularly bother me, but the guidelines to specify that you not use a specific sourceforge mirror for the source URL.  See 
http://fedoraproject.org/wiki/Packaging/SourceURL (although personally I find I often have to add one just to get things to download, since sourceforge is so incredibly unreliable).

I recommend not using the name of the package in the summary, as it tends to look rather redundant in listings.  Still, there is a change of case so I won't block the package if you think it really needs to be there.

I'm going to have to get expert assistance with the License: tag; the license given in the COPYRIGHT file is actually identical to the libtiff license, which gets its own "libtiff" license tag, but the regex code is clearly the bad "BSD+Advertising" clause which is GPL-incompatible and thus causes issues.  Reason two that I'm blocking FE-Legal for guidance.

Your changelog entries are not in one of the acceptable formats.  These are parsed automatically, so please follow the formats given in the Changelogs section of http://fedoraproject.org/wiki/Packaging/Guidelines and please also include a comment every time you change the release.

You need a dependency on the crontabs package if you want to put things in /etc/cron.daily.

You call ldconfig in your scriptlets, but you don't have any dependencies on it.  When you use the single-line scriptlets (%post -p /sbin/ldconfig) then you don't need them, but when you use multiline scriptlets you have to specify the dependencies manually.

Finally, I have significant issues with the amount of stuff this package puts under /var/spool.  I don't believe any of the files belong there at all.  Executables, certainly not.  Unless you can illustrate how the FHS allows such things, I cannot approve this package.  The modem config files need to be under /etc; the executables probably belong under /usr/libexec if they're not supposed to be run by the end user.

Checklist:
X source files do not match upstream.
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
? summary includes the name of the package.
* description is OK.
* dist tag is present.
* build root is OK.
X license field matches the actual license.
? license is open source-compatible.
* license text included in package.
X changelogs not correctly formatted.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint has acceptable complaints.
X final provides and requires:
   config(hylafax) = 5.2.4-3.fc9
   libfaxserver.so.5.2.4()(64bit)
   libfaxutil.so.5.2.4()(64bit)
   hylafax = 5.2.4-3.fc9
  =
   /bin/sh
   /sbin/chkconfig
   /sbin/service
   config(hylafax) = 5.2.4-3.fc9
   gawk
   ghostscript
   libfaxserver.so.5.2.4()(64bit)
   libfaxutil.so.5.2.4()(64bit)
   libgcc_s.so.1()(64bit)
   libgcc_s.so.1(GCC_3.0)(64bit)
   liblber-2.4.so.2()(64bit)
   libldap-2.4.so.2()(64bit)
   libpam.so.0()(64bit)
   libpam.so.0(LIBPAM_1.0)(64bit)
   libstdc++.so.6()(64bit)
   libstdc++.so.6(CXXABI_1.3)(64bit)
   libstdc++.so.6(GLIBCXX_3.4)(64bit)
   libtiff.so.3()(64bit)
   libutil.so.1()(64bit)
   libz.so.1()(64bit)
   mailx
   sharutils
X  (missing crontabs for /etc/cron.*)
X  (missing /sbin/ldconfig dependency for %post and %postun)

* %check is not present; no test suite present.  I have no way to test this 
   software.
X shared libraries are present; ldconfig called properly but dependency on it is 
   missing.
X ownership problems for /etc/cron.*
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
X scriptlets are OK, but ldconfig dependencies are missing.
* code, not content.
 documentation is small, so no -doc subpackage is necessary.
 %docs are not necessary for the proper functioning of the package.
 no headers.
 no pkgconfig files.
 no static libraries.
 no libtool .la files.


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

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@xxxxxxxxxx
http://www.redhat.com/mailman/listinfo/fedora-package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]