Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
In the ModuleInstaller we load the .module file prior to changing the class loader to allow classes from the module to autoloaded. This causes problems when we trying to deprecate constants defined in .module files in favour of constants in class file. See #3075703-45: Move search text processing to a service
Proposed resolution
Load .module and .install files after fixing the autoloader.
Remaining tasks
User interface changes
None
API changes
None
Data model changes
None
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#7 | 3144354-7.patch | 3.96 KB | alexpott |
#7 | 3144354-7.test-only.patch | 2.3 KB | alexpott |
#3 | 3144354-4.patch | 1.65 KB | alexpott |
#3 | 2-4-interdiff.txt | 779 bytes | alexpott |
#2 | 3144354-2.patch | 1.58 KB | alexpott |
Comments
Comment #2
alexpottI'm sure I've made this change on another issue somewhere... but let's see what breaks.
I tried this without the
$this->moduleHandler->load($module);
but we still need that because the .module files are not loaded by \Drupal\Core\DrupalKernelInterface::updateModules().Comment #3
alexpottTurns out the state of the module handler before updating the kernel is important. It probably should not be but let's not change that here.
Comment #5
vijaycs85+1 to RTBC. Looks good.
updateModules()
insideupdateKernel
initialize the container which means the new code loads the module with a handler with the current module list.I believe, this is what we want instead of loading from the old/obsolete container.
Comment #6
alexpottGonna some test coverage to one of the test modules for this.
Comment #7
alexpottHere's a test. Discovered that all modules are autoloadable from the test-runner. This is an artifact of how we have to make test classes autoloadable. Other project don't do this. At some point we should stop doing that too. So in order to uncover the bug we need to do the install via the UI.
Comment #9
andypostI looks ready++ just not sure about freeze!
Comment #12
catchCommitted/pushed to 9.1.x and cherry-picked back to 8.9.x, thanks!
Comment #14
dwwI was super excited to find this issue (thanks to @larowlan via Slack) since I thought it might solve a very old Update Manager bug: #2350711: Update module fails on installation -- sadly, this patch isn't sufficient. :( On a totally clean 8.9.1 Pantheon test site, even if with this patch applied, after installing the site via install.php and checking the 'Enable update notifications' checkbox, once you're logged into the live site and try to use various Update manager pages (e.g. the settings form) you get a WSOD due to
Class "\Drupal\update\UpdateSettingsForm" does not exist.
ModuleInstaller::install() jumps through all kinds of hoops to rebulid the service container, but the autoloader itself doesn't seem to kick in and find the classes. If you clear all caches (rebuilding the full container), everything starts working fine.Anyone from here available to take a look over there and offer any insights? ;) 🤞
Thanks!
-Derek
Comment #17
quietone CreditAttribution: quietone at PreviousNext commentedClosed #2843457: Provide PSR4 namespaces before loading the module code in ModuleInstaller as a duplicate, adding credit for the work in that issue which was an identical fix, though without a test.