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.
API page: http://api.drupal.org/api/drupal/modules--system--system.api.php/functio...
Describe the problem you have found:
hook_install and hook_uninstall do not actually mention that they need to go in a .install file! I guess it's something so basic as to be assumed knowledge ... but it should probably be mentioned.
hook_update_N and hook_schema both correctly specify this.
Comment | File | Size | Author |
---|---|---|---|
#19 | doc-hook_install-uninstall-must-be-in-install-file-1247626-19.patch | 1.56 KB | sven.lauer |
#17 | uninstall_patch_6.x.txt | 1.06 KB | kingandy |
#16 | uninstall_patch_6.x.txt | 483 bytes | kingandy |
#9 | 1247626-doc-hook_uninstall-9.patch | 2.12 KB | sven.lauer |
#7 | 1247626-doc_hook_install_uninstall-7.patch | 1.86 KB | sven.lauer |
Comments
Comment #1
sven.lauer CreditAttribution: sven.lauer commentedNice catch, this clearly should be documented!
(Moving to 8.x, as patches go in there first---I assume this is also true for doc patches?)
Attaching a patch that clarifies where the hook implementations should go.
Comment #3
sven.lauer CreditAttribution: sven.lauer commentedUhm ... what? Trying again.
Comment #4
sven.lauer CreditAttribution: sven.lauer commentedComment #5
jhodgdonNot technically correct, I think? You can also put these hooks in the main .module file. It's not recommended practice, but it will work.
Also, when you move an issue to d8 (and yes, docs patches have the same convention), please add the backport tag. Thanks!
Comment #6
kingandy CreditAttribution: kingandy commentedI don't think hook_uninstall, at least, can just go in .module. From the D7/8 hook_uninstall docs:
(Emphasis mine.)
I don't honestly know if hook_install suffers under the same constraint.
Comment #7
sven.lauer CreditAttribution: sven.lauer commentedAh ... yes, I was sure about hook_uninstall(), but I extrapolated to hook_install() and from D6.
Indeed, in D7/8 in
module_enable()
inmodule.inc
we haveSo yes, the module file does get loaded, so if it would contain the
hook_install()
implementation, that would work.See also http://drupal.org/node/224333#module_file_during_install (though this is clearly not intended to enable people put their hook_install() implementations in the .module file).
On the other hand, for
hook_uninstall()
, the existing comment quoted by kingandy is correct (fromdrupal_uninstall_modules()
ininstall.inc
):So, the module file does NOT get loaded on uninstall (and it should not, as modules that the current module depends on might be uninstalled and even removed from the codebase at the point hook_uninstall is called).
Basically, everything that has to be accessible during uninstall has to go into the .install file, while things like hook_install() implementations SHOULD go there, too. hook_schema must be in the .install file for this reason.
Attaching a modified patch.
Also tagging for backport to D6, where where things are as described in the first patch.
A question is whether the API really should allow to put the hook_install() implementation into the .module file, or whether the fact that this works is an unintended side effect (which does not make much of difference, but ...).
Comment #8
jhodgdonThat text looks good to me, thanks!
The line wrapping needs to be fixed here:
Comment #9
sven.lauer CreditAttribution: sven.lauer commentedRe-roll with line wrapping fix. No changes in wording.
Comment #10
jhodgdonLooks fine for d8/d7. Then I guess mark for D6 status "to be ported". Thanks!
Comment #11
catchI thought we might need to add this for hook_schema() as well, but that's already got a similar note. Committed to 8.x, moving to 7.x for webchick.
Comment #12
webchickMakes sense.
Committed and pushed to 7.x. Thanks!
Comment #13
jhodgdonStill needs backport to D6.
Comment #14
sven.lauer CreditAttribution: sven.lauer commentedForgive my ignorance, but where does the hook documentation live in D6? It is not in my checkout of 6.x ...
Never did a 6.x doc patch before. I would be willing to do the backporting, if I knew how/where.
Comment #15
sven.lauer CreditAttribution: sven.lauer commentedForgive my ignorance, but where does the hook documentation live in D6? It is not in my checkout of 6.x ...
Never did a 6.x doc patch before. I would be willing to do the backporting, if I knew how/where.
Comment #16
kingandy CreditAttribution: kingandy commentedhook_uninstall (and its doc comment) seems to be in developer/hooks/install.php. I'm afraid I've no idea if this file would be present in a standard checkout, or if the 'developer' folder is something that's been added to the API version for documentation purposes.
I've manually built a version of this patch using the source presented on api.drupal.org (http://api.drupal.org/api/drupal/developer--hooks--install.php/6/source). The hook_schema doc already mentions that it must reside in .install and needs to be the current version of the schema, and it doesn't get called automatically in 6.x anyway, so that part of the patch is basically unnecessary ... similarly a lot of the hook_uninstall comment deals with when it runs in relation to removing the schema, which could go as well (again, the schema doesn't get automatically removed in 6.x). I've ended up with about two lines of difference.
Comment #17
kingandy CreditAttribution: kingandy commentedUpdate: I completely forgot the hook_install requirement (as noted in #7, hook_install does indeed need to be in the .install file in 6.x). Modified patch attached.
Comment #18
jhodgdonOh sorry! The files are in the Documentation project git repository. Moving to that project.
So the patch will need to be rerolled.
One other thing:
"in the modules .install file." -- should be "module's".
Comment #19
sven.lauer CreditAttribution: sven.lauer commentedBringing this one home.
Re-rolled the patch against the documentation project's repository (the 6.x version). Followed kingandy's #17, as his reasoning about omitting the hook_schema related stuff is sound; fixing the apostrophe issue noted by jhodgdon
Comment #20
delraydavis CreditAttribution: delraydavis commentedI can verify that the install hook will function properly in the module file, however the uninstall hook must be in the .install file.
A little janky I know but I am in the process of writing up my first module and stumbling through is the best way to learn.
I tested this thoroughly using landmark inserts and deletes into the database in conjunction with the install and uninstall hooks.
It should be noted both hooks function perfectly in the install file. So that's what I am staying with.
Comment #21
webchickWow, interesting. I had no idea that hook_install() would work in .module. :) Nice find!
Comment #22
jhodgdonSo it sounds like we should say for d6 that hook_install() should be in the .install file, rather than "must" as in the current patch?
Comment #23
sven.lauer CreditAttribution: sven.lauer commentedThe report in #20 surprised me a little (okay, a lot), so I ran my own quick test, confirming that in D6, hook_install DOES NOT work if it is in the module file. (I did something very simple---just a hook implementation that only had a drupal_set_message() in it. The message did not show when the hook implementation was in the module file.
As I showed in #7, in D7, the .module file gets loaded on install. Here is the relevant portion of install.inc:
Note that only the install file gets loaded (via module_load_install(), which does NOT load the module file). I also do not see were else it would be loaded.
Finally, in the module update guide, it says:
http://drupal.org/node/224333#module_file_during_install
So either I am very much confused, or the hook_install() canNOT be implemented in the .module file in D6.
@delraydavis: What specific version did you use for your test?
Comment #24
jhodgdonSven: Just to clarify on your test - did the set message work when the install hook was in the .install file? I am never certain if messages are displayed during module installation. A database update or something else more durable might be a better test (though definitely more work to try).
I'm pretty sure than in D6 I have made modules with either install or enable hooks and put the entire thing in one .module file (not the best practice, but anyway)... but I would have to search around for examples...
Comment #25
sven.lauer CreditAttribution: sven.lauer commentedYes. As I was not sure either, I did a separate test run to putting the hook in the .install file, and then the message showed.
Comment #26
sven.lauer CreditAttribution: sven.lauer commentedAnd, just to be completely sure, I ran another test using variable_set() in the install hook (this is with a current git checkout of the 6.x branch).
With this minimal test_hook_install_location.module file (no .install file):
I ran the following:
Clearly, the variable is not set after the module was installed/enabled. And jsut to be sure, I followed this up with:
This shows that the hook implementation does exactly what it should do, and it is recognized if it is in the .install file, while the same implementation in the .module file did nothing.
So the behavior is exactly as the patch in #19 says.
Comment #27
sven.lauer CreditAttribution: sven.lauer commentedAnd re: the second part of #24:
Hm ... with enable hooks, this would work (in either D6 and D7, I think), but it really does not seem to be working with hook_install().
Comment #28
jhodgdonSven - THANK YOU for this extensive and definitive test! My memory must be faulty, and perhaps the other person who tested was not quite as careful.
So it looks like the patch in #19 is RTBC for the docs project, Drupal 6 version. Thanks!
Comment #29
jhodgdoncommitted and pushed.