subdirs kicks in even when subdirs module is disabled.

$alias in the _subdirs_delete_site_location() function is expected to be a string with a slash in it.

None of the provision functions seem to validate this.
We could quickly add

if (count($alias_parts) < 2) { return FALSE; }

But why are these being called? Do we have any other provision extensions that depend on a hosting module being enabled?

Related: #2046167: [META] Remaining issues with subsite support (Part II)

that's a long time problem: there is no standard mechanism to enable/disable backend module from the frontend.

i am bumping this to critical since the subdir code has been merged in 2.x and this has the potential of breaking everything.

see also #1189576: can't disable the DNS service from the frontend.

From: function drush_commandfile_list() {

* Commands implementing hook_drush_load() in MODULE.drush.load.inc with
* a return value FALSE will not be loaded.

Should we implement a hook_drush_load() which checks the frontend module status in the hostmaster context ?

Maybe we could add a 'subdir' flag in the site context, and then switch based on that?

ideally, this would be an aegir-wide parameter that any task sends a list of aegir-related modules enabled, and then we have *one* _load hook that we kick in the backend?

Maybe that will still required a hook_drush_load() in every .load.inc for every module, but at least we can simplify the message passing to be more generic from the frontend's perspective?

Just throwing ideas out there...

I think every drush/provision contrib would have to implement hook_drush_load(), at least if there is something that should be prevented from running if the hosting counterpart is not enabled.
It might even be a slight performance gain if less code needs to be processed.

Some pseudo code:

// In subdirs/subdirs.drush.load.inc
function subdirs_drush_load() {
  $this_module = 'subdirs' ;
  return provision_is_module_enabled_in_hosting_frontend($this_module); 
}


// Somewhere in provision.... a part that's loaded before all the drush_load()'s get called.
function provision_is_module_enabled_in_hosting_frontend($module_name) {
  $context = drush_get_context('HOSTING_MODULES_ENABLED');
  // should we log a debug message here? 
  return (!empty($context[$module_name]));
}

// Somewhere in ... the verification task of the master server???
$modules = ...
drush_set_context('HOSTING_MODULES_ENABLED', $modules);

Normally, there shouldn't be any code in Provision that runs before drush_load()'s. In _drush_add_commandfiles(), it'll include all *.drush.load.inc's before including any commandfiles.

However, Drush provides an '--early' option that allows pre-bootstrap code to be run. It is invoked at the top of drush_main(). Unfortunately, it appears to only be documented in the option itself:

    $options['early']               = array('description' => "Include a file (with relative or full path) and call the drush_early_hook() function (where 'hook' is the filename). The function is called pre-bootstrap and offers an opportunity to alter the drush bootstrap environment or process (returning FALSE from the function will continue the bootstrap), or return output very rapidly (e.g. from caches). See includes/complete.inc for an example.");

I'm finding it hard to trace Drush's bootstrapping, but I think drushrc.php's are parsed just before looking for command files. If so, maybe we should consider writing a /var/aegir/.drushrc.php (or /var/aegir/.drush/drushrc.php), as part of verifying the hostmaster site. It might be triggered by adding a 'verify' task somewhere in hosting features.

So this seems to work. In provision/subdirs/subdirs.drush.load.inc:

function subdirs_drush_load() {
  return array_search('subdirs', drush_get_option('hosting_features'));
}

... and in /var/aegir/.drush/drushrc.php:

$options['hosting_features'] = array (
  'subdirs' => 'subdirs',
);

Changing the value in this array to anything other that 'subdirs' blocks the code in subdirs.drush.inc from running.

Since git.drupal.org is still broken, here are a couple patches that implement what I suggested in #8 and #9. This is a general purpose solution allows us to block drush hooks by implementing something like:

function subdirs_drush_load() {
  return array_search('subdirs', drush_get_option('hosting_features', array()));
}

+++ b/subdirs/subdirs.drush.load.inc
@@ -0,0 +1,10 @@
+  return array_search('subdirs', drush_get_option('hosting_features', array()));

Why array_search()?
I think looking up the array key is faster. And the array value could just be a boolean. This could later be extended to be an array containing more info about the module ... possibly version info.
It also reduces ambiguity for a contrib developer.

This updated provision patch also adds the hook to the dns module. Which is an easy thing to use for testing... after this patch the provision-zone command no longer appears in the help listing when calling 'drush'

// Option 1
return array_search('subdirs', drush_get_option('hosting_features', array()));

// Option 2
return array_key_exists('subdirs', drush_get_option('hosting_features', array()));

// Option 3
$hosting_features = $drush_get_option('hosting_features', array())
return !empty($hosting_features['subdirs']);

Just for the sake of it, let me add:

// Option 4
return in_array('subdirs', drush_get_option('hosting_features', array()));

ie. that we just have the modules as the values in the array - why bother with anything more complicated?

Also, at this point it is a little unclear for me when this code will run and the features array actually be written into that drushrc file... Can you be a little more specific about this?

After applying the patch I verified the hostmaster site to fill the /var/aegir/.drush/drushrc.php

Saving the admin/hosting/features page triggers a verify of the hostmaster site.

I'm ok with option 4... I'll update the patches.

Updated patch for provision ... the hosting one from #10 is still good.

Assuming we're otherwise okay with this, I'll make the changes in #14, and commit. I have some nice explanatory commit messages that I'd hate to waste :)

I pushed these in provision and hosting. I'm re-setting to 'active', since there's also a suggestion in #1 to validate better.

Retested after the patch update in #14 btw, it's all good :)

I've created #2099883: Validate an alias has parts (subdirs) to checkout the validation.

And another follow-up: #2099889: More hosting_features checks in _drush_load hooks?