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
In #2401073: Assign inter-feature dependencies we introduced handling to ensure that features depended on other features that provide configuration that is a dependency. However, this is not occurring for features in the same bundle--let alone for modules outside the feature bundle..
Steps to reproduce:
- Install with the Standard install profile.
- Install Features UI and dependencies.
- Create a custom bundle, 'one'; do not include an install profile.
- Export all detected features of that bundle. Result: features including one_article are missing the dependency on one_core (which provides configuration required by configuration in one_article).
- Create a new bundle, 'two', configuring it to include the install profile.
- Export the install profile. Result: the install profile includes several block configuration items but does not have dependencies on the features from the 'one' bundle that provide configuration required by those blocks.
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | interdiff.txt | 1.71 KB | nedjo |
| #11 | 2649302-11.patch | 17.61 KB | nedjo |
| |||
| #9 | 2649302-9.patch | 15.95 KB | nedjo |
| |||
Comments
Comment #2
nedjoElevating to major as this leads to install errors when features are installed before their dependencies.
Comment #3
nedjoWorking on this.
Comment #4
nedjoThe main issue was that we were looking for packages using the package name without a bundle prefix, while, because
FeaturesManager::assignInterPackageDependencies()is invoked after::setPackageBundleNames()has been called, package names were prefixed (if part of a custom bundle). Our testFeaturesManagerTest::testAssignInterPackageDependenciesWithBundle()wasn't detecting this problem because it didn't call::setPackageBundleNames()before invokingFeaturesManager::assignInterPackageDependencies().To facilitate testing, I've converted
::setPackageBundleNames()into a public method onFeatures ManagerInterface.Comment #5
dawehnerComment #6
dawehnerWe talked about the
$bundle = NULLbit for a while. Nedjo and myself agreed that having a fallback in there is not optional and we should try to remove it.If this would not be possible (note there are still callers with = NULL around), we should at least throw an exception to better be able to understand what is going on.
On top of that we talked about possible invariants in which the call to
setPackageBundleNameswould always be true. Ensuring thiscould reduce fragility quite a bit.
Comment #7
nedjoInitial round of changes dropping the
$bundle = NULLdefault and hence the fallback.Comment #9
nedjoIgnore #7 (patch file was empty).
Initial round of changes dropping the $bundle = NULL default and hence the fallback.
Most of this is just changing the order of arguments, putting
$bundlebefore$package_namesbecause the latter has a default value.Comment #10
nedjoAdding the second change from #6, addressing fragility.
Comment #11
nedjoAdd a test for the exception added in #10, and fix a misnamed property.
Comment #13
nedjoCommitted. Moving follow-up test coverage gap to #2607782: Feature should not declare dependency on itself.