Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Give that it deals with module installation, it seems reasonable to assume that hook_modules_uninstalled() should go in an install file rather than a module file. (And assumptions are all one has to go on: #633332: hook API documentation files have no information about where hooks are to go.)
This is in fact not the case. I've just encountered this, and a read of http://api.drupal.org/api/drupal/includes--install.inc/function/drupal_u... confirms it.
I would suggest we move its expected location to the install file, by adding a module_load_all_includes('install')l to drupal_uninstall_modules().
Comments
Comment #1
jhodgdonI disagree with this idea.
hook_modules_uninstalled is a way for a module to respond when other modules are installed/uninstalled, so it is not in the same group as enable/disable/install/etc. Yes it is related to installation, but it is not part of that particular module being installed, so IMO it doesn't belong in the .install file.
Comment #2
joachim CreditAttribution: joachim commentedAs I've always understood it, the install file exists for optimization: code that is only run when big system changes such as code updates or module addition/removal take place.
By that criterion, these hooks belong in there too. The rest of the time, during normal site operation, having them sit in .module is a waste of memory and clutters the code.
Comment #3
jhodgdonWell, your understanding is different from mine. My understanding of .install files is that it is for code relating to installing/enabling/uninstalling/disabling the module itself. That should be clarified...
Comment #4
joachim CreditAttribution: joachim commentedWell the thing is that the hook_modules_*() hooks are new to D7. Prior to that, 'relating to changing the system status of the module itself' was all there was. I guess nobody's had the discussion about whether install files are for relating to changing the system status of other modules too.
And so here we are :)
Comment #5
jhodgdonOK. Well that isn't the only new hook that would be moved, if your idea is the one that is adopted.... There is also hook_schema_alter() and possibly others that relate to these processes? They should definitely all be consistent. And I would guess some code change would be needed in order to move them?
Comment #6
sven.lauer CreditAttribution: sven.lauer commentedI concur with jhodgdon's understanding of the current intention of .install files. To my mind, if we want to optimize / minimize loading of code that is only used rarely, we should use a hook_hook_info()-group and rely on the autoloading mechanism for those. Then, hook_modules_*() could (but need not) go into a MODULE_NAME.modules.inc (or MODULE_NAME.module_install.inc or something). The autoload mechanism is there for exactly this purpose, from the hook_hook_info() doc:
.install files, on the other hand, are special as they collect hooks that are intended to be run only on the implementing module's (de)installation and (de|en)abling. Actually, the optimization argument cuts both ways: Loading the .install files of ALL modules if any given module is installed/uninstalled/enabled/disabled is overkill. Having a separate include file for hook_module_*() implementations to go in would be more efficient (also, easier to implement, as the include file would be autoloaded).
Comment #8
dawehner