Warning: The command callback function dog_command has a deprecated name. Skipping hook functions.

I noticed the same error as well. The patch fixes the error notices and basic Drush/Dog functions still work. Let me know if further testing is needed.

I saw the error too, and changed dog_command() to drush_dog_command(), and also noticed the endless loop. The other annoying thing is that it *looks* like there's supposed to be a way to prevent this error from being spat, but it seems to not actually be accessible. Argh. I'm nursing a growing...dislike for all the magic hook invocations drush does, so I'm leaning towards chopping as much of it out as possible. And since dog makes every attempt to run with the *utter* minimum bootstrap (to improve reliability), there's little point to those hooks anyway. Dog will provide its own extension mechanisms.

But clearly, I didn't try the change you made. Interesting...I'll have to look at that, see why it fixes it. I grabbed that cufa() logic from something in core drush itself, but I'll have a step-through and see if that change is ok. Truth is though, I'm probably going to represent the command objects as classes, which'll make this whole problem evaporate completely.

Really, you want to avoid the 'callback' element in command definition. All those drush hooks are expected to work for drush developers. You don't want to piss them off - they are your patch contributors and users.

I'll see if I can fix this up for you.

Drush5 does not warn about this in debug mode because it fires all the right hooks even when you use the 'callback' element. So I think we can set this issue to Won't Fix and just ask interested parties to use drush5 if they need the drush hooks. drush5 should be released by london drupalcon.

For the record, here is dog-init with the --show-invoke global option. You can see drush looking many hook implementations.

~/tmp$ drush dog-init -d --show-invoke
Bootstrap to phase 0. [0.03 sec, 3.37 MB]                            [bootstrap]
Drush bootstrap phase : _drush_bootstrap_drush() [0.03 sec, 3.63 MB] [bootstrap]
Loading drushrc "/Users/mw/.drush/drushrc.php" into "home.drush"     [bootstrap]
scope. [0.03 sec, 3.63 MB]
Bootstrap to phase 0. [0.1 sec, 9.25 MB]                             [bootstrap]
Found command: dog-init (commandfile=dog) [0.1 sec, 9.25 MB]         [bootstrap]
Downloading release history from                                        [notice]
http://updates.drupal.org/release-history/drupal/7.x [0.12 sec, 9.29
MB]
Cloning 'git://git.drupal.org/project/drupal.git' into                    [info]
/Users/mw/tmp/drupal. [1.06 sec, 9.3 MB]
Successfully initialized a new dog instance in /Users/mw/tmp/drupal. [success]
[100.45 sec, 9.3 MB]
Available drush_invoke() hooks for dog-init:                         [ok]
drush_archive_dog_init_validate
drush_cache_dog_init_validate
drush_core_dog_init_validate
drush_docs_dog_init_validate
drush_dog_init_validate
drush_drush_make_dog_init_validate
drush_drush_make_d_o_dog_init_validate
drush_field_dog_init_validate
drush_grn_dog_init_validate
drush_help_dog_init_validate
drush_image_dog_init_validate
drush_iq_dog_init_validate
drush_mw_dog_init_validate
drush_phpshell_dog_init_validate
drush_pm_dog_init_validate
drush_search_dog_init_validate
drush_shellalias_dog_init_validate
drush_site_install_dog_init_validate
drush_sitealias_dog_init_validate
drush_sql_dog_init_validate
drush_test_dog_init_validate
drush_topic_dog_init_validate
drush_upgrade_dog_init_validate
drush_user_dog_init_validate
drush_variable_dog_init_validate
drush_watchdog_dog_init_validate
drush_archive_pre_dog_init
drush_cache_pre_dog_init
drush_core_pre_dog_init
drush_docs_pre_dog_init
drush_dog_pre_dog_init
drush_drush_make_pre_dog_init
drush_drush_make_d_o_pre_dog_init
drush_field_pre_dog_init
drush_grn_pre_dog_init
drush_help_pre_dog_init
drush_image_pre_dog_init
drush_iq_pre_dog_init
drush_mw_pre_dog_init
drush_phpshell_pre_dog_init
drush_pm_pre_dog_init
drush_search_pre_dog_init
drush_shellalias_pre_dog_init
drush_site_install_pre_dog_init
drush_sitealias_pre_dog_init
drush_sql_pre_dog_init
drush_test_pre_dog_init
drush_topic_pre_dog_init
drush_upgrade_pre_dog_init
drush_user_pre_dog_init
drush_variable_pre_dog_init
drush_watchdog_pre_dog_init
drush_archive_dog_init
drush_cache_dog_init
drush_core_dog_init
drush_docs_dog_init
drush_dog_init [* Defined in /Users/mw/.drush/dog/dog.drush.inc]
drush_drush_make_dog_init
drush_drush_make_d_o_dog_init
drush_field_dog_init
drush_grn_dog_init
drush_help_dog_init
drush_image_dog_init
drush_iq_dog_init
drush_mw_dog_init
drush_phpshell_dog_init
drush_pm_dog_init
drush_search_dog_init
drush_shellalias_dog_init
drush_site_install_dog_init
drush_sitealias_dog_init
drush_sql_dog_init
drush_test_dog_init
drush_topic_dog_init
drush_upgrade_dog_init
drush_user_dog_init
drush_variable_dog_init
drush_watchdog_dog_init
drush_archive_post_dog_init
drush_cache_post_dog_init
drush_core_post_dog_init
drush_docs_post_dog_init
drush_dog_post_dog_init
drush_drush_make_post_dog_init
drush_drush_make_d_o_post_dog_init
drush_field_post_dog_init
drush_grn_post_dog_init
drush_help_post_dog_init
drush_image_post_dog_init
drush_iq_post_dog_init
drush_mw_post_dog_init
drush_phpshell_post_dog_init
drush_pm_post_dog_init
drush_search_post_dog_init
drush_shellalias_post_dog_init
drush_site_install_post_dog_init
drush_sitealias_post_dog_init
drush_sql_post_dog_init
drush_test_post_dog_init
drush_topic_post_dog_init
drush_upgrade_post_dog_init
drush_user_post_dog_init
drush_variable_post_dog_init
drush_watchdog_post_dog_init [101.37 sec, 9.31 MB]
Available rollback hooks for dog-init:                               [ok]
drush_dog_init_rollback [101.37 sec, 9.31 MB]
Good dog! [101.37 sec, 9.27 MB]                                         [notice]
Command dispatch complete [101.37 sec, 9.27 MB]                         [notice]
Peak memory usage was 10.34 MB [101.37 sec, 9.27 MB]                    [memory]

Although the errors are not present in drush5, the current code is still convoluted. The attached patch removes the dog_command() code in favor of a more standard hook_drush_init() implementation (user command do same thing). Net result is less code, with no functionality change.

Totally missed that --show-invoke global option. Awesome.

I'm pretty torn on this. I agree that the command Was expecting you (moshe) or someone at least, would get here and raise this objection eventually:

All those drush hooks are expected to work for drush developers. You don't want to piss them off - they are your patch contributors and users.

They are. However, it's not the extension mechanism I think dog should use. If you look at this commit I've just put into the oo-firstpass branch, you'll see I've added a DogCommandInterface. I'm expecting to implement all Dog commands as classes. Two basic reasons for that:

• Dog has to be able to function even when a Drupal instance is hard-borked - e.g., a syntax error in a file that would be loaded even on the slightest bootstrap. Consequently, we *never* bootstrap unless we absolutely have to. Which means that your average hook-based extension mechanisms have a much narrower range of appeal.
• We also want to *absolutely* ensure the consistency Dog's behavior across everyone using it on a given instance. To ensure that, we'll be changing the dog-init process around so that it picks up almost all of its runtime code (excepting the first parts of init and rollout) and dumps it into the site tree itself, then commits it.

Commands as classes mean we get an easy extension mechanism back, and one that we can trivially manage from within Dog by using autoload. People can write a new class file, drop it into place, and it'll pick it up instead of the built-in command. Same for the other objects that Dog uses. It also makes it *much* easier to manage moving code around, as well as the update process, since the code is logically organized into class units.

Like I said, I'm still torn on whether I *really* want to go through with this. I can't tell if I'm being a drama queen and just wanting to roll my own architecture (since I would prefer drush commands to be represented as classes in general), or if these concerns are really enough to justify the different approach.

drush commands can be in complete control of how much they bootstrap drupal. they can choose to bootstrap none of it, or just enough to figure out the drupal root, load the settings.php (DB is now available) or bootstrap to FULL. if you are worried that drupal is borked that just bootstrap to root and you are fine. Not sure how recently you looked at drush, but it is completely independent of drupal now. it is not a module anymore.

imo, you'd do well to keep doing things the drush way until it lets you down. at that point, you can build your own palace or you can improve drush itself. either is OK with me.

commands as objects, sites as objects, and so on are on the table for drush5. aegir has gone that way in recent year so it is likely that this code will flow back to drush.

Sorry, I wasn't explicit about the first step in my thought process: I know about the bootstrap levels, and a lot of the separations that have been made - I spent a couple days going through it all at the start of this project. What I'm saying is, because we want to control the environment as much as possible, we'll almost never bootstrap drush past the minimum level possible. As in, I'd like to prevent even loading up command files that are contained within modules in the current site. Just core drush, anything under ~/.drush, and any dog commands that have been attached to the instance. That's it.

Now that I think about it, though, I realize I haven't checked to see if I can control how many potential command files get bootstrapped.

Also, I've just remembered - the motivation for this stemmed from difficulties that Pantheon had had with building truly automated systems around drush. They didn't get into specifics, but the impression I got was that they'd tried to use drush, but found that being exposed to the instability of arbitrary code was more risk than they could afford. So building a part of their infrastructure around dog was predicated on it being insulated from such problems.

dog has nothing to worry about. If you don't bootstrap Drupal, then no commandfiles contained within Drupal are loaded. That happens at DRUSH_BOOTSTRAP_DRUPAL_SITE level. See http://api.drush.ws/api/function/_drush_find_commandfiles/4.x

Pantheon has different needs than dog. When Pantheon customers execute drush commands, any code that those customers have lying around in their HOME dir does get loaded. Commandfiles inside sites gets loaded too , if the command is issued from within a site and the command asks for a Drupal bootstrap. I can see how Pantheon would consider this a hostile environment for their own hosting commands like pushing code/DBs around. I personally might be more trusting of commandfiles that the user actively installed but both sides are justifiable.

dog has nothing to worry about. If you don't bootstrap Drupal, then no commandfiles contained within Drupal are loaded. That happens at DRUSH_BOOTSTRAP_DRUPAL_SITE level.

Great, great.

Pantheon has different needs than dog.

Not so sure that's true. As I they've explained it to me, Pantheon is looking to be a provider of the "collab" repo hosting, and potentially utilize dog for their internal purposes like moving code bases around (dev/test/prod). Which makes their needs a part of a pretty standard dog workflow. It also makes them an excellent test of one of dog's basic goals - make dog robust and reliable enough that machines can work with it.

Moreover, dog's going to need to attach itself to Git repository hooks, which means bootstrapping on things like Git commits. As you know, until we can get #1051748: Use APC to persist opcode cache when using CLI PHP. done, loading up all those command files on a large site could mean a commit takes ten, twenty seconds. That's why I ended up chiming in on that issue.

Still, I want to ponder on this a bit more before I go either way, so I'll probably just leave that interface as-is but not implement any commands with it yet. Especially since that unified dog_drush_init() is available - is that a recent addition, or did I just miss it? And is there already code that trying to make commands into classes that I can look at?

dog_drush_init() hook has been around for a long time but it is not widely used.

no code available yet for commands as objects.

i'd have to know more about pantheon's use case to give better info. i'll just say though that drush5 honors Envrironment Vars for HOME, ETC_PREFIX, SHARE_PREFIX and a few others. So you can dynamically control the search path for global command files. This is used by the phpunit tests to assure a clean environment no matter who's logged in and what commandfiles are stored in the usual dirs scanned by drush.

Ah, gave the init approach shot, and thought it seemed familiar. The _drush_init and _drush_exit is triggered out of drush_command, which is what I've replaced with dog_command by using the callback option. msonnabaum pointed me to that option when I asked him about a way to have a set of logic run on every command. I ultimately decided against using that hook because it would run on *all* drush commands, not just dog commands.

Still pondering on this.

OK, decided to not swim upstream and am using dog_drush_init() and dog_drush_exit(). I don't have a good enough reason to break the mold, here.

Out of curiousity, though - are you expecting to remove the ability to bypass drush_command() with our own callback sometime soon?

I think we'll keep it for the case when you really want many commands to point to same callback for some reason.

Thanks for the commit.