Never generate migration dependencies on derivatives of itself is a self_referencing migration_lookup

Took a while but this is what I see.

First I noticed that after the migrations are created the migration dependencies on the d7_taxonomy_term is changed from

migration_dependencies:
  required:
    - d7_taxonomy_vocabulary
  optional:
    - d7_field_instance

to

migration_dependencies:
  required:
    - d7_taxonomy_vocabulary
  optional:
    - d7_field_instance
    - d7_taxonomy_term

A look at the process pipeline and there is a use of migration_lookup with a migration of d7_taxonomy_term. So, somewhere there is code that thoughtfully looks for dependent migrations in the pipeline and adds it to the optional dependencies. Where? In \Drupal\migrate\Plugin\Migration::findMigrationDependencies.

And looks like we need to make a new issue to remove the use of 'iterator' and 'migration' from findMigrationDependencies because they are deprecated.

And a patch.

Funny, I meant to add the isset().

The change to the test was using the removed process plugin, 'iterator'. This patch removes that usage. Since the test has change there is a new fail patch.

I like it. This makes sense. There's no point in adding a dependency on yourself, but this will add the dependencies in still if you are looking up against a different migration, which is good to help order them.

I reviewed this with a view to commit it, but I honestly are not qualified to do so, its outside my area of comfort/expertise

I get what is happening in principle, but I'll leave this to someone more familiar with this area of migrate

+++ b/core/modules/migrate/src/Plugin/Migration.php
@@ -624,6 +624,17 @@ protected function findMigrationDependencies($process) {
+        // If the migration uses a deriver and has a migration_lookup with
+        // itself as the source migration then skip adding dependencies
+        // otherwise the migration will depend on all the variations of itself.
+        // See d7_taxonomy_term for an example.
+        $migration = $this->getBaseId();
+        if (isset($this->deriver)) {
+          if (in_array($plugin_configuration['plugin'], ['migration_lookup'], TRUE)
+            && ($plugin_configuration['migration'] == $migration)) {
+            continue;
+          }
+        }

This can be written slightly more concisely as

        // If the migration uses a deriver and has a migration_lookup with
        // itself as the source migration then skip adding dependencies
        // otherwise the migration will depend on all the variations of itself.
        // See d7_taxonomy_term for an example.
        if (isset($this->deriver)
            && $plugin_configuration['plugin'] === 'migration_lookup'
            && $plugin_configuration['migration'] == $this->getBaseId()) {
          continue;
        }

Addressing #13

@raman.b:

Thanks for updating the patch!

According to Line length and wrapping in the coding standards,

• Control structure conditions may exceed 80 characters, if they are simple to read and understand: ...
• Conditions should not be wrapped into multiple lines.

But the patch in #15 does exactly what #13 requested, and I am not going to argue with @alexpott.

I will ask for a couple of changes to the comment:

1. Add a comma before "then skip adding dependencies".
2. Add a period at the end of that phrase, and start a new sentence with "Otherwise".

Want to emphasize the migration_lookup.

Committed and pushed 0e13819245 to 9.2.x and bf40e13e24 to 9.1.x. Thanks!