CCK module developers read this!!

I'm going to reopen this so module developers see it.

Just to add reasons why this is so critical:

1) The content module updates add columns to the fields tables to keep track of the module name and whether the field is active and available.

2) The content module will add module names to the table when the modules are enabled, but won't be able to update the tables until its own updates are run.

3) If a field module's updates are run before the content module's updates, it will create fatal errors that prevent any updates from working.

4) If the tables aren't updated, the fields will be 'inactive' and will not be included in the content_fields() array, so field modules cannot rely on that function including all fields until the content module tables are enabled and content_associate_fields() is run to populate them. So you can't expect content_fields() to contain the right information unless you know that all the content module updates have processed first. Since updates run in an unpredictable order, you can't be sure of that.

5) Once the tables are processed, inactive fields will be left out of content_fields(), so you can't expect content_fields() to contain the right information for your module if your module is still disabled. If you have updates that use content_fields() to find your module's fields, and the updates are run while the module is disabled, they will find nothing and do nothing, but will report 'success'.

The fix is to prevent any processing of any field module updates until all the content module updates are finished and the field module is enabled, which is what the above function will do.

Also note our new update instructions, on the CCK project page and in the README.txt, which recommends leaving all contrib modules out of the modules folder until CCK is updated and working correctly. If people follow those instructions, many, but not all, of the above problems will be avoided. But we don't know that everyone will follow instructions, and in almost any case you won't want your updates run when your modules are disabled, so you'll still want to include this code.

Does this fix allow imagefield fields to successfully update to Drupal 6? I just can't face the heartache of another failed update attempt. All of those errors scarred me.

Thanks KarenS and yched for posting this issue, I wouldn't have seen it otherwise. Link module doesn't require any updates between Drupal 5 and Drupal 6, so the inclusion of this code is not necessary for Link, correct?

Yes, modules that don't attempt to do updates should be fine. I should have made that more clear.

And I think I saw in the cvs log that imagefield had added this code, but haven't confirmed it or tried again. I too finally had to bail out of trying to get it to update after lots of problems.

I just posted an imagefield patch at http://drupal.org/node/296195#comment-1084052 that may fix the problem with upgrading imagefields. It may also point out a problem with other field modules that are trying to change their widget type names as part of the D6 upgrade (if any).

Quite accidentally, I think I stumbled upon a very simple thing that CCK could do that would solve this problem entirely and that would not require dependent modules to add any code or anyone to move files out of the modules folder or anything bad like that....

See http://drupal.org/node/211182#comment-1108411 for a description and example code of how this might work.

Apologies if I'm mistaken -- it's late on a Friday afternoon, so I could be :)

Changing the module system weight, even if it worked to change the update order, fixes only one of many many problems with the updates. The random order of system updates is only a part of the problem, even more problems arise because updates run even if the modules are disabled, and even if the modules they depend on are disabled. After fixing problem after problem that arose from all the possible ways that things can go wrong, we settled on this method as the only fool-proof way of making sure no updates would run prematurely. Believe me, if there was a simpler solution, we would not have made the process so complicated.

You may have found a way to get the weights to work correctly to force the updates to the right order in your situation, but they don't always. In my tests, the update ignores system weights and updates some modules with heavier weights before other modules with lighter weights, so messing with weights didn't help matters. And it won't keep updates from running when the modules are disabled. And we don't want to permanently change the weights, even that worked, because other modules have changed their weights to be sure they run either before or after CCK and we'll then screw them up by changing CCK's weights.

@KarenS - Thanks for the reply, but please read my comment again... I did not suggest touching the module system weights in the database :) My suggestion was to change the order that the modules are listed on the update form. Drupal uses the $_POST variable (directly) to determine what order to run updates in (see http://api.drupal.org/api/function/update_batch/6) so manipulating the form allows you to manipulate the order in which updates are run. In my testing, this works fine.... however, I suppose this would not cover an edge case where someone was running module updates via their own script rather than via update.php.

My comment also mentioned (the beginnings of) a suggestion for using the same hook_form_alter() implementation to make sure disabled modules' updates do not run... however, I guess that might not work in the case where CCK itself is disabled. I'll think more about that.

OK, hook_form_alter() might indeed have gotten tricky when CCK is disabled ;) However, modifying the $_POST variable directly works perfectly well and is much more robust anyway, so I decided to write a patch (attached).

I have tested this patch by creating dummy update functions for content.module and text.module which contain return array(); and nothing else and trying the following scenarios:

1. With both content.module and text.module enabled, check that content.module's updates runs first.
2. With content.module enabled and text.module disabled, check that content.module's updates run and text.module's do not.
3. With both content.module and text.module disabled, check that neither update runs.

The patch worked perfectly in each case. Are there any scenarios that I am missing?

The only weakness I see is that if there is a module which provides CCK fields but does not contain a formal "dependency" on CCK then it will not be picked up by the code here. Is this a potential issue? (Every field module I looked at has a dependency on CCK in some way.) Even if it is an issue, it could easily be dealt with by requiring such modules to go with the existing approach, or by using various kinds of .info file trickery, etc...

But the bottom line is that this patch should allow CCK field module updates to run at the correct times and in the correct order, without field module authors having to do anything of their own.

Well..... it actually occurs to me that if we did some slight refactoring of the "content_alter_module_updates" function in my patch to make it a bit more flexible (in terms of what kind of data array it takes in), and if we then renamed it to "content_module_updates_alter" instead, we could almost make this available to a command-line update script by requiring that script to add a single line of code:

drupal_alter('module_updates', $module_updates);

For example, it looks like the script you linked to could add that line at the end of their drupal_get_updates() function.

The "almost" is because I don't think drupal_alter() would find it in the case where CCK is disabled... drat. But worst case, they could add something ugly like this:

if (function_exists('content_alter_module_updates')) {
  content_alter_module_updates($module_updates);
}

And that should be enough.

I guess the question is how much burden you think CCK should try to impose on someone who is trying to write an "official" update script for Drupal 6?

Setting to code needs work as per the current discussion at #211182: Updates run in unpredictable order.