From c33d0f7be84eef3f3cd3a6d4855fc315adb3c554 Mon Sep 17 00:00:00 2001 From: Angie Byron Date: Wed, 13 Jan 2010 05:08:29 +0000 Subject: #399642 follow-up by carlos8f: Replace drupal_install_modules() with an improved module_enable(). --- includes/install.inc | 46 ---------------- includes/module.inc | 84 ++++++++++++++++++++++++++++- install.php | 4 +- modules/field/tests/field.test | 4 +- modules/simpletest/drupal_web_test_case.php | 8 +-- modules/simpletest/tests/module.test | 22 ++++---- modules/system/system.admin.inc | 4 +- modules/system/system.install | 6 +-- modules/system/system.module | 15 ++++-- modules/system/system.test | 2 +- 10 files changed, 118 insertions(+), 77 deletions(-) diff --git a/includes/install.inc b/includes/install.inc index 6f876b2c3..d1496fcbd 100644 --- a/includes/install.inc +++ b/includes/install.inc @@ -535,52 +535,6 @@ function drupal_verify_profile($install_state) { return $requirements; } -/** - * Calls the install function for a given list of modules. - * - * @param $module_list - * The modules to install. - * @param $disable_modules_installed_hook - * Normally just testing wants to set this to TRUE. - * - * @return - * TRUE if installation was attempted, FALSE if one or more dependencies are - * missing. - */ -function drupal_install_modules($module_list = array(), $disable_modules_installed_hook = FALSE) { - $files = system_rebuild_module_data(); - $module_list = array_flip(array_values($module_list)); - do { - $moved = FALSE; - foreach ($module_list as $module => $weight) { - $file = $files[$module]; - if (isset($file->info['dependencies']) && is_array($file->info['dependencies'])) { - foreach ($file->info['dependencies'] as $dependency) { - if (!isset($module_list[$dependency])) { - if (!isset($files[$dependency])) { - // A dependency was not found, abort installation. - return FALSE; - } - elseif (!$files[$dependency]->status) { - // Add dependencies to $module_list and install them first. - $module_list[$dependency] = $weight - 1; - $moved = TRUE; - } - } - elseif ($module_list[$module] < $module_list[$dependency] +1) { - $module_list[$module] = $module_list[$dependency] +1; - $moved = TRUE; - } - } - } - } - } while ($moved); - asort($module_list); - $module_list = array_keys($module_list); - module_enable($module_list, $disable_modules_installed_hook); - return TRUE; -} - /** * Callback to install an individual install profile module. * diff --git a/includes/module.inc b/includes/module.inc index f440858a8..25b33011a 100644 --- a/includes/module.inc +++ b/includes/module.inc @@ -287,10 +287,53 @@ function module_load_all_includes($type, $name = NULL) { * * @param $module_list * An array of module names. + * @param $enable_dependencies + * If TRUE, dependencies will automatically be added and enabled in the + * correct order. This incurs a significant performance cost, so use FALSE + * if you know $module_list is already complete and in the correct order. * @param $disable_modules_installed_hook * Normally just testing wants to set this to TRUE. + * @return + * FALSE if one or more dependencies are missing, TRUE otherwise. */ -function module_enable($module_list, $disable_modules_installed_hook = FALSE) { +function module_enable($module_list, $enable_dependencies = TRUE, $disable_modules_installed_hook = FALSE) { + if ($enable_dependencies) { + // Get all module data so we can find dependencies and sort. + $module_data = system_rebuild_module_data(); + // Create an associative array with weights as values. + $module_list = array_flip(array_values($module_list)); + + while (list($module) = each($module_list)) { + if (!isset($module_data[$module])) { + // This module is not found in the filesystem, abort. + return FALSE; + } + if ($module_data[$module]->status) { + // Skip already enabled modules. + unset($module_list[$module]); + continue; + } + $module_list[$module] = $module_data[$module]->sort; + + // Add dependencies to the list, with a placeholder weight. + // The new modules will be processed as the while loop continues. + foreach ($module_data[$module]->info['dependencies'] as $dependency) { + if (!isset($module_list[$dependency])) { + $module_list[$dependency] = 0; + } + } + } + + if (!$module_list) { + // Nothing to do. All modules already enabled. + return TRUE; + } + + // Sort the module list by pre-calculated weights. + arsort($module_list); + $module_list = array_keys($module_list); + } + $invoke_modules = array(); // Try to install the enabled modules and collect which were installed. @@ -328,6 +371,7 @@ function module_enable($module_list, $disable_modules_installed_hook = FALSE) { if (!$disable_modules_installed_hook && !empty($modules_installed)) { module_invoke_all('modules_installed', $modules_installed); } + _system_update_bootstrap_status(); } foreach ($invoke_modules as $module) { @@ -346,6 +390,8 @@ function module_enable($module_list, $disable_modules_installed_hook = FALSE) { // enabled. module_invoke_all('modules_enabled', $invoke_modules); } + + return TRUE; } /** @@ -353,9 +399,42 @@ function module_enable($module_list, $disable_modules_installed_hook = FALSE) { * * @param $module_list * An array of module names. + * @param $disable_dependents + * If TRUE, dependent modules will automatically be added and disabled in the + * correct order. This incurs a significant performance cost, so use FALSE + * if you know $module_list is already complete and in the correct order. */ -function module_disable($module_list) { +function module_disable($module_list, $disable_dependents = TRUE) { + if ($disable_dependents) { + // Get all module data so we can find dependents and sort. + $module_data = system_rebuild_module_data(); + // Create an associative array with weights as values. + $module_list = array_flip(array_values($module_list)); + + while (list($module) = each($module_list)) { + if (!isset($module_data[$module]) || !$module_data[$module]->status) { + // This module doesn't exist or is already disabled, skip it. + unset($module_list[$module]); + continue; + } + $module_list[$module] = $module_data[$module]->sort; + + // Add dependent modules to the list, with a placeholder weight. + // The new modules will be processed as the while loop continues. + foreach ($module_data[$module]->required_by as $dependent => $dependent_data) { + if (!isset($module_list[$dependent]) && !strstr($module_data[$dependent]->filename, '.profile')) { + $module_list[$dependent] = 0; + } + } + } + + // Sort the module list by pre-calculated weights. + asort($module_list); + $module_list = array_keys($module_list); + } + $invoke_modules = array(); + foreach ($module_list as $module) { if (module_exists($module)) { // Check if node_access table needs rebuilding. @@ -385,6 +464,7 @@ function module_disable($module_list) { module_invoke_all('modules_disabled', $invoke_modules); // Update the registry to remove the newly-disabled module. registry_update(); + _system_update_bootstrap_status(); } // If there remains no more node_access module, rebuilding will be diff --git a/install.php b/install.php index 3a086bd38..0b9dddd87 100644 --- a/install.php +++ b/install.php @@ -1505,7 +1505,7 @@ function _install_module_batch($module, $module_name, &$context) { // loaded by drupal_bootstrap in subsequent batch requests, and other // modules possibly depending on it can safely perform their installation // steps. - module_enable(array($module)); + module_enable(array($module), FALSE); $context['results'][] = $module; $context['message'] = st('Installed %module module.', array('%module' => $module_name)); } @@ -1711,7 +1711,7 @@ function install_configure_form_submit($form, &$form_state) { // Enable update.module if this option was selected. if ($form_state['values']['update_status_module'][1]) { - drupal_install_modules(array('update')); + module_enable(array('update'), FALSE); // Add the site maintenance account's email address to the list of // addresses to be notified when updates are available, if selected. diff --git a/modules/field/tests/field.test b/modules/field/tests/field.test index b207bb55e..3052af9ad 100644 --- a/modules/field/tests/field.test +++ b/modules/field/tests/field.test @@ -2253,7 +2253,7 @@ class FieldCrudTestCase extends FieldTestCase { $field = field_read_field($field_name); $this->assertTrue($field_definition <= $field, t('The field was properly read.')); - module_disable($modules); + module_disable($modules, FALSE); $fields = field_read_fields(array('field_name' => $field_name), array('include_inactive' => TRUE)); $this->assertTrue(isset($fields[$field_name]) && $field_definition < $field, t('The field is properly read when explicitly fetching inactive fields.')); @@ -2265,7 +2265,7 @@ class FieldCrudTestCase extends FieldTestCase { $this->assertTrue(empty($field), t('%modules disabled. The field is marked inactive.', array('%modules' => implode(', ', $modules)))); $module = array_shift($modules); - module_enable(array($module)); + module_enable(array($module), FALSE); } // Check that the field is active again after all modules have been diff --git a/modules/simpletest/drupal_web_test_case.php b/modules/simpletest/drupal_web_test_case.php index c5d2926e9..9ba935559 100644 --- a/modules/simpletest/drupal_web_test_case.php +++ b/modules/simpletest/drupal_web_test_case.php @@ -1136,18 +1136,18 @@ class DrupalWebTestCase extends DrupalTestCase { $this->preloadRegistry(); - // Include the default profile + // Include the default profile. variable_set('install_profile', 'standard'); $profile_details = install_profile_info('standard', 'en'); // Install the modules specified by the default profile. - drupal_install_modules($profile_details['dependencies'], TRUE); + module_enable($profile_details['dependencies'], FALSE, TRUE); drupal_static_reset('_node_types_build'); if ($modules = func_get_args()) { // Install modules needed for this test. - drupal_install_modules($modules, TRUE); + module_enable($modules, TRUE, TRUE); } // Because the schema is static cached, we need to flush @@ -1158,7 +1158,7 @@ class DrupalWebTestCase extends DrupalTestCase { // Run default profile tasks. $install_state = array(); - drupal_install_modules(array('standard'), TRUE); + module_enable(array('standard'), FALSE, TRUE); // Rebuild caches. node_types_rebuild(); diff --git a/modules/simpletest/tests/module.test b/modules/simpletest/tests/module.test index 939804953..242910c7f 100644 --- a/modules/simpletest/tests/module.test +++ b/modules/simpletest/tests/module.test @@ -37,7 +37,7 @@ class ModuleUnitTest extends DrupalWebTestCase { $this->assertModuleList($module_list, t('Standard profile')); // Try to install a new module. - drupal_install_modules(array('contact')); + module_enable(array('contact')); $module_list[] = 'contact'; sort($module_list); $this->assertModuleList($module_list, t('After adding a module')); @@ -101,25 +101,25 @@ class ModuleUnitTest extends DrupalWebTestCase { } /** - * Test drupal_install_modules() and dependency resolution. + * Test dependency resolution. */ - function testDrupalInstallModules() { - drupal_install_modules(array('module_test')); + function testDependencyResolution() { + module_enable(array('module_test'), FALSE); $this->assertTrue(module_exists('module_test'), t('Test module is enabled.')); // First, create a fake missing dependency. Forum depends on poll, which // depends on a made-up module, foo. Nothing should be installed. variable_set('dependency_test', 'missing dependency'); - $result = drupal_install_modules(array('forum')); - $this->assertFalse($result, t('drupal_install_modules() returns FALSE if dependencies are missing.')); - $this->assertFalse(module_exists('forum'), t('drupal_install_modules() aborts if dependencies are missing.')); + $result = module_enable(array('forum')); + $this->assertFalse($result, t('module_enable() returns FALSE if dependencies are missing.')); + $this->assertFalse(module_exists('forum'), t('module_enable() aborts if dependencies are missing.')); - // Now, fix the missing dependency. drupal_install_modules() should work. + // Now, fix the missing dependency. module_enable() should work. variable_set('dependency_test', 'dependency'); - $result = drupal_install_modules(array('forum')); - $this->assertTrue($result, t('drupal_install_modules() returns the correct value.')); + $result = module_enable(array('forum')); + $this->assertTrue($result, t('module_enable() returns the correct value.')); // Verify that the fake dependency chain was installed. - $this->assertTrue(module_exists('poll') && module_exists('php'), t('Dependency chain was installed by drupal_install_modules().')); + $this->assertTrue(module_exists('poll') && module_exists('php'), t('Dependency chain was installed by module_enable().')); // Finally, verify that the original module was installed. $this->assertTrue(module_exists('forum'), t('Module installation with unlisted dependencies succeeded.')); } diff --git a/modules/system/system.admin.inc b/modules/system/system.admin.inc index c478e0465..fe6af7436 100644 --- a/modules/system/system.admin.inc +++ b/modules/system/system.admin.inc @@ -1203,7 +1203,7 @@ function system_modules_submit($form, &$form_state) { $sort[$module] = $files[$module]->sort; } array_multisort($sort, $modules_to_be_enabled); - module_enable($modules_to_be_enabled); + module_enable($modules_to_be_enabled, FALSE); } // Disable the modules that need disabling. if (!empty($disable_modules)) { @@ -1225,7 +1225,7 @@ function system_modules_submit($form, &$form_state) { $sort[$module] = $files[$module]->sort; } array_multisort($sort, $new_modules); - drupal_install_modules($new_modules); + module_enable($new_modules, FALSE); } $current_module_list = module_list(TRUE); diff --git a/modules/system/system.install b/modules/system/system.install index c7ef22fd3..b6c02d4e5 100644 --- a/modules/system/system.install +++ b/modules/system/system.install @@ -2082,8 +2082,7 @@ function system_update_7018() { */ function system_update_7020() { $module_list = array('field_sql_storage', 'field'); - drupal_install_modules($module_list); - module_enable($module_list); + module_enable($module_list, FALSE); } /** @@ -2305,8 +2304,7 @@ function system_update_7027() { ->condition('type', 'module') ->condition('name', $module_list) ->execute(); - drupal_install_modules($module_list); - module_enable($module_list); + module_enable($module_list, FALSE); } /** diff --git a/modules/system/system.module b/modules/system/system.module index ef5b72af0..58d135ffb 100644 --- a/modules/system/system.module +++ b/modules/system/system.module @@ -2203,7 +2203,18 @@ function system_rebuild_module_data() { ksort($modules); system_get_files_database($modules, 'module'); system_update_files_database($modules, 'module'); - // Refresh bootstrap status. + $modules = _module_build_dependencies($modules); + return $modules; +} + +/** + * Refresh bootstrap column in the system table. + * + * This is called internally by module_enable/disable() to flag modules that + * implement hooks used during bootstrap, such as hook_boot(). These modules + * are loaded earlier to invoke the hooks. + */ +function _system_update_bootstrap_status() { $bootstrap_modules = array(); foreach (bootstrap_hooks() as $hook) { foreach (module_implements($hook) as $module) { @@ -2219,8 +2230,6 @@ function system_rebuild_module_data() { $query->condition('name', $bootstrap_modules, 'NOT IN'); } $query->execute(); - $modules = _module_build_dependencies($modules); - return $modules; } /** diff --git a/modules/system/system.test b/modules/system/system.test index 03545ce94..4e49ade61 100644 --- a/modules/system/system.test +++ b/modules/system/system.test @@ -208,7 +208,7 @@ class ModuleDependencyTestCase extends ModuleTestCase { $this->assert(count($checkbox) == 1, t('Checkbox for the module is disabled.')); // Force enable the system_dependencies_test module. - module_enable(array('system_dependencies_test')); + module_enable(array('system_dependencies_test'), FALSE); // Verify that the module is forced to be disabled when submitting // the module page. -- cgit v1.2.3