Bug 241757 - Review Request: drupal - An open-source content-management platform
Summary: Review Request: drupal - An open-source content-management platform
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Lubomir Rintel
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Keywords:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-05-30 12:49 UTC by Gwyn Ciesla
Modified: 2009-01-07 18:08 UTC (History)
1 user (show)

Fixed In Version: 5.1-5.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-07-25 05:16:49 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lkundrak: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Gwyn Ciesla 2007-05-30 12:49:01 UTC
Spec URL: http://zanoni.jcomserv.net/fedora/drupal/drupal.spec
SRPM URL: http://zanoni.jcomserv.net/fedora/drupal/drupal-5.1-1.src.rpm
Description: Equipped with a powerful blend of features, Drupal can support a variety of websites ranging from personal weblogs to large community-driven websites.

Comment 1 Sébastien PRUD'HOMME 2007-05-31 21:03:16 UTC
+ build of the SRPM
+ rpmlint on SRPM and RPM
+ source is the one on www.drupal.org
+ license GPL
+ install and uninstall of the RPM
+ README.fedora explains how to finish the installation

- group Applications/System seems not appropriate: perhaps Applications/Publishing?
- doc in /usr/share/doc/drupal-5.1 should not be duplicated in /usr/share/drupal
- apache should be restarted after installation to load drupal.conf:

%postun
/sbin/service httpd condrestart > /dev/null 2>&1 || :

- perhaps install the cron job (as said in INSTALL.txt)?

Comment 2 Gwyn Ciesla 2007-06-04 18:13:16 UTC
Spec URL: http://zanoni.jcomserv.net/fedora/drupal/drupal.spec
SRPM URL: http://zanoni.jcomserv.net/fedora/drupal/drupal-5.1-2.src.rpm

Fixed group, duped docs, added apache restart.  Added it to %post as well. 
Added cron job.


Comment 3 Sébastien PRUD'HOMME 2007-06-05 21:03:43 UTC
- drupal need write access to /etc/drupal/default/settings.php during install
(i've just do a "chown apache:apache settings.php" to fix this)

- drupal status report says:
The directory 'files' does not exist. You may need to set the correct directory
at the file system settings page or change the current directory's permissions
so that it is writable.

perhaps add /var/lib/drupal (or /var/lib/drupal/files) and
/usr/share/drupal/files as a soft link to it

Comment 4 Gwyn Ciesla 2007-06-06 16:33:44 UTC
Spec URL: http://zanoni.jcomserv.net/fedora/drupal/drupal.spec
SRPM URL: http://zanoni.jcomserv.net/fedora/drupal/drupal-5.1-3.src.rpm

Corrected.  I assume having settings.php writable by default should be Ok, since
on a new install, it's locked down to localhost, and on an upgrade, it's
%config.  Of course, a malicious user on the same system could always configure
Drupal in the time between the RPM installation and when the admin goes to do it
herself, but then I suspect that's an extreme corner case. :)

Comment 5 Lubomir Kundrak 2007-06-28 17:27:19 UTC
Jon: that is _not_ an extreme corner case, but a serious security issue.
Drupal should _not_ be started before being configured. Please comment out
relevant lines in the Apache HTTPd counfiguration line snippet, and maybe
add a comment on which lines to uncomment when configured and how to configure.
This also removes the requirement for HTTPd restart.

Comment 6 Lubomir Kundrak 2007-06-28 17:57:52 UTC
Seems like I misunderstood your statement about settings.php being writable
by default, as it obviously isn't. If you were thinking about it, please
forget quickly. :) Also, it is probably a bad idea for settings.php to be
world-readable, as it holds the database password.

The cron job uses wget by default -- why not e.g. curl? And what about
dependency on wget then?

And, also the description could be written better. Please rewrite it to say
what Drupal actually is.



Comment 7 Gwyn Ciesla 2007-07-04 16:56:01 UTC
I've locked down the initial config, and made the description a bit better.

I've added Requires: wget, as it's recommended by upstream.

Spec URL: http://zanoni.jcomserv.net/fedora/drupal/drupal.spec
SRPM URL: http://zanoni.jcomserv.net/fedora/drupal/drupal-5.1-4.fc7.src.rpm

Comment 8 Lubomir Kundrak 2007-07-20 19:48:28 UTC
Jon: Please pardon the long delay.
Here's the rest of the review, other requirements are met:

* Rpmlint complains:
E: drupal non-standard-gid /var/lib/drupal apache
E: drupal non-standard-gid /etc/cron.hourly/drupal apache
  As these files have the same modes for the group as for the rest of the
  world, I see no reason for having these be owned by root group.

* BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

* Config files of drupal: /etc/drupal/all/README.txt shouldn't be here.
Documentation in /usr/share/doc is the right place for this, preferrably
README.Fedora.

Otherwise the package is superb. I'll approve it once you deal with the above.

Comment 9 Gwyn Ciesla 2007-07-20 20:04:38 UTC
Fixed README and buildroot.

Spec URL: http://zanoni.jcomserv.net/fedora/drupal/drupal.spec
SRPM URL: http://zanoni.jcomserv.net/fedora/drupal/drupal-5.1-5.fc7.src.rpm

Anything else?


Comment 10 Lubomir Kundrak 2007-07-21 08:35:33 UTC
Jon: No. As I said, the package is very fine now.
APPROVED.

Please request a CVS module now.
Thanks for the package!

Comment 11 Gwyn Ciesla 2007-07-21 17:19:29 UTC
New Package CVS Request
=======================
Package Name: drupal
Short Description: An open-source content-management platform
Owners: limb@jcomserv.net 
Branches: F-7 FC-6 EL-4 EL-5
InitialCC: 

Thank you for the review!

Comment 12 Fedora Update System 2007-07-25 05:16:41 UTC
drupal-5.1-5.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.