Sync 6.x extra updates with HEAD

There are a couple blocks of code in update.php for the 5.x-6.x update as well.

Looks like there's (at least) one more...

/**
 * Create the batch table.
 *
 * This is part of the Drupal 5.x to 6.x migration.
 */
function update_create_batch_table() {

Patch #4 re-rolled against CVS HEAD.

this is a good idea, but we should make 6.x->7.x updates work at all so we can confirm this patch.

The previous patches were missing adding implementations of hook_update_last_removed and a few @defgroup 7.x blocks. I also wanted to make sure that update functions were always at the bottom of the .install file. That's why there are big changes in comment.install in this patch. No updates are actually changed however. I also found a few 700x updates that didn't start with 7000, so I fixed those as well.

Rerolled since the user table name was rolled back to users.

Yep, we should remove those and force people to upgrade from Drupal 5 to Drupal 6 first. It is the recommended practice anyway. Marking RTBC so people can chime in if they still want to.

The "but won't someone think of the users?!?!?" part of me wants us to retain the upgrade path, since there are probably lots of sites out there that want to make the jump directly from 5.x to 7.x. But the technical arguments for removing this code are pretty compelling. So I've no problems with this patch being committed. I can do so tomorrow if you don't beat me to it. :)

Right. NowPublic plans a D5->D7 upgrade but this does not mean I need to upgrade code in there

I'm not sure I understood what you said in #18, chx. Are you OK with committing this patch?

An alternative would be to move all updates into $module.updates.inc - as a new standard. Solves the "$module.install is loaded on admin pages" and related issues.

FWIW, I also plan to upgrade most of our sites from D5->D7.

FWIW.

Before something like #22 gets committed I want to study why we load all the install files. Can't look at the code yet.

IIRC, reason is hook_requirements(), which is supposed to live in $module.install. Some paths on/below admin/* check whether all requirements are met, and output a warning if not.

ok, http://api.drupal.org/api/function/system_requirements/7 checks whether the database schema of all modules is up to date and thereby loads all hook_update_N() implementations via http://api.drupal.org/api/function/drupal_get_schema_versions/7. umph.

FWIW, there's very little chance that CCK -> Fields D7 upgrade will work from anything else than D6.
+ When work on this progresses, it might very possibly uncover the need for a 'preparation' on the D6 side (before core upgrade), thus requiring getting current with CCK D6 latest release before upgrading core (just like a D5->D6 upgrade requires you get current with CCK D5 first)

I was going to investigate this this morning, but you guys beat me to it when I was sleeping. Awesome. The Drupal community never sleeps. :-)

I'll go ahead and commit the patch in #14 and then we can continue to worry about loading of all the files in #497408: Move hook_update_N() to module.update?

Committed to CVS HEAD. Thanks!

Fatal error: Call to undefined function update_fix_d6_requirements() ...update.php on line 711

Oops. Committed to CVS HEAD. Thanks.

Looks like this patch removed a few update functions which were not actually 5.x to 6.x updates. Around the first part of the patch is blogapi_update_6001(), which explicitly has in its comments that it is a 6.4 update. A locale location column update has a similar fate (realized via http://drupal.org/node/221020#comment-2038462). Granted these were not properly marked with the "updates-6.x-extra" group, which marks updates added after 6.0 was released. What's worse though is that updates explicitly marked with updates-6.x-extra were removed in system.install the same time.

We SHOULD keep the updates-6.x-extra updates in core, since that is the only way for 7.x to support upgrading from any 6.x version (which is our tradition and intention as far as I'm informed). If we remove updates which were added in the 6.x cycle, we'd require people to update to the latest 6.x first and only then update to 7.x. That is not how we operated so far.

My suggestion to revise this patch is to try and apply it to a Drupal 6.0 checkout. Whatever does not apply there was added after 6.x and should be rolled back and marked with the updates-6.x-extra tag. The locale location column update function was added with the wrong number even, so that should be resolved in its own issue, please leave the rollback of that out here.

Note that the patch Dries committed is in #14.

Did you run through the other update functions on a check whether any others were inadvertently removed? I've just included the above as examples and suggested a procedure to verify the others.

Ok, let's ensure we did not restore only the random items I found manually but we systematically ensure that all update functions added after 6.0 are restored.

@catch said in IRC, that he thinks that we should not support updates from any 6.x version to any 7.x version. Our whole development methodology is geared towards producing code supporting that though. Issues are expected to be filed against 7.x first and resolved there first before bumped back to 6.x. So by the time a possible DB update ends up in the D6 queue, it is already in D7. That is, all DB updates added after Drupal 6.0 should be in D7. Therefore Drupal 7 should support updating from any Drupal 6.x version. Changing development approaches at this point in the D7 development would not be a good idea IMHO.

Therefore we should ensure we have all extra updates restored.

So what is the preferable way to make updates in install files before D7 released?

Mostly about #107824: Convert {watchdog}.referer and {accesslog}.url from VARCHAR to TEXT
and same #221020: locales_source.location is just varchar(255) and it is not truncated

if we already make a change in D6 schema in 6-extra section do we need a 700X update function?

The way we did it with 5.x to 6.x is that we built in checks into the 6.x update functions to skip their business if the relevant 5.x update already happened. For identical updates, I think we've also left in "no-op update functions" (named empty functions) to signal that the given update number was removed due to a previous update covering the same thing.

We need to re-add the 6.x-extra updates to our D7 system_install since we have broken update paths. Everything from system_update_6048() on up needs to be re-added, and system_update_last_removed() should be 6047.

Latest patch is missing update_6050 through 6053.

Moving to a bug report so we don't miss this before release.

Let's summarize here issues that have broken upgrade path or should be changed.

Maybe there's an issue about reworking|reordering update_N() functions in D7, so please point to it if any.

#107824: Convert {watchdog}.referer and {accesslog}.url from VARCHAR to TEXT
#334283: Add msgctxt-type context to t()
#221020: locales_source.location is just varchar(255) and it is not truncated

#215080: Performance: change {system}.type: alter table system modify column type VARCHAR(32);

While playing with update I ran across two items. First is the system tables default values in 6.14 do not match 7.x, in particular the weight field in 6.14 is set to tinyint(2) but in 7.x is int(11). This causes failure in update.inc function update_fix_d7_install_profile() Cannot upgrade from Drupal 6 to Drupal 7 - meta issue.

The other issue I ran into was the role_permissions tables and registry tables need to be created before updating. found role_permissions in system_update_7007. What I did was to change the system tables defaults and create the registry files in update.inc, had to also change the system_update_functions to check for the existence of the schema and skip table creation before running on both.

setting it up in update.inc still allows installs as expected and 6.x - > 7.x update with errors.

I don't pretend to know what else I missed. The errors I had dealt with d6 modules and filetimec errors on paths which resembled old d6 modules I hadn't disables when I tried the update. I particular it caught the cck module.

Can provide a diff if anyone is interested.

This patch sync drupal-6 extra system updates section with drupal-7

@catch locale still waiting for review in D6 #221020: locales_source.location is just varchar(255) and it is not truncated

taggin

This is a split for all modules with extra updates in d6
- modules/dblog/dblog.install
- modules/locale/locale.install
- modules/statistics/statistics.install
- modules/system/system.install - this one

Both #107824: Convert {watchdog}.referer and {accesslog}.url from VARCHAR to TEXT and #221020: locales_source.location is just varchar(255) and it is not truncated
I mark as postponed till this commited

patch

Committed to CVS HEAD. Thanks!

can we remove the critical or close this?

I am having the mentioned watchdog problem of "data too long for column..." on my multilingual site.
Is the patch attached on comment #58 the one I need to apply to fix this?

One more thing, usually the patches I've seen apply changes to ".module files". This one, however, targets the ".install" files.
This means that I need to apply it normally and then run update.php?

Am I on the right track here?

Thanks,
Nimi.

anyone? help please...

@64: suggest you open a support request or forum topic - this issue is specifically about changes needed in 7.x. Without more info (Drupal version, exact error message) it's not possible in any case to answer your questions...

Suppose this issue should stay critical until D7 is released because D6 still have issues

system_update_7025() is broken as reported at #77 #358315: drupal_lookup_path() not respects alias' order
Let's wait for commit to D-6 branch... then sync new system_update_6054()

Another old issue #39 #221020: locales_source.location is just varchar(255) and it is not truncated waiting...

Since those are covered by other issues already, no need to keep this one open + critical.