Problem/Motivation
The help_topics module has a plugin cache clearer - \Drupal\help_topics\HelpSectionManager::clearCachedDefinitions() that can trigger under certain conditions and throw exceptions.
This is caused because we call the plugin cache clearer after beginning module uninstall but before rebuild the container and the kernel. This is the wrong way around we should rebuild the container first so plugin cache cleaners have a chance to reflect the changes to the available services.
Steps to reproduce
This is happening in https://www.drupal.org/pift-ci-job/2090326 for a MR which adds dependencies to olivero_test - see #3174107: Add additional testing coverage for Olivero. Module dependencies affect the order in which things occur. This change causes help_topics to be uninstalled after search in the ConfigImportAllTest and this results in it breaking.
Proposed resolution
Call plugin cache cleaners in uninstall after the container rebuild.
Remaining tasks
User interface changes
NOne
API changes
None
Data model changes
None
Release notes snippet
N/a
Comment | File | Size | Author |
---|---|---|---|
#7 | 3218660-6.patch | 6.1 KB | alexpott |
#7 | 3218660-6.new-test-only.patch | 3.72 KB | alexpott |
Comments
Comment #2
alexpottComment #3
alexpottComment #4
alexpottHmmm actually there is a way better fix that makes more sense and will fix more modules...
This problem is caused by triggering a plugin cache clear before rebuilding the container after removing the module from core.extension. That's the wrong way around. It's amazing we've got this far with it that way around.
Comment #5
alexpottComment #7
alexpottHere's a test that doesn't involve an experimental module so we continue to have test coverage if that one doesn't make it. The test only patch is the interdiff.
Comment #8
andypostIs it ok to mix help topics test into base system fix?
Otherwise looks straightforward
Comment #9
alexpott@andypost I think it is. It proves that uninstalling search is broken when help_topics is still installed and it proves that the change in ModuleInstaller fixes it.
Comment #11
andypostThen it looks ready to go
Comment #13
larowlanI think that normally we'd want a test for a base-system bug in a base-system module on the outside chance the experimental module was removed from core.
However in the case of help-topics, it is a) unanimously supported b) close to stable c) going to end up as part of help module if it goes anywhere - so I think in that case it is fine to add the test in this namespace.
Committed 257efe5 and pushed to 9.3.x. Thanks
Because this is a major bug and the risk of disruption is low, I feel its fine to backport to 9.2.x, so did so