[drupal-devel] [feature] Dependency system to solve forum without comment problem

stefan nagtegaal drupal-devel at drupal.org
Fri Jul 1 18:08:15 UTC 2005

Issue status update for http://drupal.org/node/18447

 Project:      Drupal
 Version:      cvs
 Component:    system.module
 Category:     feature requests
 Priority:     normal
 Assigned to:  chx
 Reported by:  chx
 Updated by:   stefan nagtegaal
 Status:       patch

This isn't applying anymore..

Dries, what are your thoughts about the dependency system? I think it's
quite good, for the sake of usability, it surely will be!

Please share your thoughts so we can get this slippin' in...

stefan nagtegaal

Previous comments:

March 6, 2005 - 02:55 : chx

Attachment: http://drupal.org/files/issues/depend.patch (3.22 KB)

As discussed on the developer list. This was surprisingly easy. There is
still an issue: t('The configuration options have been saved.') is
displayed more than once if there is a dependency forced module switch
on. I have tried something (you'll see) for some reason it does not

Number of page reloads can be decreased if Drupal could be
reinitialized after system_listing_save instead of redirecting but I
doubt this would be feasible.

Although I marked this is feature request, this is more a usability
issue (for the forum-comment dependency) and as a side effect it is a
useful feature, too (for any other module "bundles").

I want criticism! I'd like to get torn apart by wild animals. Heavy! 
Eaten by some squirrels.


March 6, 2005 - 03:22 : clydefrog

Forgive me if I'm mistaken, but shouldn't forum_depends return an array?


March 6, 2005 - 03:26 : javanaut

I think allowing for both required dependencies as well as optional ones
would make it practical for a package management tool to be created.  I
like it, though, and I think being able to programatically enforce (or
at least notify users about) dependencies would assist users who refuse
to RTFM.

>From my drupal-devel comment, here's how I would suggest formatting the

function filestore2_depends() {
 return array('required'=>array('fscache'),


March 6, 2005 - 03:29 : chx

If you take a look at module_invoke_all, you will see that a string is
fine, 'cos if you return a string it does $result[]

However, an associative array is not good with module_invoke_all 'cos
for returned arrays there is an array_merge where "If the input arrays
have the same string keys, then the later value for that key will
overwrite the previous one."


March 6, 2005 - 03:33 : javanaut

Ok, how about it taking a parameter?

function filestore2_depends($dep_type='required') {
$deps = array('required'=>array('fscache'),
return $deps[$dep_type];


Then, for required dependencies, pass a 'required' argument, and
optional ones could take an 'optional' argument.


March 6, 2005 - 04:00 : chx

Attachment: http://drupal.org/files/issues/depend_0.patch (3.6 KB)

I would leave associative arrays for the future. As we do not have a
package tool, there is little use of 'optional' -- I'd like to get this
into 4.6

After testing the patch, I have changed something, which I should not
have done, so another version is attached.


March 6, 2005 - 04:11 : chx

Attachment: http://drupal.org/files/issues/depend_1.patch (4.37 KB)

This one should really work...


March 6, 2005 - 04:12 : chx

Attachment: http://drupal.org/files/issues/depend_2.patch (3.15 KB)


March 6, 2005 - 06:13 : gordon


Not only is this something that I have wanted for filestore2 for quite
a while, but has not been on my priority list. But this would be
extremely useful for the ecommerce modules, as there seems to be alot
of interdependancy.

Also on a side note, it would also be good for a module to be able to
reject an enabling. So if the module is going to stop drupal from
running if it does have its tables created, it will inform the user and
not be turned on. This could actually be done allowing the dependancy
hook to do some checking and so stop the module from being able to be


March 6, 2005 - 10:19 : mathias

This would make installing the ecommerce package easier given that
modules are no longer listed alphabetically from within their
subdirectories.  The modules are scattered all over the page unless I
adopt some sort of ec_ prefix to module naming.

I wonder how Adrian's installer handles dependencies?


March 6, 2005 - 12:42 : adrian

My installer doesn't currently handle dependencies, and I am not
convinced this should go into 4.6.

There's more to dependency checking than just 'depends' including fun
things like circular dependencies and version checking.
Once we have versioning for drupal modules / schemas .. we can look at
this again. 

Vlado apparently has dependency checking code already built, that he is
going to donate to the installer effort.


March 6, 2005 - 12:57 : chx

Attachment: http://drupal.org/files/issues/depend_3.patch (3.26 KB)

Well, I know there could be problems. However, circular dependecy is
handled with my code. You have module A, this depends on B, this
depends on C which depends on A. You tick A, press Save. Next listing
will tick B instead of you, press Save. Next listing will tick C
instead of you, press Save. Next listing will be happy, 'cos A is on.

Adrian convinced me that taking a parameter is a good thing, so it's
in. And he asked for calling the hook _info. So be it. And he pointed
out that I do not understand drupal_set_message which was true. So now
the status message is now displayed only once. Shall I protect the new
error message from such a duplication?

And please, get this simple system into 4.6 'cos (as already stated)
the forum-comment dependency has threads on drupal.org already so it
could be called a bug.


March 6, 2005 - 13:08 : chx

Attachment: http://drupal.org/files/issues/depend_4.patch (3.27 KB)

Minor things: default hook_info key shall be called 'depends' not
'required' (Adrian). Message should be 'enabled' for consistency


March 8, 2005 - 21:56 : Dries

The status message needs work (incorrect use of em, confusing wording). 
Lastly, the way messages are handled and fiddled with is peculiar at
best: the logic is spread out over different functions and complicated
because of that.  I'm not happy with the code as is.


March 12, 2005 - 12:37 : Dries

Setting this 'active'.  Waiting for new patch.


March 13, 2005 - 22:21 : chx

Attachment: http://drupal.org/files/issues/depend_5.patch (3.3 KB)

Here is new version. Hope you like it more than the previous one.


March 13, 2005 - 22:29 : chx

One s too much...


March 13, 2005 - 22:31 : chx

Attachment: http://drupal.org/files/issues/depend_6.patch (3.3 KB)

grrrr... it submitted without attaching... is there some way to make
preview required for me...?


March 14, 2005 - 12:28 : stefan nagtegaal

chx: I do like the approach you took, but I think we should return more
information to the administrator.
You did:

  drupal_set_message(t('%module is enabled because other modules
depend on it.', array ('%module' =>'<em>'. $file->name .'</em>')),

First I don't think you need to have the $type = 'error' as the second
argument.. Imo it is not an error, just a notification. Or am I wrong?

And secondly, can't we do something like:

  drupal_set_message(t("Because '%module' depends on functionality
offered by '%modules', these modules are also enabled to omit possible
problems.', array ('%module' => "<em>$file->name</em>", '%modules' =>

I would really appreciate feedback of any of you...


March 26, 2005 - 01:17 : chx

Attachment: http://drupal.org/files/issues/dependency1.patch (2.34 KB)

Whole new dependency system! This has nothing to do the with previous
patches. Those were a bit hackish.

hook_install('depends') may return a string or an array of strings. The
modules whose name is returned, will be switched on. These new modules'
dependencies are also dealt with. And so on.

I have thought of using format_plural, but the arguments make that


March 26, 2005 - 03:46 : chx

Attachment: http://drupal.org/files/issues/dependency1_0.patch (2.34 KB)

A few 's are gone according to Steven's advice.


March 26, 2005 - 05:24 : mathias

Attachment: http://drupal.org/files/issues/dependency.patch (2.62 KB)

+1, and a slice of blueberry pie.

I tested this patch against the ecommerce package and it worked as
expected.  My attached patch simply tweaks a couple minor code details
and made the the dependency message a little easier to understand.

Please add this to 4.6.  This could be viewed as a major usability
benefit for module developers and end users ;-)


March 26, 2005 - 16:55 : stefan nagtegaal

Can't we refrase this:

t("The following module has been enabled since it's dependent on
%module in order function properly: %dependents.", array ('%module' =>
'<em>'. $module .'</em>', '%dependents' => '<em>'. implode(', ',
$enabled) .'</em>')

I'm not sure why, but I don't think this is userfriendly enough..
Anisa, (I'm not sur eshe is on this list) do you have a better idea?


March 26, 2005 - 22:12 : adrian

Two things ..
a) Enabling a module is not installing a module, so the naming of the
_install hook (wether it is already being used in my install api or
not) is wrong.
b) don't add a hook when we already have one. 

hook_info used to be used for the module description, but currently is
only being used for auth. I am rolling a patch which uses hook_info()
(as I am intending to do in my install patches).


March 27, 2005 - 00:33 : chx

Attachment: http://drupal.org/files/issues/dependency_0.patch (2.61 KB)

OK I changed back to hook_info. However, hook_info PHPdoc needs a


March 28, 2005 - 02:01 : chx

Attachment: http://drupal.org/files/issues/dependency_1.patch (2.69 KB)

I contacted Anisa who had a better wording idea. Thanks!

To quote her: "Keep in mind that to the user, this is a surprise.
 They may not have read the documentation properly, and not anything
about X.  'for you' suggests that this is a service, not a
scary-computer-doing-things-on-its-own behavior. 

I added the line about permissions and settings to remind the user that
just because X module has been enabled for them, not everything will be
exactly perfect." and she had had advice about changing "permissions"
(and maybe settings) to a link, but that's not for this patch... it is
listed in the usability group's todo for every module.


March 28, 2005 - 04:42 : moshe weitzman

not just permissions - some database table(s) may need manual creation.


April 3, 2005 - 21:48 : stefan nagtegaal

Dries, what is wrong with this patch?
I think it is a great (and not even a very big code change) improvement
to the current drupal.. It's pretty straight-forward, and makes sense in
a lot of ways..

If the patch needs more work, just let it know and i'll see what I can


April 3, 2005 - 23:25 : chx

Attachment: http://drupal.org/files/issues/dependency_2.patch (2.9 KB)

reword again to include moshe's suggestion. theme placeholder usage.
proper cvs diff.


April 12, 2005 - 01:27 : chx

Any chance for 4.6?


April 12, 2005 - 13:34 : menesis

Looking at the new patch, I was surprised to see how small it is...
There's much more to dependency system than that.

I don't like the approach at all. You automatically enable the required
modules and only inform the user afterwards. As I'm used in Linux
packaging tools, I expect to be asked if I really want to enable the
required module and the module itself. Those modules might not be
installed, in this case have to tell the user to download the required
modules and properly install them - create tables, enable, configure,
and then try enabling this module again. Your patch does not deal with
that and might make the system unusable, when a broken module is
automatically enabled.

A real package/dependency system should have more different
relationships - depends on, recommends, suggests, conflicts... After
you select which modules to enable, you get a listing of additional
dependant modules with checkboxes defaulting to most likely action, and
possibly disabled. Then you confirm that and selected modules are

More information about the drupal-devel mailing list