Refactor pm enable/disable/uninstall to better use version specific engines. Fix D7 versions.

Changes:

0. I think there was a typo with `DRUSH_PM_ENABLE_MODULE_NOT FOUND`.. replaced whitespace with underscore.

1. D7 environment: `drush_get_modules()` now return `system_rebuild_module_data()` instead of using a custom algorithm. I think it was using a custom algorithm because `module_rebuild_cache()` dissapear in D7.
`system_rebuild_module_data()` return the full dependency chain. There's a lightweight option: `_system_rebuild_module_data()`. Should we change to use this in `drush_get_modules()` and explicitly call to `_module_build_dependencies()` where it is needed?

2. `pm_dependencies()` has gone. Some code has been moved up to `drush_pm_enable()` and version specific code has been moved to new function `drush_module_dependencies()`. It is now more untangled and I think more elegant as there aren't arguments passed by reference as in `pm_dependencies()` in despite of we have two more loops over module list in `pm_enable()`.

3. `drush_module_dependencies()` is a environment-dependant function. Same code duplicated for D5 and D6. D7 code is different and also handles module version incompatibility. Note this function doesn't set errors or print messages: it returns a $status array with errors related to each module.

4. new environment function `drush_module_enable()`. It is a wrapper for `module_enable($modules, FALSE)` in D6 & D7. D5 version has the install/enable modules separation logic.

----
Extra thoughs:

a) `pm_enable()` will evolve again in the future to also include themes. The idea is to move code specific for module from pm_enable() to pm_enable_modules() and put some code at the beginning of `drush_pm_enable()` to clasify projects into modules and themes and call to pm_enable_modules() a/o pm_enable_themes(). It can be done now or we can wait for the 3.x release.

b) I don't like completely current error handling of `drush_module_dependencies()`. I'd like to see it evolve to an array of all the errors found: modules missing, version incompatible.

c) I'd like also to see more friendly messages for enable/disable. ie: it is not a drush error that a module is not available. See log below.

----

I plan to apply the same logic for pm_disable().... Just providing now a patch for reviewing as you are in a hurry to release 3.x alpha.

As code has been heavily refactored it needs testing on three drupal supported versions. Here's the new error we can find in D7 when enabling a module:

jonhattan@larry:/var/www/drupal-7.x-dev/sites/d7$ drush pm-enable devel
Module devel cannot be enabled because it depends on menu  (2.x) but 7.x-dev is available                                                         [error]
There are no modules to be enabled.
An error occurred at function : drush_pm_enable                                                                                                   [error]

Minor changes:
1. Fixed the style issues.
2. D5: `_drush_get_modules()` sets `$module->info['dependents'] = array();` if empty.
3. Included proposal to change environment function's documentation:

environment.inc:

/**
 * Enable a list of modules. It is assumed the list contains all the dependencies not already enabled.
 * 
 * @param $modules
 *   Array of module names
 */
function drush_module_enable($modules) {}

environment_5.inc:
/**
 * Implementation of drush_module_enable() for Drupal 5.
 */
function _drush_module_enable($modules) {}

Major changes:
4. Refactor of `drush_pm_disable()`. Added new environment-dependant `drush_disable_module()`, and `pm_reverse_dependencies()` removed.

Other things that can be done:
* `pm_system_modules_form_submit()` is a candidate to move to environment.inc
* `drush_module_uninstall()` is also possible along with a minor refactor of `drush_pm_uninstall()`.

Well, even more than the issue's title. Should I continue with that and #3 "extra thoughts "?

So functions using engines needs a call to drush_include_engine('drupal', 'environment') at the top. bridge functions in environment.inc are removed and all functions in environment_X.inc are de-underscored. `drush_incude_engine()` already uses `drush_set_error()`.

new patch to come.

Changes:
1. environment functions: removed all bridge functions in environment.inc. Added a call to drush_include_engine('drupal', 'environment'); in all functions calling `drush_get_modules()`.
2. renamed `pm_system_modules_form_submit()` to `drush_system_modules_form_submit() ` per environment.
3. refactor `pm_uninstall_modules()`. New function `drush_uninstall_module()` per environment.
4. `drush_module_dependencies()` renamed to `drush_check_module_dependencies()`.

I think it's time to stop adding new things to this patch and continue in separate issues for modules+themes integration or better error handling / output messages.

For some reason changes to update_6.inc didn't ship in the commit: http://drupal.org/cvs?commit=318198

ok. already commited.

http://drupal.org/cvs?commit=318536

I'll take a look at it in the next days.

It seems system_rebuild_module_data() provides the dependencies array in reverse order. So the first change in this patch just reverse it to fix the issue.

Reviewing the code flow from this point on to find any drawback for this solution, I see we use array_merge() with a string keys array of modules in a loop, and after the loop it does array_unique(). It is not needed as array_merge does it right and avoid duplicates. From array_merge's documentation:

If the input arrays have the same string keys, then the later value for that key will overwrite the previous one. If, however, the arrays contain numeric keys, the later value will not overwrite the original value, but will be appended.

So the second change is a bit of performance gain.

I'm rusty... here's the patch for d5 and d6. I've tested it making devel_generate dependant on devel and devel dependant on token.

It doesn't work for d5 because the list of dependencies is not recursively calculated by drupal. Can be fixed by making drush_check_module_dependencies() a recursive function, not sure if it worth it.