Problem/Motivation

After #2208429: Extension System, Part III: ExtensionList, ModuleExtensionList and ProfileExtensionList, we now have a canonical repository for modules in a Drupal installation as well as a list of installed modules in configuration storage via 'core.extension:module' config key.

However, that issue did not remove a duplicate storage of installed modules in ModuleHandler::moduleList(). Now that we have a canonical repository for extensions, the list of installed extensions should be in those, while ModuleHandler may only hold a cached copy for performance reasons.

Proposed Resolution

  • Remove the moduleList from ModuleHandler and only retain a locally cached copy for performance reasons
  • Deprecate ModuleHandler::getModuleList() and replace with ModuleExtensionList::getInstalled()

This helps avoid potential inconsistencies in the code and enforces separation of concerns.

API Changes

ModuleHandler::getModuleList() is deprecated. New method in ExtensionList::getInstalled() that allows the list of installed extensions to be obtained as a replacement for ModuleHandler::getModuleList()

CommentFileSizeAuthor
#19 2941155-nr-bot.txt10.55 KBneeds-review-queue-bot
#18 2941155-18.patch32.39 KBpradhumanjain2311
#10 2941155-10.patch33.88 KBandypost

Issue fork drupal-2941155

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

almaudoh created an issue. See original summary.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
StatusFileSize
new 2941155-10.patch33.88 KB

initial attempt, let's see how much is broken

The related setModuleList() also needs refactoring

Status: Needs review » Needs work

The last submitted patch, 10: 2941155-10.patch, failed testing. View results

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

donquixote’s picture

donquixote commented

It looks like we are confusing the %container.modules% container parameter and the @extension.list.module service.
These have very different purposes.

The ModuleExtensionList is _not_ the "canonical repository" for installed modules.
The %container.modules% parameter is, but it is not a service, so it cannot replace what we have in ModuleHandler.

Also it contains arrays instead of Extension objects - which is actually ok, because the Extension objects from ModuleHandler are never fully initialized with all the properties they would have in ModuleExtensionList.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Child issue is fixed #2926070: Deprecate ModuleHandlerInterface::getName()

andypost opened merge request !6903

andypost’s picture

andypost
he/him
Primary language Russian
commented

Needs more work in MR

pradhumanjain2311’s picture

pradhumanjain2311 commented
Status: Needs work » Needs review
StatusFileSize
new 2941155-18.patch32.39 KB

Re-roll Patch for 11.x.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 2941155-nr-bot.txt10.55 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

nicxvan’s picture

nicxvan commented

After looking at this and reading #2941155-14: ModuleHandler should not maintain list of installed modules now that ModuleExtensionList exists I think I agree, I'm not sure we want to change this.

If we do there is also a method getInstalledExtensionNames, that is literally getting the list from module handler.

andypost’s picture

andypost
he/him
Primary language Russian
commented

with PHP 8.4 we can have setters for the property so swapping of moduleList could cause propagation

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.