Provide an upgrade path for date formats

I think missing upgrade path is critical?

I wouldn't necessarily delete the old tables, we can open a 9.x issue to do that now, like #1860986: Drop left-over tables from 8.x.

Fine with keeping the tables. But there's no need to keep them in system_schema() and there they still are :)

I tried to make a bit of a start on this. Posting up patch of progress so far, incase anyone else is interested in taking up.
Setting to needs work, as there is still some things to do.

I haven't looked into what is involved with any of the date_locale stuff yet.

Thanks for working on this!

+++ b/core/modules/system/lib/Drupal/system/Tests/Upgrade/DateUpgradePathTest.phpundefined
@@ -0,0 +1,76 @@
+ * Definition of Drupal\system\Tests\Upgrade\DateUpgradePathTest.

Should be Contains...

+++ b/core/modules/system/lib/Drupal/system/Tests/Upgrade/DateUpgradePathTest.phpundefined
@@ -0,0 +1,76 @@
+
+    // Verify standard date formats
...
+    // Verify custom date format

Missing . at the end.

+++ b/core/modules/system/system.installundefined
@@ -2331,6 +2242,39 @@ function system_update_8041() {
+        db_delete('variable')->condition('name', $variable_name, '=')->execute();

You can use update_variable_del() here.

+++ b/core/modules/system/tests/upgrade/drupal-7.date.database.phpundefined
@@ -0,0 +1,61 @@
+ */
+// Add default format for standard date formats
+db_insert('variable')->fields(array(

->fields.. should be on a new, separate line.

+++ b/core/modules/system/tests/upgrade/drupal-7.date.database.phpundefined
@@ -0,0 +1,61 @@
+// Add image effects.

Wrong comment.

I'm not 100% sure, but I think localized date formats is a new feature in 8.x and we don't need to provide a 8.x -> 8.x upgrade path. So we can probably ignore this.

Setting to needs review to check if this passes. Setting to needs work doesn't make much sense, all patches will be sent to the testbot when the issue is set to needs review. If you really don't want to have something tested, use -do-not-test.patch.

Working a bit on this.

Fixed the things pointed out in my review, added locale date format (they exist in D7 :) ) upgrade path and tests for it.

Something missing?

Here is a reroll + a test-only patch.

I added the leftover tables to #1860986: Drop left-over tables from 8.x.

Updated the update function number :)

I manually tested the upgrade path and it seems to be working well. Overall the date format localization code looks pretty broken in HEAD, AAMOF it seems localized formats are not being picked up but the data is being migrated correctly, which is all we need for what this patch is concerned.

The patch looks good and the tests capture the bug. Since I just rerolled it I feel ok to RTBC it.

Lovely!

Committed and pushed to 8.x. Thanks!

The upgrade path or its tests seem to be unstable: #1893800: Something is very, very wrong with update.php / upgrade tests... demons suspected