#399642 by sndev and carlos8f: Make drupal_install_modules() resolve...

#399642 by sndev and carlos8f: Make drupal_install_modules() resolve dependencies and trigger an error when they do not exist. (with tests)

includes/install.inc

@@ -537,6 +537,10 @@ function drupal_verify_profile($install_state) {
  *   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();
@@ -547,7 +551,18 @@ function drupal_install_modules($module_list = array(), $disable_modules_install
       $file = $files[$module];
       if (isset($file->info['dependencies']) && is_array($file->info['dependencies'])) {
         foreach ($file->info['dependencies'] as $dependency) {
-          if (isset($module_list[$dependency]) && $module_list[$module] < $module_list[$dependency] +1) {
+          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;
           }
@@ -558,6 +573,7 @@ function drupal_install_modules($module_list = array(), $disable_modules_install
   asort($module_list);
   $module_list = array_keys($module_list);
   module_enable($module_list, $disable_modules_installed_hook);
+  return TRUE;
 }
 
 /**

modules/simpletest/drupal_web_test_case.php

@@ -1140,17 +1140,14 @@ protected function setUp() {
     variable_set('install_profile', 'default');
     $profile_details = install_profile_info('default', 'en');
 
-    // Add the specified modules to the list of modules in the default profile.
     // Install the modules specified by the default profile.
     drupal_install_modules($profile_details['dependencies'], TRUE);
 
     drupal_static_reset('_node_types_build');
 
-    // Install additional modules one at a time in order to make sure that the
-    // list of modules is updated between each module's installation.
-    $modules = func_get_args();
-    foreach ($modules as $module) {
-      drupal_install_modules(array($module), TRUE);
+    if ($modules = func_get_args()) {
+      // Install modules needed for this test.
+      drupal_install_modules($modules, TRUE);
     }
 
     // Because the schema is static cached, we need to flush

modules/simpletest/tests/module.test

@@ -99,6 +99,30 @@ class ModuleUnitTest extends DrupalWebTestCase {
     $this->drupalGet('');
     $this->assertTrue(cache_get('module_implements', 'cache_bootstrap'), t('The module implements cache is populated after requesting a page.'));
   }
+
+  /**
+   * Test drupal_install_modules() and dependency resolution.
+   */
+  function testDrupalInstallModules() {
+    drupal_install_modules(array('module_test'));
+    $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.'));
+
+    // Now, fix the missing dependency. drupal_install_modules() should work.
+    variable_set('dependency_test', 'dependency');
+    $result = drupal_install_modules(array('forum'));
+    $this->assertTrue($result, t('drupal_install_modules() 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().'));
+    // Finally, verify that the original module was installed.
+    $this->assertTrue(module_exists('forum'), t('Module installation with unlisted dependencies succeeded.'));
+  }
 }
 
 /**

modules/simpletest/tests/module_test.module

@@ -9,3 +9,31 @@ function module_test_permission() {
     'module_test perm' => t('example perm for module_test module'),
   );
 }
+
+/**
+ * Implements hook_system_info_alter().
+ * 
+ * Manipulate module dependencies to test dependency chains.
+ */
+function module_test_system_info_alter(&$info, $file, $type) {
+  if (variable_get('dependency_test', FALSE) == 'missing dependency') {
+    if ($file->name == 'forum') {
+      // Make forum module depend on poll.
+      $info['dependencies'][] = 'poll';
+    }
+    elseif ($file->name == 'poll') {
+      // Make poll depend on a made-up module.
+      $info['dependencies'][] = 'foo';
+    }
+  }
+  elseif (variable_get('dependency_test', FALSE) == 'dependency') {
+    if ($file->name == 'forum') {
+      // Make the forum module depend on poll.
+      $info['dependencies'][] = 'poll';
+    }
+    elseif ($file->name == 'poll') {
+      // Make poll depend on php module.
+      $info['dependencies'][] = 'php';
+    }
+  }
+}
\ No newline at end of file