Profile configuration should be able to depend on optional configuration provided by modules

So here's the fix. What I don't understand at the moment is how the profile's optional configuration is being installed... we need to work that out before proceeding.

So this is still broken but now we're in interesting circular dependencies issues. The umami block has a dependency on content but that content is only created after the block in installed. With this it happens the other way around. It works in HEAD because optional config is installed after the profile but what umami is doing is only possible of the current code in install_install_profile and installing all optional configuration after the install profile.

We could do

  \Drupal::service('config.installer')->installOptionalConfig();
  \Drupal::service('module_installer')->install([$install_state['parameters']['profile']], FALSE);
  \Drupal::service('config.installer')->installOptionalConfig();

in install_install_profile() and that would be green. But the patch has a massive advantage in that this also fixes optional config install for modules installed after install_install_profile(). Distributions like Thunder have forms after profile installation that currently won't do a default config install. And in fact this happens in head with the option that enables the update module in the site configure form - the only reason we don't have a critical bug is that the Update module does not have any optional configuration.

What's interesting is that I tried to do

  // Enable the demo content module. This can't be specified as a dependency
  // in the demo_umami.info.yml file, as it requires configuration provided by
  // the profile (fields etc.).
  \Drupal::service('module_installer')->install(['demo_umami_content'], TRUE);

  // Resave the block to re-establish content dependencies.
  \Drupal::service('plugin.manager.block')->clearCachedDefinitions(); // or even drupal_flush_all_caches() so there must be some static cache that's getting in the way. Interesting.
  Block::load('umami_banner_home')->calculateDependencies()->save();

i.e. re-save the block to fix the dependencies after installing demo_umami_content and it didn't work.

Ah I just didn't read carefully enough - resaving the block does fix the dependencies - there's more than one block that we need to do this for. Here's a hopefully green patch!

This patch will fix a hard to find bugfix in Thunder which installs all optional configuration at the end of the install to fix this bug - see https://github.com/BurdaMagazinOrg/thunder-distribution/blob/8.x-2.x/thu...

Removed extraneous code.

Unrelated random fail.

I bet it needs todo and follow-up to debug why dependencies require resave

I don't think a follow-up is needed. This is plain chicken and egg. Config installation occurs during a module install before it's hook_install is done. So when demo_umami_content is installed during the profile's install hook. We create the optional block config but the content does not yet exist. Therefore the block dependencies cannot be correctly calculated. But content dependencies are "soft" so we allow them to be removed with no other changes to the block. Then after demo_umami_content's hook_install has fired the content exists so if we re-save them the dependencies are fixed. There's no simple way to fix this unless we can think of a way to create the content before the configuration.

So #16 opens an interesting fix for this. Modules can listen to their own preinstallation so we can import content then and then we have a better flow without having to re-save the blocks.

@rensingh99 if you assign yourself an issue it'd be great if your comment says why. Going to unassign for now - if there are things you think the issue needs done it'd be great if you can add that to your comment.

Hi @rensingh99 normally people assign issues to themselves to state something like "I'm going to roll a new patch here so talk to me before doing the same thing" - it case of doing a review many people can do that at the same time so assigning yourself an issue is not necessary. The key thing is when assigning yourself an issue is to state what you're going to do otherwise assigning yourself an issue might stop someone else from doing a review or doing other needed work.

We need the test module to be Drupal 9 compat too.

Ah we need #3096609: Allow contrib test modules to not need a core or core_version_requirement key because we copy the module out of the core directory in \Drupal\Tests\config\Functional\ConfigInstallProfileUnmetDependenciesTest - or we can add core_version_requirement: '*' but let's hope that issue lands.

#3096609: Allow contrib test modules to not need a core or core_version_requirement key landed so we're back to rtbc as the re-roll has been completed.

@hchonov check out ConfigInstallProfileOverrideTest - much overriding of all the thing going on there!

@adam-delaney you could safely ignore the changes to core/profiles/demo_umami/modules/demo_umami_content/demo_umami_content.install but this patch is targeting 8.9.x and 9.0.x and applies there just fine.

@DanielVeza good idea. All the install and uninstall hooks should have this param and document the considerations.

@catch thanks for review this. Re #40 - i think it is okay to put this in 9.0.x only but I think the benefits of putting this in 8.9.x outweigh the negatives. Just this is a tiny bit disruptive for profiles that have a module that installs default content where that default content is depended on by configuration it provides (i.e content block placements). But I think that this situation is extremely rare. Unlike the situation of profile configuration depending on optional configuration provided by modules it depends on. However I'd be happy if this fix makes it into any branch :)

Added a change record: https://www.drupal.org/node/3118908 and a release note.

@Neslee Canil Pinto the patch in #31 applies to 8.9.x and has been tested against it.