Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
We have two modules named "B" and "D" and two identical profiles named "A" and "C". Module "D" provides a filter.format.custom_format config. Module "B" provides a custom filter entity only.
The config filter.format_custom format is being overwritten in both profiles and contains the custom filter from module "B" and therefore has two module dependencies - to both "B" and "D".
The installation of profile "C" is successful.
The installation of profile "A" is unsuccessful.
During the installation of profile "A" the config installer throws the UnmetDependencyException because when trying to install module "D" the overwritten config from the profile is used and it contains a dependency to module "B", which isn't installed yet.
What I've observed is that because of the naming ModuleHandler::buildModuleDependencies() will sort the modules differently, which is not an actual problem because the dependencies are still being considered. However the dependencies of the overwritten configs are not being considered and what happens is that depending on the profile name in the case of profile "C" module "B" is installed before module "D" and in the case of profile "A" module "D" is installed before module "B". However in the second case during the installation of module "D" the config from the profile will be used and an unmet dependency will be detected, because the config from the profile has a dependency on both modules "B" and "D" and "B" isn't installed yet.
Considering this a major issue, because I couldn't think of a workaround and profile installation isn't possible.
Proposed resolution
During profile installation do not install each module with its default config taken from the profile config storage but from the module own config storage. When the profile itself is being installed then reinstall all the config contained in its own config storage.
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | interdiff-10-12.txt | 4.7 KB | hchonov |
| #12 | 2872977-12-test-only.patch | 3.03 KB | hchonov |
| #12 | 2872977-12.patch | 5.45 KB | hchonov |
Comments
Comment #2
hchonovI've talked with @tstoeckler about this issue and he offered two approaches:
I personally think that the first solution is better because in this case the config will be accessible in hooks during the installation.
Comment #3
hchonovWith the following changes made the problem from the issue summary has been solved.
Comment #4
hchonovProviding a test for the problem from the issue summary.
Comment #7
hchonovI've removed the extra test as there is already one failing test with the additions to the profile "testing_config_overrides". I also had to change the type of the config from config_object to config_entity, because otherwise the "dependencies" key is not defined in the schema.
Comment #9
hchonovActually we don't need the changes to the protected methods because we could just provide the profile storages argument with an empty array in case of an profile/site installation.
Comment #10
hchonovI've talked with @alexpott about this in IRC and he suggested that we could simply use ConfigInstaller::drupalInstallationAttempted() to determine if Drupal installation is currently running instead of carrying that information from the module installer to the config installer. This has simplified the patch a lot and reduced it only to the config installer, therefore the credit for this should go all to Alex!
Comment #12
hchonovI don't fully understand why the test
Drupal\config\Tests\ConfigInstallProfileUnmetDependenciesTestwasn't failing with the previous approach, but only with the current. But this is a good thing, because this test is testing exactly the behavior from the issue summary and it looks like this was the expected behavior. I guess at the time of implementing the test it made sense but now having a realistic use case like the one from the issue summary with the filter format config certainly demonstrates that we have to support building inter-module dependencies in overridden configs in the profile.I've removed all the tests I've added and now the only test is ConfigInstallProfileUnmetDependenciesTest, which I had to adjust to the new excepted behavior.
Comment #14
hchonovComment #18
tstoecklerThis one needs a reroll for #2978952: Deprecate Drupal\simpletest\InstallerTestBase, convert children to BTB.
Also did a review:
These are the only two usages of
getProfileStorages()so I think it makes sense to move the check in there.I like the fix to the test coverage here, but it even though the previous test was accidentally testing broken behavior, we should still have a test that proves that a missing dependency is properly caught during installation.
Other than that looks good and I agree with the approach taken here. It's a bit unfortunate to have to install certain things twice, but I don't see any way we can keep the system in a valid state each step of the way during installation.
Comment #19
alexpottThere's another issue that tackles this problem differently and I think in a better way - see #2922417: Profile provided configuration entities can have unmeetable dependencies - that issue only results in the expected single write to configuration.
Comment #20
alexpottComment #21
alexpottIn fact I'm convinced these are duplicate issues with pretty much the same approach apart from how the other one deals with simple configuration which I think is better. I would also argue that the other one has slightly better test coverage so going to plump for that issue even though this issue was opened a little bit earlier. Will credit @hchonov and @tstoeckler on that issue for their work here.