Optional configuration from dependence modules is installed before installing module when installing profile

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

Think to better show this issue it will need a test case to show the problem.

We are running into this in multiple places:

• The book module provides the optional node book type. It's currently impossible to make modules that build on top of that with config/install (e.g. a field extending the book type) even if that module declares book and node as dependencies to ensure the optional config's dependencies are satisfied.
• The search_api module provides optional fields for the English language, but those can't be used by config objects in config/install even if we can ensure that the English language and language module are both properly installed

Attached is a patch that demonstrates this in the most simple form I could come up with:

• A module that serves as target for optional config
• A module providing some optional config if the target module is enabled
• A third module that wants to ensure the optional module is enabled and build on top of the then provided optional config

I'll need someone smarter than me to let me know if the test is placed in the right Drupal core location or whether it should be moved elsewhere.

I'm moving this to "Needs review" for the test but to run, but I've already seen that the fix proposed in #2 is unfortunately not enough to make the test green, so this will need more work. The test-only.patch is also the interdiff :)

I made a mistake in my earlier test in the naming of the config files which caused the test to be inaccurate. That's now fixed in the new test. I also had to make sure that within the test the ConfigInstaller was tricked into thinking we're in install mode to accurately show how the proposed patch fixes the issue.

The test now properly demonstrates the problem (which only exists during site installation and not for modules installed later) and shows that the proposed fix solves the issue (and all other core tests passing should show that it has no adverse effects).

I'm not sure whether the comment in the ConfigInstaller needs to be updated.

Testing this patch against Open Social shows an interesting possible break for some modules:

In case Module A which is installed after module B provides a config entity in config/install that module B provides in config/optional then without this patch the installation will fail with Configuration object (object) provided by Module A already exist in active configuration.

This behavior is the same as it would be if you were to install modules after a site installation. However because optional configuration was previously only installed at the end of site-install, a rogue module that was part of the site-install process could beat the optional config to installation which makes Drupal think it was already installed.

This change might cause errors for people relying on this behavior during site-install but I don't think that should hold up this change since this is already considered incorrect from a "normal" post-site-install module installation.

There was a discussion about this on slack, which alex asked to summarize on this issue. I don't understand it well enough to summarize, but here's a link https://drupal.slack.com/archives/C1BMUQ9U6/p1703168240580519

I think this issue can stay in needs review, because we first have to decide on a direction, then we can look into the patch.

The request to summarize a few hours of conversation came just before the start of a Christmas holiday so Santa comes a bit later :) Below is my attempt to summarize the discussion.

There's a few different things being discussed in the thread:

Thing 1 - Drupal installation determinism

During a drupal installation optional configuration is installed at the end of the installation process. Once the install profile is installed optional configuration should be installed as usual. @see install_install_profile()

Quoted from ConfigInstaller::installDefaultConfig around line 153.

This was introduced in Drupal quite some time ago in #2090115: Don't install a module when its default configuration has unmet dependencies → and the primary reason was that the install process was not deterministic for modules that had equal weight in the dependency tree. The comment that was added in install_install_profile provides more details:

  // Install all available optional config. During installation the module order
  // is determined by dependencies. If there are no dependencies between modules
  // then the order in which they are installed is dependent on random factors
  // like PHP version. Optional configuration therefore might or might not be
  // created depending on this order. Ensuring that we have installed all of the
  // optional configuration whose dependencies can be met at this point removes
  // any disparities that this creates.

Alex summarised this as

The real reason is the reason stated. To have optional configuration created under the same conditions - ie. codebase. If you let it be installed through the profile install that on different systems the optional config might be installed with different codebases active. This has lead to very obscure and hard to track bugs.

But he also mentions that this problem might have gone away in the past 10 years:

Note this might have changed as PHP has made quite a few sorts stable. See https://wiki.php.net/rfc/stable_sorting. But we’d need to work that out.

(introduced in version 8 which is lower than Drupal 10's minimum).

Thing 2 - Whether config/install should be able to depend on config/optional in the first place

Alex and I disagree here. He mentioned:

I do think that there is a logic problem with required configuration depending on optional config. That’s the bit that is not pleasant. Like what happens if after installing the module that provides the optional config the user deletes the optional configuration from the site. What’s the other module going to do?

I agreed initially (which is why I didn't work on this issue 6 months ago when I first encountered it), but I no longer agree with the stance. My definition of optional configuration is: configuration that provides additional functionality if some preconditions are met.

The modules that are running into errors say: "I'm making sure the preconditions are met, but I require the optional functionality provided by the module to function properly". If we don't allow required config to depend on optional config in any scenario then optional configuration is poisonous.

Additionally if a user deletes configuration from their site then they have a chance of breaking their site, that's true regardless of where the configuration is installed from.

Thing 3 - Profiles can replace configuration

Alex mentioned:

The other thorny thing here is that profiles can replace config from modules that they are installing. It’s a special skill. And when do that they can add dependencies.

FWIW - profile configuration is swapped in during dependency checking - see \Drupal\Core\Config\ConfigInstaller::findDefaultConfigWithUnmetDependencies()

I think this is outside of the scope of this issue, because this is currently possible regardless of when config is being installed.

Thing 4 - Performance

Not installing optional config during every module that's enabled currently speeds up Drupal installation because the installer doesn't look for optional config after every module but only does this once at the end.

I believe that performance isn't great if it causes hard to solve problems. The only alternative that I see to allowing ConfigInstaller to install optional config during site installation is to go to every module that ships some config/optional config that we might want to build upon and ask them if they can move it to config/install for an optional submodule instead, but that seems a waste of Drupal's optional config system.

An example of currently optional config is the Book node-type which is really problematic if we say that required config can't depend on optional config, because it means Drupal ships a core feature that I can't build on (and there's no way for me to opt-out of book if both the book (for the book-system) and node module are installed).

Moving this to "Needs work". I still believe that Thing 1 is the only real thing that matters and that the PHP addition of stable sorting has solved this issue for us (allowing us to install optional config during Site Installation as soon as its dependencies are met, in the same way as would happen during post-site install module installation).

The proposed patch implements that solution properly, but as pointed out by Alex in the Slack discussion the comments in the ConfigInstaller and in install_install_profile are no longer accurate after this fix, so they should be updated/removed as part of this issue.

I'm not sure if my comment from #16: node's preview incorrect ("Submitted by anonymous", ...) → is considered a breaking change, I would personally lean to "no" for two reasons:

1. It only affects new site installs and does not affect upgrading sites, so in that sense it's more "new feature" than breaking behaviour
2. The issue encountered is also encountered in case a module is installed outside of site-installation, which means that the bug only exists for modules that were only ever installed during site-installation and were never installed later which is an uncommon scenario as most modules will have some usage where they're installed post-installation

Work to-do:

• Update/remove comment in ConfigInstaller
• Update/remove comment in install_install_profile