Problem/Motivation
Drupal 11.2 has been released. It introduced some interesting changes to speed up site and module installations that impact Config Provider:
- Change \Drupal\Core\Config\ConfigInstaller to support installing multiple modules with a single container rebuild
- New container_rebuild_required key in .info.yml files
The changes to core's ConfigInstallerInterface cause fatal errors on sites upgraded to 11.2.x with Config Provider installed:
PHP Fatal error: Declaration of Drupal\config_provider\ProxyClass\ConfigProviderConfigInstaller::installDefaultConfig($type, $name) must be compatible with Drupal\Core\Config\ConfigInstallerInterface::installDefaultConfig($type, $name, Drupal\Core\Config\DefaultConfigMode $mode = Drupal\Core\Config\DefaultConfigMode::All) in /modules/contrib/config_provider/src/ProxyClass/ConfigProviderConfigInstaller.php on line 73
Steps to reproduce
Try upgrading a site with Config Provider installed to Drupal 11.2.x.
Proposed resolution
Looks like we need to add a compatibility layer similar to what was added to the Config Selector module in #3492937: Add compatibility layer for Drupal 11.2 to add Drupal >= 11.2 compatibility without breaking Drupal < 11.2 compatibility.
Remaining tasks
Figure out whether to close this as a duplicate of #2800839: Address conflict with Features over config.installer class or address separately.
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | config_provider-config-installer-interface-changes-3532693-12.patch | 2.68 KB | tadean |
Issue fork config_provider-3532693
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:
Comments
Comment #2
joegraduateComment #4
joegraduateUpdated the module to decorate the core config.installer service rather than alter it in MR !20. This made it easier to employ the dynamic compatibility traits strategy employed by the config_selector module and made it possible to remove the proxy class. The proxy class was needed before because this module was extending the core ConfigInstaller class (which core declares as a lazy service in core.services.yml). It is no longer needed with the MR.
Comment #6
joegraduateTested MR !20 with an existing project that contains a custom ConfigProvider plugin and found that the decorator approach used in that MR doesn't seem to work (the custom ConfigProvider plugin I was testing did not work as expected).
Created alternate MR !21 that adds a separate backwards-compatible
ConfigProviderConfigInstallerclass, updates the existing class to be 11.2 compatible and updates the existing service provider to use the appropriateConfigProviderConfigInstallerclass depending on the installed core version. The custom ConfigProvider in our existing project did still work as expected when tested with this new MR.Comment #9
joegraduateDiscussed the 2 different MR approaches extensively with fellow maintainer @tadean.
I've updated MR !20 with the approach that we both feel the most comfortable with which registers our
ConfigProviderConfigInstallerclass as a decorator for the coreconfig.installerservice via the services.yml file instead of altering (replacing) the core service via a ServiceProvider class. This approach vastly simplifies the MR because it:ConfigProviderConfigInstallerservice classI've also hidden the other MR branch.
While discussing this issue and the potential fixes with @tadean, we realized that the changes proposed in @trackleft2's MRs to address #2800839: Address conflict with Features over config.installer class are very similar. We might want to consider closing this issue as a duplicate of that one and updating its IS with the 11.2 incompatibility details.
Comment #10
joegraduate@tadean also noted that the removal of the ProxyClass and changing the lazy behavior of the config.installer service probably constitutes a performance regression (probably especially for sites using Drupal core < 11.2?).
It does seem like it would be desirable to keep the lazy behavior / ProxyClass if possible but the changes to the ConfigInstallerInterface introduced in 11.2 (the reason for this issue) make that a much more complicated task (because 2 different versions of the ProxyClass are needed to provide compatibility with Drupal >=11.2 and Drupal <11.2).
Comment #11
joegraduateComment #12
tadean@joegraduate here's a patch versus MR !20 with a potential approach that keeps the lazy loading, but might be able to avoid the need for a backward compatibility Proxy class by inheriting from the
\Drupal\Core\ProxyClass\Config\ConfigInstaller.Some considerations: this would be less code to maintain than the two-proxy approach (in MR !21), but perhaps there are reasons to avoid inheriting from the
\Drupal\Core\ProxyClass\Config\ConfigInstaller? (Though, it is not marked internal). The approach in this patch makes no claims about the API of the Proxy by saying it's whatever the Core Proxy uses, essentially.Since all interface calls are deferred through the public functions in the lazy loading service via
lazyLoadItself(), perhaps\Drupal\Core\ProxyClass\Config\ConfigInstalleralready does everything it needs to, we just need it be available under a different class name.Comment #14
anicho01 commentedHey -
Do you all know when this will be released?
Comment #15
tadean@anicho01 sorry for the delay on getting this incorporated into a release - we're hoping to very soon.
Comment #16
tadeanDiscussed the two potential MRs with @joegraduate and we agreed that the approach in MR !20 is less likely to create problems for other modules that need to extend the core service. The performance impact of not pursing the proxy class approach appears to be relatively small compared to the overhead of introducing two versions of the proxy class to support the two different core APIs.
@joegraduate proposed the idea of a followup improvement to pass along the inner argument to the decorated service for reuse, with this MR !20 in this issue being merged for 11.2 compatibility.
Comment #19
joegraduateThanks @tadean!
Comment #21
mprell commentedWe're experiencing PreExistingConfigException during profile installation after upgrading to alpha3.
This appears related to the decorator change merged here. The followup mentioned in Comment #16 about "passing along the inner argument to the decorated service for reuse" seems critical.
I tested the patch from #2800839 which injects
@config_provider.config.installer.innerand stores it in$this->configInstaller, but thegetConfigToCreate()method still callsparent::instead of delegating to the inner service, and I think this bypasses any other decorators in the chain (like Features).Pinning to alpha2 resolves the issue but breaks Drupal 11.2 compatibility. Happy to help test any proposed fix.