RecipeRunner::processInstall() should allow for installing multiple modules at once

Note we need to think about \Drupal\Core\Recipe\RecipeRunner::toBatchOperationsInstall() too

It's likely this change will result in quite a bit of refactoring.

The tricky bit here will be

    // Disable configuration entity install but use the config directory from
    // the module.
    \Drupal::service('config.installer')->setSyncing(TRUE);
    $default_install_path = \Drupal::service('extension.list.module')->get($module)->getPath() . '/' . InstallStorage::CONFIG_INSTALL_DIRECTORY;
    // Allow the recipe to override simple configuration from the module.
    $storage = new RecipeOverrideConfigStorage(
      $recipeConfigStorage,
      new FileStorage($default_install_path, StorageInterface::DEFAULT_COLLECTION)
    );

@phenaproxima suggested wrapping all the config from all the modules together. This might work because modules cannot have conflicting config - we have the PreExistingConfiguration exception that handles that. I'm concerned that it might not work because that when you install the first module it'd discover all the configuration from the modules that are yet to be installed. Not sure how that's going to work out.

+1 on the need for this. I can install the Event Platform project (which has loads of nested dependencies) in less than 20 seconds via the Drupal UI or drush. If I implement a recipe that installs this project (and does nothing else, not even forcing the configuration to load) then the recipe runner tries for several minutes and then inevitably fails.

I will also add that this seems to severely impact the intended reusability of the recipes within Drupal CMS. I can include one such recipe in my own recipe (tested successfully with drupal_cms_admin_ui) but my recipe fails to apply if I add another (tested by adding drupal_cms_anti_spam, which only adds another 5 dependencies, based on the command line output).

Tested this locally by installing Byte. The computer in question is a pretty snappy M1 Pro. I was running Drupal CMS 2.x-dev HEAD, under DDEV.

I timed it using my phone's stopwatch, and it clocked in at 26.08 seconds with the patch applied.

Then I removed the patch and tried again: 45.18 seconds.

I'd call that a major improvement!

Things to do at this point:

• Add test coverage for the stream wrappers / work out if it is the best implementation (also check for existing issues)
• Add test coverage of the new \Drupal\Core\Recipe\RecipeMultipleModulesConfigStorage config storage implementation

@catch thanks - I've moved my streamwrapper to a separate MR attached to that issue as the fix I did here works for that too... so we should get that in first.

Merged main... should be back to green.

So the one tricky thing with this MR is that it allows side effects during module install if a module contains simple configuration from another module. Because it reads all the module's config directories at the same time. Module's should never provide simple configuration for another module but things happen. We could potentially use this fact to limit to only reading from storages that match the config prefix.... so for example if you're reading system.site it will only read the system module's config directory.

I think this would be much safer and more predictable.

I've addressed #18. The updated RecipeMultipleModulesConfigStorage will only read config starting with a module's name form that module's config folder.

I need to improve the documentation in the class as why this is important and why it works. It's all about \Drupal\Core\Config\ConfigInstaller::installDefaultConfig() lines 127-132.

I've added documentation and improved the code and test coverage. This is now definitely in the needs review stage.

Updated the issue summary with the solution from the MR.

Some sites have very strict memory requirements, so whatever approach is taken with respect to installing multiple modules per request would ideally be able to safely recover from out-of-memory errors. This gets tricky, though, because a partial install could put the site/configuration in an invalid state which may be hard to recover from.

Perhaps there should be a flag on RecipeRunner::toBatchOperations() to control this behavior if it does get added? Personally speaking, I prefer stability over speed.

A value in settings.php could be somewhat inflexible for site builders that don't have access to change the site code. It'd be great if this was part of RecipeRunner::toBatchOperations() so that it could be set when submitting the form, or sourced from a configuration object. Additionally, CLI may have different (looser) memory constraints than other PHP SAPIs based on hosting platform configuration.

@clayfreeman this is not using any different batch sizes than the module install form, drush instlaller, site install via the UI. If you want to propose adding a CLI and UI setting then this need to work for all of those things and not just recipes. For my part, I'm not sure about presenting a UI option to set. How is a user to know they've set the right value? Why would they ever change the default and how would they know?

Read through this and it mostly makes sense. I don't quite understand how or why this RecipeMultipleModulesConfigInstaller gets us the behavior we want, though.

Addressed @phenaproxima's comments. To answer #27 because it allows the config storage supplied to the module installer by the recipe runner to read from all the modules we're installing in a safe manner and also do the recipe magic of override config using the recipe's config folder.

The current change record covers:

RecipeRunner::installModule() is deprecated
RecipeRunner::installModules() is added

Do we need to document this?

new RecipeMultipleModulesConfigStorage class

Is there anything recipe author/applier facing that should be documented? Or is this all behind the scenes?

Looks like the unit test for the new config storage is failing.

Fixed the unit test.

@thejimbirch it is all behind the scenes.

:thumbs_up:

I have a couple of nits left but I see nothing, really, to complain about here.

As before, we're still delegating to the module installer, which has always accepted an array of module names to be installed in one go. The real innovation here is the read-isolation of RecipeMultipleModulesConfigStorage, and that is quite thoroughly tested; everything else about it is straightforward.