Config import fails when auto-creating new features bundles [#2674792]

When you do a config-import, config entities (like features_bundles) are created only after modules are installed. So if a config-import installs both features and a feature module, at the time it's installing the feature module no features_bundles yet exist, including the default one. When features attempts to auto-create the bundle for the feature module, it wants to base it on the default bundle, but no such bundle exists and it dies.

I'm not really sure what the best fix is. The easiest thing is to just disable features auto-creation if a config-import is in progress (using ConfigInstallerInterface::isSyncing()). Alternatively, we could keep auto-creation but base the new bundle on something else if no default bundle exists yet.

To reproduce:

* Install Drupal: drush site-install -y
* Enable features_ui: drush en -y features_ui
* Create a new features bundle "mybundle" by visiting admin/config/development/features/bundle . Save the bundle.
* Export the "Article" content type as a feature, with the new bundle:

drupal$ drush features-export --bundle=mybundle article
Package Article written to modules/custom/mybundle_article.

* Enable the new module: drush en -y mybundle_article
* Export your configuration: drush config-export -y sync
* Remove features and the new feature: drush pmu -y mybundle_article; drush pmu -y features_ui; drush pmu -y features
* Attempt to revert to the earlier config, but it dies:

drupal$ drush config-import -y sync
 Collection  Config                    Operation                
             features.bundle.default   create 
             features.bundle.mybundle  create 
             features.settings         create 
             core.extension            update
Import the listed configuration changes? (y/n): y
PHP Fatal error:  Call to a member function createDuplicate() on a non-object in /Users/vasi/Sites/devdesktop/drupal8/modules/contrib/features/src/FeaturesAssigner.php on line 276
PHP Stack trace:
PHP   1. {main}() /Applications/DevDesktop/drush/vendor/drush/drush/drush.php:0
PHP   2. drush_main() /Applications/DevDesktop/drush/vendor/drush/drush/drush.php:12
PHP   3. Drush\Boot\BaseBoot->bootstrap_and_dispatch() /Applications/DevDesktop/drush/vendor/drush/drush/includes/preflight.inc:64
PHP   4. drush_dispatch() /Applications/DevDesktop/drush/vendor/drush/drush/lib/Drush/Boot/BaseBoot.php:65
PHP   5. call_user_func_array:{/Applications/DevDesktop/drush/vendor/drush/drush/includes/command.inc:183}() /Applications/DevDesktop/drush/vendor/drush/drush/includes/command.inc:183
PHP   6. drush_command() /Applications/DevDesktop/drush/vendor/drush/drush/includes/command.inc:183
PHP   7. _drush_invoke_hooks() /Applications/DevDesktop/drush/vendor/drush/drush/includes/command.inc:215
PHP   8. call_user_func_array:{/Applications/DevDesktop/drush/vendor/drush/drush/includes/command.inc:364}() /Applications/DevDesktop/drush/vendor/drush/drush/includes/command.inc:364
PHP   9. drush_config_import() /Applications/DevDesktop/drush/vendor/drush/drush/includes/command.inc:364
PHP  10. drush_op() /Applications/DevDesktop/drush/vendor/drush/drush/commands/core/config.drush.inc:640
PHP  11. drush_call_user_func_array() /Applications/DevDesktop/drush/vendor/drush/drush/includes/drush.inc:706
PHP  12. _drush_config_import() /Applications/DevDesktop/drush/vendor/drush/drush/includes/drush.inc:720
PHP  13. Drupal\Core\Config\ConfigImporter->import() /Applications/DevDesktop/drush/vendor/drush/drush/commands/core/config.drush.inc:662
PHP  14. Drupal\Core\Config\ConfigImporter->doSyncStep() /Users/vasi/Sites/devdesktop/drupal8/core/lib/Drupal/Core/Config/ConfigImporter.php:469
PHP  15. Drupal\Core\Config\ConfigImporter->processExtensions() /Users/vasi/Sites/devdesktop/drupal8/core/lib/Drupal/Core/Config/ConfigImporter.php:492
PHP  16. Drupal\Core\Config\ConfigImporter->processExtension() /Users/vasi/Sites/devdesktop/drupal8/core/lib/Drupal/Core/Config/ConfigImporter.php:553
PHP  17. Drupal\Core\ProxyClass\Extension\ModuleInstaller->install() /Users/vasi/Sites/devdesktop/drupal8/core/lib/Drupal/Core/Config/ConfigImporter.php:789
PHP  18. Drupal\Core\Extension\ModuleInstaller->install() /Users/vasi/Sites/devdesktop/drupal8/core/lib/Drupal/Core/ProxyClass/Extension/ModuleInstaller.php:87
PHP  19. Drupal\Core\Extension\ModuleHandler->invokeAll() /Users/vasi/Sites/devdesktop/drupal8/core/lib/Drupal/Core/Extension/ModuleInstaller.php:298
PHP  20. call_user_func_array:{/Users/vasi/Sites/devdesktop/drupal8/core/lib/Drupal/Core/Extension/ModuleHandler.php:393}() /Users/vasi/Sites/devdesktop/drupal8/core/lib/Drupal/Core/Extension/ModuleHandler.php:393
PHP  21. features_modules_installed() /Users/vasi/Sites/devdesktop/drupal8/core/lib/Drupal/Core/Extension/ModuleHandler.php:393
PHP  22. Drupal::service() /Users/vasi/Sites/devdesktop/drupal8/modules/contrib/features/features.module:44
PHP  23. Drupal\Component\DependencyInjection\Container->get() /Users/vasi/Sites/devdesktop/drupal8/core/lib/Drupal.php:158
PHP  24. Drupal\Component\DependencyInjection\Container->createService() /Users/vasi/Sites/devdesktop/drupal8/core/lib/Drupal/Component/DependencyInjection/Container.php:181
PHP  25. Drupal\features\FeaturesAssigner->__construct() /Users/vasi/Sites/devdesktop/drupal8/core/lib/Drupal/Component/DependencyInjection/Container.php:281
PHP  26. Drupal\features\FeaturesAssigner->createBundlesFromPackages() /Users/vasi/Sites/devdesktop/drupal8/modules/contrib/features/src/FeaturesAssigner.php:94
PHP  27. Drupal\features\FeaturesAssigner->createBundleFromDefault() /Users/vasi/Sites/devdesktop/drupal8/modules/contrib/features/src/FeaturesAssigner.php:337

Comment	File	Size	Author
#10	2674792-10.patch	5.34 KB	vasi
#7	interdiff.txt	2.34 KB	vasi
#7	2674792-7.patch	8.33 KB	vasi
#3	2674792-auto-create-import.patch	4.45 KB	vasi

Comments

Comment #1

24 February 2016 at 13:32

vasi created an issue. See original summary.

Comment #2

nedjo

he/him/his

English

commented 25 February 2016 at 16:35

Thanks for your detailed explanation of the problem.

We could read directly from the file storage (config/features.bundle.default.yml) rather than duplicating the existing bundle, but I believe the current approach is by design in that a new bundle should inherit any customizations made to the default bundle.

So, possibly, test for the default bundle and, if not found, load from the file storage?

Comment #3

vasi commented 26 February 2016 at 06:24

Status	File	Size
new	2674792-auto-create-import.patch	4.45 KB

Ok, here's an attempt at that.

Comment #4

vasi commented 28 February 2016 at 19:49

Status:

Active

» Needs review

Comment #5

nedjo

he/him/his

English

commented 29 February 2016 at 00:30

Looking good! A couple of suggestions:

+++ b/src/FeaturesAssigner.php
@@ -273,7 +274,17 @@ class FeaturesAssigner implements FeaturesAssignerInterface {
+      $bundle_storage = \Drupal::entityTypeManager()->getStorage('features_bundle');

We should inject the entity type manager service so we can use it here.

+++ b/tests/modules/test_feature/config/install/system.cron.yml
@@ -0,0 +1,5 @@
+_core:
+  default_config_hash: 05U0n1_8zHYzxEFSWjyHCWuJyhdez2a6Z_aTIXin04E

We should leave these lines out. See #2668526: Do not export _core default_config_hash.

Comment #6

nedjo

he/him/his

English

commented 11 March 2016 at 15:14

Status:

Needs review

» Needs work

Comment #7

vasi commented 22 March 2016 at 23:20

Status:

Needs work

» Needs review

Status	File	Size
new	2674792-7.patch	8.33 KB
new	interdiff.txt	2.34 KB

Oh, it looks like we already have entityManager, we just don't declare it.

Fixed the two issues identified in #5.

Tests are expected to fail, due to a missing file in this commit: https://www.drupal.org/node/2666836#comment-10997991 .

Comment #8

23 March 2016 at 12:57

Status:

Needs review

» Needs work

The last submitted patch, 7: 2674792-7.patch, failed testing.

Comment #9

mpotter commented 23 March 2016 at 12:59

Hmm, looks like there is some extra stuff in the patch in #7, like the ForwardDependency patch and some other files.

Also, just a quick dumb question...can we just add a config dependency of features.bundle.default to the features.bundle.mybundle config? Would that let Drupal install the default bundle before any other bundles?

Comment #10

vasi commented 24 March 2016 at 01:35

Status:

Needs work

» Needs review

Status	File	Size
new	2674792-10.patch	5.34 KB

Sorry about the bad patch, this one should be fixed.

I don't think any sort of dependencies between bundles would help. At the time that we are auto-creating a bundle, we don't actually have a full config for the bundle—hence the "auto-creation". Also, we don't have any bundles in config at that point.

Comment #11

mpotter commented 24 March 2016 at 14:50

Status:

Needs review

» Reviewed & tested by the community

This looks good to me.

Comment #12

24 March 2016 at 17:15

mpotter committed 8ddce1e on 8.x-3.x authored by vasi

Issue #2674792 by vasi: Config import fails when auto-creating new...

mpotter committed bf93ecd on 8.x-3.x authored by vasi

Issue #2674792 by vasi: Config import fails when auto-creating new...

Comment #13

mpotter commented 24 March 2016 at 17:16

Status:

Reviewed & tested by the community

» Fixed

Committed to bf93ecd and 8ddce1e

Comment #14

7 April 2016 at 17:24

Status:

Fixed

» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Config import fails when auto-creating new features bundles

Comments

Comment #1

Comment #2

Comment #3

Comment #4

Comment #5

Comment #6

Comment #7

Comment #8

Comment #9

Comment #10

Comment #11

Comment #12

Comment #13

Comment #14

News items

Our community

Documentation

Drupal code base

Governance of community