Issue #1719648 by joachim, ttkaminski, kshama_deshmukh:...

Issue #1719648 by joachim, ttkaminski, kshama_deshmukh: ModuleInstaller::install() silently fails if a module isn't in the file system

core/lib/Drupal/Core/Extension/MissingDependencyException.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Extension\MissingDependencyException.
+ */
+
+namespace Drupal\Core\Extension;
+
+/**
+ * Exception class to throw when modules are missing on install.
+ *
+ * @see \Drupal\Core\Extension\ModuleInstaller::install()
+ */
+class MissingDependencyException extends \Exception {}

core/lib/Drupal/Core/Extension/ModuleInstaller.php

@@ -11,6 +11,7 @@
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Cache\CacheBackendInterface;
 use Drupal\Core\DrupalKernelInterface;
+use Drupal\Component\Utility\String;
 
 /**
  * Default implementation of the module installer.
@@ -83,9 +84,12 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
       // Get all module data so we can find dependencies and sort.
       $module_data = system_rebuild_module_data();
       $module_list = $module_list ? array_combine($module_list, $module_list) : array();
-      if (array_diff_key($module_list, $module_data)) {
+      if ($missing_modules = array_diff_key($module_list, $module_data)) {
         // One or more of the given modules doesn't exist.
-        return FALSE;
+        throw new MissingDependencyException(String::format('Unable to install modules %modules due to missing modules %missing.', array(
+          '%modules' => implode(', ', $module_list),
+          '%missing' => implode(', ', $missing_modules),
+        )));
       }
 
       // Only process currently uninstalled modules.
@@ -101,7 +105,10 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
         foreach (array_keys($module_data[$module]->requires) as $dependency) {
           if (!isset($module_data[$dependency])) {
             // The dependency does not exist.
-            return FALSE;
+            throw new MissingDependencyException(String::format('Unable to install modules: module %module is missing its dependency module %dependency.', array(
+              '%module' => $module,
+              '%dependency' => $dependency,
+            )));
           }
 
           // Skip already installed modules.

core/lib/Drupal/Core/Extension/ModuleInstallerInterface.php

@@ -31,7 +31,10 @@ interface ModuleInstallerInterface {
    *   if you know $module_list is already complete.
    *
    * @return bool
-   *   FALSE if one or more dependencies are missing, TRUE otherwise.
+   *   TRUE if the modules were successfully installed.
+   *
+   * @throws \Drupal\Core\Extension\MissingDependencyException
+   *   Thrown when a requested module, or a dependency of one, can not be found.
    *
    * @see hook_module_preinstall()
    * @see hook_install()

core/modules/config/src/Tests/ConfigInstallWebTest.php

@@ -132,8 +132,7 @@ function testInstallProfileConfigOverwrite() {
     // Turn on the test module, which will attempt to replace the
     // configuration data. This attempt to replace the active configuration
     // should be ignored.
-    $status = \Drupal::service('module_installer')->install(array('config_existing_default_config_test'));
-    $this->assertTrue($status, "The module config_existing_default_config_test was installed.");
+    \Drupal::service('module_installer')->install(array('config_existing_default_config_test'));
 
     // Verify that the test module has not been able to change the data.
     $config = $this->config($config_name);

core/modules/simpletest/src/Tests/SimpleTestTest.php

@@ -210,7 +210,7 @@ function assertNothing() {
    * Confirm that the stub test produced the desired results.
    */
   function confirmStubTestResults() {
-    $this->assertAssertion(t('Enabled modules: %modules', array('%modules' => 'non_existent_module')), 'Other', 'Fail', 'SimpleTestTest.php', 'Drupal\simpletest\Tests\SimpleTestTest->setUp()');
+    $this->assertAssertion(t('Unable to install modules %modules due to missing modules %missing.', array('%modules' => 'non_existent_module', '%missing' => 'non_existent_module')), 'Other', 'Fail', 'SimpleTestTest.php', 'Drupal\simpletest\Tests\SimpleTestTest->setUp()');
 
     $this->assertAssertion($this->passMessage, 'Other', 'Pass', 'SimpleTestTest.php', 'Drupal\simpletest\Tests\SimpleTestTest->stubTest()');
     $this->assertAssertion($this->failMessage, 'Other', 'Fail', 'SimpleTestTest.php', 'Drupal\simpletest\Tests\SimpleTestTest->stubTest()');

core/modules/simpletest/src/WebTestBase.php

@@ -916,8 +916,15 @@ protected function setUp() {
     }
     if ($modules) {
       $modules = array_unique($modules);
-      $success = $container->get('module_installer')->install($modules, TRUE);
-      $this->assertTrue($success, String::format('Enabled modules: %modules', array('%modules' => implode(', ', $modules))));
+      try {
+        $success = $container->get('module_installer')->install($modules, TRUE);
+        $this->assertTrue($success, String::format('Enabled modules: %modules', array('%modules' => implode(', ', $modules))));
+      }
+      catch (\Drupal\Core\Extension\MissingDependencyException $e) {
+        // The exception message has all the details.
+        $this->fail($e->getMessage());
+      }
+
       $this->rebuildContainer();
     }
 

core/modules/system/src/Form/ModulesListConfirmForm.php

@@ -153,6 +153,10 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
 
     // Install the given modules.
     if (!empty($this->modules['install'])) {
+      // Don't catch the exception that this can throw for missing dependencies:
+      // the form doesn't allow modules with unmet dependencies, so the only way
+      // this can happen is if the filesystem changed between form display and
+      // submit, in which case the user has bigger problems.
       $this->moduleInstaller->install(array_keys($this->modules['install']));
     }
 

core/modules/system/src/Tests/Extension/ModuleHandlerTest.php

@@ -116,8 +116,14 @@ function testDependencyResolution() {
     \Drupal::state()->set('module_test.dependency', 'missing dependency');
     drupal_static_reset('system_rebuild_module_data');
 
-    $result = $this->moduleInstaller()->install(array('color'));
-    $this->assertFalse($result, 'ModuleHandler::install() returns FALSE if dependencies are missing.');
+    try {
+      $result = $this->moduleInstaller()->install(array('color'));
+      $this->fail(t('ModuleInstaller::install() throws an exception if dependencies are missing.'));
+    }
+    catch (\Drupal\Core\Extension\MissingDependencyException $e) {
+      $this->pass(t('ModuleInstaller::install() throws an exception if dependencies are missing.'));
+    }
+
     $this->assertFalse($this->moduleHandler()->moduleExists('color'), 'ModuleHandler::install() aborts if dependencies are missing.');
 
     // Fix the missing dependency.