You are here

Remove all 7xxx update functions and tests (D6 to D7 upgrade path)

This patch makes the first steps in removing all the upgrade functions that migrate from D6 to D7. It does not remove utility functions that may still be needed in Drupal 8 and keeps in place basic documentation around areas that are guaranteed to be necessary (such as a system_update_8000() and update_fix_d8_requirements()). Further revisions would be great, but this is definitely step 2 for opening up development on Drupal 8 (right after #1089320: Update version strings and constants to 8.x).

Updated to remove additional PDO checked which aren't necessary (since you already had to have PDO in order to run D7). Not sure if this patch format is correct, git patches aren't quite straight-forward.

I can't apply this because the format of the patch is a little weird (update.inc gets patched 3 times for example). And as I never updated I followed the update instructions, shouldn't we change garland to stark?

I know that is nitpicking but if we are going to apply this patch I would like it to be done :).

Well after sitting through an incredibly tedious round of simpletests (literally 10 hours on my local), I found that the upgrade path is totally borked when attempting to update, since all of our tests assume a Drupal 6 database to start, not a D7 one. So I've updated this patch to include removal of the entire "upgrade" directory in the simpletest module. Obviously we'll want to add these tests again as necessary, but right now there is nothing to actually test, since there aren't any differences between D7 and D8 in node, comment, taxonomy or other modules that we were previously testing. The major increase in size is the removal of files that create default Drupal 6 databases. Finally figured out rolling patches against Git origin, so this patch should be good to test and in the right format.

I never upgraded in my life :) so here is my first attempt. I applied your patch, went fine. There was one file that didn't apply clean.
I followed all the steps, applied new D8 files. I went to my site and it still worked ;). But when I run update.php I get this error.

One remark: if we are keeping some of the database files in simpletest, can we rename them to d7 in stead of d6?

The patch in #8 should remove all the d6 initial database files. Those files are intended to recreate a full Drupal 6 site that can then be tested for upgrading to Drupal 7. Eventually we'll want not just to rename them but actually create a full Drupal 7 database file that provides similar functionality to test the upgrade path to Drupal 8.

I can't get this patch to apply to 8.x with patch or git apply -- I'd simply delete the tests/upgrade directory and submit a new patch, but quicksketch's patch includes changes to a bunch of other files that reference these tests.

I'm trying to help but I don't know how to rly test this. I *tried* updating (don't rly know how that works yet, just followed instructions) but I get some server errors while doing that. And when I enable the testing module and I go to the test suite I get the followng strict warning:

Strict warning: Declaration of FieldUIManageFieldsTestCase::setUp() should be compatible with that of FieldUITestCase::setUp() in _registry_check_code() (line 2683 of C:\xampp\htdocs\drupal8\includes\bootstrap.inc).

install.inc must be included or update.php gives a WSOD since drupal_get_installed_schema_version() is undefined.

+++ b/includes/update.inc@@ -68,683 +68,51 @@ function update_check_incompatibility($name, $type = 'module') {- // Allow the database system to work even if the registry has not been- // created yet.- drupal_bootstrap(DRUPAL_BOOTSTRAP_DATABASE);

Also, drupal_get_installed_schema_version() calls db_query(), so the database must be bootstrapped.

The attached patch removes that last usage of $configurable_timezones as well as addressing #29 and #30. Using the attached patch, I was able to successfully update from D7 to D8 without errors or notices.

Has anyone tried running the tests with these patches since quicksketch in #7? I've never ran tests manually before, but may give it a try if I have time today. (For those who don't know, the test bot won't test patches for 8.x until 8.x itself passes tests, which is part of why this issue is so critical -- it's holding up all 8.x patches from testing).

Yep, agreed. My patch didn't apply successfully at first either (I also use Eclipse), but so far #32 is looking great after applying successfully. Unfortunately my tests stopped at about 60% when I accidentally restarted Apache. :(

Tests are looking great so far (100% of the ones I got through), but now I have to start it all over again. Another 8 hours... sigh.

Actually, the failed test I had was some sort of semaphore lock -- when I re-tested, it passed, so I actually haven't had a test fail yet. I'd love to see this committed as is -- #32 is definitely better than where we're at now, and it would let testbot take a crack at it.

I'm currently re-running the test suite too, but I'm not very good at it -- my last run failed with an out-of-memory error, so I just restarted it doing half the tests at a time.

Actually, the failed test I had was some sort of semaphore lock -- when I re-tested, it passed, so I actually haven't had a test fail yet. I'd love to see this committed as is -- #32 is definitely better than where we're at now, and it would let testbot take a crack at it.

I had this exact same test fail the first time I ran it too. The drupal_render() test seems to occasionally fail also, but never twice in a row. Seems like some of our tests are a bit fragile. This patch doesn't touch on either drupal_render() or the lock/semaphore system.

So with all tests passing, update.php working as expected, the ability to "upgrade" from Drupal 7 to Drupal 8 all in place, I also agree this is ready to go. It'll be nice to start work on Drupal 8. :)

Sweet, thanks Dries and rfay! I'll take a look at the remaining tests. Glad to see both the drupal_render() and locking tests passing by testbot though. Strange that they don't pass all the time locally for me.

This patch renamed _update_7000_node_get_types() (and presumably others, did not check) to _update_8000_node_get_types() - those functions are linked to schema version, not Drupal version, so need to be renamed back.

@catch: I left those functions in the .install files in case we needed them during the Drupal 8 cycle, but none of these functions are actually called anywhere in Drupal 8. Couldn't we just remove them entirely? I'm not understanding why we need to rename them back, or really why they would have an effect on anything at all if they're not used.

The functions should be tied to the schema version when they were added, when there's a new schema version for the module, we'd need to add a new function with the new schema name - this allows foo_update_8000() to get a list of node types with one schema, and foo_update_8032() to get a list of node types with a different schema. Question is whether we'll need these specific functions during the 7.x-8.x upgrade path. Damien and chx added these, so maybe they have some feedback.

/me shrugs. While I think it's silly to have 7000_* named functions lingering around, it's not going to hurt anything renaming them back, since (like I said) they're not actually used anywhere. Seems like if we're going to keep them they should be given appropriate names though. Can someone explain why a function that isn't even used in D8 this is blocking D7 bug fixes?

They are named on schema version, if the schema relating to those functions doesn't change via the D8 lifecycle, then 7000 will be the appropriate name. If it changes during the D7 cycle, we may need to add 7012 versions of those functions to D7, in which case it will be very silly for the 7000 version to be in D8 but named 8000 - are you going to rename 7012 to 8012 then?

We could remove the functions, but then we'd need to add them back as soon as (for example) someone wants to write an update that adds a field. Either way is a pain in the arse.

1. Install Drupal 8, run SELECT name, shema_version FROM system; the schema version for installed modules will be 0, not 8000.

2. Install Drupal 7, 'upgrade' it to Drupal 8 - the schema version will be 0 or 7* for all installed modules.

Therefore there is currently no code in Drupal core at all, that corresponds to a schema version of 8000 yet (edit apparently there is but it is a no-op, however it only applies to system module). And even that update might just set a variable or update a field definition, rather than change the schema (in which case we don't bother changing the functions at all - until we have to).

On why this is blocking D7 bug fixes

- We branched while there were still critical data loss issues in the upgrade path in Drupal 7, then proceeded to diverge the code paths for the upgrade path, while we have a strict policy of fix in HEAD and backport. I don't think we should have branched while this was the case.

- Removing the functions entirely would have not blocked the other issue (it would mean that patch only goes into D7), leaving the functions named correctly would also have not blocked it (the patch would apply to D8 with no dependencies). It is the renaming here that means we are blocking it, because the only way I can make that patch apply is ranting in this issue until it is fixed, or re-rolling that patch to be wrong.

Also this didn't just block D7 patches, committing straight from needs review on http://drupal.org/node/1089320#comment-4237202 broke D8 HEAD too for several days. This doesn't make me feel very optimistic about all the good intentions we had at DrupalCon about a less critically broken development release during the course of the cycle.

ranting in this issue until it is fixed, or re-rolling that patch to be wrong.

It sounds like this is the philosophical hold-up. The patch could easily be re-rolled with the 8000_* functions and get that committed despite it being "wrong" because of the function name number not matching a schema version.

Well in any case, it's not going to make any difference in D8. Let's just rename the functions back and work under the assumption that they're going to get renamed anyway as soon as we need them (and then we can match the schema version appropriately).

So catch and I had an IRC chat in which he explained the whole situation to me on why these shouldn't be 8000_* and why we should keep them around. The explanation makes sense and I agree we should rename them back to 7000. Let's just rename them back and be done with it. #59 is still ready to go.