[Bug 189195] Review Request: horde - php application framework

bugzilla at redhat.com bugzilla at redhat.com
Wed Dec 27 21:32:49 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: horde - php application framework


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





------- Additional Comments From tibbs at math.uh.edu  2006-12-27 16:32 EST -------
OK, finally some time.

I note that you emit a message in %post; you generally shouldn't do this. 
(There's a good chance that the packaging committee will add a guideline banning
this sort of output soon.)

I set up a test machine with a minimal OS install, installed this package and
started httpd.  Everything works fine, so that's good.  Still, I wonder if we
couldn't have this package spit out a subpackage that pulls in all of the
optional dependencies (save php-mysql and php-pgsql, leaving the admin to
choose).  This would make it even easier for an admin to get up and running.

The stuff in comment 45 might be useful as well, although we should carefully
consider whether increasing the limits like that is safe.  Perhaps you could
include the lines but comment them out and include some useful info, like how
high you need to increase the limits to handle attachments of a certain size
(assuming it's possible to calculate that).

Now, let's deal with the rpmlint output.  I'll put any issues worth considering
at the front.  Only two things I consider blockers:

E: horde script-without-shebang /usr/share/horde/lib/Net/IMSP/Auth/imtest.php
   This could be an issue; this is executable, but it has no shebang line and
thus won't do anything useful if executed.  Perhaps it shouldn't be executable?

W: horde symlink-should-be-relative /usr/share/horde/config /etc/horde
   This is valid; the symlink should be relative to avoid breaking various odd
setups like chroots.

W: horde strange-permission registry.php 0640
W: horde mixed-use-of-spaces-and-tabs (spaces: line 143, tab: line 1)
  These are not a big deal; clean them up if you like.  I think checking into
CVS will fix the first one, as it's only complaining about the permissions of
the file in the SRPM.

E: horde htaccess-file /usr/share/horde/lib/.htaccess
E: horde htaccess-file /usr/share/horde/locale/.htaccess
E: horde htaccess-file /usr/share/horde/po/.htaccess
E: horde htaccess-file /usr/share/horde/scripts/.htaccess
E: horde htaccess-file /usr/share/horde/templates/.htaccess
  Yes, indeed, these are htaccess files, and they need to be there.

E: horde non-executable-script /usr/share/horde/scripts/temp-cleanup.cron 0644
  Not a big deal, although it does open the question of whether we should
consider running that.  I've never done so on any of my systems.

E: horde non-readable /etc/horde/conf.php 0660
E: horde non-readable /etc/horde/conf.php.dist 0640
E: horde non-readable /etc/horde/conf.xml 0660
E: horde non-readable /etc/horde/mime_drivers.php 0660
E: horde non-readable /etc/horde/mime_drivers.php.dist 0640
E: horde non-readable /etc/horde/motd.php 0660
E: horde non-readable /etc/horde/motd.php.dist 0640
E: horde non-readable /etc/horde/nls.php 0660
E: horde non-readable /etc/horde/nls.php.dist 0640
E: horde non-readable /etc/horde/prefs.php 0660
E: horde non-readable /etc/horde/prefs.php.dist 0640
E: horde non-readable /etc/horde/registry.php 0660
E: horde non-readable /etc/horde/registry.php.dist 0640
   Yes, and they need to be non-readable.

E: horde non-standard-dir-perm /etc/horde 0770
   Again, this is necessary.

E: horde non-standard-gid /etc/horde apache
E: horde non-standard-gid /etc/horde/conf.php apache
E: horde non-standard-gid /etc/horde/conf.php.dist apache
E: horde non-standard-gid /etc/horde/conf.xml apache
E: horde non-standard-gid /etc/horde/mime_drivers.php apache
E: horde non-standard-gid /etc/horde/mime_drivers.php.dist apache
E: horde non-standard-gid /etc/horde/motd.php apache
E: horde non-standard-gid /etc/horde/motd.php.dist apache
E: horde non-standard-gid /etc/horde/nls.php apache
E: horde non-standard-gid /etc/horde/nls.php.dist apache
E: horde non-standard-gid /etc/horde/prefs.php apache
E: horde non-standard-gid /etc/horde/prefs.php.dist apache
E: horde non-standard-gid /etc/horde/registry.php apache
E: horde non-standard-gid /etc/horde/registry.php.dist apache
E: horde non-standard-uid /etc/horde apache
E: horde non-standard-uid /etc/horde/conf.php apache
E: horde non-standard-uid /etc/horde/conf.php.dist apache
E: horde non-standard-uid /etc/horde/conf.xml apache
E: horde non-standard-uid /etc/horde/mime_drivers.php apache
E: horde non-standard-uid /etc/horde/mime_drivers.php.dist apache
E: horde non-standard-uid /etc/horde/motd.php apache
E: horde non-standard-uid /etc/horde/motd.php.dist apache
E: horde non-standard-uid /etc/horde/nls.php apache
E: horde non-standard-uid /etc/horde/nls.php.dist apache
E: horde non-standard-uid /etc/horde/prefs.php apache
E: horde non-standard-uid /etc/horde/prefs.php.dist apache
E: horde non-standard-uid /etc/horde/registry.php apache
E: horde non-standard-uid /etc/horde/registry.php.dist apache
   Again, these need to be owned by that UID.

W: horde conffile-without-noreplace-flag /etc/horde/conf.php.dist
W: horde conffile-without-noreplace-flag /etc/horde/conf.xml
W: horde conffile-without-noreplace-flag /etc/horde/mime_drivers.php.dist
W: horde conffile-without-noreplace-flag /etc/horde/motd.php.dist
W: horde conffile-without-noreplace-flag /etc/horde/nls.php.dist
W: horde conffile-without-noreplace-flag /etc/horde/prefs.php.dist
W: horde conffile-without-noreplace-flag /etc/horde/registry.php.dist
   It is normal for distributed example config files to be installed without
noreplace.

Review:
* source files match upstream:
   fbc56c608ac81474b846b1b4b7bb5ee7  horde-3.1.3.tar.gz
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
* latest version is being packaged.
O BuildRequires are proper (BR: perl is unnecessary, but not a blocker)
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
X rpmlint has a couple of valid complaints
* final provides and requires are sane:
   config(horde) = 3.1.3-9.fc7
   horde = 3.1.3-9.fc7
  =
   /bin/sh
   /sbin/service
   /usr/bin/php
   config(horde) = 3.1.3-9.fc7
   php >= 4.3.0
   php-pear(DB)
   php-pear(File)
   php-pear(Log)
   php-pear(Mail_Mime)
   php-xml
* %check is not present; no test suite upstream.  Manual tests show everything's
working OK.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* locales seem to be handled properly.
X file permissions are appropriate (imtest.php)
X scriptlets not OK; %post has intentional output
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

-- 
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.




More information about the Fedora-package-review mailing list