[Bug 224245] Merge Review: squirrelmail

[bugzilla at redhat.com]

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


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


Jason Tibbitts <tibbs at math.uh.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mhlavink at redhat.com,
                   |                            |tibbs at math.uh.edu




--- Comment #14 from Jason Tibbitts <tibbs at math.uh.edu>  2008-12-18 22:56:43 EDT ---
Looks like mhlavink at redhat.com owns this package now, so I'll update the CC. 
I've been staring at this, the very first merge review, for nearly two years
now and would love to see it go away.  I'm not sure if I have what it takes to
do a proper review of it, but I can at least look at the current packaging and
make some comments.

First off, I wonder how much of the above discussion about Fedora's version
being heavily patched is currently true.  I see only four patches applied; one
is a one-liner, one is a set of translation changes for, I believe, Turkish,
and the other two don't really patch in all that much (under fifteen lines
each).  It would indeed be nice to include the upstream status of those patches
in accordance with
http://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment
but I'm just not seeing how our current version is significantly patched. 
Maybe someone did some significant work on things in the last 18 months.  Or
maybe the problem is with some of the locale tweaks that happen directly in the
specfile.

It would also be nice to clean the patches up a bit, remove the unapplied
patches from CVS and get rid of the commented-out bits of the spec where those
patches used to be applied.

Here's what current rpmlint has to say about the src.rpm:

  37: E: prereq-use httpd, perl
Prereq: shouldn't be used; something is needed for a scriptlet, it should be
reflected in a fine-grained dependency like Requires(pre).  There are no
scriptlets here, so I think those should be regular runtime dependencies,
specified with Requires:.  Those happen to already be there, so I don't really
see the point in the Prereq: line.


  38: W: unversioned-explicit-provides squirrelmail-i18n
We always want to provide something with a version, because without one it
becomes impossible to ever provide a separate squirrelmail-i18n package.
However, is there anything which might actually depend on squirrelmail-i18n?
Why does it need to be provided by this package?

  217: E: use-of-RPM_SOURCE_DIR
Shouldn't this just refer to %{SOURCE1} instead?

  W: mixed-use-of-spaces-and-tabs (spaces: line 76, tab: line 76)
Nobody cares about this.  Fix it if you like.

and about the built noarch.rpm:

  W: non-standard-uid /var/lib/squirrelmail/prefs apache
  W: non-standard-gid /var/lib/squirrelmail/prefs apache
  E: non-standard-dir-perm /var/lib/squirrelmail/prefs 0700
  W: non-standard-uid /var/spool/squirrelmail/attach apache
  W: non-standard-gid /var/spool/squirrelmail/attach apache
  E: non-standard-dir-perm /var/spool/squirrelmail/attach 0700
  W: non-standard-gid /etc/squirrelmail/config.php apache
  E: non-readable /etc/squirrelmail/config.php 0640
  W: non-standard-gid /etc/squirrelmail/config_local.php apache
  E: non-readable /etc/squirrelmail/config_local.php 0640
  W: non-standard-gid /etc/squirrelmail/default_pref apache
  E: non-readable /etc/squirrelmail/default_pref 0640
  W: non-standard-gid /etc/squirrelmail/sqspell_config.php apache
  E: non-readable /etc/squirrelmail/sqspell_config.php 0640
These are all perfectly OK.

  E: htaccess-file
   /usr/share/squirrelmail/plugins/squirrelspell/modules/.htaccess
Generally it's a bad idea to use a .htaccess file in a package; better to put
such things in /etc/httpd/conf.d/squirrelmail.conf so the admin can see all of
the access restrictions in one place.  It's also common to disable htaccess
checks for reasons of speed.

  W: file-not-utf8 /usr/share/doc/squirrelmail-1.4.17/ChangeLog
It would be significantly better if anyone who gets this package would be able
to actually read the names in the ChangeLog, but such things tend to get messy
and bits in various different character sets are often added at different
times.  Fixing it is generally a manual procedure and needs to be coordinated
with upstream (because generally iconv can't fix these problems and carrying a
patch to the ChangeLog is pointless).  I don't think the package review is the
appropriate place to force that to happen.

  W: wrong-file-end-of-line-encoding
   /usr/share/doc/squirrelmail-1.4.17/ReleaseNotes/1.4/Notes-1.4.12.txt
  W: wrong-file-end-of-line-encoding
   /usr/share/doc/squirrelmail-1.4.17/ReleaseNotes/1.4/Notes-1.4.13.txt
These, however, should be fixed with a couple of quick calls to tr.

  E: zero-length
   /usr/share/squirrelmail/locale/es_ES/LC_MESSAGES/serversidefilter.po.new
Why do we ship this?

  W: devel-file-in-non-devel-package
   /usr/share/squirrelmail/plugins/filters/bulkquery/bulkquery.c
  E: non-executable-script /usr/share/squirrelmail/plugins/demo/getpot 0644
What are these for?  If they're not usable as is, perhaps they should be
removed from the actual squirrelmail installation and packaged as
documentation.

That's it for rpmlint.  I'll look at the spec later if anyone is listening on
the other end of this ticket.

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