| Project: | Drush |
| Version: | All-versions-5.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | greg.1.anderson |
| Status: | closed (fixed) |
Issue Summary
From looking through includes/commands.inc, there seems that there should be various hooks that are called for each command that is run (drush_command calling drush_invoke which calls validate, pre, post hooks)
However drush.php calls drush_dispatch which does not call drush_command so all of those valid, pre, post hooks are not called.
Is this intended and maybe I am just missing how to really take advantage of those hooks? For a better idea of what I want to do, I want to create an include svn.props.drush.inc that implements hook drush_svn_props_pre_update. For every update command that is run, I want to add a svn property with the cvs tag of the updated module to the modules folder.
Is that possible and am I heading in the wrong direction?
Thanks!
Comments
#1
The usual callback that you see in drush_dispatch is drush_command not the final command callback. drush_command does some hook_invocation and drush_invoke does more.
#2
Thanks for the response!
I see how for the download command, it is indeed drush_command which is great and works for my example above. However I was also looking to hook into pre_sql_dump as well, but drush_dispatch is not calling drush_command for sql_dump. Is there a particular reason for that?
#3
OK, it looks like commands that specify a particular callback do not currently get all their hooks run. sql query specifies a callback and thus no hooks as you have seen.
these sql command should mostly not declare explicit callback functions but thats not really relevant. this bug needs fixing.
assigning to adrian to see how he prefers to fix.
#4
Great, thanks moshe! Will await further word from adrian.
#5
the callback support was for backward compatibility
we should remove the support for callback imo, but before that we need some documentation.
#6
hmm. the 'callback' support is quite drupalish. we do that in menu system, fapi, etc. i think it is here to stay. can we get full drush_invoke support for it somehow?
marking as critical.
#7
Here it is. Note that in order for drush_invoke to work correctly, the 'callback' must begin with "drush_commandfile_". I fixed drush simpletext (just took out the callbacks) and pm_module_manage (renamed to drush_pm_module_manage) so that they would conform. For contrib modules that do not conform, a notice is printed, and drush_invoke is skipped. For all conforming commands (which should be all commands if the coding conventions are being followed), the full set of drush hooks are called.
In the future, if you want to obsolete (break) non-conforming command callback functions, drush_dispatch can make a direct call to drush_command.
#8
#9
Looks great to me. We'll leave it open for a bit to get more feedback ... I see no need to support non conforming callbacks. Don't feel strongly about it though.
Ideally, we would discuss the callback key at drush.api.php. I recently fixed up the callback function name at example.drush.inc so we are probably fine there.
#10
This updated patch includes the code from #630022: drush_invoke is too silent plus some expanded documentation.
#11
- * that wants to hook into the `dl` command.+ * that wants to hook into the `download` command.
*
- * 1. drush_mysite_dl_validate()
- * 2. drush_mysite_pre_dl()
- * 3. drush_mysite_dl()
- * 4. drush_mysite_post_dl()
+ * 1. drush_mysite_download_validate()
+ * 2. drush_mysite_pre_download()
+ * 3. drush_mysite_download()
+ * 4. drush_mysite_post_download()
isn't it pm-download and drush_mysite_pm_download_validate() etc?
I also see a conflict with the patch at #677894: untangle pm_module_manage() a bit with pm-list. Whatever is commit earlier the other must reroll. Basicly you don't need to do anything with pm-list callback and pm_module_manage() renaming as it is going to disappear.
#12
Whoops, I did in fact miss-fix example.drush.inc. This patch also makes a couple of other fixes to that file. The "post-install" patch should have been a "post download" patch, I think, and I switch "post update" to "post updatedb", as the former will not be called if the user does pm-updatecode followed by updatedb, while the later will always be called at about the right place.
I backed out all of my 'pm' modifications'; pm_module_manage can be called via the backwards-compatibility hook until #677894: untangle pm_module_manage() a bit lands.
#13
Committed.
I notice that the sql commands still use 'callback'. I just opened #690602: Review 'callback' usage in sql commands
#14
Automatically closed -- issue fixed for 2 weeks with no activity.