Proper dependency management in features

mig5 - September 10, 2009 - 07:14
Project:Hosting
Version:6.x-0.3
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs work
Description

Clone can be enabled without enabling the Migrate feature. This throws a fatal error when clicking on the Clone tab on a site node:

Fatal error: Call to undefined function hosting_task_migrate_form() in /var/aegir/drupal-6.13/profiles/hostmaster/modules/hosting/clone/hosting_clone.module on line 41

#1

mig5 - September 11, 2009 - 06:11
Status:active» needs review

Patch attached tries to introduce dependency checking in the Features form.

Might need some work.

It'll enable any additional features it detects as dependencies, if they aren't already enabled. It won't allow a feature to be disabled if another feature depends on it (i.e you cannot disable Migrate until you also disable Clone, if Clone is active).

AttachmentSize
573438.patch 2.37 KB

#2

anarcat - September 14, 2009 - 11:50
Title:Clone: does not properly depend on the Migrate feature» Proper dependency management in features
Project:Provision» Hosting
Version:HEAD» 6.x-0.3
Status:needs review» fixed

I committed a similar fix to CVS.

http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/hosting/hos...

#3

mig5 - September 14, 2009 - 14:04
Status:fixed» needs work

We should move at least the Disable stuff into hook_validate so that we can set only a form_set_error() without the drupal_set_message() 'The configuration options have been saved' from system module sneaking in.

And if we move it into hook_validate(), we need a way to reset the checkbox states (if you disable Migrate while Clone is enabled, it doesn't disable Migrate and throws the error, but the re-rendering of the form keeps the form state so that Migrate is unchecked: reloading the page altogether fixes it). I think we need form_set_value() here but I'm not a forms wizard.

Also, my earlier patch, though it may have been less elegant, only enables modules that aren't already enabled (i.e it won't enable hosting and hosting_site to enable clone and migrate, if these are already running). I do this with module_exists() in that patch, have to work out a way to replicate this in your work.

 
 

Drupal is a registered trademark of Dries Buytaert.