Create a module and enable it. Create two update functions, _1 and _2, and declare them in most-recent first order:

function mymod_update_2() {
  drupal_set_message('update 2');
}

function mymod_update_1() {
  drupal_set_message('update 1');
}

Run update.php and see the output. Notice that only update_2() runs.

Initial patch against 5.x attached but it applies to HEAD. This patch needs tests and, IMHO, should then be back-ported to 6.x and 5.x.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Damien Tournoud’s picture

Priority: Critical » Normal
Status: Active » Needs work

I'm not sure there are any real-world consequences for that bug, so that's not critical. Any way, I can confirm that bugs. All updates functions in install.php and update.php expects the result of drupal_get_schema_versions() to be sorted, while nothing in the drupal_get_schema_versions() contract guarantees that.

I have not tested the patch. On review, it looks good, but the change in the behavior of the function (ie. the results will always be ordered) should be documented in its doxygen comment.

bjaspan’s picture

Priority: Normal » Critical

The real-world consequence is that foo_update_1() will never be run and foo_update_2() will be. That's bad and complete violates the specification for update functions.

floretan’s picture

Status: Needs work » Needs review
FileSize
858 bytes

Mention the sorting in the doxygen comment.

Damien Tournoud’s picture

Priority: Critical » Normal

Tested and confirmed to work on a dummy module.

No module was ever broken due to this bug, because all follows the sound convention of putting updates stages in order, so demoting this to normal.

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
783 bytes

Rerolled for head. Trivial fix that solves a confirmed problem, marked as RTBC.

Dries’s picture

Version: 7.x-dev » 5.x-dev

Committed to CVS HEAD and DRUPAL-6. Might have to go into DRUPAL-5 too.

drumm’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 5.x.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.