all translation/localization migrations should depend on the 'language' migration

Arg, probably worth double checking all the translation migrations.

@Wim Leers, I see that I got lax on the dependencies on the translation migrations. Thanks for picking up the slack.

I count 43 migrations with 'translation' in the title and only 11 have 'language' as a dependency, leaving 32 to change. I didn't expect it to be that many.

Finally got back to review this. And it needs a reroll! This reroll is suitable for a novice.

Rerolled the patch as suggested. Please review.

@ayushmishra206, can you add the diff?

Adding the diff.

oops, #17 saved with wrong extension.

@adityasingh, Thanks you for the diff. It does show the mistakes in the reroll, which I have identified below. They should be easy to fix. Obviously, I don't know how you are doing reroll but using the method in Rerolling patches should help prevent these errors. Cheers,

Looking at the diff

1. +++ 3189463-14.patch	2021-01-29 14:01:28.786759376 +0530
   @@ -175,18 +147,6 @@
   -diff --git a/core/modules/content_translation/migrations/d7_block_translation.yml b/core/modules/content_translation/migrations/d7_block_translation.yml
   

   The changes to this file need to be restored.

2. +++ 3189463-14.patch	2021-01-29 14:01:28.786759376 +0530
   @@ -327,7 +287,7 @@
   -+      'd7_language_content_settings',
   ++      'd7_language_content_comment_settings',
   

   The migration should not be changed here.

@quietone

I don't know how you are doing reroll

I haven't re-rolled the patch.

I only shared the diff requested in #16, to help review the patch in #14. I appreciate your consistency, but we should be more vigilant while being suggestive / critical.

@adityasingh, my sincere apologies for that mistake. Thank you for pointing out my error.

I wanted to use this patch for some testing with #3191782: Fix dependency in d6 user profile translation migrations in order to tracking down the problem here, #3191782: Fix dependency in d6 user profile translation migrations. It needed a reroll, so I have done that.

24. It is failing due to a mistake in the reroll.

19.1 Another mistake. d7_block_translation should have a dependency on language
19.2. Fixed

This also adds the d6 migrations.

And again, I forgot to update the Migration tests.

A few more migrations.

According to the script below that leaves the entity translation migrations.
< core/modules/content_translation/migrations/d7_entity_translation_settings.yml
30d28
< core/modules/content_translation/migrations/d7_entity_reference_translation.yml
41d38
< core/modules/content_translation/migrations/d6_entity_reference_translation.yml

#!/usr/bin/env bash

find core/modules/*/migrations -iname \*translation\* -not -path "*/state/*" > migrations.out
find core/modules/*/migrations -iname \*translation\* -not -path "*/state/*"| xargs grep -A20 migration_dependencies | grep ' - language' | awk -F- '{print $1}' > migrations_with_language_dependency.out
diff migrations.out migrations_with_language_dependency.out

And the entity reference translation migrations.

That just leaves core/modules/content_translation/migrations/d7_entity_translation_settings.yml which I don't think needs the language migration.

@Wim Leers, thank you.

Hm, why not that one,

No clue. I must have been off with the fairies (day dreaming).

Fixed now.

This needed a reroll because d6 and d7 block_translation moved to config_translation. I also added language as a dependency to a couple more migration; d6_language_content_comment_settings, d6_language_types and language.

Changed one to many migrations.

Need to add the language migration to a test.

@Wim Leers, "mind-bendingly annoyingly useless error messages" is a far more polite expression that the one I use when I get that error. I waste a lot of time because of #3197324: Exception trace cannot be serialized because of closure.

If you know where the test fails, then something like this helps:

+    try {
+      $this->executeMediaMigrations();
+    }
+    catch (\Exception $exception) {
+      throw new AssertionFailedError(
+        $exception->getMessage() . "\n" .
+        $exception->getLine() . "\n" .
+        $exception->getTraceAsString()
+      );
+    }

(Copied from Media Migration)

Playing devil's advocate, how can we be sure that we don't inadvertently add another migration that should depend on language but does not.
Is there a way we can add a test to prevent it sneaking back.

Or in other words, can we automate some of quietone's know-how - specifically

I count 43 migrations with 'translation' in the title and only 11 have 'language' as a dependency, leaving 32 to change.

First, this happened because I simply forgot to go back and set the dependencies on all the migrations I wrote. Somehow that step never got into my workflow. :-(

After an admittedly brief time thinking about how to test for this nothing popped up. Figuring out the dependencies is a matter of experience and what is learned when writing the migration kernel test. Plus, I am not highly motivated to think about it because as far as I know all the necessary migrations have been added to core. It is very unlikely that a new one will be added; effort is better spent on other issues in the migration system.

Does that help?

Plus, I am not highly motivated to think about it because as far as I know all the necessary migrations have been added to core. It is very unlikely that a new one will be added; effort is better spent on other issues in the migration system.

Perfect, that's an excellent point

error: patch failed: core/modules/config_translation/migrations/d6_profile_field_option_translation.yml:49
error: core/modules/config_translation/migrations/d6_profile_field_option_translation.yml: patch does not apply
error: patch failed: core/modules/config_translation/migrations/d6_user_profile_field_instance_translation.yml:40
error: core/modules/config_translation/migrations/d6_user_profile_field_instance_translation.yml: patch does not apply
error: patch failed: core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateUserProfileFieldInstanceTranslationTest.php:26
error: core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateUserProfileFieldInstanceTranslationTest.php: patch does not apply
error: patch failed: core/modules/user/tests/src/Kernel/Migrate/d6/MigrateProfileFieldOptionTranslationTest.php:29
error: core/modules/user/tests/src/Kernel/Migrate/d6/MigrateProfileFieldOptionTranslationTest.php: patch does not apply

I tried to commit this, but it no longer applies

Needed a reroll for changes from #3191782: Fix dependency in d6 user profile translation migrations.

Known random failure, Drupal\BuildTests\Framework\Tests\BuildTestTest::testPortMany

Committed 24494c2 and pushed to 9.3.x. Thanks!

Backported to 9.2.x

Thanks folks

We are using a distribution that is still using Drupal 8.9 so I back ported the patch for 8.9