Right now ConfigInstaller implements getSourceStorage and it is not part of the inteferface ConfigInstallerInterface.

This method is called in ModuleInstaller:132

    if ($sync_status) {
      $source_storage = $config_installer->getSourceStorage();
    }

The contrib Module config_selector decorates the ConfigInstaller but did not implement the method getSourceStorage. This crashes during a site install with config_installer

CommentFileSizeAuthor
#3 getsourcestorage_in_interface-3001159.patch1.52 KBchristian.wiedemann

Comments

Christian.wiedemann created an issue. See original summary.

christian.wiedemann’s picture

christian.wiedemann commented
Title: getSourceStorage should be part ConfigInstallerInterface » getSourceStorage() should be part of ConfigInstallerInterface
christian.wiedemann’s picture

christian.wiedemann commented
StatusFileSize
new getsourcestorage_in_interface-3001159.patch1.52 KB
christian.wiedemann’s picture

christian.wiedemann commented
Status: Active » Needs review
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Reviewed & tested by the community

Yeah I'm not sure why #1808248: Add a separate module install/uninstall step to the config import process didn't add it to the interface. I think it's really because it is only in a very very specific and rare case that it is meant to be used. Ie. when installing a module via the ConfigImporter.

I wonder if the documentation needs to be updated to inform developers that the use case is very specialised. OTOH if you have a use-case that's not likely to stop you :)

Also this not being on the interface is causing bugs - see #3001168: Config Installer crashes with "Call to undefined method Drupal\config_selector\DecoratingConfigInstaller::getSourceStorage()" - so let's add it.

I think this is okay to go into 8.7.x under the 1-to-1 interface / class rule.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed b0b4ef0 and pushed to 8.7.x. Thanks!

catch’s picture

catch
he/him
Primary language English
commented
Status: Fixed » Needs work

Arghh just committed this, but it should have had a change record...

  • catch committed b0b4ef0 on 8.7.x 
    Issue #3001159 by Christian.wiedemann: getSourceStorage() should be part...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Fixed

Created https://www.drupal.org/node/3014010

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.