Red Hat Bugzilla – Bug 241757
Review Request: drupal - An open-source content-management platform
Last modified: 2009-01-07 13:08:58 EST
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.
+ 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)?
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.
- 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
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. :)
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.
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.
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
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.
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?
Jon: No. As I said, the package is very fine now. APPROVED. Please request a CVS module now. Thanks for the package!
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!
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.