Problem/Motivation

If you uninstall migrate_drupal and re-enable it, you get

exception 'Drupal\Core\Config\PreExistingConfigException' with message 'Configuration objects (migrate.migration.d6_action_settings, migrate.migration.d6_aggregator_feed,
migrate.migration.d6_aggregator_item, migrate.migration.d6_aggregator_settings, migrate.migration.d6_block, ...)

This is because the default configuration is not automatically deleted on uninstall.

Proposed resolution

Add an explicit dependency on the migrate_drupal module to each migration's .yml file.

Remaining tasks

Do it.

User interface changes

N/A

API changes

N/A

CommentFileSizeAuthor
#6 interdiff-1-6.txt446 bytesanavarre
#6 2459619-6.patch47.68 KBanavarre
#1 2459619-1.patch47.7 KBanavarre
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

anavarre’s picture

Status: Active » Needs review
FileSize
47.7 KB

I didn't experience that behavior in my testing. However, I think this patch is a start to address what we need here.

mikeryan’s picture

Almost exactly the same patch I made last night - but it doesn't work, uninstalling migrate_drupal leaves the configuration. Looking in the config table, the data contains s:12:"dependencies";a:0:{}. By comparison, enabling the aggregator module, which contains core.entity_view_display.aggregator_feed.aggregator_feed.default.yml, results in a config containing s:12:"dependencies";a:1:{s:6:"module";a:1:{i:0;s:10:"aggregator";}} and it does get uninstalled properly.

So, probably something funky in the Migration config entity is preventing dependencies from being properly saved? I've done some poking around but I'm at a loss...

anavarre’s picture

Something funky indeed:

$ drush @d8.local cget core.entity_view_display.aggregator_feed.aggregator_feed.default
uuid: 2681ddb2-a400-4817-9300-face9679d947
langcode: en
status: true
dependencies:
  module:
    - aggregator
(snipped)

$ drush @d8.local cget migrate.migration.d6_aggregator_feed
uuid: 545148d0-aae1-43b3-8600-bb8759780957
langcode: en
status: true
dependencies: {  }
(snipped)

At first glance it looks like a different, more involved issue than this one. Still seems we should add those dependencies anyway, right?

mikeryan’s picture

mikeryan’s picture

Status: Needs review » Needs work
+++ b/core/modules/migrate_drupal/config/install/migrate.migration.d6_user_picture_file.yml
@@ -27,3 +27,6 @@ migration_dependencies:
+dependencies:

Please merge with the existing (file) dependency.

anavarre’s picture

Status: Needs work » Needs review
FileSize
47.68 KB
446 bytes

Thanks, I had missed it.

mikeryan’s picture

Status: Needs review » Postponed
Related issues: +#2460529: Migrations need to use the configuration entity dependency system

Per https://www.drupal.org/node/2404447, it appears the dependencies need to be "enforced" (i.e., there's an additional "enforced" key between "dependencies" and "module".

However, there's a broader issue at #2460529: Migrations need to use the configuration entity dependency system which might supersede this one. Postponing for now...

alexpott’s picture

mikeryan’s picture

Status: Postponed » Closed (duplicate)