diff --git a/core/core.services.yml b/core/core.services.yml index f3282256e1..a13368228b 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -558,7 +558,7 @@ services: arguments: ['@app.root', '@config.factory', '@extension.list.theme'] theme_installer: class: Drupal\Core\Extension\ThemeInstaller - arguments: ['@theme_handler', '@config.factory', '@config.installer', '@module_handler', '@config.manager', '@asset.css.collection_optimizer', '@router.builder', '@logger.channel.default', '@state'] + arguments: ['@theme_handler', '@config.factory', '@config.installer', '@module_handler', '@config.manager', '@asset.css.collection_optimizer', '@router.builder', '@logger.channel.default', '@state', '@extension.list.module'] # @deprecated in Drupal 8.0.x and will be removed before 9.0.0. Use the other # entity* services instead. entity.manager: diff --git a/core/lib/Drupal/Core/Extension/InfoParserDynamic.php b/core/lib/Drupal/Core/Extension/InfoParserDynamic.php index 541b6a086c..2e1dae399d 100644 --- a/core/lib/Drupal/Core/Extension/InfoParserDynamic.php +++ b/core/lib/Drupal/Core/Extension/InfoParserDynamic.php @@ -120,15 +120,6 @@ public function parse($filename) { $parsed_info['install'] = $parsed_info['dependencies']; $parsed_info['dependencies'] = []; } - // Module dependencies declared by themes should be in machine name - // format. Module dependencies declared by a theme do not support version - // constraints or project prefixes. - if ($parsed_info['type'] === 'theme' && isset($parsed_info['dependencies'])) { - $non_machine_name_dependencies = preg_grep('/[^a-z0-9_]+/', $parsed_info['dependencies']); - if (!empty($non_machine_name_dependencies)) { - throw new InfoParserException("Theme module dependencies must be machine names (only underscores, digits, and lowercase letters). At least one dependency declared by $filename is not in machine name format: " . implode(',', $non_machine_name_dependencies)); - } - } } return $parsed_info; } diff --git a/core/lib/Drupal/Core/Extension/ModuleInstaller.php b/core/lib/Drupal/Core/Extension/ModuleInstaller.php index 9866739ba0..538ecdc19d 100644 --- a/core/lib/Drupal/Core/Extension/ModuleInstaller.php +++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php @@ -591,6 +591,7 @@ protected function updateKernel($module_filenames) { // After rebuilding the container we need to update the injected // dependencies. $container = $this->kernel->getContainer(); + $this->themeExtensionList = $container->get('extension.list.theme'); $this->moduleHandler = $container->get('module_handler'); } diff --git a/core/lib/Drupal/Core/Extension/ThemeInstaller.php b/core/lib/Drupal/Core/Extension/ThemeInstaller.php index 59adbae578..0fa79fa7a3 100644 --- a/core/lib/Drupal/Core/Extension/ThemeInstaller.php +++ b/core/lib/Drupal/Core/Extension/ThemeInstaller.php @@ -2,6 +2,7 @@ namespace Drupal\Core\Extension; +use Drupal\Component\Utility\Html; use Drupal\Core\Asset\AssetCollectionOptimizerInterface; use Drupal\Core\Cache\Cache; use Drupal\Core\Config\ConfigFactoryInterface; @@ -10,6 +11,8 @@ use Drupal\Core\Extension\Exception\UnknownExtensionException; use Drupal\Core\Routing\RouteBuilderInterface; use Drupal\Core\State\StateInterface; +use Drupal\Core\StringTranslation\StringTranslationTrait; +use Drupal\system\ModuleDependencyMessageTrait; use Psr\Log\LoggerInterface; /** @@ -17,6 +20,9 @@ */ class ThemeInstaller implements ThemeInstallerInterface { + use ModuleDependencyMessageTrait; + use StringTranslationTrait; + /** * @var \Drupal\Core\Extension\ThemeHandlerInterface */ @@ -62,6 +68,13 @@ class ThemeInstaller implements ThemeInstallerInterface { */ protected $logger; + /** + * The module extension list. + * + * @var \Drupal\Core\Extension\ModuleExtensionList + */ + protected $moduleExtensionList; + /** * Constructs a new ThemeInstaller. * @@ -86,8 +99,10 @@ class ThemeInstaller implements ThemeInstallerInterface { * A logger instance. * @param \Drupal\Core\State\StateInterface $state * The state store. + * @param \Drupal\Core\Extension\ModuleExtensionList $module_extension_list + * The module extension list. */ - public function __construct(ThemeHandlerInterface $theme_handler, ConfigFactoryInterface $config_factory, ConfigInstallerInterface $config_installer, ModuleHandlerInterface $module_handler, ConfigManagerInterface $config_manager, AssetCollectionOptimizerInterface $css_collection_optimizer, RouteBuilderInterface $route_builder, LoggerInterface $logger, StateInterface $state) { + public function __construct(ThemeHandlerInterface $theme_handler, ConfigFactoryInterface $config_factory, ConfigInstallerInterface $config_installer, ModuleHandlerInterface $module_handler, ConfigManagerInterface $config_manager, AssetCollectionOptimizerInterface $css_collection_optimizer, RouteBuilderInterface $route_builder, LoggerInterface $logger, StateInterface $state, ModuleExtensionList $module_extension_list = NULL) { $this->themeHandler = $theme_handler; $this->configFactory = $config_factory; $this->configInstaller = $config_installer; @@ -97,6 +112,11 @@ public function __construct(ThemeHandlerInterface $theme_handler, ConfigFactoryI $this->routeBuilder = $route_builder; $this->logger = $logger; $this->state = $state; + if ($module_extension_list === NULL) { + @trigger_error('The extension.list.module service must be passed to ' . __NAMESPACE__ . '\ThemeInstaller::__construct. It was added in Drupal 8.9.0 and will be required before Drupal 10.0.0.', E_USER_DEPRECATED); + $module_extension_list = \Drupal::service('extension.list.module'); + } + $this->moduleExtensionList = $module_extension_list; } /** @@ -124,6 +144,7 @@ public function install(array $theme_list, $install_dependencies = TRUE) { } foreach ($theme_list as $theme => $value) { + $module_list = $this->moduleExtensionList->getList(); $module_dependencies = $theme_data[$theme]->module_dependencies; $theme_dependencies = array_diff_key($theme_data[$theme]->requires, $module_dependencies); $unmet_module_dependencies = array_diff_key($module_dependencies, $installed_modules); @@ -131,7 +152,14 @@ public function install(array $theme_list, $install_dependencies = TRUE) { // Prevent themes with unmet module dependencies from being installed. if (!empty($unmet_module_dependencies)) { $unmet_module_dependencies_list = implode(', ', array_keys($unmet_module_dependencies)); - throw new MissingDependencyException("Unable to install theme: '$theme' due to unmet module dependencies: '$unmet_module_dependencies_list.'"); + throw new MissingDependencyException("Unable to install theme: '$theme' due to unmet module dependencies: '$unmet_module_dependencies_list'."); + } + + foreach ($module_dependencies as $dependency => $dependency_object) { + if ($incompatible = $this->checkDependencyMessage($module_list, $dependency, $dependency_object)) { + $sanitized_message = Html::decodeEntities(strip_tags($incompatible)); + throw new MissingDependencyException("Unable to install theme: $sanitized_message"); + } } // Add dependencies to the list of themes to install. The new themes diff --git a/core/modules/system/src/Controller/ThemeController.php b/core/modules/system/src/Controller/ThemeController.php index 0a8ad31cf8..1f3e61865c 100644 --- a/core/modules/system/src/Controller/ThemeController.php +++ b/core/modules/system/src/Controller/ThemeController.php @@ -6,6 +6,7 @@ use Drupal\Core\Config\PreExistingConfigException; use Drupal\Core\Config\UnmetDependenciesException; use Drupal\Core\Controller\ControllerBase; +use Drupal\Core\Extension\MissingDependencyException; use Drupal\Core\Extension\ThemeExtensionList; use Drupal\Core\Extension\ThemeHandlerInterface; use Drupal\Core\Extension\ThemeInstallerInterface; @@ -161,6 +162,9 @@ public function install(Request $request) { catch (UnmetDependenciesException $e) { $this->messenger()->addError($e->getTranslatedMessage($this->getStringTranslation(), $theme)); } + catch (MissingDependencyException $e) { + $this->messenger()->addError($this->t('Unable to install @theme due to missing module dependencies.', ['@theme' => $theme])); + } return $this->redirect('system.themes_page'); } diff --git a/core/modules/system/system.admin.inc b/core/modules/system/system.admin.inc index 206ed3cdf3..5ae24c07fb 100644 --- a/core/modules/system/system.admin.inc +++ b/core/modules/system/system.admin.inc @@ -139,7 +139,7 @@ function template_preprocess_system_modules_details(&$variables) { foreach ($theme->info['dependencies'] as $dependency) { if (isset($form[$dependency])) { // Add themes to the module's required by list. - $form[$dependency]['#required_by'][] = $theme->info['name'] . ' (' . t('Theme') . ')' . (!$theme->status ? ' (' . t('Disabled') . ')' : ''); + $form[$dependency]['#required_by'][] = $theme->status ? t('@theme', ['@theme (theme)' => $theme->info['name']]) : t('@theme (theme) (disabled)', ['@theme' => $theme->info['name']]); } } } diff --git a/core/modules/system/system.install b/core/modules/system/system.install index daf78612ee..7e0ffb0506 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -858,7 +858,9 @@ function system_requirements($phase) { // Display an error if a newly introduced dependency in a module is not resolved. if ($phase == 'update') { $profile = \Drupal::installProfile(); - $files = \Drupal::service('extension.list.module')->getList(); + $module_files = \Drupal::service('extension.list.module')->getList(); + $theme_files = \Drupal::service('extension.list.theme')->getList(); + $files = array_merge($module_files, $theme_files); foreach ($files as $module => $file) { // Ignore disabled modules and installation profiles. if (!$file->status || $module == $profile) { diff --git a/core/modules/system/tests/src/Functional/Form/ModulesListFormWebTest.php b/core/modules/system/tests/src/Functional/Form/ModulesListFormWebTest.php index cef12ffcc7..5d5c7b3c91 100644 --- a/core/modules/system/tests/src/Functional/Form/ModulesListFormWebTest.php +++ b/core/modules/system/tests/src/Functional/Form/ModulesListFormWebTest.php @@ -74,8 +74,7 @@ public function testModulesListFormWithInvalidInfoFile() { */ public function testRequiredByThemeMessage() { $this->createUserAdministerModules(); - $module_theme_depends_on_description = $this->getSession()->getPage()->findAll('css', '#edit-modules-test-module-required-by-theme-enable-description .admin-requirements li:contains("Test Theme Depending on Modules (Theme) (Disabled)")'); - + $module_theme_depends_on_description = $this->getSession()->getPage()->findAll('css', '#edit-modules-test-module-required-by-theme-enable-description .admin-requirements li:contains("Test Theme Depending on Modules (theme) (disabled)")'); // Confirm that that 'Test Theme Depending on Modules' is listed as being // required by the module 'Test Module Required by Theme'. $this->assertCount(1, $module_theme_depends_on_description); diff --git a/core/modules/system/tests/src/Functional/Theme/ThemeUiTest.php b/core/modules/system/tests/src/Functional/Theme/ThemeUiTest.php index 44f9f53809..c46998a380 100644 --- a/core/modules/system/tests/src/Functional/Theme/ThemeUiTest.php +++ b/core/modules/system/tests/src/Functional/Theme/ThemeUiTest.php @@ -316,4 +316,15 @@ public function testInstallModuleWithMissingDependencies() { $this->assertContains('This theme requires the listed modules to operate correctly.', $theme_container->getText()); } + /** + * Tests installing a theme with incompatible module dependencies. + */ + public function testInstallModuleWithIncompatibleDependencies() { + $this->container->get('module_installer')->install(['test_module_compatible_constraint', 'test_module_incompatible_constraint']); + $this->drupalGet('admin/appearance'); + $theme_container = $this->getSession()->getPage()->find('css', 'h3:contains("Test Theme Depending on Version Constrained Modules")')->getParent(); + $this->assertContains('Requires: Test Module Theme Depends on with Compatible ConstraintTest Module Theme Depends on with Incompatible Constraint (>=8.x-2.x) (incompatible with version 8.x-1.8)', $theme_container->getText()); + $this->assertContains('This theme requires the listed modules to operate correctly.', $theme_container->getText()); + } + } diff --git a/core/modules/system/tests/themes/test_theme_depending_on_constrained_modules/test_module_compatible_constraint/test_module_compatible_constraint.info.yml b/core/modules/system/tests/themes/test_theme_depending_on_constrained_modules/test_module_compatible_constraint/test_module_compatible_constraint.info.yml new file mode 100644 index 0000000000..0bf2374461 --- /dev/null +++ b/core/modules/system/tests/themes/test_theme_depending_on_constrained_modules/test_module_compatible_constraint/test_module_compatible_constraint.info.yml @@ -0,0 +1,5 @@ +name: Test Module Theme Depends on with Compatible Constraint +type: module +core: 8.x +package: Testing +version: '8.x-1.2' diff --git a/core/modules/system/tests/themes/test_theme_depending_on_constrained_modules/test_module_incompatible_constraint/test_module_incompatible_constraint.info.yml b/core/modules/system/tests/themes/test_theme_depending_on_constrained_modules/test_module_incompatible_constraint/test_module_incompatible_constraint.info.yml new file mode 100644 index 0000000000..dcc0fcd144 --- /dev/null +++ b/core/modules/system/tests/themes/test_theme_depending_on_constrained_modules/test_module_incompatible_constraint/test_module_incompatible_constraint.info.yml @@ -0,0 +1,5 @@ +name: Test Module Theme Depends on with Incompatible Constraint +type: module +core: 8.x +package: Testing +version: '8.x-1.8' diff --git a/core/modules/system/tests/themes/test_theme_depending_on_constrained_modules/test_theme_depending_on_constrained_modules.info.yml b/core/modules/system/tests/themes/test_theme_depending_on_constrained_modules/test_theme_depending_on_constrained_modules.info.yml new file mode 100644 index 0000000000..588dcc4dae --- /dev/null +++ b/core/modules/system/tests/themes/test_theme_depending_on_constrained_modules/test_theme_depending_on_constrained_modules.info.yml @@ -0,0 +1,7 @@ +name: Test Theme Depending on Version Constrained Modules +type: theme +core: 8.x +base theme: stark +dependencies: + - test_module_compatible_constraint (>=8.x-1.x) + - test_module_incompatible_constraint (>=8.x-2.x) diff --git a/core/tests/Drupal/KernelTests/Core/Theme/ThemeInstallerTest.php b/core/tests/Drupal/KernelTests/Core/Theme/ThemeInstallerTest.php index 7ff1ec3a12..e192b3510c 100644 --- a/core/tests/Drupal/KernelTests/Core/Theme/ThemeInstallerTest.php +++ b/core/tests/Drupal/KernelTests/Core/Theme/ThemeInstallerTest.php @@ -144,12 +144,12 @@ public function testInstallNameTooLong() { * * @dataProvider providerTestInstallThemeWithUnmetModuleDependencies */ - public function testInstallThemeWithUnmetModuleDependencies($theme_name, $missing_dependencies, $installed_modules) { + public function testInstallThemeWithUnmetModuleDependencies($theme_name, $installed_modules, $message) { $this->container->get('module_installer')->install($installed_modules); $themes = $this->themeHandler()->listInfo(); $this->assertEmpty(array_keys($themes)); $this->expectException(MissingDependencyException::class); - $this->expectExceptionMessage("Unable to install theme: '$theme_name' due to unmet module dependencies: '$missing_dependencies'"); + $this->expectExceptionMessage($message); $this->themeInstaller()->install([$theme_name]); } @@ -160,28 +160,38 @@ public function providerTestInstallThemeWithUnmetModuleDependencies() { return [ 'theme with uninstalled module dependencies' => [ 'test_theme_depending_on_modules', - 'test_module_required_by_theme, test_another_module_required_by_theme.', [], + "Unable to install theme: 'test_theme_depending_on_modules' due to unmet module dependencies: 'test_module_required_by_theme, test_another_module_required_by_theme'.", ], 'theme with a base theme with uninstalled module dependencies' => [ 'test_theme_with_a_base_theme_depending_on_modules', - 'test_module_required_by_theme, test_another_module_required_by_theme.', [], + "Unable to install theme: 'test_theme_with_a_base_theme_depending_on_modules' due to unmet module dependencies: 'test_module_required_by_theme, test_another_module_required_by_theme'.", ], 'theme and base theme have uninstalled module dependencies' => [ 'test_theme_mixed_module_dependencies', - 'help, test_module_required_by_theme, test_another_module_required_by_theme.', [], + "Unable to install theme: 'test_theme_mixed_module_dependencies' due to unmet module dependencies: 'help, test_module_required_by_theme, test_another_module_required_by_theme'.", ], 'theme with already installed module dependencies, base theme module dependencies are not installed' => [ 'test_theme_mixed_module_dependencies', - 'test_module_required_by_theme, test_another_module_required_by_theme.', ['help'], + "Unable to install theme: 'test_theme_mixed_module_dependencies' due to unmet module dependencies: 'test_module_required_by_theme, test_another_module_required_by_theme'.", ], 'theme with module dependencies not installed, base theme module dependencies are already installed, ' => [ 'test_theme_mixed_module_dependencies', - 'help.', ['test_module_required_by_theme', 'test_another_module_required_by_theme'], + "Unable to install theme: 'test_theme_mixed_module_dependencies' due to unmet module dependencies: 'help'.", + ], + 'theme depending on a module that does not exist' => [ + 'test_theme_depending_on_nonexisting_module', + [], + "Unable to install theme: 'test_theme_depending_on_nonexisting_module' due to unmet module dependencies: 'test_module_non_existing", + ], + 'theme depending on an installed but incompatible module' => [ + 'test_theme_depending_on_constrained_modules', + ['test_module_compatible_constraint', 'test_module_incompatible_constraint'], + "Unable to install theme: Test Module Theme Depends on with Incompatible Constraint (>=8.x-2.x) (incompatible with version 8.x-1.8)", ], ]; } diff --git a/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php b/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php index 8196735c4f..2e8c9ce1ca 100644 --- a/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php +++ b/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php @@ -534,91 +534,4 @@ public function testUnparsableCoreVersionRequirement() { $this->infoParser->parse(vfsStream::url('modules/fixtures/unparsable_core_version_requirement.info.txt')); } - /** - * Tests theme dependencies not in machine name format. - * - * @param string $theme_yml - * YML of a theme depending on modules. - * @param string $exception_message - * The expected exception message. - * - * @dataProvider providerTestUnparsableThemeModuleDependency - */ - public function testUnparsableThemeModuleDependency($theme_yml, $exception_message) { - vfsStream::setup('modules'); - vfsStream::create([ - 'fixtures' => [ - 'unparsable_module_dependency.info.yml' => $theme_yml, - ], - ]); - $this->expectException(InfoParserException::class); - $this->expectExceptionMessage($exception_message); - $this->infoParser->parse(vfsStream::url('modules/fixtures/unparsable_module_dependency.info.yml')); - } - - /** - * Data provider for testUnparsableThemeModuleDependency(). - */ - public function providerTestUnparsableThemeModuleDependency() { - $has_version_constraints = <<4.1) -UNPARSABLE; - - $has_project_prefix = << [ - $has_version_constraints, - 'Theme module dependencies must be machine names (only underscores, digits, and lowercase letters). At least one dependency declared by vfs://modules/fixtures/unparsable_module_dependency.info.yml is not in machine name format: module_with_some_version_metadata (>4.1)', - ], - 'has_project_prefix' => [ - $has_project_prefix, - 'Theme module dependencies must be machine names (only underscores, digits, and lowercase letters). At least one dependency declared by vfs://modules/fixtures/unparsable_module_dependency.info.yml is not in machine name format: drupal::module_with_a_project_prefix', - ], - 'has_a_space' => [ - $has_a_space, - 'Theme module dependencies must be machine names (only underscores, digits, and lowercase letters). At least one dependency declared by vfs://modules/fixtures/unparsable_module_dependency.info.yml is not in machine name format: apparently_i_have_a space', - ], - 'has_capital' => [ - $has_capital, - 'Theme module dependencies must be machine names (only underscores, digits, and lowercase letters). At least one dependency declared by vfs://modules/fixtures/unparsable_module_dependency.info.yml is not in machine name format: capitalLetters', - ], - ]; - } - }