Bug 241757 - Review Request: drupal - An open-source content-management platform
Review Request: drupal - An open-source content-management platform
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Lubomir Rintel
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-05-30 08:49 EDT by Jon Ciesla
Modified: 2009-01-07 13:08 EST (History)
1 user (show)

See Also:
Fixed In Version: 5.1-5.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-07-25 01:16:49 EDT
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)

  None (edit)
Description Jon Ciesla 2007-05-30 08:49:01 EDT
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 17:03:16 EDT
+ 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 Jon Ciesla 2007-06-04 14:13:16 EDT
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 17:03:43 EDT
- 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 Jon Ciesla 2007-06-06 12:33:44 EDT
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 13:27:19 EDT
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 13:57:52 EDT
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 Jon Ciesla 2007-07-04 12:56:01 EDT
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 15:48:28 EDT
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 Jon Ciesla 2007-07-20 16:04:38 EDT
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 04:35:33 EDT
Jon: No. As I said, the package is very fine now.
APPROVED.

Please request a CVS module now.
Thanks for the package!
Comment 11 Jon Ciesla 2007-07-21 13:19:29 EDT
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 01:16:41 EDT
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.